Fix :: FAR :: Remove corrupted .ctor symbol reference

eng/release/insert-into-vs.yml

@@ -15,7 +15,7 @@ stages:
   - job: Insert_VS
     pool:
       name: NetCore1ESPool-Svc-Internal
-      image: windows.vs2022preview.amd64
+      image: windows.vs2026preview.scout.amd64
     variables:
     - group: DotNet-VSTS-Infra-Access
     - name: InsertAccessToken

src/Compiler/Checking/CheckDeclarations.fs

@@ -2878,6 +2878,13 @@ module EstablishTypeDefinitionCores =
         let noCLIMutableAttributeCheck() =
             if hasCLIMutable then errorR (Error(FSComp.SR.tcThisTypeMayNotHaveACLIMutableAttribute(), m))
 
+        // Check attribute targets for error reporting only — results are discarded.
+        // Suspend the sink to avoid duplicate symbol entries from re-resolving attribute constructors.
+        let checkAttributeTargetsErrors attrTarget =
+            if reportAttributeTargetsErrors then
+                use _holder = TemporarilySuspendReportingTypecheckResultsToSink cenv.tcSink
+                TcAttributesWithPossibleTargets TcCanFail.IgnoreMemberResoutionError cenv envinner attrTarget synAttrs |> ignore
+
         let isStructRecordOrUnionType = 
             match synTyconRepr with
             | SynTypeDefnSimpleRepr.Record _ 
@@ -2915,11 +2922,7 @@ module EstablishTypeDefinitionCores =
                 // Run InferTyconKind to raise errors on inconsistent attribute sets
                 InferTyconKind g (SynTypeDefnKind.Union, attrs, [], [], inSig, true, m) |> ignore
 
-                if reportAttributeTargetsErrors then
-                    if hasStructAttr then
-                        TcAttributesWithPossibleTargets TcCanFail.IgnoreMemberResoutionError cenv envinner AttributeTargets.Struct synAttrs |> ignore
-                    else
-                        TcAttributesWithPossibleTargets TcCanFail.IgnoreMemberResoutionError cenv envinner AttributeTargets.Class synAttrs |> ignore
+                checkAttributeTargetsErrors (if hasStructAttr then AttributeTargets.Struct else AttributeTargets.Class)
 
                 // Note: the table of union cases is initially empty
                 Construct.MakeUnionRepr []
@@ -2940,11 +2943,7 @@ module EstablishTypeDefinitionCores =
                 // Run InferTyconKind to raise errors on inconsistent attribute sets
                 InferTyconKind g (SynTypeDefnKind.Record, attrs, [], [], inSig, true, m) |> ignore
 
-                if reportAttributeTargetsErrors then
-                    if hasStructAttr then
-                        TcAttributesWithPossibleTargets TcCanFail.IgnoreMemberResoutionError cenv envinner AttributeTargets.Struct synAttrs |> ignore
-                    else
-                        TcAttributesWithPossibleTargets TcCanFail.IgnoreMemberResoutionError cenv envinner AttributeTargets.Class synAttrs |> ignore
+                checkAttributeTargetsErrors (if hasStructAttr then AttributeTargets.Struct else AttributeTargets.Class)
 
                 // Note: the table of record fields is initially empty
                 TFSharpTyconRepr (Construct.NewEmptyFSharpTyconData TFSharpRecord)
@@ -2959,29 +2958,24 @@ module EstablishTypeDefinitionCores =
                     let kind = 
                         match kind with
                         | SynTypeDefnKind.Class ->
-                            if reportAttributeTargetsErrors then
-                                TcAttributesWithPossibleTargets TcCanFail.IgnoreMemberResoutionError cenv envinner AttributeTargets.Class synAttrs |> ignore
+                            checkAttributeTargetsErrors AttributeTargets.Class
                             TFSharpClass
                         | SynTypeDefnKind.Interface ->
-                            if reportAttributeTargetsErrors then
-                                TcAttributesWithPossibleTargets TcCanFail.IgnoreMemberResoutionError cenv envinner AttributeTargets.Interface synAttrs |> ignore
+                            checkAttributeTargetsErrors AttributeTargets.Interface
                             TFSharpInterface
                         | SynTypeDefnKind.Delegate _ ->
-                            if reportAttributeTargetsErrors then
-                                TcAttributesWithPossibleTargets TcCanFail.IgnoreMemberResoutionError cenv envinner AttributeTargets.Delegate synAttrs |> ignore
+                            checkAttributeTargetsErrors AttributeTargets.Delegate
                             TFSharpDelegate (MakeSlotSig("Invoke", g.unit_ty, [], [], [], None))
                         | SynTypeDefnKind.Struct ->
-                            if reportAttributeTargetsErrors then
-                                TcAttributesWithPossibleTargets TcCanFail.IgnoreMemberResoutionError cenv envinner AttributeTargets.Struct synAttrs |> ignore
+                            checkAttributeTargetsErrors AttributeTargets.Struct
                             TFSharpStruct 
                         | _ -> error(InternalError("should have inferred tycon kind", m))
 
                     TFSharpTyconRepr (Construct.NewEmptyFSharpTyconData kind)
 
             | SynTypeDefnSimpleRepr.Enum _ ->
                 noCLIMutableAttributeCheck()
-                if reportAttributeTargetsErrors then
-                    TcAttributesWithPossibleTargets TcCanFail.IgnoreMemberResoutionError cenv envinner AttributeTargets.Enum synAttrs |> ignore
+                checkAttributeTargetsErrors AttributeTargets.Enum
                 TFSharpTyconRepr (Construct.NewEmptyFSharpTyconData TFSharpEnum)
 
         // OK, now fill in the (partially computed) type representation

src/Compiler/Checking/Expressions/CheckExpressions.fs

@@ -801,13 +801,6 @@ let ForNewConstructors tcSink (env: TcEnv) mObjTy methodName meths =
     let callSink (item, minst) = CallMethodGroupNameResolutionSink tcSink (mObjTy, env.NameEnv, item, origItem, minst, ItemOccurrence.Use, env.AccessRights)
     let sendToSink minst refinedMeths = 
         callSink (Item.CtorGroup(methodName, refinedMeths), minst)
-        // #14902: Also register as Item.Value for Find All References
-        for meth in refinedMeths do
-            match meth with
-            | FSMeth(_, _, vref, _) when vref.IsConstructor ->
-                let shiftedRange = Range.mkRange mObjTy.FileName (Position.mkPos mObjTy.StartLine (mObjTy.StartColumn + 1)) mObjTy.End
-                CallNameResolutionSink tcSink (shiftedRange, env.NameEnv, Item.Value vref, minst, ItemOccurrence.Use, env.AccessRights)
-            | _ -> ()
     match meths with
     | [] ->
         AfterResolution.DoNothing

src/Compiler/Checking/NameResolution.fs

@@ -1998,6 +1998,15 @@ let ItemsAreEffectivelyEqual g orig other =
     | Item.Trait traitInfo1, Item.Trait traitInfo2 ->
         traitInfo1.MemberLogicalName = traitInfo2.MemberLogicalName
 
+    // Cross-match constructor value refs with constructor groups (for FAR from additional constructor new())
+    | ValUse vref1, Item.CtorGroup(_, meths)
+    | Item.CtorGroup(_, meths), ValUse vref1 ->
+        meths
+        |> List.exists (fun meth ->
+            match meth.ArbitraryValRef with
+            | Some vref2 -> valRefDefnEq g vref1 vref2
+            | _ -> false)
+
     | _ -> false
 
 /// Given the Item 'orig' - returns function 'other: Item -> bool', that will yield true if other and orig represents the same item and false - otherwise
@@ -2006,6 +2015,7 @@ let ItemsAreEffectivelyEqualHash (g: TcGlobals) orig =
     | EntityUse tcref -> tyconRefDefnHash g tcref
     | Item.TypeVar (nm, _)-> hash nm
     | Item.Trait traitInfo -> hash traitInfo.MemberLogicalName
+    | ValUse vref when vref.IsConstructor && vref.IsMember -> hash vref.MemberApparentEntity.LogicalName
     | ValUse vref -> valRefDefnHash g vref
     | ActivePatternCaseUse (_, _, idx)-> hash idx
     | MethodUse minfo -> minfo.ComputeHashCode()

tests/FSharp.Compiler.ComponentTests/FSharpChecker/CommonWorkflows.fs

@@ -154,8 +154,7 @@ let GetAllUsesOfAllSymbols() =
             return checkProjectResults.GetAllUsesOfAllSymbols()
         } |> Async.RunSynchronously
 
-    // #14902: Count is 80 due to constructor double registration
-    if result.Length <> 80 then failwith $"Expected 80 symbolUses, got {result.Length}:\n%A{result}"
+    if result.Length <> 79 then failwith $"Expected 79 symbolUses, got {result.Length}:\n%A{result}"
 
 [<Fact>]
 let ``We don't lose subsequent diagnostics when there's error in one file`` () =

tests/FSharp.Compiler.ComponentTests/FSharpChecker/FindReferences.fs

@@ -57,6 +57,15 @@ let expectLinesInclude expectedLines (ranges: range list) =
     for line in expectedLines do
         Assert.True(actualLines.Contains(line), $"Expected reference on line {line}. Ranges: {ranges}")
 
+/// Helper for tests that create a synthetic project and check all symbols in a file
+let checkAllSymbols (source: string) (check: FSharpCheckFileResults -> seq<FSharpSymbolUse> -> unit) =
+    SyntheticProject.Create({ sourceFile "Source" [] with Source = source })
+        .Workflow {
+            checkFile "Source" (fun (result: FSharpCheckFileResults) ->
+                let allUses = result.GetAllUsesOfAllSymbolsInFile()
+                check result allUses)
+        }
+
 /// Asserts a minimum number of references.
 let expectMinRefs minCount (ranges: range list) =
     Assert.True(ranges.Length >= minCount, $"Expected at least {minCount} references, got {ranges.Length}")
@@ -1050,19 +1059,15 @@ type MyClass(x: int) =
 let a = MyClass()
 let b = MyClass(5)
 """
-        SyntheticProject.Create({ sourceFile "Source" [] with Source = source })
-            .Workflow {
-                checkFile "Source" (fun (result: FSharpCheckFileResults) ->
-                    let allUses = result.GetAllUsesOfAllSymbolsInFile()
-                    let additionalCtorDef = 
-                        allUses 
-                        |> Seq.tryFind (fun su -> su.IsFromDefinition && su.Range.StartLine = 4 && su.Range.StartColumn = 4)
-                    Assert.True(additionalCtorDef.IsSome, "Should find additional constructor at (4,4)")
-                    match additionalCtorDef.Value.Symbol with
-                    | :? FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue as mfv ->
-                        Assert.True(mfv.IsConstructor, "Symbol should be a constructor")
-                    | _ -> Assert.Fail("Expected FSharpMemberOrFunctionOrValue"))
-            }
+        checkAllSymbols source (fun _ allUses ->
+            let additionalCtorDef = 
+                allUses 
+                |> Seq.tryFind (fun su -> su.IsFromDefinition && su.Range.StartLine = 4 && su.Range.StartColumn = 4)
+            Assert.True(additionalCtorDef.IsSome, "Should find additional constructor at (4,4)")
+            match additionalCtorDef.Value.Symbol with
+            | :? FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue as mfv ->
+                Assert.True(mfv.IsConstructor, "Symbol should be a constructor")
+            | _ -> Assert.Fail("Expected FSharpMemberOrFunctionOrValue"))
 
 module ExternalDllOptimization =
 
@@ -1141,20 +1146,17 @@ open System.Linq
 let arr = [| "a"; "b"; "c" |]
 let first = arr.First()
 """
-        SyntheticProject.Create({ sourceFile "Source" [] with Source = source })
-            .Workflow {
-                checkFile "Source" (fun (result: FSharpCheckFileResults) ->
-                    let firstUses = 
-                        result.GetAllUsesOfAllSymbolsInFile()
-                        |> Seq.filter (fun su -> su.Symbol.DisplayName = "First")
-                        |> Seq.toArray
-                    Assert.True(firstUses.Length >= 1, $"Expected at least 1 use of First, found {firstUses.Length}")
-                    for su in firstUses do
-                        match su.Symbol with
-                        | :? FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue as mfv ->
-                            Assert.True(mfv.IsExtensionMember, "First should be an extension member")
-                        | _ -> ())
-            }
+        checkAllSymbols source (fun _ allUses ->
+            let firstUses = 
+                allUses
+                |> Seq.filter (fun su -> su.Symbol.DisplayName = "First")
+                |> Seq.toArray
+            Assert.True(firstUses.Length >= 1, $"Expected at least 1 use of First, found {firstUses.Length}")
+            for su in firstUses do
+                match su.Symbol with
+                | :? FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue as mfv ->
+                    Assert.True(mfv.IsExtensionMember, "First should be an extension member")
+                | _ -> ())
 
 /// https://github.com/dotnet/fsharp/issues/5545
 module SAFEBookstoreSymbols =
@@ -1205,4 +1207,106 @@ let altDb : DatabaseType = PostgreSQL
             .Workflow {
                 placeCursor "Source" 7 5 "type DatabaseType = " ["DatabaseType"]
                 findAllReferences (fun ranges -> expectLinesInclude [7; 12; 19] ranges)
-            }
+            }
+
+/// https://github.com/dotnet/fsharp/issues/19336
+module ConstructorDuplicateRegression =
+
+    [<Fact>]
+    let ``No duplicate ctor symbol for attribute`` () =
+        let source = """
+type MyAttr() = inherit System.Attribute()
+
+[<MyAttr>] type Foo = { X: int }
+"""
+        checkAllSymbols source (fun _ allUses ->
+            // Issue 19336: PR 19252 added a duplicate constructor at a shifted range.
+            // Expected: member .ctor at (5,2--5,8)
+            // Regression added: member .ctor at (5,3--5,8) (StartColumn + 1)
+            let ctorSymbols =
+                allUses
+                |> Seq.filter (fun su ->
+                    su.Range.StartLine = 5
+                    && (match su.Symbol with :? FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue as m -> m.IsConstructor | _ -> false))
+                |> Seq.map (fun su -> su.Range.StartColumn, su.Range.EndColumn)
+                |> Seq.toArray
+            // Exactly 1 constructor at column 2 — no duplicates, no shifted ranges.
+            Assert.Equal<(int * int) array>([| (2, 8) |], ctorSymbols))
+
+    [<Fact>]
+    let ``No duplicate ctor symbol for optional param desugaring`` () =
+        let source = """
+type T() =
+    static member M1(?a) = ()
+"""
+        checkAllSymbols source (fun _ allUses ->
+            // Issue 19336: The desugared [<OptionalArgumentAttribute>] generates constructor calls.
+            // The regression added duplicate constructors at shifted ranges (StartColumn + 1).
+            // Check that no constructor on any line has a range shifted from its correct position.
+            let allCtors =
+                allUses
+                |> Seq.filter (fun su ->
+                    match su.Symbol with :? FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue as m -> m.IsConstructor | _ -> false)
+                |> Seq.toArray
+            // The primary constructor definition at type T() is at line 3.
+            let primaryCtorDefs =
+                allCtors
+                |> Array.filter (fun su -> su.Range.StartLine = 3 && su.IsFromDefinition)
+            Assert.True(primaryCtorDefs.Length >= 1, sprintf "Expected primary ctor definition, got %d" primaryCtorDefs.Length)
+            // No constructor should have a shifted range (StartColumn differs by 1 from another ctor at same line)
+            let ctorsByLine =
+                allCtors
+                |> Array.groupBy (fun su -> su.Range.StartLine, su.Range.EndColumn)
+            for ((line, endCol), group) in ctorsByLine do
+                let startCols = group |> Array.map (fun su -> su.Range.StartColumn) |> Array.distinct
+                if startCols.Length > 1 then
+                    Assert.Fail(sprintf "Line %d, endCol %d: multiple start columns for constructors: %A (shifted range regression)" line endCol startCols))
+
+    [<Fact>]
+    let ``No duplicate ctor symbol for generic object expression`` () =
+        let source = """
+type Base<'T>() =
+    abstract M1: 'T -> unit
+    default _.M1 _ = ()
+
+let x =
+    { new Base<int>() with
+        override this.M1 _ = () }
+"""
+        checkAllSymbols source (fun _ allUses ->
+            // Line 8: { new Base<int>() with ... }
+            // Exact expected symbols: Base(10,14), int(15,18), Base(10,19)
+            // The regression would add a 4th: .ctor(11,19) at a shifted range
+            let line8Uses =
+                allUses
+                |> Seq.filter (fun su -> su.Range.StartLine = 8)
+                |> Seq.map (fun su -> su.Symbol.DisplayName, su.Range.StartColumn, su.Range.EndColumn)
+                |> Seq.toArray
+            Assert.Equal<(string * int * int) array>(
+                [| ("Base", 10, 14); ("int", 15, 18); ("Base", 10, 19) |],
+                line8Uses))
+
+    [<Fact>]
+    let ``FAR from additional constructor new() finds call sites`` () =
+        let source = """
+type MyClass(x: int) =
+    new() = MyClass(0)
+
+let a = MyClass()
+"""
+        checkAllSymbols source (fun result allUses ->
+            // Find the additional constructor definition at "new"
+            let ctorDef =
+                allUses
+                |> Seq.tryFind (fun su -> su.IsFromDefinition && su.Range.StartLine = 4 && su.Range.StartColumn = 4)
+            Assert.True(ctorDef.IsSome, "Should find additional constructor definition at (4,4)")
+            let uses = result.GetUsesOfSymbolInFile(ctorDef.Value.Symbol)
+            // Exact: definition at (4,4)-(4,7) and call site at (6,8)-(6,15)
+            // No Array.distinct — duplicates must be caught, not hidden
+            let rawUses =
+                uses
+                |> Array.map (fun su -> su.Range.StartLine, su.Range.StartColumn, su.Range.EndColumn)
+                |> Array.sort
+            Assert.Equal<(int * int * int) array>(
+                [| (4, 4, 7); (6, 8, 15) |],
+                rawUses))

tests/FSharp.Compiler.Service.Tests/EditorTests.fs

@@ -780,7 +780,6 @@ type Class1() =
     |> shouldEqual
         [|("LiteralAttribute", (3, 10, 3, 17))
           ("member .ctor", (3, 10, 3, 17))
-          ("member .ctor", (3, 11, 3, 17)) // #14902
           ("val ModuleValue", (3, 20, 3, 31))
           ("val op_Addition", (6, 26, 6, 27))
           ("val ModuleValue", (6, 14, 6, 25))
@@ -792,11 +791,9 @@ type Class1() =
           ("member .ctor", (10, 5, 10, 11))
           ("LiteralAttribute", (11, 10, 11, 17))
           ("member .ctor", (11, 10, 11, 17))
-          ("member .ctor", (11, 11, 11, 17)) // #14902
           ("val ClassValue", (11, 20, 11, 30))
           ("LiteralAttribute", (12, 17, 12, 24))
           ("member .ctor", (12, 17, 12, 24))
-          ("member .ctor", (12, 18, 12, 24)) // #14902
           ("val StaticClassValue", (12, 27, 12, 43))
           ("val ClassValue", (14, 12, 14, 22))
           ("val StaticClassValue", (15, 12, 15, 28))