Disallow abstract member with access modifiers in sig file

docs/release-notes/.FSharp.Compiler.Service/9.0.200.md

@@ -3,7 +3,8 @@
 * Fix false negatives for passing null to "obj" arguments. Only "obj | null" can now subsume any type ([PR #17757](https://github.com/dotnet/fsharp/pull/17757))
 * Fix internal error when calling 'AddSingleton' and other overloads only differing in generic arity ([PR #17804](https://github.com/dotnet/fsharp/pull/17804))
 * Fix extension methods support for non-reference system assemblies ([PR #17799](https://github.com/dotnet/fsharp/pull/17799))
-* Ensure `frameworkTcImportsCache` mutations are thread-safe. ([PR #17795](https://github.com/dotnet/fsharp/pull/17795))
+* Ensure `frameworkTcImportsCache` mutations are threadsafe. ([PR #17795](https://github.com/dotnet/fsharp/pull/17795))
+* Disallow abstract member with access modifiers in sig file. ([PR #17802](https://github.com/dotnet/fsharp/pull/17802))
 * Fix concurrency issue in `ILPreTypeDefImpl`  ([PR #17812](https://github.com/dotnet/fsharp/pull/17812))
 
 ### Added

src/Compiler/Service/FSharpSource.fsi

@@ -26,7 +26,7 @@ type internal FSharpSource =
     abstract TimeStamp: DateTime
 
     /// Gets the internal text container. Text may be on-disk, in a stream, or a source text.
-    abstract internal GetTextContainer: unit -> Async<TextContainer>
+    abstract GetTextContainer: unit -> Async<TextContainer>
 
     /// Creates a FSharpSource from disk. Only used internally.
     static member internal CreateFromFile: filePath: string -> FSharpSource

src/Compiler/SyntaxTree/ParseHelpers.fs

@@ -1229,3 +1229,11 @@ let mkValField
         mkSynField parseState idOpt typ isMutable access attribs mStaticOpt rangeStart (Some leadingKeyword)
 
     SynMemberDefn.ValField(field, field.Range)
+
+let leadingKeywordIsAbstract =
+    function
+    | SynLeadingKeyword.Abstract _
+    | SynLeadingKeyword.AbstractMember _
+    | SynLeadingKeyword.StaticAbstract _
+    | SynLeadingKeyword.StaticAbstractMember _ -> true
+    | _ -> false

src/Compiler/SyntaxTree/ParseHelpers.fsi

@@ -302,3 +302,5 @@ val mkSynField:
     rangeStart: range ->
     leadingKeyword: SynLeadingKeyword option ->
         SynField
+
+val leadingKeywordIsAbstract: SynLeadingKeyword -> bool

src/Compiler/pars.fsy

@@ -985,8 +985,12 @@ classMemberSpfn:
                match optLiteralValue with
                | None -> m
                | Some e -> unionRanges m e.Range
-
+        
        let flags, leadingKeyword = $3
+       if leadingKeywordIsAbstract leadingKeyword then 
+        [ $5; getterAccess; setterAccess ]
+        |> List.iter (function None -> () | Some access -> errorR(Error(FSComp.SR.parsAccessibilityModsIllegalForAbstract(), access.Range)))
+
        let flags = flags (getSetAdjuster arity)
        let trivia = { LeadingKeyword = leadingKeyword; InlineKeyword = $4; WithKeyword = mWith; EqualsRange = mEquals }
        let valSpfn = SynValSig($1, id, explicitValTyparDecls, ty, arity, isInline, false, doc, vis2, optLiteralValue, mWhole, trivia)
@@ -2046,21 +2050,24 @@ classDefnMember:
        let ty = SynType.FromParseError(mInterface.EndRange)
        [ SynMemberDefn.Interface(ty, None, None, rhs2 parseState 1 3) ] }
 
-  | opt_attributes opt_access abstractMemberFlags opt_inline nameop opt_explicitValTyparDecls COLON topTypeWithTypeConstraints classMemberSpfnGetSet opt_ODECLEND
-     { let ty, arity = $8
-       let isInline, doc, id, explicitValTyparDecls = (Option.isSome $4), grabXmlDoc(parseState, $1, 1), $5, $6
-       let mWith, (getSet, getSetRangeOpt, getterAccess, setterAccess) = $9
+  | opt_attributes opt_access abstractMemberFlags opt_access opt_inline nameop opt_explicitValTyparDecls COLON topTypeWithTypeConstraints classMemberSpfnGetSet opt_ODECLEND
+     { if Option.isSome $2 then errorR(Error(FSComp.SR.parsVisibilityDeclarationsShouldComePriorToIdentifier(), rhs parseState 2))
+       let ty, arity = $9
+       let isInline, doc, id, explicitValTyparDecls = (Option.isSome $5), grabXmlDoc(parseState, $1, 1), $6, $7
+       let mWith, (getSet, getSetRangeOpt, getterAccess, setterAccess) = $10
        let getSetAdjuster arity = match arity, getSet with SynValInfo([], _), SynMemberKind.Member -> SynMemberKind.PropertyGet | _ -> getSet
        let mWhole =
            let m = rhs parseState 1
            match getSetRangeOpt with
            | None -> unionRanges m ty.Range
            | Some gs -> unionRanges m gs.Range
            |> unionRangeWithXmlDoc doc
-       if Option.isSome $2 || Option.isSome getterAccess || Option.isSome setterAccess
-       then errorR(Error(FSComp.SR.parsAccessibilityModsIllegalForAbstract(), mWhole))
+
+       [ $4; getterAccess; setterAccess ]
+       |> List.iter (function None -> () | Some access -> errorR(Error(FSComp.SR.parsAccessibilityModsIllegalForAbstract(), access.Range)))
+
        let mkFlags, leadingKeyword = $3
-       let trivia = { LeadingKeyword = leadingKeyword; InlineKeyword = $4; WithKeyword = mWith; EqualsRange = None }
+       let trivia = { LeadingKeyword = leadingKeyword; InlineKeyword = $5; WithKeyword = mWith; EqualsRange = None }
        let vis2 = SynValSigAccess.Single(None)
        let valSpfn = SynValSig($1, id, explicitValTyparDecls, ty, arity, isInline, false, doc, vis2, None, mWhole, trivia)
        let trivia: SynMemberDefnAbstractSlotTrivia = { GetSetKeywords = getSetRangeOpt }

tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/AccessibilityAnnotations/PermittedLocations/PermittedLocations.fs

@@ -27,12 +27,12 @@ module AccessibilityAnnotations_PermittedLocations =
         |> verifyCompile
         |> shouldFail
         |> withDiagnostics [
-            (Error 561, Line 18, Col 5, Line 18, Col 62, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
-            (Error 561, Line 19, Col 5, Line 19, Col 62, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
-            (Error 561, Line 20, Col 5, Line 20, Col 62, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
-            (Error 10, Line 21, Col 14, Line 21, Col 20, "Unexpected keyword 'public' in member definition. Expected identifier, '(', '(*)' or other token.")
-            (Error 561, Line 21, Col 14, Line 22, Col 62, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
-            (Error 10, Line 23, Col 14, Line 23, Col 22, "Unexpected keyword 'internal' in member definition. Expected identifier, '(', '(*)' or other token.")
+            (Error 531, Line 18, Col 5, Line 18, Col 11, "Accessibility modifiers should come immediately prior to the identifier naming a construct")
+            (Error 531, Line 19, Col 5, Line 19, Col 12, "Accessibility modifiers should come immediately prior to the identifier naming a construct")
+            (Error 531, Line 20, Col 5, Line 20, Col 13, "Accessibility modifiers should come immediately prior to the identifier naming a construct")
+            (Error 561, Line 21, Col 14, Line 21, Col 20, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+            (Error 561, Line 22, Col 14, Line 22, Col 21, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+            (Error 561, Line 23, Col 14, Line 23, Col 22, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
         ]
 
     // SOURCE=E_accessibilityOnInterface01.fs              SCFLAGS="--test:ErrorRanges"         # E_accessibilityOnInterface01.fs
@@ -42,7 +42,7 @@ module AccessibilityAnnotations_PermittedLocations =
         |> verifyCompile
         |> shouldFail
         |> withDiagnostics [
-            (Error 561, Line 13, Col 5, Line 13, Col 67, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+            (Error 531, Line 13, Col 5, Line 13, Col 11, "Accessibility modifiers should come immediately prior to the identifier naming a construct")
         ]
 
     // SOURCE=E_accessibilityOnInterface02.fs              SCFLAGS="--test:ErrorRanges"         # E_accessibilityOnInterface02.fs
@@ -52,7 +52,7 @@ module AccessibilityAnnotations_PermittedLocations =
         |> verifyCompile
         |> shouldFail
         |> withDiagnostics [
-            (Error 561, Line 15, Col 5, Line 15, Col 68, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+            (Error 531, Line 15, Col 5, Line 15, Col 12, "Accessibility modifiers should come immediately prior to the identifier naming a construct")
         ]
 
     // SOURCE=E_accessibilityOnInterface03.fs              SCFLAGS="--test:ErrorRanges"         # E_accessibilityOnInterface03.fs
@@ -62,7 +62,7 @@ module AccessibilityAnnotations_PermittedLocations =
         |> verifyCompile
         |> shouldFail
         |> withDiagnostics [
-            (Error 561, Line 15, Col 5, Line 15, Col 69, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+            (Error 531, Line 15, Col 5, Line 15, Col 13, "Accessibility modifiers should come immediately prior to the identifier naming a construct")
         ]
 
     // SOURCE=E_accessibilityOnInterface04.fs              SCFLAGS="--test:ErrorRanges"         # E_accessibilityOnInterface04.fs
@@ -72,7 +72,7 @@ module AccessibilityAnnotations_PermittedLocations =
         |> verifyCompile
         |> shouldFail
         |> withDiagnostics [
-            (Error 10, Line 15, Col 14, Line 15, Col 20, "Unexpected keyword 'public' in member definition. Expected identifier, '(', '(*)' or other token.")
+            (Error 561, Line 15, Col 14, Line 15, Col 20, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
         ]
 
     // SOURCE=E_accessibilityOnInterface05.fs              SCFLAGS="--test:ErrorRanges"         # E_accessibilityOnInterface05.fs
@@ -82,7 +82,7 @@ module AccessibilityAnnotations_PermittedLocations =
         |> verifyCompile
         |> shouldFail
         |> withDiagnostics [
-            (Error 10, Line 15, Col 14, Line 15, Col 21, "Unexpected keyword 'private' in member definition. Expected identifier, '(', '(*)' or other token.")
+            (Error 561, Line 15, Col 14, Line 15, Col 21, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
         ]
 
     // SOURCE=E_accessibilityOnInterface06.fs              SCFLAGS="--test:ErrorRanges"         # E_accessibilityOnInterface06.fs
@@ -92,7 +92,7 @@ module AccessibilityAnnotations_PermittedLocations =
         |> verifyCompile
         |> shouldFail
         |> withDiagnostics [
-            (Error 10, Line 15, Col 14, Line 15, Col 22, "Unexpected keyword 'internal' in member definition. Expected identifier, '(', '(*)' or other token.")
+            (Error 561, Line 15, Col 14, Line 15, Col 22, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
         ]
 
     // SOURCE=E_accessibilityOnRecords.fs                  SCFLAGS="--test:ErrorRanges"         # E_accessibilityOnRecords.fs
@@ -160,3 +160,32 @@ module AccessibilityAnnotations_PermittedLocations =
         |> withDiagnostics [
             (Error 531, Line 8, Col 13, Line 8, Col 20, "Accessibility modifiers should come immediately prior to the identifier naming a construct")
         ]
+
+    [<Fact>]
+    let ``Signature File Test: abstract member cannot have access modifiers`` () =
+        Fsi """module Program
+
+type A =
+    abstract internal B: int ->int
+    abstract member internal E: int ->int
+    abstract member C: int with internal get, private set
+    abstract internal D: int with get, set
+    static abstract internal B2: int ->int
+    static abstract member internal E2: int ->int
+    static abstract member C2: int with internal get, private set
+    static abstract internal D2: int with get, set""" 
+        |> withOptions ["--nowarn:3535"]
+        |> verifyCompile
+        |> shouldFail
+        |> withDiagnostics [
+            (Error 0561, Line 4, Col 14, Line 4, Col 22, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+            (Error 0561, Line 5, Col 21, Line 5, Col 29, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+            (Error 0561, Line 6, Col 33, Line 6, Col 41, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+            (Error 0561, Line 6, Col 47, Line 6, Col 54, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+            (Error 0561, Line 7, Col 14, Line 7, Col 22, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+            (Error 0561, Line 8, Col 21, Line 8, Col 29, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+            (Error 0561, Line 9, Col 28, Line 9, Col 36, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+            (Error 0561, Line 10, Col 41, Line 10, Col 49, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+            (Error 0561, Line 10, Col 55, Line 10, Col 62, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+            (Error 0561, Line 11, Col 21, Line 11, Col 29, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+        ]

tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/MemberDefinitions/MethodsAndProperties/AutoPropsWithModifierBeforeGetSet.fs

@@ -110,11 +110,12 @@ let ``Abstract Properties Test: access modifiers are not allowed`` () =
     |> typecheck
     |> shouldFail
     |> withDiagnostics [
-        (Error 0561, Line 6, Col 5, Line 6, Col 51, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
-        (Error 0561, Line 8, Col 5, Line 8, Col 51, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
-        (Error 0561, Line 10, Col 5, Line 10, Col 60, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
-        (Error 0561, Line 12, Col 5, Line 12, Col 46, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
-        (Error 0561, Line 14, Col 5, Line 14, Col 46, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+        (Error 0561, Line 6, Col 34, Line 6, Col 42, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+        (Error 0561, Line 8, Col 39, Line 8, Col 47, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+        (Error 0561, Line 10, Col 34, Line 10, Col 42, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+        (Error 0561, Line 10, Col 48, Line 10, Col 56, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+        (Error 0561, Line 12, Col 34, Line 12, Col 42, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
+        (Error 0561, Line 14, Col 34, Line 14, Col 42, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
     ]
 
 [<Fact>]
@@ -132,7 +133,7 @@ type A =
     |> verifyCompile
     |> shouldFail
     |> withDiagnostics [
-        (Error 240, Line 1, Col 1, Line 9, Col 42, "The signature file 'Program' does not have a corresponding implementation file. If an implementation file exists then check the 'module' and 'namespace' declarations in the signature and implementation files match.")
+        (Error 0561, Line 9, Col 31, Line 9, Col 38, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
     ]
 
 [<Fact>]