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

FSharpSymbol.ImplementationLocation points to definition in FSI file across multiple projects #545

Closed
vasily-kirichenko opened this issue Mar 23, 2016 · 8 comments
Labels

Comments

@vasily-kirichenko
Copy link
Contributor

File1.fsi

namespace Lib2
type TypeWithSignature = 
    { Field1: int }

File1.fs

namespace Lib2
type TypeWithSignature = 
    { Field1: int }

Library1.fs in different project

open Lib2
let x = { Field1 = 1 }

FSharpSymbolUse.Symbol for Field1 in Library1.fs has identical DeclarationLocation, ImplementationLocation and SignatureLocation:

image

This bug makes it very hard to implement this VFPT feature: fsprojects-archive/zzarchive-VisualFSharpPowerTools#1351

@7sharp9
Copy link
Member

7sharp9 commented Mar 23, 2016

Yeah Ive got annoyed by that at times, jumping to the fsi is annoying

@dsyme dsyme added the bug label Mar 30, 2016
@dsyme dsyme mentioned this issue Mar 30, 2016
@dsyme
Copy link
Contributor

dsyme commented Mar 30, 2016

Added a fix - next time you could probably have got this one :)

@dsyme dsyme closed this as completed Mar 30, 2016
@vasily-kirichenko
Copy link
Contributor Author

I tried #548 and the bug is still here:

image

@dsyme
Copy link
Contributor

dsyme commented Mar 30, 2016

Ah, I hadn't tried the multi-project case. Well, the PR fixed a real bug in any case :)

The cross-project case may not be that easily fixable, since the F# metadata propagated between projects has only space for one range, see https://github.com/fsharp/FSharp.Compiler.Service/blob/master/src/fsharp/TastPickle.fs#L1975 for example.

When compiling a set of related projects using FCS in a single multi-project graph it's possible that we can propagate the extra information in a lookaside table of some kind.

@dsyme dsyme reopened this Mar 30, 2016
@vasily-kirichenko
Copy link
Contributor Author

It does not work for all cases even within single project.

Symbol returned in fs file (ok):

image

Symbol returned for the same symbol in fsi file (wrong):

image

@dsyme dsyme removed the fix ready label Mar 30, 2016
@dsyme dsyme changed the title FSharpSymbol.ImplementationLocation points to definition in FSI file FSharpSymbol.ImplementationLocation points to definition in FSI file across multipe projects Mar 30, 2016
@dsyme dsyme changed the title FSharpSymbol.ImplementationLocation points to definition in FSI file across multipe projects FSharpSymbol.ImplementationLocation points to definition in FSI file across multiple projects Mar 30, 2016
@dsyme
Copy link
Contributor

dsyme commented Mar 30, 2016

Thanks

I believe it works correctly for results from whole project analysis using ParseAndCheckProject - at least that is under test and appears to work.

I assume the above signature symbols are just from checking a single signature file using ParseAndCheckFileInProject? That case is also tricky, since as things are done now the corresponding matching implementation file is not checked for the single file, so the matching implementation location is not even known. I'm not sure what to do about this, it feels like it needs an API addition.

The locations should be correct for results from GetBackgroundCheckResultsForFileInProject if you first ask for the results for the implementation file, and then the signature file.

@vasily-kirichenko
Copy link
Contributor Author

Thanks.
OK, I wrote this code:

let (|SignatureFile|_|) file = 
    if Path.GetExtension file = ".fsi" then
        let implFile = Path.ChangeExtension(file, ".fs")
        if File.Exists implFile then Some implFile else None
    else None

let! results = 
    match currentFile with
    // it's signature file and the impl file presents
    | SignatureFile implementationFile ->
        async {
            let source = 
                openDocumentsTracker.TryGetDocumentText implementationFile 
                |> Option.getOrTry (fun _ -> File.ReadAllText implemen
            // check impl file
            let! _ = instance.ParseAndCheckFileInProject(opts, implementationFile, source, stale)
            // check sig file
            let! _ = instance.ParseAndCheckFileInProject(opts, currentFile, source, stale)
            // get background check results for sig file
            return! instance.GetBackgroundCheckResultsForFileInProject(opts, currentFile)
        }
    // this is how it previously works
    | _ -> instance.ParseAndCheckFileInProject(opts, currentFile, source, stale) 
    |> liftAsync

When I try results.GetSymbolUseAtLocation, it returns None. However, results.GetAllUsesOfAllSymbolsInFile returns the proper symbol use:

image

Maybe GetSymbolUseAtLocation uses ImplementationLocation hence None?

@dsyme
Copy link
Contributor

dsyme commented Mar 31, 2016

Perhaps this:

            let! _ = instance.GetBackgroundCheckResultsForFileInProject(opts, implementationFile)
           // get background check results for sig file
            return! instance.GetBackgroundCheckResultsForFileInProject(opts, currentFile)

However, assuming this works at all, it will only work for the contents of the signature and implementation files as reported by the IFileSystem - not for a source text passed as a parameter.

Calls to ParseAndCheckFileInProject on a signature file will currently always return symbols where the ImplementationLocation is None. We really need a way to check both the updated signature and implementation together (or else you save the files - enough for IFileSystem to report updated results - and use the above technique)

@dsyme dsyme closed this as completed Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants