From e52520d9e965e79bdc37f69b340406eb390a58d9 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Sun, 20 Aug 2023 14:34:51 +0200 Subject: [PATCH 1/5] Improve error reporting on anonymous record declarations --- src/Compiler/Checking/CheckExpressions.fs | 17 +++-- .../Types/RecordTypes/AnonymousRecords.fs | 70 +++++++++++++++++++ 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index c7b4b6da3e2..b4679719b77 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -4511,20 +4511,25 @@ and TcTupleType kindOpt (cenv: cenv) newOk checkConstraints occ env tpenv isStru else let argsR,tpenv = TcTypesAsTuple cenv newOk checkConstraints occ env tpenv args m TType_tuple(tupInfo, argsR), tpenv + +and CheckAnonRecdTypeDuplicateFields (ident: _ -> Ident) elems = + elems |> List.iteri (fun i uc1 -> + elems |> List.iteri (fun j uc2 -> + let id1 = (ident uc1) + let id2 = (ident uc2) + if j > i && id1.idText = id2.idText then + errorR(Error(FSComp.SR.tcAnonRecdTypeDuplicateFieldId(id1.idText), id1.idRange)))) + elems and TcAnonRecdType (cenv: cenv) newOk checkConstraints occ env tpenv isStruct args m = let tupInfo = mkTupInfo isStruct + let idents = args |> List.choose(fun (lid, _) -> Some lid) + CheckAnonRecdTypeDuplicateFields id idents |> ignore let tup = args |> List.map (fun (_, t) -> SynTupleTypeSegment.Type t) let argsR,tpenv = TcTypesAsTuple cenv newOk checkConstraints occ env tpenv tup m let unsortedFieldIds = args |> List.map fst |> List.toArray let anonInfo = AnonRecdTypeInfo.Create(cenv.thisCcu, tupInfo, unsortedFieldIds) - // Check for duplicate field IDs - unsortedFieldIds - |> Array.countBy (fun fieldId -> fieldId.idText) - |> Array.iter (fun (idText, count) -> - if count > 1 then error (Error (FSComp.SR.tcAnonRecdTypeDuplicateFieldId(idText), m))) - // Sort into canonical order let sortedFieldTys, sortedCheckedArgTys = List.zip args argsR |> List.indexed |> List.sortBy (fun (i,_) -> unsortedFieldIds[i].idText) |> List.map snd |> List.unzip diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/AnonymousRecords.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/AnonymousRecords.fs index b5074a29007..ba7c8c099e6 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/AnonymousRecords.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/AnonymousRecords.fs @@ -221,3 +221,73 @@ let x = {| abcd = {| ab = 4; cd = 1 |} |} """ |> compile |> shouldSucceed + + [] + let ``Anonymous Records field appears multiple times in this anonymous record definition`` () = + Fsx """ +let x(f: {| A: int; A: int |}) = () + """ + |> compile + |> shouldFail + |> withDiagnostics [ + (Error 3523, Line 2, Col 13, Line 2, Col 14, "The field 'A' appears multiple times in this anonymous record type.") + ] + + [] + let ``Anonymous Records field appears multiple times in this anonymous record definition 2`` () = + Fsx """ +let x(f: {| A: int; A: int; A:int |}) = () + """ + |> compile + |> shouldFail + |> withDiagnostics [ + (Error 3523, Line 2, Col 13, Line 2, Col 14, "The field 'A' appears multiple times in this anonymous record type.") + (Error 3523, Line 2, Col 21, Line 2, Col 22, "The field 'A' appears multiple times in this anonymous record type.") + ] + + [] + let ``Anonymous Records field appears multiple times in this anonymous record declaration 3`` () = + Fsx """ +let f(x:{| A: int; B: int; A:string; B: int |}) = () + """ + |> compile + |> shouldFail + |> withDiagnostics [ + (Error 3523, Line 2, Col 12, Line 2, Col 13, "The field 'A' appears multiple times in this anonymous record type.") + (Error 3523, Line 2, Col 20, Line 2, Col 21, "The field 'B' appears multiple times in this anonymous record type.") + ] + + [] + let ``Anonymous Records field appears multiple times in this anonymous record declaration 4`` () = + Fsx """ +let f(x:{| A: int; C: string; A: int; B: int |}) = () + """ + |> compile + |> shouldFail + |> withDiagnostics [ + (Error 3523, Line 2, Col 12, Line 2, Col 13, "The field 'A' appears multiple times in this anonymous record type.") + ] + + [] + let ``Anonymous Records field appears multiple times in this anonymous record declaration 5`` () = + Fsx """ +let f(x:{| A: int; C: string; A: int; B: int; A: int |}) = () + """ + |> compile + |> shouldFail + |> withDiagnostics [ + (Error 3523, Line 2, Col 12, Line 2, Col 13, "The field 'A' appears multiple times in this anonymous record type.") + (Error 3523, Line 2, Col 31, Line 2, Col 32, "The field 'A' appears multiple times in this anonymous record type.") + ] + + [] + let ``Anonymous Records field with in double backticks appears multiple times in this anonymous record declaration`` () = + Fsx """ +let f(x:{| ``A``: int; B: int; A:string; B: int |}) = () + """ + |> compile + |> shouldFail + |> withDiagnostics [ + (Error 3523, Line 2, Col 12, Line 2, Col 17, "The field 'A' appears multiple times in this anonymous record type.") + (Error 3523, Line 2, Col 24, Line 2, Col 25, "The field 'B' appears multiple times in this anonymous record type.") + ] From f0d2702b893244c7d01cee91abf432687d269aaa Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Sun, 20 Aug 2023 14:41:48 +0200 Subject: [PATCH 2/5] one more test --- .../Types/RecordTypes/AnonymousRecords.fs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/AnonymousRecords.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/AnonymousRecords.fs index ba7c8c099e6..32588c31f47 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/AnonymousRecords.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/RecordTypes/AnonymousRecords.fs @@ -291,3 +291,15 @@ let f(x:{| ``A``: int; B: int; A:string; B: int |}) = () (Error 3523, Line 2, Col 12, Line 2, Col 17, "The field 'A' appears multiple times in this anonymous record type.") (Error 3523, Line 2, Col 24, Line 2, Col 25, "The field 'B' appears multiple times in this anonymous record type.") ] + + [] + let ``Anonymous Records field appears multiple times in this anonymous record declaration 6`` () = + Fsx """ +let foo: {| A: int; C: string; A: int; B: int; A: int |} = failwith "foo" + """ + |> compile + |> shouldFail + |> withDiagnostics [ + (Error 3523, Line 2, Col 13, Line 2, Col 14, "The field 'A' appears multiple times in this anonymous record type.") + (Error 3523, Line 2, Col 32, Line 2, Col 33, "The field 'A' appears multiple times in this anonymous record type.") + ] From bbf7455ae53a024ecdaee75a6298d5268932da34 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Sun, 20 Aug 2023 18:37:00 +0200 Subject: [PATCH 3/5] check for >1 idents --- src/Compiler/Checking/CheckExpressions.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index b4679719b77..16309f0c6b2 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -4519,12 +4519,12 @@ and CheckAnonRecdTypeDuplicateFields (ident: _ -> Ident) elems = let id2 = (ident uc2) if j > i && id1.idText = id2.idText then errorR(Error(FSComp.SR.tcAnonRecdTypeDuplicateFieldId(id1.idText), id1.idRange)))) - elems and TcAnonRecdType (cenv: cenv) newOk checkConstraints occ env tpenv isStruct args m = let tupInfo = mkTupInfo isStruct let idents = args |> List.choose(fun (lid, _) -> Some lid) - CheckAnonRecdTypeDuplicateFields id idents |> ignore + if idents.Length > 1 then + CheckAnonRecdTypeDuplicateFields id idents let tup = args |> List.map (fun (_, t) -> SynTupleTypeSegment.Type t) let argsR,tpenv = TcTypesAsTuple cenv newOk checkConstraints occ env tpenv tup m let unsortedFieldIds = args |> List.map fst |> List.toArray From 76349fc534e69424c42e974f1d610c669ca33e2c Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Mon, 21 Aug 2023 12:52:22 +0200 Subject: [PATCH 4/5] Update src/Compiler/Checking/CheckExpressions.fs Co-authored-by: Adam Boniecki <20281641+abonie@users.noreply.github.com> --- src/Compiler/Checking/CheckExpressions.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index 16309f0c6b2..0493c74dd5a 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -4522,7 +4522,7 @@ and CheckAnonRecdTypeDuplicateFields (ident: _ -> Ident) elems = and TcAnonRecdType (cenv: cenv) newOk checkConstraints occ env tpenv isStruct args m = let tupInfo = mkTupInfo isStruct - let idents = args |> List.choose(fun (lid, _) -> Some lid) + let idents = args |> List.map fst if idents.Length > 1 then CheckAnonRecdTypeDuplicateFields id idents let tup = args |> List.map (fun (_, t) -> SynTupleTypeSegment.Type t) From b648e495f144fbf15904f70d888e57e48d17f69f Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Mon, 21 Aug 2023 13:58:09 +0200 Subject: [PATCH 5/5] PR feedback --- src/Compiler/Checking/CheckExpressions.fs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index 0493c74dd5a..92b189c016d 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -4512,22 +4512,19 @@ and TcTupleType kindOpt (cenv: cenv) newOk checkConstraints occ env tpenv isStru let argsR,tpenv = TcTypesAsTuple cenv newOk checkConstraints occ env tpenv args m TType_tuple(tupInfo, argsR), tpenv -and CheckAnonRecdTypeDuplicateFields (ident: _ -> Ident) elems = - elems |> List.iteri (fun i uc1 -> - elems |> List.iteri (fun j uc2 -> - let id1 = (ident uc1) - let id2 = (ident uc2) - if j > i && id1.idText = id2.idText then - errorR(Error(FSComp.SR.tcAnonRecdTypeDuplicateFieldId(id1.idText), id1.idRange)))) +and CheckAnonRecdTypeDuplicateFields (elems: Ident array) = + elems |> Array.iteri (fun i (uc1: Ident) -> + elems |> Array.iteri (fun j (uc2: Ident) -> + if j > i && uc1.idText = uc2.idText then + errorR(Error(FSComp.SR.tcAnonRecdTypeDuplicateFieldId(uc1.idText), uc1.idRange)))) and TcAnonRecdType (cenv: cenv) newOk checkConstraints occ env tpenv isStruct args m = let tupInfo = mkTupInfo isStruct - let idents = args |> List.map fst - if idents.Length > 1 then - CheckAnonRecdTypeDuplicateFields id idents + let unsortedFieldIds = args |> List.map fst |> List.toArray + if unsortedFieldIds.Length > 1 then + CheckAnonRecdTypeDuplicateFields unsortedFieldIds let tup = args |> List.map (fun (_, t) -> SynTupleTypeSegment.Type t) let argsR,tpenv = TcTypesAsTuple cenv newOk checkConstraints occ env tpenv tup m - let unsortedFieldIds = args |> List.map fst |> List.toArray let anonInfo = AnonRecdTypeInfo.Create(cenv.thisCcu, tupInfo, unsortedFieldIds) // Sort into canonical order