"Open declaration can be removed" - but actually it cannot

[forki]

Repro steps

1. clone https://github.com/fsprojects/Canopy.Mobile
2. run paket restore
3. open sln in VS2017
4. open Canopy.Mobile.fs

If you accept then you get compile errors

Expected behavior

Don't show the suggestion

/cc @vasily-kirichenko

[vasily-kirichenko]

Why it cannot be removed?

[forki]

because then compile errors happen

[forki]

it's a false postive

[matthid]

@forki I guess @vasily-kirichenko is asking for the compiler error or the statement which needs the open ;)

[forki]

typical name resolutions issues.

[vasily-kirichenko]

No, I'm asking for the source code of a symbol that makes the open statement needed.

[vasily-kirichenko]

(I guess it's the exception definition)

[forki]

see screenshot

[forki]

    /// Contains canopy specific exception
    module canopy.mobile.Exceptions

    open System

    type CanopyException(message) = inherit Exception(message)
    type CanopyElementNotFoundException(message) = inherit CanopyException(message)

^^ is the other file

[vasily-kirichenko]

Thanks.

[vasily-kirichenko]

Ah, relative opens. They are not supported. Use full ns/module names.

[forki]

What do you mean they are not supported? That's proper F# - the tooling should adjust and not my code, right? If there is no way to make it work with relative opens then it should be disabled for relative opens, right?

Also not that open Wait is not greyed out

[vasily-kirichenko]

Right :) The problem is that opens are not symbols, they are text from untyped AST. However, if it works properly for Wait, it maybe the case that exceptions are not processed.

[vasily-kirichenko]

Sorry, Ctrl+F5 crashes VS immediately on my machine :))

[dsyme]

What do you mean they are not supported? That's proper F# - the tooling should adjust and not my code, right? If there is no way to make it work with relative opens then it should be disabled for relative opens, right?

Perhaps a code fix to turn relative opens into fully-qualified opens.

[BentTranberg]

I believe this is an exact duplicate of the older #3306.

[cartermp]

This issue seems to be mitigated. You can still get a lightbulb which will remove it, but Exceptions is not greyed out anymore on 15.5.3.

[TysonMN]

What is a "relative open"?

[TysonMN]

Issue #7111 was just marked as a duplicate of this one. That issue includes a very minimal working example.

The consensus here seems to be that the present issue is related to the concept of a "relative open", which is contrasted with a "fully-qualified open". I have only ever heard these terms in this issue.

Is there documentation that explains what these terms mean? If not, can someone explain what these terms mean in this issue?

Because of my lack of knowledge (both generally of F# and specifically about the answer to those two questions) and the not-so-minimal working example provided in the present issue, I am unable to verify for myself that issue #7111 is a duplicate of this one.

[abelbraaksma]

@bender2k14, a relative open is when you don't fully specify the whole namespace, or the namespace before the module. A fully qualified open is when, in contrast, you do specify the FQN (fully qualified name) in the open statement.

open System // fqn
open IO   // relative, will raise FS0893, which shows these terms, see below

FS0893: This declaration opens the namespace or module 'System.IO' through a partially qualified path. Adjust this code to use the full path of the namespace. This change will make your code more robust as new constructs are added to the F# and CLI libraries.

Here is a valid way:

module X =
    module Y =
        let z = 10
    module Z =
        open Y    // relative open of MyNamespace.X.Y
        let a = z  // use of X.Y.z, as can be seen, in this case the "unusued open" warning disappears

Btw, I agree that the documentation is rather limited, and also doesn't mention the peculiarities in opening namespaces vs opening modules, or nested modules. https://docs.microsoft.com/en-us/dotnet/fsharp/language-reference/import-declarations-the-open-keyword.

I am unable to verify for myself that issue #7111 is a duplicate of this one.

I have seen several issues of false positives w.r.t. unused opens. Inlined members, type aliases and some more complex namespace + type/module hierarchies, type forwarding and more can all lead to this situation where the suggestion to remove the unused open is invalid.

I think it would be good to collect a couple of simple repros so that working on the issue in a PR becomes easier. The code that does this currently is in UnusedOpensDiagnosticAnalyzer.fs.

[cartermp]

Slight correction, the analyzer itself lives here: https://github.com/dotnet/fsharp/blob/master/src/fsharp/service/ServiceAnalysis.fs#L12

As mentioned earlier in this issue there is a problem related to the data that is being operated on here. It can likely be fiddled with a bit.

[TysonMN]

Thanks @abelbraaksma. That is very helpful.

I think it would be good to collect a couple of simple repros so that working on the issue in a PR becomes easier.

Are there any existing unit tests for this unused open analyzer? If so, I might be able to add tests for the cases that have been reported in this and the linked issues.

[abelbraaksma]

I assume there are, but where exactly I don't know. I haven't played with this part of the tooling yet.

[cartermp]

In this case there are some fairly comprehensive tests here: https://github.com/dotnet/fsharp/blob/master/vsintegration/tests/UnitTests/UnusedOpensTests.fs

[TysonMN]

I think it would be good to collect a couple of simple repros so that working on the issue in a PR becomes easier. The code that does this currently is in UnusedOpensDiagnosticAnalyzer.fs.

I tried and failed to reproduce the bugs reported in this issue and the two linked issues. Here is the environment I used.
Operating system: Windows 10
.NET Runtime kind: .NET Core 3.1
Editing Tools: Visual Studio Enterprise 2019 version 16.6.5

1. I followed the steps in the first comment ("Open declaration can be removed" - but actually it cannot #3347 (comment)) of this issue. No open was reported as unused...not even open Exception.
2. Linked issue Visual Studio incorrectly suggests "Open declaration can be removed" #3306: Not surprising that I couldn't reproduce because it was already claimed to be fixed in Visual Studio incorrectly suggests "Open declaration can be removed" #3306 (comment).
3. Linked issue Open incorrectly considered unused #7111: This was the issue I created that was marked as a duplicate of this one.

Seems like all these bugs are fixed. I recommend closing this issue.

In this case there are some fairly comprehensive tests here: https://github.com/dotnet/fsharp/blob/master/vsintegration/tests/UnitTests/UnusedOpensTests.fs

Thanks @cartermp. I was so excited to contribute something to F# that I confirmed that the 67 unused open tests pass on my machine before confirming that I had a MWE to make into a test. I see there are more open issues in this repo, so I will investigate them now.

[cartermp]

Thanks @bender2k14 - I also failed to reproduce the issue in the latest internal preview build of the F# tools. It could be that something changed since this report was initially filed that influences the lack of repro, but looking at the history of the codebase it doesn't appear to be the case. I'll close this one out.