Skip to content

Commit 8b0900c

Browse files
ijklamKevinRansom
ijklam
and
KevinRansom
authored
Disallow abstract member with access modifiers in sig file (#17802)
* Disallow abstract member with access modifiers in sig file * release note * format * fix * fix test * use access modifier range to show error * update tests * update tests * show both FS0531 and FS0561 --------- Co-authored-by: ijklam <43789618+Tangent-90@users.noreply.github.com> Co-authored-by: Kevin Ransom (msft) <codecutter@hotmail.com>
1 parent ee8af8a commit 8b0900c

File tree

7 files changed

+82
-28
lines changed

7 files changed

+82
-28
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
* 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))
44
* Fix internal error when calling 'AddSingleton' and other overloads only differing in generic arity ([PR #17804](https://github.com/dotnet/fsharp/pull/17804))
55
* Fix extension methods support for non-reference system assemblies ([PR #17799](https://github.com/dotnet/fsharp/pull/17799))
6-
* Ensure `frameworkTcImportsCache` mutations are thread-safe. ([PR #17795](https://github.com/dotnet/fsharp/pull/17795))
6+
* Ensure `frameworkTcImportsCache` mutations are threadsafe. ([PR #17795](https://github.com/dotnet/fsharp/pull/17795))
7+
* Disallow abstract member with access modifiers in sig file. ([PR #17802](https://github.com/dotnet/fsharp/pull/17802))
78
* Fix concurrency issue in `ILPreTypeDefImpl` ([PR #17812](https://github.com/dotnet/fsharp/pull/17812))
89
* Fix nullness inference for member val and other OO scenarios ([PR #17845](https://github.com/dotnet/fsharp/pull/17845))
910
* Fix internal error when analyzing incomplete inherit member ([PR #17905](https://github.com/dotnet/fsharp/pull/17905))

‎src/Compiler/Service/FSharpSource.fsi

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ type internal FSharpSource =
2626
abstract TimeStamp: DateTime
2727

2828
/// Gets the internal text container. Text may be on-disk, in a stream, or a source text.
29-
abstract internal GetTextContainer: unit -> Async<TextContainer>
29+
abstract GetTextContainer: unit -> Async<TextContainer>
3030

3131
/// Creates a FSharpSource from disk. Only used internally.
3232
static member internal CreateFromFile: filePath: string -> FSharpSource

‎src/Compiler/SyntaxTree/ParseHelpers.fs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1229,3 +1229,11 @@ let mkValField
12291229
mkSynField parseState idOpt typ isMutable access attribs mStaticOpt rangeStart (Some leadingKeyword)
12301230

12311231
SynMemberDefn.ValField(field, field.Range)
1232+
1233+
let leadingKeywordIsAbstract =
1234+
function
1235+
| SynLeadingKeyword.Abstract _
1236+
| SynLeadingKeyword.AbstractMember _
1237+
| SynLeadingKeyword.StaticAbstract _
1238+
| SynLeadingKeyword.StaticAbstractMember _ -> true
1239+
| _ -> false

‎src/Compiler/SyntaxTree/ParseHelpers.fsi

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,3 +302,5 @@ val mkSynField:
302302
rangeStart: range ->
303303
leadingKeyword: SynLeadingKeyword option ->
304304
SynField
305+
306+
val leadingKeywordIsAbstract: SynLeadingKeyword -> bool

‎src/Compiler/pars.fsy

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -985,8 +985,12 @@ classMemberSpfn:
985985
match optLiteralValue with
986986
| None -> m
987987
| Some e -> unionRanges m e.Range
988-
988+
989989
let flags, leadingKeyword = $3
990+
if leadingKeywordIsAbstract leadingKeyword then
991+
[ $2; $5; getterAccess; setterAccess ]
992+
|> List.iter (function None -> () | Some access -> errorR(Error(FSComp.SR.parsAccessibilityModsIllegalForAbstract(), access.Range)))
993+
990994
let flags = flags (getSetAdjuster arity)
991995
let trivia = { LeadingKeyword = leadingKeyword; InlineKeyword = $4; WithKeyword = mWith; EqualsRange = mEquals }
992996
let valSpfn = SynValSig($1, id, explicitValTyparDecls, ty, arity, isInline, false, doc, vis2, optLiteralValue, mWhole, trivia)
@@ -2046,21 +2050,24 @@ classDefnMember:
20462050
let ty = SynType.FromParseError(mInterface.EndRange)
20472051
[ SynMemberDefn.Interface(ty, None, None, rhs2 parseState 1 3) ] }
20482052

2049-
| opt_attributes opt_access abstractMemberFlags opt_inline nameop opt_explicitValTyparDecls COLON topTypeWithTypeConstraints classMemberSpfnGetSet opt_ODECLEND
2050-
{ let ty, arity = $8
2051-
let isInline, doc, id, explicitValTyparDecls = (Option.isSome $4), grabXmlDoc(parseState, $1, 1), $5, $6
2052-
let mWith, (getSet, getSetRangeOpt, getterAccess, setterAccess) = $9
2053+
| opt_attributes opt_access abstractMemberFlags opt_access opt_inline nameop opt_explicitValTyparDecls COLON topTypeWithTypeConstraints classMemberSpfnGetSet opt_ODECLEND
2054+
{ if Option.isSome $2 then errorR(Error(FSComp.SR.parsVisibilityDeclarationsShouldComePriorToIdentifier(), rhs parseState 2))
2055+
let ty, arity = $9
2056+
let isInline, doc, id, explicitValTyparDecls = (Option.isSome $5), grabXmlDoc(parseState, $1, 1), $6, $7
2057+
let mWith, (getSet, getSetRangeOpt, getterAccess, setterAccess) = $10
20532058
let getSetAdjuster arity = match arity, getSet with SynValInfo([], _), SynMemberKind.Member -> SynMemberKind.PropertyGet | _ -> getSet
20542059
let mWhole =
20552060
let m = rhs parseState 1
20562061
match getSetRangeOpt with
20572062
| None -> unionRanges m ty.Range
20582063
| Some gs -> unionRanges m gs.Range
20592064
|> unionRangeWithXmlDoc doc
2060-
if Option.isSome $2 || Option.isSome getterAccess || Option.isSome setterAccess
2061-
then errorR(Error(FSComp.SR.parsAccessibilityModsIllegalForAbstract(), mWhole))
2065+
2066+
[ $2; $4; getterAccess; setterAccess ]
2067+
|> List.iter (function None -> () | Some access -> errorR(Error(FSComp.SR.parsAccessibilityModsIllegalForAbstract(), access.Range)))
2068+
20622069
let mkFlags, leadingKeyword = $3
2063-
let trivia = { LeadingKeyword = leadingKeyword; InlineKeyword = $4; WithKeyword = mWith; EqualsRange = None }
2070+
let trivia = { LeadingKeyword = leadingKeyword; InlineKeyword = $5; WithKeyword = mWith; EqualsRange = None }
20642071
let vis2 = SynValSigAccess.Single(None)
20652072
let valSpfn = SynValSig($1, id, explicitValTyparDecls, ty, arity, isInline, false, doc, vis2, None, mWhole, trivia)
20662073
let trivia: SynMemberDefnAbstractSlotTrivia = { GetSetKeywords = getSetRangeOpt }

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

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,15 @@ module AccessibilityAnnotations_PermittedLocations =
2727
|> verifyCompile
2828
|> shouldFail
2929
|> withDiagnostics [
30-
(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.")
31-
(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.")
32-
(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.")
33-
(Error 10, Line 21, Col 14, Line 21, Col 20, "Unexpected keyword 'public' in member definition. Expected identifier, '(', '(*)' or other token.")
34-
(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.")
35-
(Error 10, Line 23, Col 14, Line 23, Col 22, "Unexpected keyword 'internal' in member definition. Expected identifier, '(', '(*)' or other token.")
30+
(Error 531, Line 18, Col 5, Line 18, Col 11, "Accessibility modifiers should come immediately prior to the identifier naming a construct")
31+
(Error 561, Line 18, Col 5, Line 18, Col 11, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
32+
(Error 531, Line 19, Col 5, Line 19, Col 12, "Accessibility modifiers should come immediately prior to the identifier naming a construct")
33+
(Error 561, Line 19, Col 5, Line 19, Col 12, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
34+
(Error 531, Line 20, Col 5, Line 20, Col 13, "Accessibility modifiers should come immediately prior to the identifier naming a construct")
35+
(Error 561, Line 20, Col 5, Line 20, Col 13, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
36+
(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.")
37+
(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.")
38+
(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.")
3639
]
3740

3841
// SOURCE=E_accessibilityOnInterface01.fs SCFLAGS="--test:ErrorRanges" # E_accessibilityOnInterface01.fs
@@ -42,7 +45,8 @@ module AccessibilityAnnotations_PermittedLocations =
4245
|> verifyCompile
4346
|> shouldFail
4447
|> withDiagnostics [
45-
(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.")
48+
(Error 531, Line 13, Col 5, Line 13, Col 11, "Accessibility modifiers should come immediately prior to the identifier naming a construct")
49+
(Error 561, Line 13, Col 5, Line 13, Col 11, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
4650
]
4751

4852
// SOURCE=E_accessibilityOnInterface02.fs SCFLAGS="--test:ErrorRanges" # E_accessibilityOnInterface02.fs
@@ -52,7 +56,8 @@ module AccessibilityAnnotations_PermittedLocations =
5256
|> verifyCompile
5357
|> shouldFail
5458
|> withDiagnostics [
55-
(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.")
59+
(Error 531, Line 15, Col 5, Line 15, Col 12, "Accessibility modifiers should come immediately prior to the identifier naming a construct")
60+
(Error 561, Line 15, Col 5, Line 15, Col 12, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
5661
]
5762

5863
// SOURCE=E_accessibilityOnInterface03.fs SCFLAGS="--test:ErrorRanges" # E_accessibilityOnInterface03.fs
@@ -62,7 +67,8 @@ module AccessibilityAnnotations_PermittedLocations =
6267
|> verifyCompile
6368
|> shouldFail
6469
|> withDiagnostics [
65-
(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.")
70+
(Error 531, Line 15, Col 5, Line 15, Col 13, "Accessibility modifiers should come immediately prior to the identifier naming a construct")
71+
(Error 561, Line 15, Col 5, Line 15, Col 13, "Accessibility modifiers are not allowed on this member. Abstract slots always have the same visibility as the enclosing type.")
6672
]
6773

6874
// SOURCE=E_accessibilityOnInterface04.fs SCFLAGS="--test:ErrorRanges" # E_accessibilityOnInterface04.fs
@@ -72,7 +78,7 @@ module AccessibilityAnnotations_PermittedLocations =
7278
|> verifyCompile
7379
|> shouldFail
7480
|> withDiagnostics [
75-
(Error 10, Line 15, Col 14, Line 15, Col 20, "Unexpected keyword 'public' in member definition. Expected identifier, '(', '(*)' or other token.")
81+
(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.")
7682
]
7783

7884
// SOURCE=E_accessibilityOnInterface05.fs SCFLAGS="--test:ErrorRanges" # E_accessibilityOnInterface05.fs
@@ -82,7 +88,7 @@ module AccessibilityAnnotations_PermittedLocations =
8288
|> verifyCompile
8389
|> shouldFail
8490
|> withDiagnostics [
85-
(Error 10, Line 15, Col 14, Line 15, Col 21, "Unexpected keyword 'private' in member definition. Expected identifier, '(', '(*)' or other token.")
91+
(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.")
8692
]
8793

8894
// SOURCE=E_accessibilityOnInterface06.fs SCFLAGS="--test:ErrorRanges" # E_accessibilityOnInterface06.fs
@@ -92,7 +98,7 @@ module AccessibilityAnnotations_PermittedLocations =
9298
|> verifyCompile
9399
|> shouldFail
94100
|> withDiagnostics [
95-
(Error 10, Line 15, Col 14, Line 15, Col 22, "Unexpected keyword 'internal' in member definition. Expected identifier, '(', '(*)' or other token.")
101+
(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.")
96102
]
97103

98104
// SOURCE=E_accessibilityOnRecords.fs SCFLAGS="--test:ErrorRanges" # E_accessibilityOnRecords.fs
@@ -160,3 +166,32 @@ module AccessibilityAnnotations_PermittedLocations =
160166
|> withDiagnostics [
161167
(Error 531, Line 8, Col 13, Line 8, Col 20, "Accessibility modifiers should come immediately prior to the identifier naming a construct")
162168
]
169+
170+
[<Fact>]
171+
let ``Signature File Test: abstract member cannot have access modifiers`` () =
172+
Fsi """module Program
173+
174+
type A =
175+
abstract internal B: int ->int
176+
abstract member internal E: int ->int
177+
abstract member C: int with internal get, private set
178+
abstract internal D: int with get, set
179+
static abstract internal B2: int ->int
180+
static abstract member internal E2: int ->int
181+
static abstract member C2: int with internal get, private set
182+
static abstract internal D2: int with get, set"""
183+
|> withOptions ["--nowarn:3535"]
184+
|> verifyCompile
185+
|> shouldFail
186+
|> withDiagnostics [
187+
(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.")
188+
(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.")
189+
(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.")
190+
(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.")
191+
(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.")
192+
(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.")
193+
(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.")
194+
(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.")
195+
(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.")
196+
(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.")
197+
]

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,12 @@ let ``Abstract Properties Test: access modifiers are not allowed`` () =
110110
|> typecheck
111111
|> shouldFail
112112
|> withDiagnostics [
113-
(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.")
114-
(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.")
115-
(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.")
116-
(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.")
117-
(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.")
113+
(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.")
114+
(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.")
115+
(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.")
116+
(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.")
117+
(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.")
118+
(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.")
118119
]
119120

120121
[<Fact>]
@@ -132,7 +133,7 @@ type A =
132133
|> verifyCompile
133134
|> shouldFail
134135
|> withDiagnostics [
135-
(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.")
136+
(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.")
136137
]
137138

138139
[<Fact>]

0 commit comments

Comments
 (0)