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

FindBackgroundReferencesInFile on Active Pattern Case finds all other Cases too #14206

Closed
Booksbaum opened this issue Nov 1, 2022 · 0 comments · Fixed by #14966
Closed

FindBackgroundReferencesInFile on Active Pattern Case finds all other Cases too #14206

Booksbaum opened this issue Nov 1, 2022 · 0 comments · Fixed by #14966
Assignees
Labels
Area-LangService-FindAllReferences Find all references across projects and solutions Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Milestone

Comments

@Booksbaum
Copy link
Contributor

"Find All References" in Visual Studio (and Ionide) on an Active Pattern Case sometimes not just finds current case, but all other cases too:
VS-FindAllReferences-Project-EvenOdd
-> "Find All References" on Even -- but finds Odd too

But this isn't always the case:
VS-FindAllReferences-Script-Even
Here VS correctly finds just Even occurrences.

Difference: First one is a fs file inside a Project, second one is a Script file (even one not saved on disk)


Also an issue for Renaming:
Instead of just renaming Even, Odd gets renamed at same time (to same name)



Side Note:
Images above show a presentation error too:
let |Even|Odd|(|Even|Odd|) v = instead of let (|Even|Odd|) v =
Link location (click on result) works fine



The difference in behaviour can be explained by looking at FindUsagesService:

match symbolUse.GetDeclarationLocation document with
| Some SymbolDeclarationLocation.CurrentDocument ->
let symbolUses = checkFileResults.GetUsesOfSymbolInFile(symbolUse.Symbol)
for symbolUse in symbolUses do
match RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, symbolUse.Range) with
| Some textSpan ->
do! onFound document textSpan symbolUse.Range |> liftAsync
| _ ->
()
| scope ->
let projectsToCheck =
match scope with
| Some (SymbolDeclarationLocation.Projects (declProjects, false)) ->
[ for declProject in declProjects do
yield declProject
yield! declProject.GetDependentProjects() ]
|> List.distinct
| Some (SymbolDeclarationLocation.Projects (declProjects, true)) -> declProjects
// The symbol is declared in .NET framework, an external assembly or in a C# project within the solution.
// In order to find all its usages we have to check all F# projects.
| _ -> Seq.toList document.Project.Solution.Projects
let! _ = SymbolHelpers.getSymbolUsesInProjects (symbolUse.Symbol, projectsToCheck, onFound) |> liftAsync
()

-> different lookup methods depending on local to current doc or public:

  • current doc: checkFileResults.GetUsesOfSymbolInFile
  • otherwise: SymbolHelpers.getSymbolUsesInProjects which boils down to checker.FindBackgroundReferencesInFile (actual call)

(Ionide uses same functions for same scopes -> explains same behavior as in VS)


So it seems FSharpCheckFileResults.GetUsesOfSymbolInFile and FSharpChecker.FindBackgroundReferencesInFile return different results for Active Pattern Cases -- with FSharpChecker.FindBackgroundReferencesInFile being wrong



Repro steps

(Test Script)

Tests find usages on last Even in:

let (|Even|Odd|) v = 
  if v % 2 = 0 then Even else Odd
match 2 with
| Even -> ()
| Odd -> ()

git clone https://gist.github.com/275f6af13410812ba97a0647b9d2408f.git test

cd test

dotnet fsi --nologo --exec .\Test.fsx

Output:

# Project
* uses: Ok
* refs: Error
    Expected 3, but was 6: [|...\src\Project.fs (2,6--2,10);
  ...\src\Project.fs (2,11--2,14);
  ...\src\Project.fs (3,20--3,24);
  ...\src\Project.fs (3,30--3,33);
  ...\src\Project.fs (5,2--5,6);
  ...\src\Project.fs (6,2--6,5)|]
# Script
* uses: Ok
* refs: Error
    Expected 3, but was 6: [|...\src\Script.fsx (2,6--2,10);
  ...\src\Script.fsx (2,11--2,14);
  ...\src\Script.fsx (3,20--3,24);
  ...\src\Script.fsx (3,30--3,33);
  ...\src\Script.fsx (5,2--5,6);
  ...\src\Script.fsx (6,2--6,5)|]
# InMemory
* uses: Ok
* refs: Error
    Exception: System.AggregateException: [...]
    // Expected, because no existing file
  • uses: GetUsesOfSymbolInFile
  • refs: FindBackgroundReferencesInFile
  • Ok when 3 occurrences found, otherwise Error

-> tests prove: Different results for GetUsesOfSymbolInFile and FindBackgroundReferencesInFile and wrong results for FindBackgroundReferencesInFile

Expected behavior

FindBackgroundReferencesInFile should only return locations of search Active Pattern Case.

Actual behavior

FindBackgroundReferencesInFile returns locations of other Cases too

Known workarounds

Use GetUsesOfSymbolInFile instead of FindBackgroundReferencesInFile

Related information

  • Win 11 x64
  • dotnet --version: 6.0.402, 7.0.100-rc.2.22477.23
  • FCS: 41.0.5, 41.0.6, 42.7.100-preview.22473.1
  • Microsoft Visual Studio Community 2022 (64-bit) - Current Version 17.3.6
@github-actions github-actions bot added this to the Backlog milestone Nov 1, 2022
@vzarytovskii vzarytovskii moved this to Not Planned in F# Compiler and Tooling Nov 1, 2022
@0101 0101 added the Area-LangService-FindAllReferences Find all references across projects and solutions label Nov 1, 2022
@0101 0101 added Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code. and removed Needs-Triage labels Nov 28, 2022
@0101 0101 self-assigned this Mar 21, 2023
@github-project-automation github-project-automation bot moved this from Not Planned to Done in F# Compiler and Tooling Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-LangService-FindAllReferences Find all references across projects and solutions Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants