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 7 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
3 changes: 2 additions & 1 deletion docs/release-notes/FSharp.Compiler.Service/8.0.200.md
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
- Miscellaneous fixes to parens analysis - https://github.com/dotnet/fsharp/pull/16262
- Miscellaneous fixes to parens analysis - https://github.com/dotnet/fsharp/pull/16262
- Raise a new error when interfaces with auto properties are implemented on constructor-less types - https://github.com/dotnet/fsharp/pull/16352
21 changes: 18 additions & 3 deletions src/Compiler/Checking/CheckDeclarations.fs
Original file line number Diff line number Diff line change
Expand Up @@ -4224,9 +4224,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 Expand Up @@ -4345,6 +4342,16 @@ module TcDeclarations =
match trepr with
| SynTypeDefnRepr.ObjectModel(kind, members, m) ->
let members = desugarGetSetMembers members
let implementedAutoProperties =
members
|> List.choose (
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
fun mem ->
match mem with
| SynMemberDefn.Interface (members= Some defs) ->
let autoProps = defs |> List.filter(isAutoProperty)
Some autoProps
| _ -> None)
|> List.concat

CheckMembersForm members

Expand Down Expand Up @@ -4386,6 +4393,14 @@ module TcDeclarations =
members |> List.tryPick (function
| SynMemberDefn.ImplicitCtor (ctorArgs = SynSimplePats.SimplePats _ as spats) -> Some spats
| _ -> None)

// Raise a new error if we try to have an interface implementation with auto properties on constructor-less types,
if not implementedAutoProperties.IsEmpty then
for mem in implementedAutoProperties do
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
match mem with
| SynMemberDefn.AutoProperty(range = m) when implicitCtorSynPats.IsNone ->
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
errorR(Error(FSComp.SR.tcAutoPropertyRequiresImplicitConstructionSequence(), m))
| _ -> ()

// An ugly bit of code to pre-determine if a type has a nullary constructor, prior to establishing the
// members of the type
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 @@ -91,4 +91,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,47 @@ 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
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved