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 83a1b23aa9f..35cb6319ddc 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md @@ -15,6 +15,7 @@ * Fix release inline optimization, which leads to MethodAccessException if used with `assembly:InternalsVisibleTo`` attribute. ([Issue #16105](https://github.com/dotnet/fsharp/issues/16105), ([PR #16737](https://github.com/dotnet/fsharp/pull/16737)) * Enforce AttributeTargets on let values and functions. ([PR #16692](https://github.com/dotnet/fsharp/pull/16692)) * Enforce AttributeTargets on union case declarations. ([PR #16764](https://github.com/dotnet/fsharp/pull/16764)) +* Disallow using base to invoke an abstract base method. ([Issue #13926](https://github.com/dotnet/fsharp/issues/13926), [PR #16773](https://github.com/dotnet/fsharp/pull/16773)) ### Added diff --git a/docs/release-notes/.Language/8.0.md b/docs/release-notes/.Language/8.0.md index ad829f0a6b9..1e2dc00f2cd 100644 --- a/docs/release-notes/.Language/8.0.md +++ b/docs/release-notes/.Language/8.0.md @@ -1,3 +1,7 @@ -### Added +### Fixed -* `while!` ([Language suggestion #1038](https://github.com/fsharp/fslang-suggestions/issues/1038), [PR #14238](https://github.com/dotnet/fsharp/pull/14238)) \ No newline at end of file +* Disallow using base to invoke an abstract base method ([Issue #13926](https://github.com/dotnet/fsharp/issues/13926), [PR #16773](https://github.com/dotnet/fsharp/pull/16773)) + +### Added + +* `while!` ([Language suggestion #1038](https://github.com/fsharp/fslang-suggestions/issues/1038), [PR #14238](https://github.com/dotnet/fsharp/pull/14238)) diff --git a/src/Compiler/AbstractIL/il.fs b/src/Compiler/AbstractIL/il.fs index 7f303178553..fecefad1434 100644 --- a/src/Compiler/AbstractIL/il.fs +++ b/src/Compiler/AbstractIL/il.fs @@ -5640,10 +5640,8 @@ let resolveILMethodRefWithRescope r (td: ILTypeDef) (mref: ILMethodRef) = possibles |> List.filter (fun md -> mref.CallingConv = md.CallingConv - && - // REVIEW: this uses equality on ILType. For CMOD_OPTIONAL this is not going to be correct - (md.Parameters, argTypes) - ||> List.lengthsEqAndForall2 (fun p1 p2 -> r p1.Type = p2) + && (md.Parameters, argTypes) + ||> List.lengthsEqAndForall2 (fun p1 p2 -> r p1.Type = p2) && // REVIEW: this uses equality on ILType. For CMOD_OPTIONAL this is not going to be correct r md.Return.Type = retType) diff --git a/src/Compiler/Checking/PostInferenceChecks.fs b/src/Compiler/Checking/PostInferenceChecks.fs index e15d9e9463f..852be12aff0 100644 --- a/src/Compiler/Checking/PostInferenceChecks.fs +++ b/src/Compiler/Checking/PostInferenceChecks.fs @@ -1307,14 +1307,14 @@ and CheckILBaseCall cenv env (ilMethRef, enclTypeInst, methInst, retTypes, tyarg match tryTcrefOfAppTy g baseVal.Type with | ValueSome tcref when tcref.IsILTycon -> try - // This is awkward - we have to explicitly re-resolve back to the IL metadata to determine if the method is abstract. - // We believe this may be fragile in some situations, since we are using the Abstract IL code to compare - // type equality, and it would be much better to remove any F# dependency on that implementation of IL type - // equality. It would be better to make this check in tc.fs when we have the Abstract IL metadata for the method to hand. - let mdef = resolveILMethodRef tcref.ILTyconRawMetadata ilMethRef + let mdef = + match tcref.ILTyconInfo with + | TILObjectReprData(scoref, _, _) -> + resolveILMethodRefWithRescope (rescopeILType scoref) tcref.ILTyconRawMetadata ilMethRef + if mdef.IsAbstract then errorR(Error(FSComp.SR.tcCannotCallAbstractBaseMember(mdef.Name), m)) - with _ -> () // defensive coding + with _ -> () | _ -> () CheckTypeInstNoByrefs cenv env m tyargs diff --git a/src/Compiler/FSharp.Compiler.Service.fsproj b/src/Compiler/FSharp.Compiler.Service.fsproj index e1fe24ea421..5c11a4eebbf 100644 --- a/src/Compiler/FSharp.Compiler.Service.fsproj +++ b/src/Compiler/FSharp.Compiler.Service.fsproj @@ -76,8 +76,8 @@ - - + + @@ -365,8 +365,8 @@ - - + + @@ -504,12 +504,12 @@ - - - - - - + + + + + + diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/ImportDeclarations/ImportDeclarations.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/ImportDeclarations/ImportDeclarations.fs index 1f93d4b3bab..851746df35c 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/ImportDeclarations/ImportDeclarations.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/ImportDeclarations/ImportDeclarations.fs @@ -107,4 +107,75 @@ module ImportDeclarations = |> verifyCompileAndRun |> shouldSucceed + [] + let ImplementImportedAbstractBaseMethodsFailsIfUsed ()= + FSharp """ +module Testing +open System.Text.Json +open System.Text.Json.Serialization + +type StringTrimJsonSerializer(o: JsonSerializerOptions) = + inherit JsonConverter() + + override this.Read(reader, _, _) = + match reader.TokenType with + | JsonTokenType.String -> reader.GetString().Trim() + | _ -> JsonException("Type is not a string") |> raise + + override this.Write(writer, objectToWrite, options) = base.Write(writer, objectToWrite, options) + +type SomeType = { AField: string } + +let serialize item = + let options = JsonSerializerOptions() + StringTrimJsonSerializer options |> options.Converters.Add + JsonSerializer.Serialize(item, options) + +[] +let main _ = + { AField = "a" } + |> serialize + |> ignore + 0""" + |> verifyCompile + |> shouldFail + |> withDiagnostics [ + (Error 1201, Line 15, Col 59, Line 15, Col 101, "Cannot call an abstract base member: 'Write'") + ] + + + [] + let ImplementImportedAbstractBaseMethodsFailsIfNotUsed ()= + FSharp """ +module Testing + +open System.Text.Json +open System.Text.Json.Serialization + +type StringTrimJsonSerializer(o: JsonSerializerOptions) = + inherit JsonConverter() + override this.Read(reader, _, _) = + match reader.TokenType with + | JsonTokenType.String -> reader.GetString().Trim() + | _ -> JsonException("Type is not a string") |> raise + override this.Write(writer, objectToWrite, options) = base.Write(writer, objectToWrite, options) + +type SomeType = { AField: int } + +let serialize item = + let options = JsonSerializerOptions() + StringTrimJsonSerializer options |> options.Converters.Add + JsonSerializer.Serialize(item, options) + +[] +let main _ = + { AField = 1 } + |> serialize + |>ignore + 0""" + |> verifyCompile + |> shouldFail + |> withDiagnostics [ + (Error 1201, Line 13, Col 59, Line 13, Col 101, "Cannot call an abstract base member: 'Write'") + ] diff --git a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/SuggestionsTests.fs b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/SuggestionsTests.fs index 50837863d5a..76bf2d3e88c 100644 --- a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/SuggestionsTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/SuggestionsTests.fs @@ -139,12 +139,12 @@ open Collectons [] let ``Suggest Namespaces`` () = FSharp """ -open System.Collectons +open System.Lema """ |> typecheck |> shouldFail - |> withSingleDiagnostic (Error 39, Line 2, Col 13, Line 2, Col 23, - "The namespace 'Collectons' is not defined.") + |> withSingleDiagnostic (Error 39, Line 2, Col 13, Line 2, Col 17, + "The namespace 'Lema' is not defined.") [] let ``Suggest Record Labels`` () = diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index 38937398473..e1704555421 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -19,7 +19,7 @@ $(DefineConstants);DEBUG true - true + true @@ -228,9 +228,9 @@ - - - + + + @@ -280,7 +280,7 @@ - + @@ -313,14 +313,12 @@ - - %(RelativeDir)TestSource\%(Filename)%(Extension) - - - + + %(RelativeDir)TestSource\%(Filename)%(Extension) + %(RelativeDir)\BaseLine\%(Filename)%(Extension) @@ -331,6 +329,7 @@ +