Don't show completions on nested module identifier (dotnet#13089)

* Don't show completions on nested module identifier * Recover from incomplete module declaration * Blah * Fixes * Fix parser * Fix parser * Format * Add AST tests * Revert LexFilter changes * Readd LexFilter * Update baselines * Format * Update baselines --------- Co-authored-by: Tomas Grosup <[email protected]>
kant2002 · Apr 1, 2023 · f6c864c · f6c864c
1 parent ffc6dd5
commit f6c864c
Show file tree

Hide file tree

Showing 12 changed files with 152 additions and 19 deletions.
diff --git a/src/Compiler/Service/ServiceParsedInputOps.fs b/src/Compiler/Service/ServiceParsedInputOps.fs
@@ -1474,6 +1474,12 @@ module ParsedInput =
                             Some(CompletionContext.OpenDeclaration isOpenType)
                         else
                             None
+
+                    // module Namespace.Top
+                    // module Neste|
+                    | SynModuleDecl.NestedModule(moduleInfo = SynComponentInfo(longId = [ ident ])) when rangeContainsPos ident.idRange pos ->
+                        Some CompletionContext.Invalid
+
                     | _ -> defaultTraverse decl
 
                 member _.VisitType(_, defaultTraverse, ty) =

diff --git a/src/Compiler/SyntaxTree/LexFilter.fs b/src/Compiler/SyntaxTree/LexFilter.fs
@@ -1562,7 +1562,7 @@ type LexFilterImpl (
         //  Otherwise it's a 'head' module declaration, so ignore it
 
         //  Here prevToken is either 'module', 'rec', 'global' (invalid), '.', or ident, because we skip attribute tokens and access modifier tokens
-        | _, CtxtModuleHead (moduleTokenPos, prevToken, lexingModuleAttributes) :: _ ->
+        | _, CtxtModuleHead (moduleTokenPos, prevToken, lexingModuleAttributes) :: rest ->
             match prevToken, token with
             | _, GREATER_RBRACK when lexingModuleAttributes = LexingModuleAttributes
                                      && moduleTokenPos.Column < tokenStartPos.Column ->
@@ -1587,19 +1587,28 @@ type LexFilterImpl (
                 pushCtxt tokenTup (CtxtModuleBody (moduleTokenPos, false))
                 pushCtxtSeqBlock(true, AddBlockEnd)
                 returnToken tokenLexbufState token
-            | _ -> 
-                if debug then dprintf "CtxtModuleHead: start of file, CtxtSeqBlock\n"
-                popCtxt()
-                // Don't push a new context if next token is EOF, since that raises an offside warning
-                match tokenTup.Token with 
-                | EOF _ -> 
-                    returnToken tokenLexbufState token
+            | _ ->
+                match rest with
+                | [ CtxtSeqBlock _ ] ->
+                    if debug then dprintf "CtxtModuleHead: start of file, CtxtSeqBlock\n"
+                    popCtxt()
+                    // Don't push a new context if next token is EOF, since that raises an offside warning
+                    match tokenTup.Token with 
+                    | EOF _ -> 
+                        returnToken tokenLexbufState token
+                    | _ ->
+                        // We have reached other tokens without encountering '=' or ':', so this is a module declaration spanning the whole file
+                        delayToken tokenTup 
+                        pushCtxt tokenTup (CtxtModuleBody (moduleTokenPos, true))
+                        pushCtxtSeqBlockAt (tokenTup, true, AddBlockEnd) 
+                        hwTokenFetch false
                 | _ ->
-                    // We have reached other tokens without encountering '=' or ':', so this is a module declaration spanning the whole file
-                    delayToken tokenTup 
-                    pushCtxt tokenTup (CtxtModuleBody (moduleTokenPos, true))
-                    pushCtxtSeqBlockAt (tokenTup, true, AddBlockEnd) 
-                    hwTokenFetch false
+                    // Adding a new nested module, EQUALS hasn't been typed yet
+                    // and we've encountered declarations below
+                    if debug then dprintf "CtxtModuleHead: not start of file, popping CtxtModuleHead\n"
+                    popCtxt()
+                    reprocessWithoutBlockRule()
+
         //  Offside rule for SeqBlock.  
         //      f x
         //      g x

diff --git a/src/Compiler/pars.fsy b/src/Compiler/pars.fsy
@@ -665,6 +665,17 @@ moduleSpfn:
         let trivia: SynModuleSigDeclNestedModuleTrivia = { ModuleKeyword = Some mModule; EqualsRange = $4 }
         SynModuleSigDecl.NestedModule(info, isRec, $5, m, trivia) }
 
+  | opt_attributes opt_access moduleIntro error
+      { let mModule, isRec, path, vis, attribs2 = $3
+        let xmlDoc = grabXmlDoc(parseState, $1, 1)
+        if not (isSingleton path) then raiseParseErrorAt (rhs parseState 3) (FSComp.SR.parsModuleDefnMustBeSimpleName())
+        if isRec then raiseParseErrorAt (rhs parseState 3) (FSComp.SR.parsInvalidUseOfRec())
+        let info = SynComponentInfo($1 @ attribs2, None, [], path, xmlDoc, false, vis, rhs parseState 3)
+        if Option.isSome $2 then errorR(Error(FSComp.SR.parsVisibilityDeclarationsShouldComePriorToIdentifier(), rhs parseState 2))
+        let mWhole = rhs2 parseState 1 3 |> unionRangeWithXmlDoc xmlDoc
+        let trivia: SynModuleSigDeclNestedModuleTrivia = { ModuleKeyword = Some mModule; EqualsRange = None }
+        SynModuleSigDecl.NestedModule(info, isRec, [], mWhole, trivia) }
+
   | opt_attributes opt_access typeKeyword tyconSpfn tyconSpfnList
       { if Option.isSome $2 then errorR (Error (FSComp.SR.parsVisibilityDeclarationsShouldComePriorToIdentifier (), rhs parseState 2))
         let leadingKeyword = SynTypeDefnLeadingKeyword.Type (rhs parseState 3)
@@ -1285,6 +1296,15 @@ moduleDefn:
             let m = match mEndOpt with | None -> m | Some mEnd -> unionRanges m mEnd
             [ SynModuleDecl.NestedModule(info, isRec, def, false, m, trivia) ] }
 
+  /* incomplete 'module' definitions */
+  | opt_attributes opt_access moduleIntro error
+      { let xmlDoc = grabXmlDoc(parseState, $1, 1)
+        let mWhole = rhs2 parseState 1 3 |> unionRangeWithXmlDoc xmlDoc
+        let attribs, (mModule, isRec, path, vis, attribs2) = $1, $3
+        let info = SynComponentInfo(attribs @ attribs2, None, [], path, xmlDoc, false, vis, rhs parseState 3)
+        let trivia: SynModuleDeclNestedModuleTrivia = { ModuleKeyword = Some mModule; EqualsRange = None }
+        [ SynModuleDecl.NestedModule(info, isRec, [], false, mWhole, trivia) ] }
+
   /* unattached custom attributes */
   | attributes recover
       { errorR(Error(FSComp.SR.parsAttributeOnIncompleteCode(), rhs parseState 1))

diff --git a/tests/fsharp/typecheck/sigs/neg41.bsl b/tests/fsharp/typecheck/sigs/neg41.bsl
@@ -1,4 +1,2 @@
 
-neg41.fs(3,1,3,5): parse error FS0010: Unexpected start of structured construct in definition. Expected '=' or other token.
-
-neg41.fs(4,1,4,1): parse error FS0010: Incomplete structured construct at or before this point in implementation file
+neg41.fs(3,1,3,5): parse error FS0010: Unexpected keyword 'type' in definition. Expected '=' or other token.
diff --git a/tests/fsharp/typecheck/sigs/neg42.bsl b/tests/fsharp/typecheck/sigs/neg42.bsl
@@ -1,4 +1,2 @@
 
-neg42.fsi(3,1,3,5): parse error FS0010: Unexpected start of structured construct in signature file. Expected ':', '=' or other token.
-
-neg42.fsi(4,1,4,1): parse error FS0010: Incomplete structured construct at or before this point in signature file
+neg42.fsi(3,1,3,5): parse error FS0010: Unexpected keyword 'type' in signature file. Expected ':', '=' or other token.
diff --git a/tests/service/data/SyntaxTree/ModuleOrNamespace/EmptyModuleOrNamespaceShouldBePresent.fs b/tests/service/data/SyntaxTree/ModuleOrNamespace/EmptyModuleOrNamespaceShouldBePresent.fs
@@ -0,0 +1 @@
+module A.B.C
diff --git a/tests/service/data/SyntaxTree/ModuleOrNamespace/EmptyModuleOrNamespaceShouldBePresent.fs.bsl b/tests/service/data/SyntaxTree/ModuleOrNamespace/EmptyModuleOrNamespaceShouldBePresent.fs.bsl
@@ -0,0 +1,10 @@
+ImplFile
+  (ParsedImplFileInput
+     ("/root/ModuleOrNamespace/EmptyModuleOrNamespaceShouldBePresent.fs", false,
+      QualifiedNameOfFile A.B.C, [], [],
+      [SynModuleOrNamespace
+         ([A; B; C], false, NamedModule, [],
+          PreXmlDoc ((1,0), FSharp.Compiler.Xml.XmlDocCollector), [], None,
+          (1,0--1,12), { LeadingKeyword = Module (1,0--1,6) })], (true, false),
+      { ConditionalDirectives = []
+        CodeComments = [] }, set []))
diff --git a/tests/service/data/SyntaxTree/NestedModule/IncompleteNestedModuleShouldBePresent.fs b/tests/service/data/SyntaxTree/NestedModule/IncompleteNestedModuleShouldBePresent.fs
@@ -0,0 +1,5 @@
+module A.B
+
+module C
+
+let a = ()
diff --git a/tests/service/data/SyntaxTree/NestedModule/IncompleteNestedModuleShouldBePresent.fs.bsl b/tests/service/data/SyntaxTree/NestedModule/IncompleteNestedModuleShouldBePresent.fs.bsl
@@ -0,0 +1,29 @@
+ImplFile
+  (ParsedImplFileInput
+     ("/root/NestedModule/IncompleteNestedModuleShouldBePresent.fs", false,
+      QualifiedNameOfFile A.B, [], [],
+      [SynModuleOrNamespace
+         ([A; B], false, NamedModule,
+          [NestedModule
+             (SynComponentInfo
+                ([], None, [], [C],
+                 PreXmlDoc ((3,0), FSharp.Compiler.Xml.XmlDocCollector), false,
+                 None, (3,0--3,8)), false, [], false, (3,0--3,8),
+              { ModuleKeyword = Some (3,0--3,6)
+                EqualsRange = None });
+           Let
+             (false,
+              [SynBinding
+                 (None, Normal, false, false, [],
+                  PreXmlDoc ((5,0), FSharp.Compiler.Xml.XmlDocCollector),
+                  SynValData
+                    (None, SynValInfo ([], SynArgInfo ([], false, None)), None),
+                  Named (SynIdent (a, None), false, None, (5,4--5,5)), None,
+                  Const (Unit, (5,8--5,10)), (5,4--5,5), Yes (5,0--5,10),
+                  { LeadingKeyword = Let (5,0--5,3)
+                    InlineKeyword = None
+                    EqualsRange = Some (5,6--5,7) })], (5,0--5,10))],
+          PreXmlDoc ((1,0), FSharp.Compiler.Xml.XmlDocCollector), [], None,
+          (1,0--5,10), { LeadingKeyword = Module (1,0--1,6) })], (true, false),
+      { ConditionalDirectives = []
+        CodeComments = [] }, set []))
diff --git a/tests/service/data/SyntaxTree/NestedModule/IncompleteNestedModuleSigShouldBePresent.fsi b/tests/service/data/SyntaxTree/NestedModule/IncompleteNestedModuleSigShouldBePresent.fsi
@@ -0,0 +1,5 @@
+module A.B
+
+module C
+
+val a: unit
diff --git a/tests/service/data/SyntaxTree/NestedModule/IncompleteNestedModuleSigShouldBePresent.fsi.bsl b/tests/service/data/SyntaxTree/NestedModule/IncompleteNestedModuleSigShouldBePresent.fsi.bsl
@@ -0,0 +1,27 @@
+SigFile
+  (ParsedSigFileInput
+     ("/root/NestedModule/IncompleteNestedModuleSigShouldBePresent.fsi",
+      QualifiedNameOfFile A.B, [], [],
+      [SynModuleOrNamespaceSig
+         ([A; B], false, NamedModule,
+          [NestedModule
+             (SynComponentInfo
+                ([], None, [], [C],
+                 PreXmlDoc ((3,0), FSharp.Compiler.Xml.XmlDocCollector), false,
+                 None, (3,0--3,8)), false, [], (3,0--3,8),
+              { ModuleKeyword = Some (3,0--3,6)
+                EqualsRange = None });
+           Val
+             (SynValSig
+                ([], SynIdent (a, None), SynValTyparDecls (None, true),
+                 LongIdent (SynLongIdent ([unit], [], [None])),
+                 SynValInfo ([], SynArgInfo ([], false, None)), false, false,
+                 PreXmlDoc ((5,0), FSharp.Compiler.Xml.XmlDocCollector), None,
+                 None, (5,0--5,11), { LeadingKeyword = Val (5,0--5,3)
+                                      InlineKeyword = None
+                                      WithKeyword = None
+                                      EqualsRange = None }), (5,0--5,11))],
+          PreXmlDoc ((1,0), FSharp.Compiler.Xml.XmlDocCollector), [], None,
+          (1,0--5,11), { LeadingKeyword = Module (1,0--1,6) })],
+      { ConditionalDirectives = []
+        CodeComments = [] }, set []))
diff --git a/vsintegration/tests/FSharp.Editor.Tests/CompletionProviderTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CompletionProviderTests.fs
@@ -736,6 +736,31 @@ open type System.Ma
         let expected = [ "Management"; "Math" ] // both namespace and static type
         VerifyCompletionList(fileContents, "System.Ma", expected, [])
 
+    [<Fact>]
+    let ``No completion on nested module identifier, incomplete`` () =
+        let fileContents =
+            """
+    module Namespace.Top
+
+    module Nest
+
+    let a = ()
+    """
+
+        VerifyNoCompletionList(fileContents, "Nest")
+
+    [<Fact>]
+    let ``No completion on nested module identifier`` () =
+        let fileContents =
+            """
+    namespace N
+
+    module Nested =
+        do ()
+    """
+
+        VerifyNoCompletionList(fileContents, "Nested")
+
     [<Fact>]
     let ``No completion on type name at declaration site`` () =
         let fileContents =