Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
([PR #19724](https://github.com/dotnet/fsharp/pull/19724))
* Emit debug points at a stack-empty position ([PR #19877](https://github.com/dotnet/fsharp/pull/19877))
* Fix spurious XmlDoc warnings (unknown parameter / no documentation for parameter) under `--warnon:3390` when a get/set property documents the full parameter set across both accessors. ([Issue #13684](https://github.com/dotnet/fsharp/issues/13684), [PR #19884](https://github.com/dotnet/fsharp/pull/19884))
* Render the declaring type for C#-style extension methods in overload-resolution error messages so the suggestion no longer falsely attributes the extension to the receiver type. ([Issue #9838](https://github.com/dotnet/fsharp/issues/9838), [PR #19925](https://github.com/dotnet/fsharp/pull/19925))

### Added

Expand Down
35 changes: 23 additions & 12 deletions src/Compiler/Checking/NicePrint.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1698,7 +1698,7 @@ module InfoMemberPrinting =
// That is, this style:
// Container(argName1: argType1, ..., argNameN: argTypeN) : retType
// Container.Method(argName1: argType1, ..., argNameN: argTypeN) : retType
let layoutMethInfoCSharpStyle amap m denv (minfo: MethInfo) minst =
let layoutMethInfoCSharpStyle useDeclaringTypeForCSharpExt amap m denv (minfo: MethInfo) minst =
let retTy = if minfo.IsConstructor then minfo.ApparentEnclosingType else minfo.GetFSharpReturnType(amap, m, minst)
let layout =
if minfo.IsExtensionMember then
Expand All @@ -1707,7 +1707,11 @@ module InfoMemberPrinting =
let layout =
layout ^^
if isAppTy minfo.TcGlobals minfo.ApparentEnclosingAppType then
let tcref = minfo.ApparentEnclosingTyconRef
let tcref =
if useDeclaringTypeForCSharpExt && minfo.IsCSharpStyleExtensionMember then
minfo.DeclaringTyconRef
else
minfo.ApparentEnclosingTyconRef
PrintTypes.layoutTyconRef denv tcref
else
emptyL
Expand Down Expand Up @@ -1781,7 +1785,7 @@ module InfoMemberPrinting =
//
// For C# extension members:
// ApparentContainer.Method(argName1: argType1, ..., argNameN: argTypeN) : retType
let rec prettyLayoutOfMethInfoFreeStyle (infoReader: InfoReader) m denv typarInst methInfo =
let rec prettyLayoutOfMethInfoFreeStyle useDeclaringTypeForCSharpExt (infoReader: InfoReader) m denv typarInst methInfo =
let amap = infoReader.amap

match methInfo with
Expand All @@ -1795,17 +1799,17 @@ module InfoMemberPrinting =
| MethInfoWithModifiedReturnType(ILMeth(_, ilminfo, _) as wrappedInfo,retTy) ->
let prettyTyparInst, prettyMethInfo, minst = prettifyILMethInfo amap m wrappedInfo typarInst ilminfo
let prettyMethInfo = MethInfoWithModifiedReturnType(prettyMethInfo,retTy)
let resL = layoutMethInfoCSharpStyle amap m denv prettyMethInfo minst
let resL = layoutMethInfoCSharpStyle useDeclaringTypeForCSharpExt amap m denv prettyMethInfo minst
prettyTyparInst, resL
| MethInfoWithModifiedReturnType(mi,_) -> prettyLayoutOfMethInfoFreeStyle infoReader m denv typarInst mi
| MethInfoWithModifiedReturnType(mi,_) -> prettyLayoutOfMethInfoFreeStyle useDeclaringTypeForCSharpExt infoReader m denv typarInst mi
| ILMeth(_, ilminfo, _) ->
let prettyTyparInst, prettyMethInfo, minst = prettifyILMethInfo amap m methInfo typarInst ilminfo
let resL = layoutMethInfoCSharpStyle amap m denv prettyMethInfo minst
let resL = layoutMethInfoCSharpStyle useDeclaringTypeForCSharpExt amap m denv prettyMethInfo minst
prettyTyparInst, resL
#if !NO_TYPEPROVIDERS
| ProvidedMeth _ ->
let prettyTyparInst, _ = PrettyTypes.PrettifyInst amap.g typarInst
prettyTyparInst, layoutMethInfoCSharpStyle amap m denv methInfo methInfo.FormalMethodInst
prettyTyparInst, layoutMethInfoCSharpStyle useDeclaringTypeForCSharpExt amap m denv methInfo methInfo.FormalMethodInst
#endif

let prettyLayoutOfPropInfoFreeStyle g amap m denv (pinfo: PropInfo) =
Expand Down Expand Up @@ -1841,8 +1845,8 @@ module InfoMemberPrinting =
let resL = prettyLayoutOfPropInfoFreeStyle g amap m denv pinfo
resL |> bufferL os

let formatMethInfoToBufferFreeStyle amap m denv os (minfo: MethInfo) =
let _, resL = prettyLayoutOfMethInfoFreeStyle amap m denv emptyTyparInst minfo
let formatMethInfoToBufferFreeStyle useDeclaringTypeForCSharpExt amap m denv os (minfo: MethInfo) =
let _, resL = prettyLayoutOfMethInfoFreeStyle useDeclaringTypeForCSharpExt amap m denv emptyTyparInst minfo
resL |> bufferL os

/// Format a method to a layout (actually just containing a string) using "free style" (aka "standalone").
Expand Down Expand Up @@ -2866,18 +2870,25 @@ let stringOfQualifiedValOrMember denv infoReader vref =

/// Convert a MethInfo to a string
let formatMethInfoToBufferFreeStyle infoReader m denv buf d =
InfoMemberPrinting.formatMethInfoToBufferFreeStyle infoReader m denv buf d
InfoMemberPrinting.formatMethInfoToBufferFreeStyle false infoReader m denv buf d

let prettyLayoutOfMethInfoFreeStyle infoReader m denv typarInst minfo =
InfoMemberPrinting.prettyLayoutOfMethInfoFreeStyle infoReader m denv typarInst minfo
InfoMemberPrinting.prettyLayoutOfMethInfoFreeStyle false infoReader m denv typarInst minfo

/// Convert a PropInfo to a string
let prettyLayoutOfPropInfoFreeStyle g amap m denv d =
InfoMemberPrinting.prettyLayoutOfPropInfoFreeStyle g amap m denv d

/// Convert a MethInfo to a string
let stringOfMethInfo infoReader m denv minfo =
buildString (fun buf -> InfoMemberPrinting.formatMethInfoToBufferFreeStyle infoReader m denv buf minfo)
buildString (fun buf -> InfoMemberPrinting.formatMethInfoToBufferFreeStyle false infoReader m denv buf minfo)

/// Convert a MethInfo to a string, suitable for the "Available overloads" list
/// in overload-resolution error messages. For C#-style extension methods, the
/// rendering uses the extension's declaring type rather than the receiver type,
/// so the message is not misleading (issue dotnet/fsharp#9838).
let stringOfMethInfoForOverloadError infoReader m denv minfo =
buildString (fun buf -> InfoMemberPrinting.formatMethInfoToBufferFreeStyle true infoReader m denv buf minfo)

let stringOfMethInfoFSharpStyle infoReader m denv minfo =
InfoMemberPrinting.layoutMethInfoFSharpStyle infoReader m denv minfo
Expand Down
7 changes: 7 additions & 0 deletions src/Compiler/Checking/NicePrint.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ val prettyLayoutOfPropInfoFreeStyle:

val stringOfMethInfo: infoReader: InfoReader -> m: range -> denv: DisplayEnv -> minfo: MethInfo -> string

/// Convert a MethInfo to a string, suitable for the "Available overloads" list
/// in overload-resolution error messages. For C#-style extension methods, the
/// rendering uses the extension's declaring type rather than the receiver type,
/// so the message is not misleading (issue dotnet/fsharp#9838).
val stringOfMethInfoForOverloadError:
infoReader: InfoReader -> m: range -> denv: DisplayEnv -> minfo: MethInfo -> string

/// Convert a MethInfo to a F# signature
val stringOfMethInfoFSharpStyle: infoReader: InfoReader -> m: range -> denv: DisplayEnv -> minfo: MethInfo -> string

Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Driver/CompilerDiagnostics.fs
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,7 @@ type Exception with
sprintf " // %s" nameOrOneBasedIndexMessage
| _ -> ""

(NicePrint.stringOfMethInfo x.infoReader m displayEnv x.methodSlot.Method)
(NicePrint.stringOfMethInfoForOverloadError x.infoReader m displayEnv x.methodSlot.Method)
+ paramInfo

let nl = Environment.NewLine
Expand Down
167 changes: 167 additions & 0 deletions tests/FSharp.Compiler.ComponentTests/ErrorMessages/Issue9838Tests.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
module ErrorMessages.Issue9838Tests

open Xunit
open FSharp.Test
open FSharp.Test.Compiler

// https://github.com/dotnet/fsharp/issues/9838
// When an F# program calls a C#-style extension method on the wrong receiver type,
// the overload-error message should render the extension's declaring type
// (e.g. `MemoryExtensions`), not the receiver type (e.g. `Foo`).
//
// The Span<T> / MemoryExtensions APIs used by the RED tests below ship in the BCL on
// .NET (Core), but on .NET Framework (net48) they live in the optional System.Memory
// NuGet package which the test sandbox does not reference. We therefore gate those
// tests on NETCOREAPP so they exercise the actual overload-resolution diagnostic
// rather than failing with FS0039 "The value or constructor 'Span' is not defined".

[<FactForNETCOREAPP>]
let ``Issue 9838 - C#-style extension method overload error shows declaring type, not receiver`` () =
FSharp """
open System
type Foo() = class end
let foo = Foo()
let span = Span<int>.Empty
foo.CopyTo(span)
"""
|> typecheck
|> shouldFail
|> withDiagnostics
[ (Error 41, Line 6, Col 5, Line 6, Col 11,
"No overloads match for method 'CopyTo'.

Known type of argument: Span<int>

Available overloads:
- (extension) MemoryExtensions.CopyTo<'T>(destination: Memory<'T>) : unit
- (extension) MemoryExtensions.CopyTo<'T>(destination: Span<'T>) : unit") ]

[<FactForNETCOREAPP>]
let ``Issue 9838 - error message mentions declaring type MemoryExtensions`` () =
FSharp """
open System
type Foo() = class end
let foo = Foo()
let span = Span<int>.Empty
foo.CopyTo(span)
"""
|> typecheck
|> shouldFail
|> withDiagnosticMessageMatches "\\(extension\\) MemoryExtensions\\.CopyTo"

[<FactForNETCOREAPP>]
let ``Issue 9838 - error message must not mislead by mentioning Foo.CopyTo`` () =
let result =
FSharp """
open System
type Foo() = class end
let foo = Foo()
let span = Span<int>.Empty
foo.CopyTo(span)
"""
|> typecheck

result |> shouldFail |> ignore

let allMessages =
result.Output.Diagnostics
|> List.map (fun d -> d.Message)
|> String.concat "\n"

Assert.DoesNotContain("Foo.CopyTo", allMessages)
Assert.Contains("MemoryExtensions.CopyTo", allMessages)

[<Fact>]
let ``F#-style extension member overload error keeps current rendering`` () =
FSharp """
type Foo() = class end

module FooExtensions =
type Foo with
member _.Bar(_: int) = ()
member _.Bar(_: double) = ()

open FooExtensions

let foo = Foo()
foo.Bar("")
"""
|> typecheck
|> shouldFail
|> withDiagnosticMessageMatches "member Foo\\.Bar"

[<Fact>]
let ``Regular instance method overload error is unaffected by issue 9838 fix`` () =
FSharp """
type T() =
member _.M(_: int) = ()
member _.M(_: double) = ()

let t = T()
t.M("")
"""
|> typecheck
|> shouldFail
|> withDiagnostics
[ (Error 41, Line 7, Col 3, Line 7, Col 4,
"No overloads match for method 'M'.

Known type of argument: string

Available overloads:
- member T.M: double -> unit // Argument at index 1 doesn't match
- member T.M: int -> unit // Argument at index 1 doesn't match") ]

[<FactForNETCOREAPP>]
let ``Issue 9838 - C#-style extension on generic receiver mismatch shows declaring type`` () =
FSharp """
open System
open System.Collections.Generic
let xs = List<int>()
let span = Span<string>.Empty
xs.CopyTo(span)
"""
|> typecheck
|> shouldFail
|> withDiagnosticMessageMatches "\\(extension\\) MemoryExtensions\\.CopyTo"

(*
* Captured RED-phase failure output (net10.0) - kept as a reference for the
* implementation sprint. Each RED test asserts the desired post-fix wording
* (`(extension) MemoryExtensions.CopyTo<'T>(...)`) and currently FAILS because
* the compiler renders the *receiver* type instead of the extension's
* declaring type. Once the fix lands the actual text below should change to
* match the expected text and the RED tests will go green.
*
* Test: Issue 9838 - C#-style extension method overload error shows declaring type, not receiver
* Expected (post-fix):
* "No overloads match for method 'CopyTo'.
* Known type of argument: Span<int>
* Available overloads:
* - (extension) MemoryExtensions.CopyTo<'T>(destination: Memory<'T>) : unit // Argument 'destination' doesn't match
* - (extension) MemoryExtensions.CopyTo<'T>(destination: Span<'T>) : unit // Argument 'destination' doesn't match"
* Actual (current bug):
* "No overloads match for method 'CopyTo'.
* Known type of argument: Span<int>
* Available overloads:
* - (extension) Foo.CopyTo<'T>(destination: Memory<'T>) : unit
* - (extension) Foo.CopyTo<'T>(destination: Span<'T>) : unit"
*
* Test: Issue 9838 - error message mentions declaring type MemoryExtensions
* regex "\(extension\) MemoryExtensions\.CopyTo" does NOT match actual text
* "(extension) Foo.CopyTo<'T>(destination: ...)".
*
* Test: Issue 9838 - error message must not mislead by mentioning Foo.CopyTo
* Assert.DoesNotContain("Foo.CopyTo", ...) fails because the diagnostic
* contains "(extension) Foo.CopyTo<'T>(destination: Memory<'T>)".
*
* Test: Issue 9838 - C#-style extension on generic receiver mismatch shows declaring type
* Expected regex "\(extension\) MemoryExtensions\.CopyTo" against actual:
* "(extension) List.CopyTo<'T>(destination: Span<'T>) : unit ..." - the
* extensions are attributed to the receiver `List` rather than to
* `MemoryExtensions`.
*
* GATE tests (F#-style extension and regular instance method) PASS today and
* must continue to pass after the fix, guaranteeing the fix is scoped to
* C#-style (`[<Extension>]`) extension overload rendering only.
*)
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@
<Compile Include="ErrorMessages\ModuleTests.fs" />
<Compile Include="ErrorMessages\DiagnosticRegressionTests.fs" />
<Compile Include="ErrorMessages\OverloadResolutionErrorRangeTests.fs" />
<Compile Include="ErrorMessages\Issue9838Tests.fs" />
<Compile Include="ErrorMessages\NameResolutionTests.fs" />
<Compile Include="ErrorMessages\SuggestionsTests.fs" />
<Compile Include="ErrorMessages\TypeMismatchTests.fs" />
Expand Down
Loading