Skip to content

Commit 2c2e497

Browse files
CopilotTheAngryByrd
Copilot
and
TheAngryByrd
authored
Fix Find All References for Active Pattern Cases (#1436)
* Initial plan * Fix Find All References for Active Pattern Cases by filtering to specific case Co-authored-by: TheAngryByrd <[email protected]> * Add comprehensive tests for Active Pattern Find All References Tests verify that: - Finding references for 'Even' returns only 'Even', not 'Odd' - Finding references for 'Odd' returns only 'Odd', not 'Even' - Partial Active Patterns work correctly - Multi-case patterns (3+ cases) correctly filter each case independently Co-authored-by: TheAngryByrd <[email protected]> * Format code with fantomas Co-authored-by: TheAngryByrd <[email protected]> * Add tests for finding references from active pattern declarations - Added test for full active pattern from declaration - Added test for partial active pattern from definition - Updated comment to clarify filtering behavior - All 18 tests now pass (9 for BackgroundCompiler + 9 for TransparentCompiler) Co-authored-by: TheAngryByrd <[email protected]> * Add logic to find case usages when querying from active pattern declaration - Updated test expectations to include case usages (ParseInt) - Added code to extract pattern name and find matching cases - When querying from (|ParseInt|_|), now searches for ParseInt case usages - Handles both partial and full active patterns Co-authored-by: TheAngryByrd <[email protected]> * Fix finding case usages from active pattern declaration - Use case.Group.DeclaringEntity to match cases to their pattern - Compare declaring entities and pattern names - Properly handle Option types in comparisons - This enables finding ParseInt usages when querying from (|ParseInt|_|) Co-authored-by: TheAngryByrd <[email protected]> * Attempt to fix case usage finding with simpler pattern matching - Changed approach to match case names within pattern DisplayName - Check if case name appears between pipes in pattern name - Handle both full patterns (|Even|Odd|) and partial patterns (|ParseInt|_|) - Test still failing - needs further debugging of FCS symbol behavior Co-authored-by: TheAngryByrd <[email protected]> * Refactor active pattern tests for consistent formatting and readability * Add tests for finding references in nested and complex patterns * Implement case usage detection for partial active patterns and add tests for cross-file references * Enhance active pattern case usage detection for partial patterns in match expressions * Refactor range tests to improve reference checking and add script support * Clean up whitespace in checkRangesScript function for improved readability * Add support for extracting case names from partial active patterns and enhance related tests * formatting * Add functions to extract case names from active patterns and refactor related logic * Enhance documentation for findPartialActivePatternCaseUsages and optimize case name extraction in AdaptiveServerState --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: TheAngryByrd <[email protected]> Co-authored-by: Jimmy Byrd <[email protected]>
1 parent 30c97f2 commit 2c2e497

File tree

6 files changed

+695
-48
lines changed

6 files changed

+695
-48
lines changed

‎src/FsAutoComplete.Core/Commands.fs‎

Lines changed: 272 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,192 @@ module Commands =
9292
let fantomasLogger = LogProvider.getLoggerByName "Fantomas"
9393
let commandsLogger = LogProvider.getLoggerByName "Commands"
9494

95+
/// Extracts the case name(s) from an active pattern name string.
96+
/// For partial patterns like "|CaseName|_|" returns "CaseName"
97+
/// For full patterns like "|Even|Odd|" returns the first case name
98+
let extractActivePatternCaseName (name: string) : string =
99+
let parts = name.Split('|') |> Array.filter (fun s -> s <> "" && s <> "_")
100+
if parts.Length > 0 then parts.[0] else name
101+
102+
/// Extracts all case names from an active pattern display name.
103+
/// For "(|ParseInt|_|)" returns ["ParseInt"]
104+
/// For "(|Even|Odd|)" returns ["Even"; "Odd"]
105+
let extractActivePatternCaseNames (displayName: string) : string list =
106+
displayName.TrimStart('|', '(').TrimEnd('|', '_', ')').Split('|')
107+
|> Array.filter (fun s -> not (String.IsNullOrWhiteSpace(s)))
108+
|> Array.toList
109+
110+
/// Finds usages of partial active pattern cases by walking the AST.
111+
///
112+
/// Returns syntactic ranges from the AST where the specified case names appear in pattern matches.
113+
/// For qualified identifiers (e.g., `MyModule.ParseInt`), returns the full qualified range to match FCS behavior.
114+
/// Recursively traverses all pattern forms, including nested and composite patterns (tuples, lists, or/and/as patterns, etc.).
115+
///
116+
/// Note: Only syntactic (AST) ranges are returned; semantic information is not considered.
117+
let findPartialActivePatternCaseUsages (caseNames: string list) (parseResults: FSharpParseFileResults) : range list =
118+
let rec walkPat (pat: SynPat) =
119+
seq {
120+
match pat with
121+
| SynPat.LongIdent(longDotId = synLongIdent; argPats = args) ->
122+
// Check if this is a potential case usage
123+
match synLongIdent with
124+
| SynLongIdent(id = idents) ->
125+
match idents with
126+
| [] -> ()
127+
| [ singleIdent ] when List.contains singleIdent.idText caseNames ->
128+
// Single identifier that matches a case name
129+
yield singleIdent.idRange
130+
| multipleIdents ->
131+
// Qualified identifier (e.g., MyModule.ParseInt)
132+
let lastIdent = List.last multipleIdents
133+
134+
if List.contains lastIdent.idText caseNames then
135+
// Return the full qualified range to match what FCS returns
136+
yield synLongIdent.Range
137+
138+
// Recursively check arguments
139+
match args with
140+
| SynArgPats.Pats pats -> yield! List.collect (walkPat >> Seq.toList) pats
141+
| SynArgPats.NamePatPairs(pats = pairs; range = _) ->
142+
yield! List.collect (fun (NamePatPairField(pat = pat)) -> walkPat pat |> Seq.toList) pairs
143+
144+
| SynPat.Paren(pat, _) -> yield! walkPat pat
145+
| SynPat.Tuple(elementPats = pats) -> yield! List.collect (walkPat >> Seq.toList) pats
146+
| SynPat.ArrayOrList(_, pats, _) -> yield! List.collect (walkPat >> Seq.toList) pats
147+
| SynPat.Ands(pats, _) -> yield! List.collect (walkPat >> Seq.toList) pats
148+
| SynPat.Or(lhsPat = pat1; rhsPat = pat2) ->
149+
yield! walkPat pat1
150+
yield! walkPat pat2
151+
| SynPat.As(lpat, rpat, _) ->
152+
yield! walkPat lpat
153+
yield! walkPat rpat
154+
| SynPat.Typed(pat, _, _) -> yield! walkPat pat
155+
| SynPat.Attrib(pat, _, _) -> yield! walkPat pat
156+
| SynPat.ListCons(lpat, rpat, _, _) ->
157+
yield! walkPat lpat
158+
yield! walkPat rpat
159+
| _ -> ()
160+
}
161+
162+
let rec walkExpr (expr: SynExpr) =
163+
let walkMatchClauses clauses =
164+
seq {
165+
for clause in clauses do
166+
match clause with
167+
| SynMatchClause(pat = pat; resultExpr = resultExpr) ->
168+
yield! walkPat pat
169+
yield! walkExpr resultExpr
170+
}
171+
172+
seq {
173+
match expr with
174+
| SynExpr.Match(expr = matchExpr; clauses = clauses) ->
175+
yield! walkExpr matchExpr
176+
yield! walkMatchClauses clauses
177+
| SynExpr.MatchBang(expr = matchExpr; clauses = clauses) ->
178+
yield! walkExpr matchExpr
179+
yield! walkMatchClauses clauses
180+
| SynExpr.TryWith(tryExpr = tryExpr; withCases = clauses) ->
181+
yield! walkExpr tryExpr
182+
yield! walkMatchClauses clauses
183+
| SynExpr.MatchLambda(matchClauses = clauses) -> yield! walkMatchClauses clauses
184+
| SynExpr.Lambda(args = args; body = body) ->
185+
match args with
186+
| SynSimplePats.SimplePats(pats = pats) ->
187+
for spat in pats do
188+
match spat with
189+
| SynSimplePat.Typed(pat, _, _)
190+
| SynSimplePat.Attrib(pat, _, _) ->
191+
match pat with
192+
| SynSimplePat.Id(ident, _, _, _, _, _) when List.contains ident.idText caseNames -> yield ident.idRange
193+
| _ -> ()
194+
| SynSimplePat.Id(ident, _, _, _, _, _) when List.contains ident.idText caseNames -> yield ident.idRange
195+
| _ -> ()
196+
197+
yield! walkExpr body
198+
// Continue walking nested expressions
199+
| SynExpr.App(funcExpr = e1; argExpr = e2) ->
200+
yield! walkExpr e1
201+
yield! walkExpr e2
202+
| SynExpr.LetOrUse(bindings = bindings; body = body) ->
203+
// Walk the binding expressions
204+
for binding in bindings do
205+
match binding with
206+
| SynBinding(expr = expr) -> yield! walkExpr expr
207+
208+
// Walk the body
209+
yield! walkExpr body
210+
| SynExpr.Sequential(expr1 = e1; expr2 = e2) ->
211+
yield! walkExpr e1
212+
yield! walkExpr e2
213+
| SynExpr.IfThenElse(ifExpr = e1; thenExpr = e2; elseExpr = e3opt) ->
214+
yield! walkExpr e1
215+
yield! walkExpr e2
216+
217+
match e3opt with
218+
| Some e3 -> yield! walkExpr e3
219+
| None -> ()
220+
| SynExpr.Paren(expr, _, _, _) -> yield! walkExpr expr
221+
| SynExpr.Typed(expr = expr) -> yield! walkExpr expr
222+
// Computation expressions (seq, async, task, etc.)
223+
| SynExpr.ComputationExpr(expr = expr) -> yield! walkExpr expr
224+
// Array or list expressions
225+
| SynExpr.ArrayOrList(exprs = exprs) -> yield! List.collect (walkExpr >> Seq.toList) exprs
226+
| SynExpr.ArrayOrListComputed(expr = expr) -> yield! walkExpr expr
227+
// For loops
228+
| SynExpr.For(doBody = body) -> yield! walkExpr body
229+
| SynExpr.ForEach(enumExpr = enumExpr; bodyExpr = body) ->
230+
yield! walkExpr enumExpr
231+
yield! walkExpr body
232+
// While loop
233+
| SynExpr.While(whileExpr = whileExpr; doExpr = doExpr) ->
234+
yield! walkExpr whileExpr
235+
yield! walkExpr doExpr
236+
// Tuples
237+
| SynExpr.Tuple(exprs = exprs) -> yield! List.collect (walkExpr >> Seq.toList) exprs
238+
// Record expressions
239+
| SynExpr.Record(copyInfo = copyInfo; recordFields = fields) ->
240+
match copyInfo with
241+
| Some(expr, _) -> yield! walkExpr expr
242+
| None -> ()
243+
244+
for (SynExprRecordField(expr = exprOpt)) in fields do
245+
match exprOpt with
246+
| Some expr -> yield! walkExpr expr
247+
| None -> ()
248+
| _ -> ()
249+
}
250+
251+
let rec walkDecl (decl: SynModuleDecl) =
252+
seq {
253+
match decl with
254+
| SynModuleDecl.Let(bindings = bindings) ->
255+
for binding in bindings do
256+
match binding with
257+
| SynBinding(expr = expr) -> yield! walkExpr expr
258+
| SynModuleDecl.Expr(expr, _) -> yield! walkExpr expr
259+
| SynModuleDecl.NestedModule(decls = decls) -> yield! List.collect (walkDecl >> Seq.toList) decls
260+
| SynModuleDecl.Types(typeDefs, _) ->
261+
for typeDef in typeDefs do
262+
match typeDef with
263+
| SynTypeDefn(members = members) ->
264+
for memb in members do
265+
match memb with
266+
| SynMemberDefn.Member(SynBinding(expr = expr), _) -> yield! walkExpr expr
267+
| SynMemberDefn.LetBindings(bindings, _, _, _) ->
268+
for SynBinding(expr = expr) in bindings do
269+
yield! walkExpr expr
270+
| _ -> ()
271+
| _ -> ()
272+
}
273+
274+
match parseResults.ParseTree with
275+
| ParsedInput.ImplFile(ParsedImplFileInput(contents = modules)) ->
276+
modules
277+
|> List.collect (fun (SynModuleOrNamespace(decls = decls)) -> decls |> List.collect (walkDecl >> Seq.toList))
278+
|> List.distinctBy (fun r -> r.Start, r.End)
279+
| _ -> []
280+
95281
let addFile (fsprojPath: string) fileVirtPath =
96282
async {
97283
try
@@ -750,7 +936,25 @@ module Commands =
750936
asyncResult {
751937
let symbol = symbolUse.Symbol
752938

753-
let symbolNameCore = symbol.DisplayNameCore
939+
let symbolNameCore =
940+
match symbol with
941+
| :? FSharpActivePatternCase as apc ->
942+
// For active pattern cases, extract just the case name without bars
943+
// apc.Name may include bars like "|LetterOrDigit|_|" for partial patterns
944+
if apc.Name.StartsWith("|") then
945+
extractActivePatternCaseName apc.Name
946+
else
947+
apc.Name
948+
| :? FSharpMemberOrFunctionOrValue as mfv when mfv.IsActivePattern ->
949+
// For active pattern functions, extract just the case name for partial patterns
950+
// For full patterns like "(|Even|Odd|)", use DisplayNameCore as-is
951+
let displayName = symbol.DisplayNameCore
952+
953+
if displayName.Contains("|_|") then
954+
extractActivePatternCaseName displayName
955+
else
956+
displayName
957+
| _ -> symbol.DisplayNameCore
754958

755959
let tryAdjustRanges (text: IFSACSourceText, ranges: seq<Range>) =
756960
let ranges = ranges |> Seq.map (fun range -> range.NormalizeDriveLetterCasing())
@@ -779,13 +983,73 @@ module Commands =
779983
let! ct = Async.CancellationToken
780984
let symbolUses = tyRes.GetCheckResults.GetUsesOfSymbolInFile(symbol, ct)
781985

782-
let symbolUses: _ seq =
783-
if includeDeclarations then
784-
symbolUses
785-
else
786-
symbolUses |> Seq.filter (fun u -> not u.IsFromDefinition)
787-
788-
let ranges = symbolUses |> Seq.map (fun u -> u.Range)
986+
let (symbolUses: _ seq), additionalRangesForPartialPatterns =
987+
let baseFiltered: _ seq =
988+
if includeDeclarations then
989+
symbolUses
990+
else
991+
symbolUses |> Seq.filter (fun u -> not u.IsFromDefinition)
992+
993+
// For Active Pattern Cases, FCS returns all cases in the pattern, not just the specific one
994+
// We need to filter to only the symbol that matches our query
995+
// BUT: if querying from the Active Pattern declaration itself (FSharpMemberOrFunctionOrValue),
996+
// we also want to find the case usages
997+
match symbolUse.Symbol with
998+
| :? FSharpActivePatternCase as apc ->
999+
// Querying from a specific case - filter to just that case
1000+
let filtered =
1001+
baseFiltered
1002+
|> Seq.filter (fun u ->
1003+
match u.Symbol with
1004+
| :? FSharpActivePatternCase as foundApc -> foundApc.Name = apc.Name
1005+
| _ -> false)
1006+
1007+
// For partial active patterns in .fsx files, FCS may not return case usages
1008+
// We walk the AST and deduplicate with what FCS found
1009+
let isPartialPattern = apc.Group.IsTotal |> not
1010+
1011+
let caseUsageRanges =
1012+
if isPartialPattern then
1013+
findPartialActivePatternCaseUsages [ apc.Name ] tyRes.GetParseResults
1014+
else
1015+
// Complete patterns - FCS handles these correctly
1016+
[]
1017+
1018+
filtered, caseUsageRanges
1019+
| :? FSharpMemberOrFunctionOrValue as mfv when
1020+
mfv.IsActivePattern
1021+
|| (mfv.DisplayName.StartsWith("(|") && mfv.DisplayName.EndsWith("|)"))
1022+
->
1023+
// Querying from the active pattern function declaration
1024+
// For partial active patterns like (|ParseInt|_|), include case usages in match expressions
1025+
// For complete active patterns like (|Even|Odd|), only return the function declaration
1026+
// Note: IsActivePattern is true for direct definitions, but let-bound values need DisplayName check
1027+
1028+
let patternDisplayName = mfv.DisplayName
1029+
// DisplayName includes parens: "(|ParseInt|_|)" so we need to check for "|_|)" not just "|_|"
1030+
let isPartialActivePattern = patternDisplayName.Contains("|_|")
1031+
1032+
if not isPartialActivePattern then
1033+
// For complete patterns, only return the pattern function declaration
1034+
baseFiltered, []
1035+
else
1036+
// For partial patterns, find case usages by walking the AST
1037+
let caseNames = extractActivePatternCaseNames patternDisplayName
1038+
1039+
let caseUsageRanges =
1040+
findPartialActivePatternCaseUsages caseNames tyRes.GetParseResults
1041+
1042+
// For partial patterns, FCS doesn't include case usages in pattern matches
1043+
// We found them by walking the AST, so we return them as additional ranges
1044+
baseFiltered, caseUsageRanges
1045+
| _ -> baseFiltered, []
1046+
1047+
let ranges =
1048+
let baseRanges = symbolUses |> Seq.map (fun u -> u.Range)
1049+
// Add any additional ranges we found from walking the AST for partial active patterns
1050+
// Deduplicate based on range start and end positions
1051+
Seq.append baseRanges (Seq.ofList additionalRangesForPartialPatterns)
1052+
|> Seq.distinctBy (fun r -> r.Start, r.End)
7891053
// Note: tryAdjustRanges is designed to only be able to fail iff `errorOnFailureToFixRange` is `true`
7901054
let! ranges = tryAdjustRanges (text, ranges)
7911055
let ranges = dict [ (text.FileName, Seq.toArray ranges) ]

0 commit comments

Comments
 (0)