Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Go To Definition fails if containing file is referenced with forward slash #4016

Closed
chadunit opened this issue Nov 25, 2017 · 4 comments · Fixed by #7376
Closed

Go To Definition fails if containing file is referenced with forward slash #4016

chadunit opened this issue Nov 25, 2017 · 4 comments · Fixed by #7376
Labels
Area-LangService-API Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Milestone

Comments

@chadunit
Copy link

chadunit commented Nov 25, 2017

UPDATE: Issue with forward slashes, not junctions.

See #4016 (comment).

Go To Definition fails for a symbol if the containing file is in a directory that is linked via NTFS junction or symbolic link.

Go To Definition fails for a symbol if the containing file is in a directory that is referenced with forward slashes in the .fsproj file.

In a .NET Standard F# project, this fails with error "Cannot navigate to the symbol under the caret."

In a .NET Framework F# project, this works.

In a .NET Standard C# project, this works.

Repro steps

In Visual Studio, create a Class Library (.NET Standard) F# project.

Create a .fs file in a subdir.

Create a junction to the subdir. (e.g. in Powershell run: New-Item myjunction -ItemType Junction -Value origsubdir)

Edit your .fsproj and manually reference the file using a forward slash.

In your main Library.fs, reference something from that file and then try to F12 to it. Results in error: "Cannot navigate to the symbol under the caret."

(Note that intellisense/compilation are working; just F12 fails.)

Related information

Visual Studio Community 2017 Preview
Version 15.5.0 Preview 4.0
Visual F# Tools (Experimental) 15.4.1.17112401 (from feed: https://dotnet.myget.org/F/fsharp-preview/vsix)
dotnet 2.1.1-preview-007165

@cartermp cartermp added this to the VS 2017 Updates milestone Nov 25, 2017
@dsyme dsyme added Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code. labels Nov 28, 2017
@chadunit
Copy link
Author

chadunit commented Dec 2, 2017

Update:

This is not an issue with junctions. It is an issue with forward slash path separators in the project file. (In trying to create a minimal repro I had hand-edited the fsproj and used forward slashes without realizing that could be the issue.) However it is still true that only .NET Standard F# fails with forward slashes; I verified with hand-edited F# "classic" and C# projects that they worked with forward slashes.

The navigation failure essentially stems from here:

https://github.com/Microsoft/visualfsharp/blob/afb8b75ae9523f69d20b7f853c0c8d939ddd089d/src/fsharp/NameResolution.fs#L1463

The equality fails because one side has forward slashes and the other backslashes.

The root cause seems to be here:

https://github.com/Microsoft/visualfsharp/blob/afb8b75ae9523f69d20b7f853c0c8d939ddd089d/vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs#L229-L233

The sources coming in retain the forward slashes verbatim from the project file.

Would the following fix for the above nested function be suitable? Using Path.GetFullPath (which uses winapi GetFullPathNameW) to convert to a canonical form with backslashes.

let fullPath p =
	if Path.IsPathRooted(p) then p
	else Path.Combine(Path.GetDirectoryName(path), p)
	|> Path.GetFullPathSafe

@cartermp
Copy link
Contributor

cartermp commented Dec 2, 2017

@chadunit I think that's a good thing to try. @KevinRansom for FYI as he implemented this.

@chadunit chadunit changed the title Go To Definition cannot navigate across NTFS junction/symbolic link Go To Definition fails if containing file is referenced with forward slash Dec 2, 2017
@piaste
Copy link

piaste commented Dec 20, 2017

Oh woah, thanks a lot for figuring this out! I'd been using forward slashes to avoid any potential issues with the Linux build server, and I had just assumed F12 was still buggy.

@cartermp cartermp modified the milestones: VS 2017 Updates, 16.0 Jun 20, 2018
@cartermp cartermp modified the milestones: 16.0, Unknown Aug 25, 2018
@yuriyostapenko
Copy link

This is still the problem with latest versions of VS 2017 and 2019 Preview.
In addition to "Go To Definition", it reversely affects "Find All References".

And dotnet new ... F# template for ASP.NET Core inserts forward slash in fsproj.

I'd gladly try to create a PR, but given the fix is already suggested, that feels like stealing credit 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-LangService-API Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants