SyntaxTraversal.Traverse → ParsedInput.tryPick (etc.) (#1241)

* Sample ParsedInput API usage * A few more * A few more * Fantomas * Update in place * Cleanup * Use right pos * Unused `path` → `_path` * No need to do our own looping
ionide · Mar 3, 2024 · 4343ccc · 4343ccc
1 parent a5ad3c0
commit 4343ccc
Show file tree

Hide file tree

Showing 26 changed files with 622 additions and 1,420 deletions.
diff --git a/build/ScaffoldCodeFix.fs b/build/ScaffoldCodeFix.fs
@@ -38,48 +38,13 @@ let mkCodeFixImplementation codeFixName =
 
 open FSharp.Compiler.Symbols
 open FSharp.Compiler.Syntax
+open FSharp.Compiler.Text
 open FsToolkit.ErrorHandling
 open Ionide.LanguageServerProtocol.Types
 open FsAutoComplete.CodeFix.Types
 open FsAutoComplete
 open FsAutoComplete.LspHelpers
 
-// The syntax tree can be an intimidating set of types to work with.
-// It is a tree structure but it consists out of many different types.
-// See https://fsharp.github.io/fsharp-compiler-docs/reference/fsharp-compiler-syntax.html
-// It can be useful to inspect a syntax tree via a code sample using https://fsprojects.github.io/fantomas-tools/#/ast
-// For example `let a b c = ()` in
-// https://fsprojects.github.io/fantomas-tools/#/ast?data=N4KABGBEAmCmBmBLAdrAzpAXFSAacUiaAYmolmPAIYA2as%%2BEkAxgPZwWQ2wAuYVYAEZhmYALxgAFAEo8BSLAAeAByrJoFHgCcArrBABfIA
-// Let's say we want to find the (FCS) range for identifier `a`.
-let visitSyntaxTree
-  (cursor: FSharp.Compiler.Text.pos)
-  (tree: ParsedInput)
-  =
-  // We will use a syntax visitor to traverse the tree from the top to the node of interest.
-  // See https://github.com/dotnet/fsharp/blob/main/src/Compiler/Service/ServiceParseTreeWalk.fsi
-  // We implement the different members of interest and allow the default traversal to move to the lower levels we care about.
-  let visitor =
-    // A visitor will report the first item it finds.
-    // Think of it as `List.tryPick`
-    // It is not an ideal solution to find all nodes inside a tree, be aware of that.
-    // For example finding all function names.
-    {{ new SyntaxVisitorBase<FSharp.Compiler.Text.range>() with
-        // We know that `a` will be part of a `SynPat.LongIdent`
-        // This was visible in the online tool.
-        member _.VisitPat(path, defaultTraverse, synPat) =
-          match synPat with
-          | SynPat.LongIdent(longDotId = SynLongIdent(id = [ functionNameIdent ])) ->
-            // When our code fix operates on the user's code there is no way of knowing what will be inside the syntax tree.
-            // So we need to be careful and verify that the pattern is indeed matching the position of the cursor.
-            if FSharp.Compiler.Text.Range.rangeContainsPos functionNameIdent.idRange cursor then
-              Some functionNameIdent.idRange
-            else
-              None
-          | _ -> None }}
-
-  // Invoke the visitor and kick off the traversal.
-  SyntaxTraversal.Traverse(cursor, tree, visitor)
-
 // TODO: add proper title for code fix
 let title = "%s{codeFixName} Codefix"
 
@@ -98,9 +63,29 @@ let fix
       let! (parseAndCheckResults:ParseAndCheckResults, line:string, sourceText:IFSACSourceText) =
         getParseResultsForFile fileName fcsPos
 
-      // As an example, we want to check whether the users cursor is inside a function definition name.
-      // We will traverse the syntax tree to verify this is the case.
-      match visitSyntaxTree fcsPos parseAndCheckResults.GetParseResults.ParseTree with
+      // The syntax tree can be an intimidating set of types to work with.
+      // It is a tree structure but it consists out of many different types.
+      // See https://fsharp.github.io/fsharp-compiler-docs/reference/fsharp-compiler-syntax.html
+      // It can be useful to inspect a syntax tree via a code sample using https://fsprojects.github.io/fantomas-tools/#/ast
+      // For example `let a b c = ()` in
+      // https://fsprojects.github.io/fantomas-tools/#/ast?data=N4KABGBEAmCmBmBLAdrAzpAXFSAacUiaAYmolmPAIYA2as%%2BEkAxgPZwWQ2wAuYVYAEZhmYALxgAFAEo8BSLAAeAByrJoFHgCcArrBABfIA
+      // Let's say we want to find the (FCS) range for identifier `a` if the user's cursor is inside the function name.
+      // We will query the syntax tree to verify this is the case.
+      let maybeFunctionNameRange =
+        (fcsPos, parseAndCheckResults.GetParseResults.ParseTree)
+        ||> ParsedInput.tryPick (fun _path node ->
+          match node with
+          // We know that `a` will be part of a `SynPat.LongIdent`
+          // This was visible in the online tool.
+          | SyntaxNode.SynPat(SynPat.LongIdent(longDotId = SynLongIdent(id = [ functionNameIdent ]))) when
+            // When our code fix operates on the user's code there is no way of knowing what will be inside the syntax tree.
+            // So we need to be careful and verify that the pattern is indeed matching the position of the cursor.
+            Range.rangeContainsPos functionNameIdent.idRange fcsPos
+            ->
+            Some functionNameIdent.idRange
+          | _ -> None)
+
+      match maybeFunctionNameRange with
       | None ->
         // The cursor is not in a position we are interested in.
         // This code fix should not trigger any suggestions so we return an empty list.

diff --git a/src/FsAutoComplete.Core/AbstractClassStubGenerator.fs b/src/FsAutoComplete.Core/AbstractClassStubGenerator.fs
@@ -27,72 +27,6 @@ type AbstractClassData =
     | ObjExpr(baseTy = t)
     | ExplicitImpl(baseTy = t) -> expandTypeParameters t
 
-let private (|ExplicitCtor|_|) =
-  function
-  | SynMemberDefn.Member(
-      memberDefn = SynBinding(valData = SynValData(memberFlags = Some({ MemberKind = SynMemberKind.Constructor })))) ->
-    Some()
-  | _ -> None
-
-/// checks to see if a type definition inherits an abstract class, and if so collects the members defined at that
-let private walkTypeDefn (SynTypeDefn(_, repr, members, implicitCtor, _, _)) =
-  option {
-    let reprMembers =
-      match repr with
-      | SynTypeDefnRepr.ObjectModel(_, members, _) -> members // repr members already includes the implicit ctor if present
-      | _ -> Option.toList implicitCtor
-
-    let allMembers = reprMembers @ members
-
-    let! inheritType, inheritMemberRange = // this must exist for abstract types
-      allMembers
-      |> List.tryPick (function
-        | SynMemberDefn.ImplicitInherit(inheritType, _, _, range) -> Some(inheritType, range)
-        | _ -> None)
-
-    let furthestMemberToSkip, otherMembers =
-      ((inheritMemberRange, []), allMembers)
-      // find the last of the following kinds of members, as object-programming members must come after these
-      // * implicit/explicit constructors
-      // * `inherit` expressions
-      // * class-level `do`/`let` bindings (`do` bindings are actually `LetBindings` in the AST)
-      ||> List.fold (fun (m, otherMembers) memb ->
-        match memb with
-        | (SynMemberDefn.ImplicitCtor _ | ExplicitCtor | SynMemberDefn.ImplicitInherit _ | SynMemberDefn.LetBindings _) as possible ->
-          let c = Range.rangeOrder.Compare(m, possible.Range)
-
-          let m' = if c < 0 then possible.Range else m
-
-          m', otherMembers
-        | otherMember -> m, otherMember :: otherMembers)
-
-    let otherMembersInDeclarationOrder = otherMembers |> List.rev
-    return AbstractClassData.ExplicitImpl(inheritType, otherMembersInDeclarationOrder, furthestMemberToSkip)
-
-  }
-
-/// find the declaration of the abstract class being filled in at the given position
-let private tryFindAbstractClassExprInParsedInput
-  (pos: Position)
-  (parsedInput: ParsedInput)
-  : AbstractClassData option =
-  SyntaxTraversal.Traverse(
-    pos,
-    parsedInput,
-    { new SyntaxVisitorBase<_>() with
-        member _.VisitExpr(path, traverseExpr, defaultTraverse, expr) =
-          match expr with
-          | SynExpr.ObjExpr(
-              objType = baseTy; withKeyword = withKeyword; bindings = bindings; newExprRange = newExprRange) ->
-            Some(AbstractClassData.ObjExpr(baseTy, bindings, newExprRange, withKeyword))
-          | _ -> defaultTraverse expr
-
-        override _.VisitModuleDecl(_, defaultTraverse, decl) =
-          match decl with
-          | SynModuleDecl.Types(types, _) -> List.tryPick walkTypeDefn types
-          | _ -> defaultTraverse decl }
-  )
-
 /// Walk the parse tree for the given document and look for the definition of any abstract classes in use at the given pos.
 /// This looks for implementations of abstract types in object expressions, as well as inheriting of abstract types inside class type declarations.
 let tryFindAbstractClassExprInBufferAtPos
@@ -102,7 +36,56 @@ let tryFindAbstractClassExprInBufferAtPos
   =
   asyncOption {
     let! parseResults = codeGenService.ParseFileInProject document.FileName
-    return! tryFindAbstractClassExprInParsedInput pos parseResults.ParseTree
+
+    return!
+      (pos, parseResults.ParseTree)
+      ||> ParsedInput.tryPick (fun _path node ->
+        match node with
+        | SyntaxNode.SynExpr(SynExpr.ObjExpr(
+            objType = baseTy; withKeyword = withKeyword; bindings = bindings; newExprRange = newExprRange)) ->
+          Some(ObjExpr(baseTy, bindings, newExprRange, withKeyword))
+
+        | SyntaxNode.SynTypeDefn(SynTypeDefn(_, repr, members, implicitCtor, _, _)) ->
+          option {
+            let reprMembers =
+              match repr with
+              | SynTypeDefnRepr.ObjectModel(_, members, _) -> members // repr members already includes the implicit ctor if present
+              | _ -> Option.toList implicitCtor
+
+            let allMembers = reprMembers @ members
+
+            let! inheritType, inheritMemberRange = // this must exist for abstract types
+              allMembers
+              |> List.tryPick (function
+                | SynMemberDefn.ImplicitInherit(inheritType, _, _, range) -> Some(inheritType, range)
+                | _ -> None)
+
+            let furthestMemberToSkip, otherMembers =
+              ((inheritMemberRange, []), allMembers)
+              // find the last of the following kinds of members, as object-programming members must come after these
+              // * implicit/explicit constructors
+              // * `inherit` expressions
+              // * class-level `do`/`let` bindings (`do` bindings are actually `LetBindings` in the AST)
+              ||> List.fold (fun (m, otherMembers) memb ->
+                match memb with
+                | SynMemberDefn.ImplicitCtor _
+                | SynMemberDefn.Member(
+                  memberDefn = SynBinding(
+                    valData = SynValData(memberFlags = Some { MemberKind = SynMemberKind.Constructor })))
+                | SynMemberDefn.ImplicitInherit _
+                | SynMemberDefn.LetBindings _ as possible ->
+                  let c = Range.rangeOrder.Compare(m, possible.Range)
+
+                  let m' = if c < 0 then possible.Range else m
+
+                  m', otherMembers
+                | otherMember -> m, otherMember :: otherMembers)
+
+            let otherMembersInDeclarationOrder = otherMembers |> List.rev
+            return ExplicitImpl(inheritType, otherMembersInDeclarationOrder, furthestMemberToSkip)
+          }
+
+        | _ -> None)
   }
 
 let getMemberNameAndRanges abstractClassData =