From 015a59cdadf068fea6a5fd8d029d43690b7d0a31 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Thu, 4 Apr 2024 15:40:47 +0100 Subject: [PATCH 1/4] Improve error reporting: ambiguous override method in object expression --- src/Compiler/Checking/MethodOverrides.fs | 7 +++-- .../ObjectExpressions/ObjectExpressions.fs | 30 ++++++++++++++++++- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/Compiler/Checking/MethodOverrides.fs b/src/Compiler/Checking/MethodOverrides.fs index 5abf08578f..d45eff06b0 100644 --- a/src/Compiler/Checking/MethodOverrides.fs +++ b/src/Compiler/Checking/MethodOverrides.fs @@ -416,7 +416,7 @@ module DispatchSlotChecking = fail(Error(FSComp.SR.typrelMemberDoesNotHaveCorrectNumberOfTypeParameters(FormatOverride denv overrideBy, FormatMethInfoSig g amap m denv dispatchSlot), overrideBy.Range)) elif not (IsTyparKindMatch compiledSig overrideBy) then fail(Error(FSComp.SR.typrelMemberDoesNotHaveCorrectKindsOfGenericParameters(FormatOverride denv overrideBy, FormatMethInfoSig g amap m denv dispatchSlot), overrideBy.Range)) - else + else fail(Error(FSComp.SR.typrelMemberCannotImplement(FormatOverride denv overrideBy, NicePrint.stringOfMethInfo infoReader m denv dispatchSlot, FormatMethInfoSig g amap m denv dispatchSlot), overrideBy.Range)) | overrideBy :: _ -> errorR(Error(FSComp.SR.typrelOverloadNotFound(FormatMethInfoSig g amap m denv dispatchSlot, FormatMethInfoSig g amap m denv dispatchSlot), overrideBy.Range)) @@ -427,8 +427,9 @@ module DispatchSlotChecking = else // Error will be reported below in CheckOverridesAreAllUsedOnce () - | _ -> - fail(Error(FSComp.SR.typrelOverrideWasAmbiguous(FormatMethInfoSig g amap m denv dispatchSlot), m)) + | possibleOverrides -> + let overrideBy = List.head possibleOverrides + fail(Error(FSComp.SR.typrelOverrideWasAmbiguous(FormatMethInfoSig g amap overrideBy.Range denv dispatchSlot), overrideBy.Range)) | _ -> fail(Error(FSComp.SR.typrelMoreThenOneOverride(FormatMethInfoSig g amap m denv dispatchSlot), m)) if missingOverloadImplementation.Count > 0 then diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Expressions/ObjectExpressions/ObjectExpressions.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Expressions/ObjectExpressions/ObjectExpressions.fs index 03e543491a..a53252356d 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Expressions/ObjectExpressions/ObjectExpressions.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Expressions/ObjectExpressions/ObjectExpressions.fs @@ -330,4 +330,32 @@ consoleLogger.Log("Hello World") """ |> withLangVersion80 |> compileExeAndRun - |> shouldSucceed \ No newline at end of file + |> shouldSucceed + + [] + let ``Error reporting ambiguous override method in object expression`` () = + Fsx """ +type IExample = + abstract member Overloaded : string -> bool + abstract member Overloaded : int -> bool + +let failingExample x = + { new IExample with + member __.Overloaded (_ : string) = x + member __.Overloaded (_ : int) = x } + """ + |> typecheck + |> shouldFail + |> withDiagnostics [ + (Error 3213, Line 8, Col 19, Line 8, Col 29, "The member 'Overloaded: string -> 'a' matches multiple overloads of the same method. +Please restrict it to one of the following: + Overloaded: int -> bool + Overloaded: string -> bool.") + (Error 3213, Line 9, Col 19, Line 9, Col 29, "The member 'Overloaded: int -> 'a' matches multiple overloads of the same method. +Please restrict it to one of the following: + Overloaded: int -> bool + Overloaded: string -> bool.") + (Error 358, Line 8, Col 19, Line 8, Col 29, "The override for 'Overloaded: int -> bool' was ambiguous") + (Error 358, Line 8, Col 19, Line 8, Col 29, "The override for 'Overloaded: string -> bool' was ambiguous") + (Error 783, Line 7, Col 11, Line 7, Col 19, "At least one override did not correctly implement its corresponding abstract member") + ] From b6b2a77a20b8f295e7d6f3dcf78b4bf0f40fb786 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Thu, 4 Apr 2024 15:50:10 +0100 Subject: [PATCH 2/4] release-notes --- docs/release-notes/.FSharp.Compiler.Service/8.0.300.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md index a3aa647689..d015536794 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md @@ -1,6 +1,7 @@ ### Fixed * Fix a false positive of the `[]` analysis in combination with `yield!`. ([PR #16933](https://github.com/dotnet/fsharp/pull/16933)) +* Improve error reporting: ambiguous override method in object expression. ([PR #16985](https://github.com/dotnet/fsharp/pull/16985)) * Don't blow the stack when traversing deeply nested sequential expressions. ([PR #16882](https://github.com/dotnet/fsharp/pull/16882)) * Fix wrong range start of INTERP_STRING_END. ([PR #16774](https://github.com/dotnet/fsharp/pull/16774), [PR #16785](https://github.com/dotnet/fsharp/pull/16785)) * Fix missing warning for recursive calls in list comprehensions. ([PR #16652](https://github.com/dotnet/fsharp/pull/16652)) From 9e76d134929dd62e44b5bbb7b0225cbee788b59c Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Thu, 4 Apr 2024 17:55:47 +0100 Subject: [PATCH 3/4] update tests --- src/Compiler/Checking/MethodOverrides.fs | 6 +++--- .../fsharp/Compiler/Language/DefaultInterfaceMemberTests.fs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Compiler/Checking/MethodOverrides.fs b/src/Compiler/Checking/MethodOverrides.fs index d45eff06b0..6151fcaea9 100644 --- a/src/Compiler/Checking/MethodOverrides.fs +++ b/src/Compiler/Checking/MethodOverrides.fs @@ -427,9 +427,9 @@ module DispatchSlotChecking = else // Error will be reported below in CheckOverridesAreAllUsedOnce () - | possibleOverrides -> - let overrideBy = List.head possibleOverrides - fail(Error(FSComp.SR.typrelOverrideWasAmbiguous(FormatMethInfoSig g amap overrideBy.Range denv dispatchSlot), overrideBy.Range)) + | ambiguousOverrides -> + let ambiguousOverride = List.head ambiguousOverrides + fail(Error(FSComp.SR.typrelOverrideWasAmbiguous(FormatMethInfoSig g amap ambiguousOverride.Range denv dispatchSlot), ambiguousOverride.Range)) | _ -> fail(Error(FSComp.SR.typrelMoreThenOneOverride(FormatMethInfoSig g amap m denv dispatchSlot), m)) if missingOverloadImplementation.Count > 0 then diff --git a/tests/fsharp/Compiler/Language/DefaultInterfaceMemberTests.fs b/tests/fsharp/Compiler/Language/DefaultInterfaceMemberTests.fs index 289d171226..e202ddc50c 100644 --- a/tests/fsharp/Compiler/Language/DefaultInterfaceMemberTests.fs +++ b/tests/fsharp/Compiler/Language/DefaultInterfaceMemberTests.fs @@ -938,7 +938,7 @@ type Test2 () = CompilerAssert.CompileWithErrors(fsCmpl, [| (FSharpDiagnosticSeverity.Error, 3350, (10, 15, 10, 22), "Feature 'default interface member consumption' is not available in F# 4.6. Please use language version " + targetVersion + " or greater.") - (FSharpDiagnosticSeverity.Error, 358, (10, 15, 10, 22), "The override for 'M<'U> : 'U * int -> unit' was ambiguous") + (FSharpDiagnosticSeverity.Error, 358, (14, 19, 14, 20), "The override for 'M<'U> : 'U * int -> unit' was ambiguous") |]) #else From 12246851805a9913a86e1c48dca140a480783e8a Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Fri, 5 Apr 2024 09:24:39 +0100 Subject: [PATCH 4/4] PR feedback --- src/Compiler/Checking/MethodOverrides.fs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Compiler/Checking/MethodOverrides.fs b/src/Compiler/Checking/MethodOverrides.fs index 6151fcaea9..b166d7fc07 100644 --- a/src/Compiler/Checking/MethodOverrides.fs +++ b/src/Compiler/Checking/MethodOverrides.fs @@ -427,8 +427,7 @@ module DispatchSlotChecking = else // Error will be reported below in CheckOverridesAreAllUsedOnce () - | ambiguousOverrides -> - let ambiguousOverride = List.head ambiguousOverrides + | ambiguousOverride :: _ -> fail(Error(FSComp.SR.typrelOverrideWasAmbiguous(FormatMethInfoSig g amap ambiguousOverride.Range denv dispatchSlot), ambiguousOverride.Range)) | _ -> fail(Error(FSComp.SR.typrelMoreThenOneOverride(FormatMethInfoSig g amap m denv dispatchSlot), m))