Hardening: eOpenedTypeOperators NameMultiMap + CheckEntityDefn dedup

src/Compiler/Checking/NameResolution.fs

@@ -450,8 +450,8 @@ type NameResolutionEnv =
       /// Other extension members unindexed by type
       eUnindexedExtensionMembers: ExtensionMember list
 
-      /// Static operator methods from 'open type' declarations, available for SRTP resolution
-      eOpenedTypeOperators: MethInfo list
+      /// Static operator methods from 'open type' declarations, available for SRTP resolution, indexed by logical name.
+      eOpenedTypeOperators: NameMultiMap<MethInfo>
 
       /// Typars (always available by unqualified names). Further typars can be
       /// in the tpenv, a structure folded through each top-level definition.
@@ -475,7 +475,7 @@ type NameResolutionEnv =
           eFullyQualifiedTyconsByDemangledNameAndArity = LayeredMap.Empty
           eIndexedExtensionMembers = TyconRefMultiMap<_>.Empty
           eUnindexedExtensionMembers = []
-          eOpenedTypeOperators = []
+          eOpenedTypeOperators = Map.empty
           eTypars = Map.empty }
 
     member nenv.DisplayEnv = nenv.eDisplayEnv
@@ -930,6 +930,9 @@ let AddTyconByAccessNames bulkAddMode (tcrefs: TyconRef[]) (tab: LayeredMultiMap
 /// Add a record field to the corresponding sub-table of the name resolution environment
 let AddRecdField (rfref: RecdFieldRef) tab = NameMultiMap.add rfref.FieldName rfref tab
 
+/// Index a MethInfo by its logical name into a NameMultiMap sub-table of the environment.
+let AddMethInfoByLogicalName (minfo: MethInfo) tab = NameMultiMap.add minfo.LogicalName minfo tab
+
 /// Add a set of union cases to the corresponding sub-table of the environment
 let AddUnionCases1 (tab: Map<_, _>) (ucrefs: UnionCaseRef list) =
     (tab, ucrefs) ||> List.fold (fun acc ucref ->
@@ -1271,9 +1274,12 @@ let rec AddStaticContentOfTypeToNameEnv (g:TcGlobals) (amap: Import.ImportMap) a
 
     let nenv = { nenv with eUnqualifiedItems = nenv.eUnqualifiedItems.AddMany items }
 
+    let allMethInfos =
+        IntrinsicMethInfosOfType infoReader None ad AllowMultiIntfInstantiations.Yes PreferOverrides m ty
+
     let methodGroupItems =
         // Methods
-        IntrinsicMethInfosOfType infoReader None ad AllowMultiIntfInstantiations.Yes PreferOverrides m ty
+        allMethInfos
         |> ChooseMethInfosForNameEnv g m ty
         // Combine methods and extension method groups of the same type
         |> List.map (fun pair ->
@@ -1296,7 +1302,7 @@ let rec AddStaticContentOfTypeToNameEnv (g:TcGlobals) (amap: Import.ImportMap) a
     // These are intentionally excluded from eUnqualifiedItems by ChooseMethInfosForNameEnv
     // but need to be available for SRTP constraint solving.
     let operatorMethods =
-        IntrinsicMethInfosOfType infoReader None ad AllowMultiIntfInstantiations.Yes PreferOverrides m ty
+        allMethInfos
         |> List.filter (fun minfo ->
             not (minfo.IsInstance || minfo.IsClassConstructor || minfo.IsConstructor)
             && typeEquiv g minfo.ApparentEnclosingType ty
@@ -1305,7 +1311,13 @@ let rec AddStaticContentOfTypeToNameEnv (g:TcGlobals) (amap: Import.ImportMap) a
     if operatorMethods.IsEmpty then
         nenv
     else
-        { nenv with eOpenedTypeOperators = operatorMethods @ nenv.eOpenedTypeOperators }
+        let eOpenedTypeOperators =
+            // Preserve source-declaration order of `operatorMethods` within each bucket
+            // (matches the prior list-prepend semantics). `List.foldBack` lands the first
+            // declared method at the head of the bucket; do not switch to `List.fold` or
+            // `NameMultiMap.initBy` without reversing first. See `docs/name-resolution-operators.md`.
+            List.foldBack AddMethInfoByLogicalName operatorMethods nenv.eOpenedTypeOperators
+        { nenv with eOpenedTypeOperators = eOpenedTypeOperators }
 
 and private AddNestedTypesOfTypeToNameEnv infoReader (amap: Import.ImportMap) ad m nenv ty =
     let tinst, tcrefs = GetNestedTyconRefsOfType infoReader amap (ad, None, TypeNameResolutionStaticArgsInfo.Indefinite, true, m) ty
@@ -1739,14 +1751,16 @@ let SelectExtensionMethInfosForTrait (traitInfo: TraitConstraintInfo, m: range,
     // Also include static operator methods from 'open type' declarations.
     // These are not registered as extension members but should participate in SRTP resolution.
     // Each method is yielded once (paired with the first support type) to avoid duplicates
-    // that would confuse overload resolution.
+    // that would confuse overload resolution. Bucket order is source-declaration order
+    // within each `open type` and most-recently-opened-first across `open type`s; see
+    // `AddStaticContentOfTypeToNameEnv` and `docs/name-resolution-operators.md`.
     let openTypeResults =
         match traitInfo.SupportTypes with
         | [] -> []
         | firstSupportTy :: _ ->
-            [ for minfo in nenv.eOpenedTypeOperators do
-                if minfo.LogicalName = nm then
-                    yield (firstSupportTy, minfo) ]
+            nenv.eOpenedTypeOperators
+            |> NameMultiMap.find nm
+            |> List.map (fun minfo -> (firstSupportTy, minfo))
 
     extResults @ openTypeResults
 

src/Compiler/Checking/NameResolution.fsi

@@ -234,7 +234,7 @@ type NameResolutionEnv =
         eUnindexedExtensionMembers: ExtensionMember list
 
         /// Static operator methods from 'open type' declarations, available for SRTP resolution
-        eOpenedTypeOperators: MethInfo list
+        eOpenedTypeOperators: NameMultiMap<MethInfo>
 
         /// Typars (always available by unqualified names). Further typars can be
         /// in the tpenv, a structure folded through each top-level definition.

src/Compiler/Checking/PostInferenceChecks.fs

@@ -2398,26 +2398,20 @@ let CheckEntityDefn cenv env (tycon: Entity) =
             | true, h -> h
             | _ -> []
 
+        // Index MethInfos by LogicalName; used for fresh-build groupings below.
+        let methInfosByLogicalName (xs: MethInfo list) : NameMultiMap<MethInfo> =
+            NameMultiMap.initBy (fun m -> m.LogicalName) xs
+
         // precompute methods grouped by MethInfo.LogicalName
-        let hashOfImmediateMeths =
-                let h = Dictionary<string, _>()
-                for minfo in immediateMeths do
-                    match h.TryGetValue minfo.LogicalName with
-                    | true, methods ->
-                        h[minfo.LogicalName] <- minfo :: methods
-                    | false, _ ->
-                        h[minfo.LogicalName] <- [minfo]
-                h
+        let immediateMethsByLogicalName = methInfosByLogicalName immediateMeths
         let getOtherMethods (minfo : MethInfo) =
-            [
-                //we have added all methods to the dictionary on the previous step
-                let methods = hashOfImmediateMeths[minfo.LogicalName]
-                for m in methods do
-                    // use referential identity to filter out 'minfo' method
-                    if not(Object.ReferenceEquals(m, minfo)) then
-                        yield m
-            ]
+            [ for m in NameMultiMap.find minfo.LogicalName immediateMethsByLogicalName do
+                // use referential identity to filter out 'minfo' method
+                if not (Object.ReferenceEquals(m, minfo)) then
+                    yield m ]
 
+        // Scan-so-far: each duplicate pair reported once, when the second member is seen.
+        // Symmetrizing (via NameMultiMap) would double-emit — see immediateMethsByLogicalName above.
         let hashOfImmediateProps = Dictionary<string, _>()
         for minfo in immediateMeths do
             let nm = minfo.LogicalName
@@ -2498,7 +2492,7 @@ let CheckEntityDefn cenv env (tycon: Entity) =
                 | None -> m
                 | Some vref -> vref.DefinitionRange
 
-            if hashOfImmediateMeths.ContainsKey nm then
+            if immediateMethsByLogicalName.ContainsKey nm then
                 errorR(Error(FSComp.SR.chkPropertySameNameMethod(nm, NicePrint.minimalStringOfType cenv.denv ty), m))
 
             let others = getHash hashOfImmediateProps nm
@@ -2546,18 +2540,14 @@ let CheckEntityDefn cenv env (tycon: Entity) =
             hashOfImmediateProps[nm] <- pinfo :: others
 
         if not (isInterfaceTy g ty) then
-            let hashOfAllVirtualMethsInParent = Dictionary<string, _>()
-            for minfo in allVirtualMethsInParent do
-                let nm = minfo.LogicalName
-                let others = getHash hashOfAllVirtualMethsInParent nm
-                hashOfAllVirtualMethsInParent[nm] <- minfo :: others
+            let parentVirtualMethsByLogicalName = methInfosByLogicalName allVirtualMethsInParent
             for minfo in immediateMeths do
                 if not minfo.IsDispatchSlot && not minfo.IsVirtual && minfo.IsInstance then
                     let nm = minfo.LogicalName
                     let m = (match minfo.ArbitraryValRef with None -> m | Some vref -> vref.DefinitionRange)
-                    let parentMethsOfSameName = getHash hashOfAllVirtualMethsInParent nm
+                    let parentMethsOfSameName = NameMultiMap.find nm parentVirtualMethsByLogicalName
                     let checkForDup erasureFlag (minfo2: MethInfo) = minfo2.IsDispatchSlot && MethInfosEquivByNameAndSig erasureFlag true g cenv.amap m minfo minfo2
-                    match parentMethsOfSameName |> List.tryFind (checkForDup EraseAll) with
+                    match parentMethsOfSameName |> List.tryFindBack (checkForDup EraseAll) with
                     | None -> ()
                     | Some minfo ->
                         let mtext = NicePrint.stringOfMethInfo cenv.infoReader m cenv.denv minfo
@@ -2570,7 +2560,7 @@ let CheckEntityDefn cenv env (tycon: Entity) =
                 if minfo.IsDispatchSlot then
                     let nm = minfo.LogicalName
                     let m = (match minfo.ArbitraryValRef with None -> m | Some vref -> vref.DefinitionRange)
-                    let parentMethsOfSameName = getHash hashOfAllVirtualMethsInParent nm
+                    let parentMethsOfSameName = NameMultiMap.find nm parentVirtualMethsByLogicalName
                     let checkForDup erasureFlag minfo2 = MethInfosEquivByNameAndSig erasureFlag true g cenv.amap m minfo minfo2
 
                     if parentMethsOfSameName |> List.exists (checkForDup EraseAll) then

tests/FSharp.Compiler.ComponentTests/Conformance/Members/CheckEntityDefnEdgeCases.fs

@@ -0,0 +1,97 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+namespace Conformance.Members
+
+open Xunit
+open FSharp.Test.Compiler
+
+// Regression coverage for duplicate-member diagnostics emitted by CheckEntityDefn
+// in src/Compiler/Checking/PostInferenceChecks.fs (lines ~2483-2566).
+module CheckEntityDefnEdgeCases =
+
+    // (a) Duplicate abstract method inherited from a base type, where the two
+    // signatures differ only in an erased unit-of-measure type argument.
+    // Exercises chkDuplicateMethodInheritedType(WithSuffix) (FS0442).
+    [<Fact>]
+    let ``Duplicate abstract method differing only in erased measure (FS0442)`` () =
+        FSharp """
+module Test
+[<Measure>] type m
+
+[<AbstractClass>]
+type A() =
+    abstract DoStuff : int -> int
+
+[<AbstractClass>]
+type B() =
+    inherit A()
+    abstract DoStuff : int<m> -> int<m>
+        """
+        |> typecheck
+        |> shouldFail
+        |> withErrorCode 442
+
+    // (b) Property and method on the same type share a name.
+    // Exercises chkPropertySameNameMethod (FS0434).
+    [<Fact>]
+    let ``Property and method with same name (FS0434)`` () =
+        FSharp """
+module Test
+type T() =
+    member val P = 0 with get
+    member _.P() = 1
+        """
+        |> typecheck
+        |> shouldFail
+        |> withErrorCode 434
+
+    // (c) A new member in a derived class has the same name/signature as an
+    // inherited abstract member but is not declared as override.
+    // Exercises tcNewMemberHidesAbstractMember(WithSuffix) (FS0864 warning).
+    [<Fact>]
+    let ``New member hides inherited abstract member (FS0864 warning)`` () =
+        FSharp """
+module Test
+type Base() =
+    abstract M : int -> int
+    default _.M x = x
+
+type Derived() =
+    inherit Base()
+    member _.M(x: int) = x + 1
+        """
+        |> withOptions [ "--warnaserror+" ]
+        |> typecheck
+        |> shouldFail
+        |> withErrorCode 864
+
+    // (d) Indexer and non-indexer property share a name on the same type.
+    // Exercises chkPropertySameNameIndexer (FS0436).
+    [<Fact>]
+    let ``Indexer vs non-indexer property with same name (FS0436)`` () =
+        FSharp """
+module Test
+type T() =
+    let mutable x = 0
+    member _.P with get () = x and set v = x <- v
+    member _.P with get (i: int) = i
+        """
+        |> typecheck
+        |> shouldFail
+        |> withErrorCode 436
+
+    // (e) Getter/setter for the same (non-indexer) property have different types.
+    // Exercises chkGetterAndSetterHaveSamePropertyType (FS3172).
+    [<Fact>]
+    let ``Getter and setter with different property types (FS3172)`` () =
+        FSharp """
+module Test
+type T() =
+    let mutable x = 0
+    member _.P
+        with get () : int = x
+        and set (v: string) = ignore v
+        """
+        |> typecheck
+        |> shouldFail
+        |> withErrorCode 3172

tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/ExtensionConstraintsTests.fs

@@ -34,6 +34,70 @@ module ExtensionConstraintsTests =
     let ``Most recently opened extension wins`` () =
         compileAndRunPreview "ExtensionPrecedence.fs"
 
+    [<Fact>]
+    let ``open type with homograph operators yields all overloads for SRTP`` () =
+        compileAndRunPreview "OpenTypeOperatorHomographOrder.fs"
+
+    [<Fact>]
+    let ``open type homograph operators across multiple holder types accumulate`` () =
+        compileAndRunPreview "OpenTypeOperatorHomographMultipleHolders.fs"
+
+    [<Fact>]
+    let ``open type nested in a module scopes extension operator correctly`` () =
+        compileAndRunPreview "OpenTypeOperatorNestedModule.fs"
+
+    [<Fact>]
+    let ``local let binding shadows open type extension operator`` () =
+        compileAndRunPreview "OpenTypeOperatorShadowing.fs"
+
+    [<Fact>]
+    let ``open type SRTP dispatch selects overload per argument type across holders`` () =
+        compileAndRunPreview "OpenTypeOperatorSRTPDispatch.fs"
+
+    [<Fact>]
+    let ``open type homograph overloads on single holder differ by parameter type`` () =
+        compileAndRunPreview "OpenTypeOperatorOverloadByParam.fs"
+
+    [<Fact>]
+    let ``open type operator with CompiledName attribute resolves by F# symbol`` () =
+        compileAndRunPreview "OpenTypeOperatorCompiledName.fs"
+
+    [<Fact>]
+    let ``open type extension operator crosses assembly boundary`` () =
+        let library =
+            FSharp """
+module OpLib
+
+[<AbstractClass; Sealed>]
+type Ops =
+    static member inline (+!) (a: int, b: int) = a + b + 7
+    static member inline (+!) (a: string, b: string) = a + b + "_X"
+            """
+            |> withName "OpLib"
+            |> asLibrary
+            |> withLangVersionPreview
+
+        FSharp """
+module Consumer
+open OpLib
+open type Ops
+
+let r1 : int = 10 +! 20
+if r1 <> 37 then failwith (sprintf "Expected 37, got %d" r1)
+
+let r2 : string = "a" +! "b"
+if r2 <> "ab_X" then failwith (sprintf "Expected 'ab_X', got '%s'" r2)
+
+let inline combine (a: ^T) (b: ^T) = a +! b
+let r3 : int = combine 1 2
+if r3 <> 10 then failwith (sprintf "Expected 10, got %d" r3)
+        """
+        |> asExe
+        |> withLangVersionPreview
+        |> withReferences [library]
+        |> compileAndRun
+        |> shouldSucceed
+
     [<Fact>]
     let ``Extension operators respect accessibility`` () =
         compileAndRunPreview "ExtensionAccessibility.fs"

tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorCompiledName.fs

@@ -0,0 +1,21 @@
+// Regression: a holder-type operator whose CLR emission is renamed via
+// [<CompiledName("...")>] must still be resolvable through 'open type' at
+// the F# source level. The LogicalName (op_PlusPlus etc.) is what feeds
+// into eOpenedTypeOperators keying and IsLogicalOpName filtering, so
+// CompiledName must not interfere with either.
+
+module OpenTypeOperatorCompiledName
+
+[<AbstractClass; Sealed>]
+type Ops =
+    [<CompiledName("CustomAdd")>]
+    static member inline (++) (a: int, b: int) = a + b + 100
+
+open type Ops
+
+let r1 : int = 1 ++ 2
+if r1 <> 103 then failwith $"Expected 103, got {r1}"
+
+let inline bump (a: ^T) (b: ^T) = a ++ b
+let r2 : int = bump 5 6
+if r2 <> 111 then failwith $"Expected 111, got {r2}"

tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorHomographMultipleHolders.fs

@@ -0,0 +1,28 @@
+// Regression: homograph operators declared across TWO separate holder types
+// each opened via 'open type'. Exercises cross-call bucket accumulation of
+// eOpenedTypeOperators (NameMultiMap<MethInfo>) — each 'open type' is a
+// separate AddStaticContentOfTypeToNameEnv call; both must contribute to the
+// same bucket keyed by LogicalName.
+
+module OpenTypeOperatorHomographMultipleHolders
+
+[<AbstractClass; Sealed>]
+type OpsA =
+    static member inline (+!) (a: int, b: int) = a + b + 10
+
+[<AbstractClass; Sealed>]
+type OpsB =
+    static member inline (+!) (a: float, b: float) = a + b + 100.0
+    static member inline (+!) (a: string, b: string) = a + b + "_B"
+
+open type OpsA
+open type OpsB
+
+let r1 : int = 1 +! 2
+if r1 <> 13 then failwith $"Expected 13, got {r1}"
+
+let r2 : float = 1.5 +! 2.5
+if r2 <> 104.0 then failwith $"Expected 104.0, got {r2}"
+
+let r3 : string = "hi" +! "world"
+if r3 <> "hiworld_B" then failwith $"Expected 'hiworld_B', got '{r3}'"