Skip to content

Fix #19664: Static extension overload resolution for generic types #19736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
T-Gro wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix #19664: Static extension overload resolution for generic types #19736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1215ed4
Add failing regression test for issue #19664 (static extension overlo…
T-Gro May 12, 2026
3dcc376
Fix #19664: include static extensions when resolving Type<args>.Membe…
T-Gro May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/11.0.100.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
* Fix parallel compilation of scripts ([PR #19649](https://github.com/dotnet/fsharp/pull/19649))
* Fix parser recovery, name resolution, and code completion for unfinished enum patterns ([PR #19708](https://github.com/dotnet/fsharp/pull/19708))
* Parser: fix unexpected diagnostics in debug builds, improve error messages ([PR #19730](https://github.com/dotnet/fsharp/pull/19730))
* Fix overload resolution of static member extension on generic types when the call uses explicit type arguments and an intrinsic overload of the same name exists. ([Issue #19664](https://github.com/dotnet/fsharp/issues/19664), [PR #19736](https://github.com/dotnet/fsharp/pull/19736))

### Added

Expand Down
14 changes: 14 additions & 0 deletions src/Compiler/Checking/NameResolution.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2874,6 +2874,20 @@ let rec ResolveLongIdentInTypePrim (ncenv: NameResolver) nenv lookupKind (resInf
| Some(MethodItem msets) when isLookUpExpr ->
let minfos = msets |> ExcludeHiddenOfMethInfos g ncenv.amap m

// If the intrinsic candidate set already disagrees with the strict
Copy link
Copy Markdown
Member

@abonie abonie May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Type System - low/medium] The trigger condition is computed after ExcludeHiddenOfMethInfos (line 2875). If an intrinsic overload of the name is hidden by a derived override, minfos will not reflect what is actually present on the type, and the relaxation can fail to fire (false negative) or fire spuriously (false positive) relative to what overload resolution will see. At minimum, leave a comment explaining why hidden-exclusion is the right input for the trigger check; ideally evaluate against the unfiltered msets for the trigger, while still emitting minfos into the meth group.

// instance/static filter (e.g. expression-form `Type<TArg>.Member` is
// resolved as a dot-expression with LookupIsInstance.Yes but yields
// static intrinsic members), relax the filter for the extension lookup
// so static extension members are also considered. See issue #19664.
let isInstanceFilter =
Copy link
Copy Markdown
Member

@abonie abonie May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Type System / Name Resolution - design, medium] This change patches the symptom at the candidate-gathering site, not the root cause. The dot-expression Type<TArg>.Member is classified as LookupIsInstance.Yes by ResolveExprDotLongIdent (line 4185) even when the LHS denotes a type, not an instance. The adhocDotSearchAll fallback at line 4204 already retries with Ambivalent - but only when the first lookup fails. When intrinsic statics exist for the name, the first lookup succeeds with a meth group that excludes static extensions, the fallback is skipped, and overload resolution later raises FS0505.

Consider whether the right fix is upstream: either (i) classify Type<TArg>.Member as Ambivalent in ResolveExprDotLongIdent when the LHS resolves to a type expression, or (ii) take the fallback path whenever the successful result yields no compatible overload. The current fix permanently couples extension-filter relaxation to the shape of the intrinsic set, which is fragile (see the ExcludeHiddenOfMethInfos comment) and silently changes behavior on the No path that has nothing to do with this bug.

match isInstanceFilter with
| LookupIsInstance.Ambivalent -> isInstanceFilter
| LookupIsInstance.Yes when minfos |> List.exists (fun m -> not m.IsInstance) ->
LookupIsInstance.Ambivalent
| LookupIsInstance.No when minfos |> List.exists (fun m -> m.IsInstance) ->
Copy link
Copy Markdown
Member

@abonie abonie May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Overload Resolution / Test Coverage - Behavioral, medium] This LookupIsInstance.No arm relaxes the extension filter on every module-qualified or unqualified expression resolution (entry points at lines 3063 and 3199) whenever the intrinsic candidate set for the name contains any instance member. That is a much broader trigger than the reported bug (#19664 only concerns Type<TArg>.Member via ResolveExprDotLongIdent, which uses LookupIsInstance.Yes).

Consequences worth justifying:

  1. For a call like M.Foo(x) where T.Foo has both static and instance intrinsic overloads, in-scope instance extensions named Foo will now be added to the candidate set. Previously they were filtered out. Overload resolution should reject them on receiver-shape mismatch in well-typed code, but the candidate-set expansion can change error wording (FS0041 ambiguity instead of FS0501) and can in principle change selection in SRTP / weak inference contexts.
  2. There is no test covering this arm. Per the repo's review rules (every behavioral change has a corresponding test), please either (a) add a positive + negative test for the No arm, or (b) restrict the relaxation to the Yes case actually motivated by the issue and leave a TODO for No.

The Yes-only fix is the minimal change required to close #19664; expanding to No is a latent behavior change without coverage.

LookupIsInstance.Ambivalent
| _ -> isInstanceFilter

Copy link
Copy Markdown
Member

@abonie abonie May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Diagnostic Quality - low] Worth verifying: when no overload matches after this relaxation, does the diagnostic still point at the same set of candidates as before? FS0501 (member does not take N arguments) vs FS0505 (is not a static method) vs FS0041 (overload ambiguity) can swap depending on the meth-group contents. Add a negative test (e.g., StaticGeneric<int>.Bar(1, 2, 3) - no overload of arity 3) and pin the expected diagnostic so future regressions surface.

// fold the available extension members into the overload resolution
let extensionMethInfos = ExtensionMethInfosOfTypeInScope ResultCollectionSettings.AllResults ncenv.InfoReader nenv ad optFilter isInstanceFilter m ty

Expand Down
42 changes: 42 additions & 0 deletions ...omponentTests/Conformance/BasicGrammarElements/MethodResolution/StaticMethodResolution.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information.

namespace Conformance.BasicGrammarElements

open FSharp.Test.Compiler
open Xunit

module StaticMethodResolution =

// Regression test for https://github.com/dotnet/fsharp/issues/19664
//
// When a static extension method is defined in a *different* [<AutoOpen>] module than
// the generic type it extends, and shares its name with an intrinsic static member,
// resolving the call via the explicit-type-argument syntax `Type<TArg>.Member(...)`
// previously failed with FS0505. The non-generic dotted form `Type.Member(...)`
// resolved correctly, so any regression test that omits the explicit type arguments
// does not actually exercise the bug. See the discussion at
// https://github.com/dotnet/fsharp/issues/19675#issuecomment-4373059900.
[<Fact>]
let ``Static extension on generic type resolves with explicit type arguments (issue 19664)`` () =
Fsx """
module Extensions =

type StaticGeneric<'T> =
static member Bar() = ()
static member Bar(_: int, _: int) = ()

[<AutoOpen>]
module StaticGenericExtensions =
type StaticGeneric<'T> with
static member Bar(_: int) = ()

module Program =
open Extensions

StaticGeneric<int>.Bar() // intrinsic, 0 args
StaticGeneric<int>.Bar(42) // regressed: extension, 1 arg, see issue 19664 (FS0505)
StaticGeneric<int>.Bar(42, 0) // intrinsic, 2 args
"""
|> withOptions ["--nowarn:1125"]
Copy link
Copy Markdown
Member

@abonie abonie May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Test Coverage - medium] --nowarn:1125 suppresses FS1125 The instantiation of the generic type '%s' is missing and can't be inferred.... Every call site in this test passes explicit type arguments (StaticGeneric<int>.Bar(...)), so FS1125 should not fire - and if it does, that itself is suspicious. It may indicate that the extension call is binding against a freshened type variable rather than the user-written int.

Please:

  1. Remove --nowarn:1125 and confirm the test still passes. If it fails, FS1125 firing on the extension overload needs investigation before merging - it suggests the fix lets the call through name resolution but downstream inference treats the generic instantiation as missing.
  2. If FS1125 is genuinely required, add an inline comment explaining why, and pin it with withDiagnostics as an expected warning rather than a silent suppression.

Copy link
Copy Markdown
Contributor

@evgTSV evgTSV May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that test case --nowarn:1125 is exactly redundant. Before some changes it supressed a warn for the call StaticGeneric.Bar(42) (without explicit generic).

|> typecheck
Copy link
Copy Markdown
Member

@abonie abonie May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Test Coverage - medium] Test gaps to fill before merge:

  • Ambiguity coverage: add a case where an instance extension and a static intrinsic share name and arity, to confirm the relaxation does not introduce a spurious FS0041. E.g. type StaticGeneric<'T> with member _.Bar(_: int) = () (instance extension) alongside the static intrinsic, called as StaticGeneric<int>.Bar(42) - must still produce a deterministic, well-worded diagnostic.
  • Symmetric LookupIsInstance.No arm: the fix touches both arms but no test exercises the No arm - module-qualified M.T.Foo(...) where T.Foo is a static intrinsic with an in-scope instance extension Foo.
  • IDE path: ResolveExprDotLongIdent is also called from quick-info / completion paths. A FSharpChecker-level smoke test that quick-info on StaticGeneric<int>.Bar returns the merged overload group would lock the IDE-visible behavior.
  • Non-generic but same shape: confirm Type.Member (no type args) continues to resolve identically - the same code path is hit.

|> shouldSucceed
1 change: 1 addition & 0 deletions tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
<Compile Include="Conformance\BasicGrammarElements\MemberDefinitions\OptionalDefaultParamArgs\OptionalDefaultParamArgs.fs" />
<Compile Include="Conformance\BasicGrammarElements\MemberDefinitions\OverloadingMembers\OverloadingMembers.fs" />
<Compile Include="Conformance\BasicGrammarElements\MethodResolution\MethodResolution.fs" />
<Compile Include="Conformance\BasicGrammarElements\MethodResolution\StaticMethodResolution.fs" />
<Compile Include="Conformance\BasicGrammarElements\ModuleAbbreviations\ModuleAbbreviations.fs" />
<Compile Include="Conformance\BasicGrammarElements\ModuleDefinitions\ModuleDefinitions.fs" />
<Compile Include="Conformance\BasicGrammarElements\NamespaceDeclGroups\NamespaceDeclGroups.fs" />
Expand Down
Loading