Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raise an error if we try to have interfaces with auto properties on constructor-less types #16352

Merged
merged 19 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
3346982
Raise a new error if we try to have auto properties on constructor-le…
edgarfgp Nov 28, 2023
7bfc575
Merge branch 'main' into internal-error-constructor-less-types
edgarfgp Nov 28, 2023
52926e2
Update release notes
edgarfgp Nov 28, 2023
d74427e
Merge branch 'main' into internal-error-constructor-less-types
edgarfgp Nov 28, 2023
78f2266
improve record test
edgarfgp Nov 29, 2023
5772d8a
Merge branch 'internal-error-constructor-less-types' of github.com:ed…
edgarfgp Nov 29, 2023
44129c5
Merge branch 'main' into internal-error-constructor-less-types
edgarfgp Nov 29, 2023
b81a7d4
Update src/Compiler/Checking/CheckDeclarations.fs
edgarfgp Nov 30, 2023
b288781
Update src/Compiler/Checking/CheckDeclarations.fs
edgarfgp Nov 30, 2023
8cc8e12
Update tests/FSharp.Compiler.ComponentTests/ErrorMessages/ClassesTest…
edgarfgp Nov 30, 2023
f6e4c3c
Fix indentation
edgarfgp Nov 30, 2023
011f004
Merge branch 'main' into internal-error-constructor-less-types
edgarfgp Nov 30, 2023
da882d3
Merge branch 'main' into internal-error-constructor-less-types
edgarfgp Dec 1, 2023
428da91
Merge branch 'main' into internal-error-constructor-less-types
edgarfgp Dec 4, 2023
605470c
Merge branch 'main' into internal-error-constructor-less-types
edgarfgp Dec 5, 2023
48a9851
Merge branch 'main' into internal-error-constructor-less-types
edgarfgp Dec 7, 2023
25d0610
Fix PR comments
edgarfgp Dec 7, 2023
9198d7c
use for loop
edgarfgp Dec 7, 2023
aac480d
Merge branch 'main' into internal-error-constructor-less-types
edgarfgp Dec 7, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/release-notes/FSharp.Compiler.Service/8.0.200.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
- Parens: Keep parentheses around non-struct tuples in `&` patterns - https://github.com/dotnet/fsharp/pull/16391
- Parens analysis: fix some parenthesization corner-cases in record expressions - https://github.com/dotnet/fsharp/pull/16370
- Fixes #16359 - correctly handle imports with 0 length public key tokens - https://github.com/dotnet/fsharp/pull/16363
- Raise a new error when interfaces with auto properties are implemented on constructor-less types - https://github.com/dotnet/fsharp/pull/16352
14 changes: 11 additions & 3 deletions src/Compiler/Checking/CheckDeclarations.fs
Original file line number Diff line number Diff line change
Expand Up @@ -4223,12 +4223,23 @@ module TcDeclarations =
| _ -> ()
| ds ->
// Check for duplicated parameters in abstract methods
// Check for an interface implementation with auto properties on constructor-less types
for slot in ds do
if isAbstractSlot slot then
match slot with
| SynMemberDefn.AbstractSlot (slotSig = synVal; range = m) ->
CheckDuplicatesArgNames synVal m
| _ -> ()

if isInterface slot then
match slot with
| SynMemberDefn.Interface (members= Some defs) ->
for au in defs do
match au with
| SynMemberDefn.AutoProperty(isStatic = false; range = m) ->
errorR(Error(FSComp.SR.tcAutoPropertyRequiresImplicitConstructionSequence(), m))
| _ -> ()
| _ -> ()

// Classic class construction
let _, ds = List.takeUntil (allFalse [isMember;isAbstractSlot;isInterface;isInherit;isField;isTycon]) ds
Expand All @@ -4253,9 +4264,6 @@ module TcDeclarations =
| SynMemberDefn.LetBindings (range=m) :: _ -> errorR(Error(FSComp.SR.tcTypeDefinitionsWithImplicitConstructionMustHaveLocalBindingsBeforeMembers(), m))
| _ -> ()




/// Split auto-properties into 'let' and 'member' bindings
let private SplitAutoProps members =
let membersIncludingAutoProps, vals_Inherits_Abstractslots =
Expand Down
21 changes: 20 additions & 1 deletion tests/FSharp.Compiler.ComponentTests/Diagnostics/Records.fs
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,23 @@ let t3 (x: RecTy) (a: AnotherNestedRecTy) = { x with D.C.c = { a with A = 3; B =
|> withDiagnostics [
(Warning 3560, Line 9, Col 26, Line 9, Col 65, "This copy-and-update record expression changes all fields of record type 'Test.NestdRecTy'. Consider using the record construction syntax instead.")
(Warning 3560, Line 15, Col 62, Line 15, Col 85, "This copy-and-update record expression changes all fields of record type 'Test.AnotherNestedRecTy'. Consider using the record construction syntax instead.")
]
]

[<Fact>]
let ``Error when implementing interface with auto property in record type``() =
FSharp """
type Foo =
abstract member X : string with get, set
abstract GetValue: unit -> string

type FooImpl =
{ name: string }
interface Foo with
member val X = "" with get, set
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
member x.GetValue() = x.name
"""
|> withLangVersion80
|> asExe
|> compile
|> shouldFail
|> withSingleDiagnostic (Error 912, Line 9, Col 5, Line 9, Col 36, "This declaration element is not permitted in an augmentation")
Original file line number Diff line number Diff line change
Expand Up @@ -704,4 +704,82 @@ type X =
abstract member Bar<'T> : string -> unit
"""
|> typecheck
|> shouldSucceed

[<Fact>]
let ``Error if we try to have auto properties on constructor-less types`` () =
Fsx """
type Foo =
abstract member X : string with get, set
abstract member Y : string with get, set
abstract member Z : string with get, set

type FooImpl =
interface Foo with
member val X = "" with get, set
member val Y = "" with get, set
member this.Z
with get() = ""
and set(value) = ()
"""
|> asExe
|> compile
|> shouldFail
|> withDiagnostics [
(Error 3133, Line 9, Col 9, Line 9, Col 40, "'member val' definitions are only permitted in types with a primary constructor. Consider adding arguments to your type definition, e.g. 'type X(args) = ...'.");
(Error 3133, Line 10, Col 9, Line 10, Col 40, "'member val' definitions are only permitted in types with a primary constructor. Consider adding arguments to your type definition, e.g. 'type X(args) = ...'.")
]

[<Fact>]
let ``No error if we try to have auto properties on types with primary constructor`` () =
Fsx """
type Foo =
abstract member X : string with get, set
abstract member Y : string with get, set
abstract member Z : string with get, set

type FooImpl() =
interface Foo with
member val X = "" with get, set
member val Y = "" with get, set
member this.Z
with get() = ""
and set(value) = ()
"""
|> typecheck
|> shouldSucceed

[<Fact>]
let ``No error if we try to have auto properties on types with primary constructor with args`` () =
Fsx """
type Foo =
abstract member X : string with get, set
abstract member Y : string with get, set
abstract member Z : string with get, set

type FooImpl(x) =
interface Foo with
member val X = "" with get, set
member val Y = "" with get, set
member this.Z
with get() = ""
and set(value) = ()
"""
|> typecheck
|> shouldSucceed

[<Fact>]
let ``No error if we try to have static autoprop on a type without constructor`` () =
Fsx """
#nowarn "3535" //We accept that static abstracts are an advanced feature
[<Interface>]
type Foo =
static abstract member X : string with get, set

[<Class>]
type FooImpl =
interface Foo with
static member val X = "" with get, set
"""
|> typecheck
|> shouldSucceed
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved