From d0c9143955fe23983d3691f8883690369ce61269 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Wed, 25 Feb 2026 14:06:07 +0100 Subject: [PATCH 1/5] Fix #19336 --- .../Checking/Expressions/CheckExpressions.fs | 7 - src/Compiler/Checking/NameResolution.fs | 10 ++ .../FSharpChecker/CommonWorkflows.fs | 3 +- .../FSharpChecker/FindReferences.fs | 165 +++++++++++++++--- 4 files changed, 148 insertions(+), 37 deletions(-) diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index be76ee77ac..7448479b47 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressions.fs @@ -787,13 +787,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 diff --git a/src/Compiler/Checking/NameResolution.fs b/src/Compiler/Checking/NameResolution.fs index 9d34ae7622..e92d468cf5 100644 --- a/src/Compiler/Checking/NameResolution.fs +++ b/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() diff --git a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/CommonWorkflows.fs b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/CommonWorkflows.fs index 3457203873..87d2f8aa8e 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/CommonWorkflows.fs +++ b/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}" [] let ``We don't lose subsequent diagnostics when there's error in one file`` () = diff --git a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/FindReferences.fs b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/FindReferences.fs index ab7eca78ab..cea75741f2 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/FindReferences.fs +++ b/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 -> 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,111 @@ let altDb : DatabaseType = PostgreSQL .Workflow { placeCursor "Source" 7 5 "type DatabaseType = " ["DatabaseType"] findAllReferences (fun ranges -> expectLinesInclude [7; 12; 19] ranges) - } \ No newline at end of file + } + +/// https://github.com/dotnet/fsharp/issues/19336 +module ConstructorDuplicateRegression = + + [] + let ``No duplicate ctor symbol for attribute`` () = + let source = """ +type MyAttr() = inherit System.Attribute() + +[] 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 + // All constructors must be at column 2 (the correct range). + // The regression had a constructor at column 3 (shifted by +1). + for (startCol, endCol) in ctorSymbols do + Assert.Equal(2, startCol) + Assert.Equal(8, endCol) + // At least one constructor must exist. + Assert.True(ctorSymbols.Length >= 1, sprintf "Expected at least 1 constructor, got %d" ctorSymbols.Length)) + + [] + 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 [] 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)) + + [] + let ``No duplicate ctor symbol for generic object expression`` () = + let source = """ +type Base<'T>() = + abstract M1: 'T -> unit + default _.M1 _ = () + +let x = + { new Base() with + override this.M1 _ = () } +""" + checkAllSymbols source (fun _ allUses -> + // Line 8: { new Base() 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)) + + [] + 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)) \ No newline at end of file From 242bedbdbbd95b2e40670af00c779b6338df42d6 Mon Sep 17 00:00:00 2001 From: Adam Boniecki <20281641+abonie@users.noreply.github.com> Date: Wed, 25 Feb 2026 14:12:16 +0100 Subject: [PATCH 2/5] Update image used for insert into VS step (#19357) --- eng/release/insert-into-vs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/release/insert-into-vs.yml b/eng/release/insert-into-vs.yml index 7545747226..7ea2007c50 100644 --- a/eng/release/insert-into-vs.yml +++ b/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 From 5bb73e40afd41a94af0e7317a3c8c3a063a9ca7a Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Wed, 25 Feb 2026 14:06:07 +0100 Subject: [PATCH 3/5] Fix #19336 --- src/Compiler/Checking/CheckDeclarations.fs | 34 ++-- .../Checking/Expressions/CheckExpressions.fs | 7 - src/Compiler/Checking/NameResolution.fs | 10 ++ .../FSharpChecker/CommonWorkflows.fs | 3 +- .../FSharpChecker/FindReferences.fs | 160 +++++++++++++++--- 5 files changed, 157 insertions(+), 57 deletions(-) diff --git a/src/Compiler/Checking/CheckDeclarations.fs b/src/Compiler/Checking/CheckDeclarations.fs index beeaa83e4d..80c70d1bcc 100644 --- a/src/Compiler/Checking/CheckDeclarations.fs +++ b/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,20 +2958,16 @@ 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)) @@ -2980,8 +2975,7 @@ module EstablishTypeDefinitionCores = | 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 diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index bc67c3f5c7..cd30572539 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/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 diff --git a/src/Compiler/Checking/NameResolution.fs b/src/Compiler/Checking/NameResolution.fs index 9d34ae7622..e92d468cf5 100644 --- a/src/Compiler/Checking/NameResolution.fs +++ b/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() diff --git a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/CommonWorkflows.fs b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/CommonWorkflows.fs index 3457203873..87d2f8aa8e 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/CommonWorkflows.fs +++ b/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}" [] let ``We don't lose subsequent diagnostics when there's error in one file`` () = diff --git a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/FindReferences.fs b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/FindReferences.fs index ab7eca78ab..d7a5fc860c 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/FindReferences.fs +++ b/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 -> 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) - } \ No newline at end of file + } + +/// https://github.com/dotnet/fsharp/issues/19336 +module ConstructorDuplicateRegression = + + [] + let ``No duplicate ctor symbol for attribute`` () = + let source = """ +type MyAttr() = inherit System.Attribute() + +[] 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)) + + [] + 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 [] 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)) + + [] + let ``No duplicate ctor symbol for generic object expression`` () = + let source = """ +type Base<'T>() = + abstract M1: 'T -> unit + default _.M1 _ = () + +let x = + { new Base() with + override this.M1 _ = () } +""" + checkAllSymbols source (fun _ allUses -> + // Line 8: { new Base() 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)) + + [] + 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)) \ No newline at end of file From 72d6c4946c34772ef5ddfe3ba73356219cecbc7a Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Wed, 25 Feb 2026 19:30:31 +0100 Subject: [PATCH 4/5] Update test expectations: remove #14902 shifted .ctor entries and duplicate attribute entries The PR removes the ForNewConstructors shifted-range hack (#14902) and suspends the sink during checkAttributeTargetsErrors, so tests no longer see those duplicate/corrupted symbol references. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../EditorTests.fs | 3 --- .../ProjectAnalysisTests.fs | 25 ------------------- 2 files changed, 28 deletions(-) diff --git a/tests/FSharp.Compiler.Service.Tests/EditorTests.fs b/tests/FSharp.Compiler.Service.Tests/EditorTests.fs index 89247ca429..6ce135326f 100644 --- a/tests/FSharp.Compiler.Service.Tests/EditorTests.fs +++ b/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)) diff --git a/tests/FSharp.Compiler.Service.Tests/ProjectAnalysisTests.fs b/tests/FSharp.Compiler.Service.Tests/ProjectAnalysisTests.fs index 2b3f5cb89a..ff97a8c7ed 100644 --- a/tests/FSharp.Compiler.Service.Tests/ProjectAnalysisTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/ProjectAnalysisTests.fs @@ -491,9 +491,6 @@ let ``Test project1 all uses of all symbols`` () = "file2", ((18, 6), (18, 18)), ["class"]); ("DefaultValueAttribute", "Microsoft.FSharp.Core.DefaultValueAttribute", "file2", ((18, 6), (18, 18)), ["member"]); - // #14902 - ("``.ctor``", "Microsoft.FSharp.Core.``.ctor``", "file2", - ((18, 7), (18, 18)), ["member"]); ("x", "N.D3.x", "file2", ((19, 16), (19, 17)), ["field"; "default"; "mutable"]); ("D3", "N.D3", "file2", ((15, 5), (15, 7)), ["class"]); @@ -556,16 +553,12 @@ let ``Test project1 all uses of all symbols`` () = ("M", "M", "file2", ((38, 22), (38, 23)), ["module"]); ("C", "M.C", "file2", ((38, 22), (38, 25)), ["class"]); ("C", "M.C", "file2", ((38, 22), (38, 25)), ["member"; "ctor"]); - // #14902 - ("``.ctor``", "M.C.``.ctor``", "file2", ((38, 23), (38, 25)), ["member"; "ctor"]); ("mmmm1", "N.mmmm1", "file2", ((38, 4), (38, 9)), ["val"]); ("M", "M", "file2", ((39, 12), (39, 13)), ["module"]); ("CAbbrev", "M.CAbbrev", "file2", ((39, 12), (39, 21)), ["abbrev"]); ("M", "M", "file2", ((39, 28), (39, 29)), ["module"]); ("CAbbrev", "M.CAbbrev", "file2", ((39, 28), (39, 37)), ["abbrev"]); ("C", "M.C", "file2", ((39, 28), (39, 37)), ["member"; "ctor"]); - // #14902 - ("``.ctor``", "M.C.``.ctor``", "file2", ((39, 29), (39, 37)), ["member"; "ctor"]); ("mmmm2", "N.mmmm2", "file2", ((39, 4), (39, 9)), ["val"]); ("N", "N", "file2", ((1, 7), (1, 8)), ["module"])] @@ -2381,10 +2374,6 @@ let ``Test Project14 all symbols`` () = allUsesOfAllSymbols |> shouldEqual [| - ("StructAttribute", "StructAttribute", "file1", ((4, 2), (4, 8)), ["attribute"]); - ("member .ctor", "StructAttribute", "file1", ((4, 2), (4, 8)), []); - // #14902 - ("member .ctor", "``.ctor``", "file1", ((4, 3), (4, 8)), []); ("StructAttribute", "StructAttribute", "file1", ((4, 2), (4, 8)), ["attribute"]); ("member .ctor", "StructAttribute", "file1", ((4, 2), (4, 8)), []); ("int", "int", "file1", ((5, 9), (5, 12)), ["type"]); @@ -2542,14 +2531,6 @@ let ``Test Project16 all symbols`` () = allUsesOfAllSymbols |> shouldEqual [|("ClassAttribute", "ClassAttribute", "sig1", ((8, 6), (8, 11)), ["attribute"], ["class"]); ("member .ctor", "ClassAttribute", "sig1", ((8, 6), (8, 11)), [], ["member"]); - // #14902 - ("member .ctor", "``.ctor``", "sig1", ((8, 7), (8, 11)), [], ["member"]); - ("ClassAttribute", "ClassAttribute", "sig1", ((8, 6), (8, 11)), ["attribute"], ["class"]); - ("member .ctor", "ClassAttribute", "sig1", ((8, 6), (8, 11)), [], ["member"]); - ("ClassAttribute", "ClassAttribute", "sig1", ((12, 6), (12, 11)), ["attribute"], ["class"]); - ("member .ctor", "ClassAttribute", "sig1", ((12, 6), (12, 11)), [], ["member"]); - // #14902 - ("member .ctor", "``.ctor``", "sig1", ((12, 7), (12, 11)), [], ["member"]); ("ClassAttribute", "ClassAttribute", "sig1", ((12, 6), (12, 11)), ["attribute"], ["class"]); ("member .ctor", "ClassAttribute", "sig1", ((12, 6), (12, 11)), [], ["member"]); ("int", "int", "sig1", ((16, 19), (16, 22)), ["type"], ["abbrev"]); @@ -5177,12 +5158,6 @@ let ``Test Project40 all symbols`` () = ("f", ((4, 4), (4, 5)), ["val"]); ("CompilationRepresentationAttribute", ((6, 2), (6, 27)), ["class"]); ("CompilationRepresentationAttribute", ((6, 2), (6, 27)), ["member"]); - // #14902 - ("``.ctor``", ((6, 3), (6, 27)), ["member"]); - ("CompilationRepresentationFlags", ((6, 28), (6, 58)), ["enum"; "valuetype"]); - ("UseNullAsTrueValue", ((6, 28), (6, 77)), ["field"; "static"; "8"]); - ("CompilationRepresentationAttribute", ((6, 2), (6, 27)), ["class"]); - ("CompilationRepresentationAttribute", ((6, 2), (6, 27)), ["member"]); ("CompilationRepresentationFlags", ((6, 28), (6, 58)), ["enum"; "valuetype"]); ("UseNullAsTrueValue", ((6, 28), (6, 77)), ["field"; "static"; "8"]); ("string", ((9, 11), (9, 17)), ["abbrev"]); From f81b8ffa4a9bdd3a47ca80b4e2827c58da3eb94d Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Thu, 26 Feb 2026 11:10:15 +0100 Subject: [PATCH 5/5] Add release notes for #19336 fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/release-notes/.FSharp.Compiler.Service/10.0.300.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md index 858f45b424..42b0b781a2 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md @@ -15,6 +15,7 @@ * Fixed Find All References on discriminated union cases not including case tester properties (e.g., `.IsCase`). ([Issue #16621](https://github.com/dotnet/fsharp/issues/16621), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Fixed Find All References on record types not including copy-and-update expressions. ([Issue #15290](https://github.com/dotnet/fsharp/issues/15290), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) * Fixed Find All References on constructor definitions not finding all constructor usages. ([Issue #14902](https://github.com/dotnet/fsharp/issues/14902), [PR #19252](https://github.com/dotnet/fsharp/pull/19252)) +* Fixed Find All References producing corrupted duplicate constructor symbol references with shifted ranges, and removed duplicate attribute symbols from type definition error-reporting. ([Issue #19336](https://github.com/dotnet/fsharp/issues/19336), [PR #19358](https://github.com/dotnet/fsharp/pull/19358)) * Fixed semantic classification regression where copy-and-update record fields were colored as type names, and union case tester dot was colored as union case. ([PR #19311](https://github.com/dotnet/fsharp/pull/19311)) * Fix false FS1182 (unused variable) warning for query expression variables used in where, let, join, and select clauses. ([Issue #422](https://github.com/dotnet/fsharp/issues/422)) * Fix FS0229 B-stream misalignment when reading metadata from assemblies compiled with LangVersion < 9.0, introduced by [#17706](https://github.com/dotnet/fsharp/pull/17706). ([PR #19260](https://github.com/dotnet/fsharp/pull/19260))