Better error reporting for unions duplicated fields (#17521)

* Better error reporting for unions duplicated fields

* update tests

* release notes

Author: edgarfgp

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

@@ -27,4 +27,5 @@
 * Enforce `AttributeTargets` on unions. ([PR #17389](https://github.com/dotnet/fsharp/pull/17389))
 * Ensure that isinteractive multi-emit backing fields are not public. ([Issue #17439](https://github.com/dotnet/fsharp/issues/17438)), ([PR #17439](https://github.com/dotnet/fsharp/pull/17439))
 * Enable FSharp 9.0 Language Version ([Issue #17497](https://github.com/dotnet/fsharp/issues/17438)), [PR](https://github.com/dotnet/fsharp/pull/17500)))
+* Better error reporting for unions with duplicated fields. ([PR #17521](https://github.com/dotnet/fsharp/pull/17521))
 ### Breaking Changes

src/Compiler/Checking/CheckDeclarations.fs

@@ -500,18 +500,26 @@ module TcRecdUnionAndEnumDeclarations =
             if not (String.isLeadingIdentifierCharacterUpperCase name) && name <> opNameCons && name <> opNameNil then
                 errorR(NotUpperCaseConstructor(id.idRange))
 
-    let ValidateFieldNames (synFields: SynField list, tastFields: RecdField list) = 
+    let private CheckUnionDuplicateFields (elems: Ident list) =
+        elems |> List.iteri (fun i (uc1: Ident) -> 
+            elems |> List.iteri (fun j (uc2: Ident) -> 
+                if j > i && uc1.idText = uc2.idText then 
+                   errorR(Error(FSComp.SR.tcFieldNameIsUsedModeThanOnce(uc1.idText), uc1.idRange))))
+
+    let ValidateFieldNames (synFields: SynField list, tastFields: RecdField list) =
+        let fields = synFields |> List.choose (function SynField(idOpt = Some ident) -> Some ident | _ -> None)
+        if fields.Length > 1 then
+            CheckUnionDuplicateFields fields
+        
         let seen = Dictionary()
         (synFields, tastFields) ||> List.iter2 (fun sf f ->
             match seen.TryGetValue f.LogicalName with
             | true, synField ->
                 match sf, synField with
-                | SynField(idOpt = Some id), SynField(idOpt = Some _) ->
-                    error(Error(FSComp.SR.tcFieldNameIsUsedModeThanOnce(id.idText), id.idRange))
                 | SynField(idOpt = Some id), SynField(idOpt = None)
                 | SynField(idOpt = None), SynField(idOpt = Some id) ->
-                    error(Error(FSComp.SR.tcFieldNameConflictsWithGeneratedNameForAnonymousField(id.idText), id.idRange))
-                | _ -> assert false
+                    errorR(Error(FSComp.SR.tcFieldNameConflictsWithGeneratedNameForAnonymousField(id.idText), id.idRange))
+                | _ -> ()
             | _ ->
                 seen.Add(f.LogicalName, sf))
 

tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/ExceptionDefinitions/E_ExnFieldConflictingName.fs

@@ -5,4 +5,6 @@
 
 exception AAA of Data1 : int * string
 
-exception BBB of A : int * A : string
+exception BBB of A : int * A : string
+
+exception CCC of A : int * A : string * A : int

tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/ExceptionDefinitions/ExceptionDefinitions.fs

@@ -226,8 +226,10 @@ module ExceptionDefinition =
         |> compile
         |> shouldFail
         |> withDiagnostics [
-            (Error 3176, Line 6, Col 18, Line 6, Col 23, "Named field 'Data1' conflicts with autogenerated name for anonymous field.")
-            (Error 3176, Line 8, Col 28, Line 8, Col 29, "Named field 'A' is used more than once.")
+            (Error 3176, Line 6, Col 18, Line 6, Col 23, "Named field 'Data1' conflicts with autogenerated name for anonymous field.");
+            (Error 3176, Line 8, Col 18, Line 8, Col 19, "Named field 'A' is used more than once.");
+            (Error 3176, Line 10, Col 18, Line 10, Col 19, "Named field 'A' is used more than once.");
+            (Error 3176, Line 10, Col 28, Line 10, Col 29, "Named field 'A' is used more than once.")
         ]
 
     // SOURCE=E_FieldNameUsedMulti.fs                      SCFLAGS="--test:ErrorRanges"        # E_FieldNameUsedMulti.fs

tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/E_UnionFieldConflictingName.fs

@@ -7,4 +7,4 @@ type MyDU =
     | Case1 of Item2 : int * string
 
 type MyDU2 = 
-    | Case1 of A : int * A : string
+    | Case1 of A : int * A : string * A : int

tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionStructTypes.fs

@@ -402,7 +402,9 @@ type StructUnion =
         |> typecheck
         |> shouldFail
         |> withDiagnostics [
-            (Error 3176, Line 5, Col 27, Line 5, Col 31, "Named field 'item' is used more than once.")
+            (Error 3176, Line 5, Col 12, Line 5, Col 16, "Named field 'item' is used more than once.");
+            (Error 3585, Line 5, Col 12, Line 5, Col 16, "If a multicase union type is a struct, then all fields with the same name must be of the same type. This rule applies also to the generated 'Item' name in case of unnamed fields.");
+            (Error 3585, Line 5, Col 27, Line 5, Col 31, "If a multicase union type is a struct, then all fields with the same name must be of the same type. This rule applies also to the generated 'Item' name in case of unnamed fields.")
         ]
 
     [<Fact>]
@@ -417,7 +419,10 @@ type StructUnion =
         |> typecheck
         |> shouldFail
         |> withDiagnostics [
-            (Error 3176, Line 5, Col 27, Line 5, Col 31, "Named field 'Item' is used more than once.")
+            (Error 3176, Line 5, Col 12, Line 5, Col 16, "Named field 'Item' is used more than once.");
+            (Error 3585, Line 5, Col 12, Line 5, Col 16, "If a multicase union type is a struct, then all fields with the same name must be of the same type. This rule applies also to the generated 'Item' name in case of unnamed fields.");
+            (Error 3585, Line 5, Col 27, Line 5, Col 31, "If a multicase union type is a struct, then all fields with the same name must be of the same type. This rule applies also to the generated 'Item' name in case of unnamed fields.");
+            (Error 3585, Line 6, Col 12, Line 6, Col 18, "If a multicase union type is a struct, then all fields with the same name must be of the same type. This rule applies also to the generated 'Item' name in case of unnamed fields.")
         ]
 
     [<Fact>]

tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionTypes.fs

@@ -272,7 +272,8 @@ module UnionTypes =
         |> verifyCompile
         |> shouldFail
         |> withDiagnostics [
-            (Error 3176, Line 7, Col 16, Line 7, Col 21, "Named field 'Item2' conflicts with autogenerated name for anonymous field.")
+            (Error 3176, Line 7, Col 16, Line 7, Col 21, "Named field 'Item2' conflicts with autogenerated name for anonymous field.");
+            (Error 3176, Line 10, Col 16, Line 10, Col 17, "Named field 'A' is used more than once.");
             (Error 3176, Line 10, Col 26, Line 10, Col 27, "Named field 'A' is used more than once.")
         ]
 
@@ -750,3 +751,59 @@ type MyId =
             (Error 23, Line 7, Col 17, Line 7, Col 20, "The member 'IdA' can not be defined because the name 'IdA' clashes with the union case 'IdA' in this type or module")
             (Error 23, Line 17, Col 17, Line 17, Col 20, "The member 'IdC' can not be defined because the name 'IdC' clashes with the union case 'IdC' in this type or module")
         ]
+        
+
+    [<Fact>]
+    let ``Union field appears multiple times in union declaration`` () =
+        Fsx """
+type X =
+    | A of a: int * a: int
+        """
+        |> typecheck
+        |> shouldFail
+        |> withDiagnostics [
+            (Error 3176, Line 3, Col 12, Line 3, Col 13, "Named field 'a' is used more than once.")
+        ]
+
+    [<Fact>]
+    let ``Union field appears multiple times in union declaration 2`` () =
+        Fsx """
+type X =
+    | A of a: int * a: int
+    | B of a: int * a: int
+        """
+        |> typecheck
+        |> shouldFail
+        |> withDiagnostics [
+            (Error 3176, Line 3, Col 12, Line 3, Col 13, "Named field 'a' is used more than once.")
+            (Error 3176, Line 4, Col 12, Line 4, Col 13, "Named field 'a' is used more than once.")
+        ]
+
+    [<Fact>]
+    let ``Union field appears multiple times in union declaration 3`` () =
+        Fsx """
+type X =
+    | A of a: int * a: int * a: int
+        """
+        |> typecheck
+        |> shouldFail
+        |> withDiagnostics [
+            (Error 3176, Line 3, Col 12, Line 3, Col 13, "Named field 'a' is used more than once.")
+            (Error 3176, Line 3, Col 21, Line 3, Col 22, "Named field 'a' is used more than once.")
+        ]
+
+    [<Fact>]
+    let ``Union field appears multiple times in union declaration 4`` () =
+        Fsx """
+type X =
+    | A of a: int * a: int
+let x = A (1, 2)
+match x with
+| A(a = 1) -> ()
+| _ -> ()
+        """
+        |> typecheck
+        |> shouldFail
+        |> withDiagnostics [
+            (Error 3176, Line 3, Col 12, Line 3, Col 13, "Named field 'a' is used more than once.")
+        ]