From 12ae67efc46505525471ad5655c49330baf45f7a Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 29 May 2026 13:15:49 +0200 Subject: [PATCH 01/10] [RED] Add failing tests for issue #16432 - single FS0039 on inherit of unknown type Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../InheritsDeclarations.fs | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/ClassTypes/InheritsDeclarations/InheritsDeclarations.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/ClassTypes/InheritsDeclarations/InheritsDeclarations.fs index ab983ded7ab..03a40c24131 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/ClassTypes/InheritsDeclarations/InheritsDeclarations.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/ClassTypes/InheritsDeclarations/InheritsDeclarations.fs @@ -56,3 +56,90 @@ module InheritsDeclarations = |> ignoreWarnings |> compile |> shouldSucceed + + // Regression tests for https://github.com/dotnet/fsharp/issues/16432: + // inheriting from an unknown type should emit FS0039 only once, + // not three times (once per name-resolution site in CheckDeclarations). + + [] + let ``Inherit nonexistent type reports single FS0039`` () = + let result = + FSharp """ +type MyClass() = + inherit NonExistentBase() +""" + |> typecheck + + let fs39 = + result.Output.Diagnostics + |> List.filter (fun d -> d.Error = (Error 39)) + + Assert.True( + fs39.Length = 1, + sprintf "Expected exactly 1 FS0039 but got %d. Diagnostics:\n%A" fs39.Length result.Output.Diagnostics + ) + + [] + let ``Two different undefined names still report separately`` () = + let result = + FSharp """ +type MyClass() = + inherit NonExistentBase() + member _.X = undefinedValue +""" + |> typecheck + + let fs39 = + result.Output.Diagnostics + |> List.filter (fun d -> d.Error = (Error 39)) + + Assert.True( + fs39.Length >= 2, + sprintf "Expected >= 2 FS0039, got %d. Diagnostics:\n%A" fs39.Length result.Output.Diagnostics + ) + + [] + let ``Inherit with generic nonexistent type single error`` () = + let result = + FSharp """ +type MyClass() = + inherit MissingGeneric() +""" + |> typecheck + + let fs39 = + result.Output.Diagnostics + |> List.filter (fun d -> d.Error = (Error 39)) + + Assert.True( + fs39.Length = 1, + sprintf "Expected exactly 1 FS0039 but got %d. Diagnostics:\n%A" fs39.Length result.Output.Diagnostics + ) + + [] + let ``Valid inherit produces no FS0039`` () = + FSharp """ +open System +type MyClass() = + inherit Exception("test") +""" + |> typecheck + |> shouldSucceed + + [] + let ``Interface inherit nonexistent single error`` () = + let result = + FSharp """ +type IMyInterface = + inherit INonExistent +""" + |> typecheck + + let fs39 = + result.Output.Diagnostics + |> List.filter (fun d -> d.Error = (Error 39)) + + Assert.True( + fs39.Length = 1, + sprintf "Expected exactly 1 FS0039 but got %d. Diagnostics:\n%A" fs39.Length result.Output.Diagnostics + ) From 57ffff0ff642334d66c716f7ccbb829140c7a9e5 Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 29 May 2026 13:21:17 +0200 Subject: [PATCH 02/10] fix(checker): dedup FS0039 within a single inherit clause (#16432) Multiple typecheck passes (EstablishTypeDefinitionCores FirstPass and SecondPass, plus Phase2AInherit's TcType/TcNewExpr) each emit the same FS0039 for an undefined base type in an 'inherit'/'interface inherit' clause, surfacing three identical diagnostics. Introduce a DedupInheritDiagnosticsLogger overlay scoped via UseTransformedDiagnosticsLogger to TcMutRecDefinitions. The dedup key is the formatted message of UndefinedName so duplicate FS0039s for the same identifier collapse to one while unrelated diagnostics, including FS0039 for distinct identifiers, pass through unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Compiler/Checking/CheckDeclarations.fs | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/Compiler/Checking/CheckDeclarations.fs b/src/Compiler/Checking/CheckDeclarations.fs index 4a4361e9808..20919675fdc 100644 --- a/src/Compiler/Checking/CheckDeclarations.fs +++ b/src/Compiler/Checking/CheckDeclarations.fs @@ -48,6 +48,30 @@ open FSharp.Compiler.TypeRelations open FSharp.Compiler.TypeProviders #endif +/// Wraps an inner DiagnosticsLogger and suppresses subsequent identical +/// 'UndefinedName' (FS0039) errors raised for the same identifier text within +/// its lifetime. This addresses the case where an undefined base type in an +/// 'inherit' / 'interface inherit' clause is reported multiple times because +/// the type-establishment pipeline resolves the same SynType in several passes +/// (FirstPass / SecondPass / Phase2AInherit). The key is the formatted +/// diagnostic message so all other diagnostics, and FS0039 emissions for +/// distinct identifiers, pass through unchanged. +type private DedupInheritDiagnosticsLogger(inner: DiagnosticsLogger) = + inherit DiagnosticsLogger("DedupInheritDiagnosticsLogger") + let seen = HashSet() + + let rec isDuplicateUndefinedName (e: exn) = + match e with + | UndefinedName _ -> not (seen.Add e.Message) + | WrappedError(inner, _) -> isDuplicateUndefinedName inner + | _ -> false + + override _.DiagnosticSink(diagnostic: PhasedDiagnostic) = + if not (isDuplicateUndefinedName diagnostic.Exception) then + inner.DiagnosticSink diagnostic + + override _.ErrorCount = inner.ErrorCount + type cenv = TcFileState //------------------------------------------------------------------------- @@ -4611,6 +4635,16 @@ module TcDeclarations = let g = cenv.g + // Suppress duplicate FS0039 ('UndefinedName') diagnostics emitted across the + // multiple establishment passes for a mutually recursive type-definition group. + // For example, an undefined base type in an 'inherit' / 'interface inherit' + // clause is otherwise reported once each in FirstPass, SecondPass and the + // Phase2AInherit member-checking pass. Dedup is keyed by the formatted + // 'UndefinedName' message so unrelated diagnostics pass through unchanged. + use _ = + UseTransformedDiagnosticsLogger(fun inner -> + DedupInheritDiagnosticsLogger(inner) :> DiagnosticsLogger) + // Split the definitions into "core representations" and "members". The code to process core representations // is shared between processing of signature files and implementation files. let mutRecDefnsAfterSplit = mutRecDefns |> MutRecShapes.mapTycons (fun i -> SplitTyconDefn g i) From c2735ab02dadcf3248d1284763c214111ff200f1 Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 29 May 2026 14:16:08 +0200 Subject: [PATCH 03/10] [GREEN] Dedup FS0039 across inherit-clause type-checking passes Adds a cenv-scoped `(range, idText)` dedup of `UndefinedName` diagnostics applied around Phase 1F inherit type checking and Phase 2A `Phase2AInherit` processing in CheckDeclarations.fs. Resolves the triple-reporting in dotnet/fsharp#16432 without changing the surface API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Compiler/Checking/CheckBasics.fs | 7 ++++ src/Compiler/Checking/CheckBasics.fsi | 6 ++++ src/Compiler/Checking/CheckDeclarations.fs | 38 +++++++++------------- 3 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/Compiler/Checking/CheckBasics.fs b/src/Compiler/Checking/CheckBasics.fs index d8fdd288af3..44fe20b4949 100644 --- a/src/Compiler/Checking/CheckBasics.fs +++ b/src/Compiler/Checking/CheckBasics.fs @@ -313,6 +313,12 @@ type TcFileState = argInfoCache: ConcurrentDictionary + /// Tracks (range, identifier-text) keys of `UndefinedName` diagnostics that have already + /// been reported during this cenv's lifetime so they can be deduplicated when the same + /// identifier is re-resolved by multiple passes (e.g. Phase 1F vs. Phase 2A of an + /// `inherit` clause). See issue dotnet/fsharp#16432. + reportedUndefinedNames: HashSet + // forward call TcPat: WarnOnUpperFlag -> TcFileState -> TcEnv -> PrelimValReprInfo option -> TcPatValFlags -> TcPatLinearEnv -> TType -> SynPat -> (TcPatPhase2Input -> Pattern) * TcPatLinearEnv @@ -363,6 +369,7 @@ type TcFileState = isInternalTestSpanStackReferring = isInternalTestSpanStackReferring diagnosticOptions = diagnosticOptions argInfoCache = ConcurrentDictionary() + reportedUndefinedNames = HashSet(HashIdentity.Structural) TcPat = tcPat TcSimplePats = tcSimplePats TcSequenceExpressionEntry = tcSequenceExpressionEntry diff --git a/src/Compiler/Checking/CheckBasics.fsi b/src/Compiler/Checking/CheckBasics.fsi index 0191cf018f2..7ce10fbb7f1 100644 --- a/src/Compiler/Checking/CheckBasics.fsi +++ b/src/Compiler/Checking/CheckBasics.fsi @@ -274,6 +274,12 @@ type TcFileState = /// we're always dealing with the same instance and the updates don't get lost argInfoCache: ConcurrentDictionary + /// Tracks (range, identifier-text) keys of `UndefinedName` diagnostics that have already + /// been reported during this cenv's lifetime so they can be deduplicated when the same + /// identifier is re-resolved by multiple passes (e.g. Phase 1F vs. Phase 2A of an + /// `inherit` clause). See issue dotnet/fsharp#16432. + reportedUndefinedNames: HashSet + // forward call TcPat: WarnOnUpperFlag diff --git a/src/Compiler/Checking/CheckDeclarations.fs b/src/Compiler/Checking/CheckDeclarations.fs index 20919675fdc..184ac387d14 100644 --- a/src/Compiler/Checking/CheckDeclarations.fs +++ b/src/Compiler/Checking/CheckDeclarations.fs @@ -48,21 +48,16 @@ open FSharp.Compiler.TypeRelations open FSharp.Compiler.TypeProviders #endif -/// Wraps an inner DiagnosticsLogger and suppresses subsequent identical -/// 'UndefinedName' (FS0039) errors raised for the same identifier text within -/// its lifetime. This addresses the case where an undefined base type in an -/// 'inherit' / 'interface inherit' clause is reported multiple times because -/// the type-establishment pipeline resolves the same SynType in several passes -/// (FirstPass / SecondPass / Phase2AInherit). The key is the formatted -/// diagnostic message so all other diagnostics, and FS0039 emissions for -/// distinct identifiers, pass through unchanged. -type private DedupInheritDiagnosticsLogger(inner: DiagnosticsLogger) = - inherit DiagnosticsLogger("DedupInheritDiagnosticsLogger") - let seen = HashSet() +/// A diagnostics-logger wrapper used while type-checking an `inherit` clause. It drops any +/// `UndefinedName` diagnostic whose `(id.idRange, id.idText)` key has already been seen in +/// the same `cenv`, so duplicate FS0039s caused by re-resolution across Phase 1F / Phase 2A +/// are suppressed. All other diagnostics are forwarded unchanged. See issue #16432. +type private InheritDedupDiagnosticsLogger(seen: HashSet, inner: DiagnosticsLogger) = + inherit DiagnosticsLogger("InheritDedupDiagnosticsLogger") let rec isDuplicateUndefinedName (e: exn) = match e with - | UndefinedName _ -> not (seen.Add e.Message) + | UndefinedName(_, _, id, _) -> not (seen.Add(id.idRange, id.idText)) | WrappedError(inner, _) -> isDuplicateUndefinedName inner | _ -> false @@ -74,6 +69,10 @@ type private DedupInheritDiagnosticsLogger(inner: DiagnosticsLogger) = type cenv = TcFileState +let private useInheritDedupLogger (cenv: cenv) = + UseTransformedDiagnosticsLogger(fun inner -> + InheritDedupDiagnosticsLogger(cenv.reportedUndefinedNames, inner) :> DiagnosticsLogger) + //------------------------------------------------------------------------- // Mutually recursive shapes //------------------------------------------------------------------------- @@ -1390,6 +1389,7 @@ module MutRecBindingChecking = // Phase2B: typecheck the argument to an 'inherits' call and build the new object expr for the inherit-call | Phase2AInherit (synBaseTy, arg, baseValOpt, m) -> + use _ = useInheritDedupLogger cenv let inheritsExpr, tpenv = try let baseTy, tpenv = TcType cenv NoNewTypars CheckCxs ItemOccurrence.Use WarnOnIWSAM.Yes envInstance tpenv synBaseTy @@ -3341,7 +3341,9 @@ module EstablishTypeDefinitionCores = let kind = InferTyconKind g (kind, attrs, slotsigs, fields, inSig, isConcrete, m) let inherits = inherits |> List.map (fun (ty, m, _) -> (ty, m)) - let inheritedTys = fst (List.mapFold (mapFoldFst (TcTypeAndRecover cenv NoNewTypars checkConstraints ItemOccurrence.UseInType WarnOnIWSAM.No envinner)) tpenv inherits) + let inheritedTys = + use _ = useInheritDedupLogger cenv + fst (List.mapFold (mapFoldFst (TcTypeAndRecover cenv NoNewTypars checkConstraints ItemOccurrence.UseInType WarnOnIWSAM.No envinner)) tpenv inherits) let implementedTys, inheritedTys = match kind with | SynTypeDefnKind.Interface -> @@ -4635,16 +4637,6 @@ module TcDeclarations = let g = cenv.g - // Suppress duplicate FS0039 ('UndefinedName') diagnostics emitted across the - // multiple establishment passes for a mutually recursive type-definition group. - // For example, an undefined base type in an 'inherit' / 'interface inherit' - // clause is otherwise reported once each in FirstPass, SecondPass and the - // Phase2AInherit member-checking pass. Dedup is keyed by the formatted - // 'UndefinedName' message so unrelated diagnostics pass through unchanged. - use _ = - UseTransformedDiagnosticsLogger(fun inner -> - DedupInheritDiagnosticsLogger(inner) :> DiagnosticsLogger) - // Split the definitions into "core representations" and "members". The code to process core representations // is shared between processing of signature files and implementation files. let mutRecDefnsAfterSplit = mutRecDefns |> MutRecShapes.mapTycons (fun i -> SplitTyconDefn g i) From e34c6123af0fa9b44ffd585079fa639e0aa6fae7 Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 29 May 2026 14:23:19 +0200 Subject: [PATCH 04/10] chore(#16432): release notes for inherit FS0039 dedup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/release-notes/.FSharp.Compiler.Service/11.0.100.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 70ca7428416..c83239dc6c5 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,5 +1,6 @@ ### Fixed +* Report `FS0039` only once when a class `inherit` clause references an undefined base type. ([Issue #16432](https://github.com/dotnet/fsharp/issues/16432)) * Fix FS0421 "The address of the variable cannot be used at this point" incorrectly raised for the discard pattern `let _ = &expr` when `let x = &expr` compiles. ([Issue #18841](https://github.com/dotnet/fsharp/issues/18841), [PR #19811](https://github.com/dotnet/fsharp/pull/19811)) * Honor `--nowarn` and `--warnaserror` for warnings emitted during command-line option parsing ([Issue #19576](https://github.com/dotnet/fsharp/issues/19576), [PR #19776](https://github.com/dotnet/fsharp/pull/19776)) * Fix `[]` prefix attributes being silently dropped on class members, and fix false-positive `AllowMultiple=false` errors when `[]` and `[]` are applied to the same binding. ([Issue #17904](https://github.com/dotnet/fsharp/issues/17904), [Issue #19020](https://github.com/dotnet/fsharp/issues/19020), [PR #19738](https://github.com/dotnet/fsharp/pull/19738)) From 88a176eff817f398d4b3d2346f9f241b8bbf3806 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Fri, 29 May 2026 15:09:51 +0200 Subject: [PATCH 05/10] [REFACTOR] Format and address expert review for #16432 - Switch reportedUndefinedNames to ConcurrentDictionary for consistency with the sibling argInfoCache field and to be defensive against any future parallel cenv use. - Forward CheckForRealErrorsIgnoringWarnings to the wrapped logger so the InheritDedupDiagnosticsLogger is a faithful pass-through for everything other than the intended UndefinedName dedup. - Document the rationale for unwrapping only WrappedError in isDuplicateUndefinedName. Expert reviewer item 1 (merging suggestions from later UndefinedName into the first) intentionally not applied: in the inherit-clause case the colliding diagnostics carry identical suggestion sets, and merging would couple this site to NameResolution.AddResults internals. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Compiler/Checking/CheckBasics.fs | 4 ++-- src/Compiler/Checking/CheckBasics.fsi | 2 +- src/Compiler/Checking/CheckDeclarations.fs | 12 ++++++++++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/Compiler/Checking/CheckBasics.fs b/src/Compiler/Checking/CheckBasics.fs index 44fe20b4949..32ca10577d5 100644 --- a/src/Compiler/Checking/CheckBasics.fs +++ b/src/Compiler/Checking/CheckBasics.fs @@ -317,7 +317,7 @@ type TcFileState = /// been reported during this cenv's lifetime so they can be deduplicated when the same /// identifier is re-resolved by multiple passes (e.g. Phase 1F vs. Phase 2A of an /// `inherit` clause). See issue dotnet/fsharp#16432. - reportedUndefinedNames: HashSet + reportedUndefinedNames: ConcurrentDictionary // forward call TcPat: WarnOnUpperFlag -> TcFileState -> TcEnv -> PrelimValReprInfo option -> TcPatValFlags -> TcPatLinearEnv -> TType -> SynPat -> (TcPatPhase2Input -> Pattern) * TcPatLinearEnv @@ -369,7 +369,7 @@ type TcFileState = isInternalTestSpanStackReferring = isInternalTestSpanStackReferring diagnosticOptions = diagnosticOptions argInfoCache = ConcurrentDictionary() - reportedUndefinedNames = HashSet(HashIdentity.Structural) + reportedUndefinedNames = ConcurrentDictionary() TcPat = tcPat TcSimplePats = tcSimplePats TcSequenceExpressionEntry = tcSequenceExpressionEntry diff --git a/src/Compiler/Checking/CheckBasics.fsi b/src/Compiler/Checking/CheckBasics.fsi index 7ce10fbb7f1..80b4baec7e5 100644 --- a/src/Compiler/Checking/CheckBasics.fsi +++ b/src/Compiler/Checking/CheckBasics.fsi @@ -278,7 +278,7 @@ type TcFileState = /// been reported during this cenv's lifetime so they can be deduplicated when the same /// identifier is re-resolved by multiple passes (e.g. Phase 1F vs. Phase 2A of an /// `inherit` clause). See issue dotnet/fsharp#16432. - reportedUndefinedNames: HashSet + reportedUndefinedNames: ConcurrentDictionary // forward call TcPat: diff --git a/src/Compiler/Checking/CheckDeclarations.fs b/src/Compiler/Checking/CheckDeclarations.fs index 184ac387d14..6107f9520d3 100644 --- a/src/Compiler/Checking/CheckDeclarations.fs +++ b/src/Compiler/Checking/CheckDeclarations.fs @@ -3,6 +3,7 @@ module internal FSharp.Compiler.CheckDeclarations open System +open System.Collections.Concurrent open System.Collections.Generic open System.Threading @@ -52,12 +53,17 @@ open FSharp.Compiler.TypeProviders /// `UndefinedName` diagnostic whose `(id.idRange, id.idText)` key has already been seen in /// the same `cenv`, so duplicate FS0039s caused by re-resolution across Phase 1F / Phase 2A /// are suppressed. All other diagnostics are forwarded unchanged. See issue #16432. -type private InheritDedupDiagnosticsLogger(seen: HashSet, inner: DiagnosticsLogger) = +type private InheritDedupDiagnosticsLogger + (seen: ConcurrentDictionary, inner: DiagnosticsLogger) = inherit DiagnosticsLogger("InheritDedupDiagnosticsLogger") + // Only `WrappedError` is unwrapped here: it is the single wrapper that the + // `inherit` type-checking path can place around an `UndefinedName`. Other diagnostic + // exception families (`StopProcessingExn`/`ReportedError` short-circuit before reaching + // the sink, and `DiagnosticWithSuggestions` etc. are peer errors, not wrappers). let rec isDuplicateUndefinedName (e: exn) = match e with - | UndefinedName(_, _, id, _) -> not (seen.Add(id.idRange, id.idText)) + | UndefinedName(_, _, id, _) -> not (seen.TryAdd(struct (id.idRange, id.idText), ())) | WrappedError(inner, _) -> isDuplicateUndefinedName inner | _ -> false @@ -67,6 +73,8 @@ type private InheritDedupDiagnosticsLogger(seen: HashSet, inner: override _.ErrorCount = inner.ErrorCount + override _.CheckForRealErrorsIgnoringWarnings = inner.CheckForRealErrorsIgnoringWarnings + type cenv = TcFileState let private useInheritDedupLogger (cenv: cenv) = From 966314a49f2c2f0187a37ae658ce91a9f0eae767 Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 29 May 2026 16:08:17 +0200 Subject: [PATCH 06/10] docs: add PR link to release notes for #16432 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/release-notes/.FSharp.Compiler.Service/11.0.100.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index c83239dc6c5..4e0d93e732d 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,6 +1,6 @@ ### Fixed -* Report `FS0039` only once when a class `inherit` clause references an undefined base type. ([Issue #16432](https://github.com/dotnet/fsharp/issues/16432)) +* Report `FS0039` only once when a class `inherit` clause references an undefined base type. ([Issue #16432](https://github.com/dotnet/fsharp/issues/16432), [PR #19862](https://github.com/dotnet/fsharp/pull/19862)) * Fix FS0421 "The address of the variable cannot be used at this point" incorrectly raised for the discard pattern `let _ = &expr` when `let x = &expr` compiles. ([Issue #18841](https://github.com/dotnet/fsharp/issues/18841), [PR #19811](https://github.com/dotnet/fsharp/pull/19811)) * Honor `--nowarn` and `--warnaserror` for warnings emitted during command-line option parsing ([Issue #19576](https://github.com/dotnet/fsharp/issues/19576), [PR #19776](https://github.com/dotnet/fsharp/pull/19776)) * Fix `[]` prefix attributes being silently dropped on class members, and fix false-positive `AllowMultiple=false` errors when `[]` and `[]` are applied to the same binding. ([Issue #17904](https://github.com/dotnet/fsharp/issues/17904), [Issue #19020](https://github.com/dotnet/fsharp/issues/19020), [PR #19738](https://github.com/dotnet/fsharp/pull/19738)) From d34b8156b57b68ef7c5403f6b270687231a79711 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Fri, 5 Jun 2026 12:21:46 +0200 Subject: [PATCH 07/10] [RESEARCH] Document 3 call sites for inherit FS0039 (#16432) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .tools/ralph/research-notes.md | 216 +++++++++++++++++++++++++++++++++ 1 file changed, 216 insertions(+) create mode 100644 .tools/ralph/research-notes.md diff --git a/.tools/ralph/research-notes.md b/.tools/ralph/research-notes.md new file mode 100644 index 00000000000..fe0a54c4872 --- /dev/null +++ b/.tools/ralph/research-notes.md @@ -0,0 +1,216 @@ +# Research Notes: dotnet/fsharp#16432 / PR #19862 + +## Summary + +For an `inherit T()` clause where `T` is undefined, FSC emits FS0039 ("The type +`T` is not defined") **three times** at the exact same `(line, column)` range. +The current PR (#19862) masks the duplicates with a `cenv`-scoped +`InheritDedupDiagnosticsLogger` keyed on `(idRange, idText)` of the +`UndefinedName` exception. Empirical tracing confirms the root cause: the +identical `SynType` node embedded in the inherit clause is type-checked +**three** times by three independent passes of `CheckDeclarations`: +Phase 1D (FirstPass) and Phase 1F (SecondPass) of +`TcTyconDefnCore_Phase1D_Phase1F_EstablishSuperTypesAndInterfaceTypes`, then +Phase 2A `Phase2AInherit` of `MutRecBindingChecking`. Each pass independently +invokes name resolution on the same `SynType`, and each independent failure +emits FS0039 through the diagnostics sink. + +## The three call sites (verified) + +| # | Phase | File:Line | Call | Occurrence | CheckCxs | env | +| - | ---------------------- | -------------------------------- | ---------------- | ----------- | ------------ | ------------ | +| 1 | Phase 1D (FirstPass) | `CheckDeclarations.fs:3354` | `TcTypeAndRecover` | `UseInType` | `NoCheckCxs` | `envinner` | +| 2 | Phase 1F (SecondPass) | `CheckDeclarations.fs:3354` | `TcTypeAndRecover` | `UseInType` | `CheckCxs` | `envinner` | +| 3 | Phase 2A | `CheckDeclarations.fs:1399` (inside `try`, call at line 1403) | `TcType` | `Use` | `CheckCxs` | `envInstance` | + +Drivers for sites #1 and #2 (same function, two passes), in the same file: + +```text +CheckDeclarations.fs:4180 (envMutRecPrelim, withAttrs) |> TcTyconDefnCore_Phase1D_Phase1F_EstablishSuperTypesAndInterfaceTypes cenv tpenv inSig FirstPass +CheckDeclarations.fs:4216 (envMutRecPrelim, withAttrs) |> TcTyconDefnCore_Phase1D_Phase1F_EstablishSuperTypesAndInterfaceTypes cenv tpenv inSig SecondPass +``` + +Each pass walks `SynTypeDefnSimpleRepr.General(_, inherits, ...)` and feeds +the inherit `SynType` to `TcTypeAndRecover`. The recovery point that actually +calls `errorRecovery` (i.e. emits the diagnostic) is +`TcTypeOrMeasureAndRecover` at `CheckExpressions.fs:5217`: + +```fsharp +and TcTypeOrMeasureAndRecover kindOpt (cenv: cenv) newOk checkConstraints occ iwsam env tpenv ty = + let g = cenv.g + try + TcTypeOrMeasure kindOpt cenv newOk checkConstraints occ iwsam env tpenv ty + with RecoverableException e -> + errorRecovery e ty.Range + ... + recoveryTy, tpenv +``` + +Site #3 uses raw `TcType` and recovers via its own outer `try/with +RecoverableException e -> errorRecovery e m; mkUnit g m, tpenv` at +`CheckDeclarations.fs:1407-1410`. All three sites ultimately funnel into +`errorRecovery`, which routes through the active `DiagnosticsLogger`. + +## Why each pass exists (with quotes from source) + +- **Phase 1D / Phase 1F** — both runs share one function whose header + documents the dual-pass design (`CheckDeclarations.fs:3322-3324`): + + > `// Third phase: check and publish the super types. Run twice, once before constraints are established` + > `// and once after` + + The first pass (`NoCheckCxs`) is needed so the *shape* of the super-type + hierarchy is published into the symbol table before user-supplied + constraints are checked. The second pass (`CheckCxs`) re-resolves the same + syntax now with full constraint checking enabled. + +- **Phase 2A** (`MutRecBindingChecking`, `CheckDeclarations.fs:1398-1399`): + + > `// Phase2B: typecheck the argument to an 'inherits' call and build the new object expr for the inherit-call` + > `| Phase2AInherit (synBaseTy, arg, baseValOpt, m) ->` + + This is the *expression* type-checking pass. It needs to resolve + `synBaseTy` again because at this point it builds the actual `TcNewExpr` + for the constructor call to the base type (the previous two passes only + computed `TType`s for the inheritance graph; they did not elaborate the + `inherit Base(args)` expression). It also uses `envInstance`, which is the + enriched environment containing the implicit-constructor args, not + `envinner`. + +## Evidence — failing test output with PR mechanism neutralised + +`useInheritDedupLogger` was changed locally to a pass-through: + +```fsharp +let private useInheritDedupLogger (cenv: cenv) = + ignore cenv + UseTransformedDiagnosticsLogger(fun inner -> inner) +``` + +Running: + +```bash +dotnet test --project tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj \ + -c Debug --no-build -- --filter-method "*Inherit nonexistent type reports single FS0039*" --output Detailed +``` + +Verbatim assertion failure (test source: `tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/ClassTypes/InheritsDeclarations/InheritsDeclarations.fs:77`): + +``` +Expected exactly 1 FS0039 but got 3. Diagnostics: +[{ Error = Error 39 + Range = { StartLine = 3 + StartColumn = 12 + EndLine = 3 + EndColumn = 27 } + NativeRange = (3,12--3,27) + Message = "The type 'NonExistentBase' is not defined." + SubCategory = "typecheck" }; + { Error = Error 39 + Range = { StartLine = 3 + StartColumn = 12 + EndLine = 3 + EndColumn = 27 } + NativeRange = (3,12--3,27) + Message = "The type 'NonExistentBase' is not defined." + SubCategory = "typecheck" }; + { Error = Error 39 + Range = { StartLine = 3 + StartColumn = 12 + EndLine = 3 + EndColumn = 27 } + NativeRange = (3,12--3,27) + Message = "The type 'NonExistentBase' is not defined." + SubCategory = "typecheck" }] +``` + +All three diagnostics carry **identical** `NativeRange = (3,12--3,27)` and +identical message, confirming they originate from re-resolving the *same* +syntactic node, not from three different syntactic occurrences. + +## Evidence — `eprintfn` trace + +Per-site `eprintfn` was added at sites #1/#2 (`CheckDeclarations.fs:3353`) +and #3 (`CheckDeclarations.fs:1400`). Test stdout captured: + +``` +[TRACE 16432] Phase1D/1F inherit-resolve pass=FirstPass tycon=MyClass +[TRACE 16432] Phase1D/1F inherit-resolve pass=SecondPass tycon=MyClass +[TRACE 16432] Phase2A inherit-resolve range=(3,12--3,27) +``` + +Exactly three resolutions for the one `inherit NonExistentBase()` clause; the +Phase 2A range `(3,12--3,27)` matches the range of every emitted FS0039, +confirming the three diagnostics are produced by these three sites and only +these three sites. + +## SynType identity across phases + +`CheckDeclarations.fs:4579-4583` collects inherits from member definitions +into the `SynTypeDefnSimpleRepr.General.inherits` list: + +```fsharp +let inherits = + members |> List.choose (function + | SynMemberDefn.Inherit (Some ty, idOpt, m, _) -> Some(ty, m, idOpt) + | SynMemberDefn.ImplicitInherit (ty, _, idOpt, m, _) -> Some(ty, m, idOpt) + | _ -> None) +``` + +The very same `ty: SynType` object — produced once by the parser — flows two +ways: + +1. Into the `SynTypeDefnSimpleRepr.General(_, inherits, ...)` list consumed + by Phase 1D/1F (`CheckDeclarations.fs:3348-3354`), where each pass + destructures `(ty, m, _)` and passes `ty` to `TcTypeAndRecover`. +2. Remains inside the original `SynMemberDefn.ImplicitInherit (ty, _, _, _, _)` + (resp. `SynMemberDefn.Inherit (Some ty, ...)`) which is what + `MutRecBindingChecking` later receives as `Phase2AInherit (synBaseTy, arg, + baseValOpt, m)` — i.e. **`synBaseTy` is the same `SynType` reference**. + +Therefore `synBaseTy.Range == ty.Range` is true *by construction*, not by +coincidence. The trace above provides empirical confirmation: the Phase 2A +range `(3,12--3,27)` equals the range of every reported diagnostic, which is +the parser-assigned range of that one `SynType` node. + +## Cache-key rationale + +Because the `SynType` node is shared across passes, the pair +`(tcref.Stamp, ty.Range)` — equivalently `(tcref.Stamp, synBaseTy.Range)` at +the Phase 2A site — **uniquely identifies a single inherit clause across all +three passes**. `tcref.Stamp` disambiguates two different tycons that happen +to share a source range (e.g. when the syntax came from a synthetic source), +and `ty.Range` distinguishes multiple inherits within the same tycon +(currently only legal once per class, but the key is robust to interface +implementations and to future relaxation). + +Notably, the PR's current key `(idRange, idText)` of the `UndefinedName` +exception works too because `idRange` for an inherit identifier coincides +with the embedded `SynType`'s identifier range — but it is keyed on the +diagnostic payload rather than on the syntactic node itself. The root-cause +key `(tcref.Stamp, ty.Range)` is preferable because it can short-circuit the +*resolution attempt* rather than just suppress the resulting diagnostic. + +## Recommended next step (root-cause fix) + +Replace the diagnostics-sink dedup with a per-`cenv` +`ConcurrentDictionary` (call it +`inheritResolutionFailed`). At each of the three call sites, when the +`TcType*` call raises a `RecoverableException` whose underlying exception is +an `UndefinedName`, add `(tcref.Stamp, ty.Range)` to the set *and* let the +first pass emit its diagnostic normally. On the subsequent two passes, before +invoking `TcTypeAndRecover` / `TcType`, look up the key; if present, +short-circuit to `g.obj_ty_ambivalent` / `mkUnit g m` directly without +calling name resolution at all. This avoids: + +- emitting duplicate FS0039s (the primary goal), +- doing redundant work in name resolution and the tc-sink for the two + later passes, and +- masking *unrelated* recoverable diagnostics (constraint failures, type + provider failures) that the current sink-level dedup would also silently + drop if they happened to share the `(idRange, idText)` key with a previously + reported `UndefinedName`. + +Only `UndefinedName` should mark the cache key; other recoverable failures +must still flow through the diagnostics sink so that constraint, IWSAM, and +type-provider diagnostics surface correctly on Phase 1F and Phase 2A. From 4968066d88459b15c812495d5982e299e5033371 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Fri, 5 Jun 2026 12:42:04 +0200 Subject: [PATCH 08/10] Root-cause fix for inherit FS0039 triple-report (#16432) Replace the InheritDedupDiagnosticsLogger sink-wrapper (PR #19862) with a per-cenv 'inheritResolutionFailed' marker. Phase 1D records (tycon.Stamp, synBaseTy.Range) when name resolution fails with UndefinedName. Phase 1F and Phase 2A consult the marker and skip re-resolution entirely, so the diagnostic is emitted once and the redundant work is eliminated. Non- UndefinedName failures (constraint, type-provider, etc.) continue to flow through normal recovery in every pass. Addresses review comment 3324874944 on PR #19862. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../.FSharp.Compiler.Service/11.0.100.md | 2 +- src/Compiler/Checking/CheckBasics.fs | 13 +-- src/Compiler/Checking/CheckBasics.fsi | 11 ++- src/Compiler/Checking/CheckDeclarations.fs | 91 +++++++++++-------- 4 files changed, 65 insertions(+), 52 deletions(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 1925b1fb408..72c8f60968d 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,6 +1,6 @@ ### Fixed -* Report `FS0039` only once when a class `inherit` clause references an undefined base type. ([Issue #16432](https://github.com/dotnet/fsharp/issues/16432), [PR #19862](https://github.com/dotnet/fsharp/pull/19862)) +* Fixed: Inheriting from an undefined type now reports `FS0039` exactly once instead of three times. Phase 1F and Phase 2A of inherit-clause type-checking now skip re-resolving a syntactic clause whose Phase 1D resolution already failed with `UndefinedName`, eliminating both the duplicate diagnostic and the redundant work. ([Issue #16432](https://github.com/dotnet/fsharp/issues/16432), [PR #19862](https://github.com/dotnet/fsharp/pull/19862)) * Reject non-function bindings for single-case and partial active pattern names with FS1209, matching the existing multi-case behavior. ([PR #19763](https://github.com/dotnet/fsharp/pull/19763)) * Fix FS0421 "The address of the variable cannot be used at this point" incorrectly raised for the discard pattern `let _ = &expr` when `let x = &expr` compiles. ([Issue #18841](https://github.com/dotnet/fsharp/issues/18841), [PR #19811](https://github.com/dotnet/fsharp/pull/19811)) * Honor `--nowarn` and `--warnaserror` for warnings emitted during command-line option parsing ([Issue #19576](https://github.com/dotnet/fsharp/issues/19576), [PR #19776](https://github.com/dotnet/fsharp/pull/19776)) diff --git a/src/Compiler/Checking/CheckBasics.fs b/src/Compiler/Checking/CheckBasics.fs index 32ca10577d5..a1308886066 100644 --- a/src/Compiler/Checking/CheckBasics.fs +++ b/src/Compiler/Checking/CheckBasics.fs @@ -313,11 +313,12 @@ type TcFileState = argInfoCache: ConcurrentDictionary - /// Tracks (range, identifier-text) keys of `UndefinedName` diagnostics that have already - /// been reported during this cenv's lifetime so they can be deduplicated when the same - /// identifier is re-resolved by multiple passes (e.g. Phase 1F vs. Phase 2A of an - /// `inherit` clause). See issue dotnet/fsharp#16432. - reportedUndefinedNames: ConcurrentDictionary + /// Marks `(tycon.Stamp, synBaseTy.Range)` pairs for which the inherit-clause type + /// failed to resolve with an `UndefinedName` diagnostic in an earlier pass. Subsequent + /// passes (Phase 1F, Phase 2A) consult this marker to skip re-resolving the same + /// syntactic inherit clause, eliminating the redundant work and the duplicate FS0039. + /// See issue dotnet/fsharp#16432. + inheritResolutionFailed: ConcurrentDictionary // forward call TcPat: WarnOnUpperFlag -> TcFileState -> TcEnv -> PrelimValReprInfo option -> TcPatValFlags -> TcPatLinearEnv -> TType -> SynPat -> (TcPatPhase2Input -> Pattern) * TcPatLinearEnv @@ -369,7 +370,7 @@ type TcFileState = isInternalTestSpanStackReferring = isInternalTestSpanStackReferring diagnosticOptions = diagnosticOptions argInfoCache = ConcurrentDictionary() - reportedUndefinedNames = ConcurrentDictionary() + inheritResolutionFailed = ConcurrentDictionary() TcPat = tcPat TcSimplePats = tcSimplePats TcSequenceExpressionEntry = tcSequenceExpressionEntry diff --git a/src/Compiler/Checking/CheckBasics.fsi b/src/Compiler/Checking/CheckBasics.fsi index 80b4baec7e5..a98686a5ac8 100644 --- a/src/Compiler/Checking/CheckBasics.fsi +++ b/src/Compiler/Checking/CheckBasics.fsi @@ -274,11 +274,12 @@ type TcFileState = /// we're always dealing with the same instance and the updates don't get lost argInfoCache: ConcurrentDictionary - /// Tracks (range, identifier-text) keys of `UndefinedName` diagnostics that have already - /// been reported during this cenv's lifetime so they can be deduplicated when the same - /// identifier is re-resolved by multiple passes (e.g. Phase 1F vs. Phase 2A of an - /// `inherit` clause). See issue dotnet/fsharp#16432. - reportedUndefinedNames: ConcurrentDictionary + /// Marks `(tycon.Stamp, synBaseTy.Range)` pairs for which the inherit-clause type + /// failed to resolve with an `UndefinedName` diagnostic in an earlier pass. Subsequent + /// passes (Phase 1F, Phase 2A) consult this marker to skip re-resolving the same + /// syntactic inherit clause, eliminating the redundant work and the duplicate FS0039. + /// See issue dotnet/fsharp#16432. + inheritResolutionFailed: ConcurrentDictionary // forward call TcPat: diff --git a/src/Compiler/Checking/CheckDeclarations.fs b/src/Compiler/Checking/CheckDeclarations.fs index 6107f9520d3..e0b04b04ff5 100644 --- a/src/Compiler/Checking/CheckDeclarations.fs +++ b/src/Compiler/Checking/CheckDeclarations.fs @@ -3,7 +3,6 @@ module internal FSharp.Compiler.CheckDeclarations open System -open System.Collections.Concurrent open System.Collections.Generic open System.Threading @@ -49,37 +48,20 @@ open FSharp.Compiler.TypeRelations open FSharp.Compiler.TypeProviders #endif -/// A diagnostics-logger wrapper used while type-checking an `inherit` clause. It drops any -/// `UndefinedName` diagnostic whose `(id.idRange, id.idText)` key has already been seen in -/// the same `cenv`, so duplicate FS0039s caused by re-resolution across Phase 1F / Phase 2A -/// are suppressed. All other diagnostics are forwarded unchanged. See issue #16432. -type private InheritDedupDiagnosticsLogger - (seen: ConcurrentDictionary, inner: DiagnosticsLogger) = - inherit DiagnosticsLogger("InheritDedupDiagnosticsLogger") - - // Only `WrappedError` is unwrapped here: it is the single wrapper that the - // `inherit` type-checking path can place around an `UndefinedName`. Other diagnostic - // exception families (`StopProcessingExn`/`ReportedError` short-circuit before reaching - // the sink, and `DiagnosticWithSuggestions` etc. are peer errors, not wrappers). - let rec isDuplicateUndefinedName (e: exn) = - match e with - | UndefinedName(_, _, id, _) -> not (seen.TryAdd(struct (id.idRange, id.idText), ())) - | WrappedError(inner, _) -> isDuplicateUndefinedName inner - | _ -> false - - override _.DiagnosticSink(diagnostic: PhasedDiagnostic) = - if not (isDuplicateUndefinedName diagnostic.Exception) then - inner.DiagnosticSink diagnostic - - override _.ErrorCount = inner.ErrorCount - - override _.CheckForRealErrorsIgnoringWarnings = inner.CheckForRealErrorsIgnoringWarnings - +/// Helper for marking inherit-clause failures so later passes can skip duplicate work. +/// See `isUndefinedNameFailure` below. type cenv = TcFileState -let private useInheritDedupLogger (cenv: cenv) = - UseTransformedDiagnosticsLogger(fun inner -> - InheritDedupDiagnosticsLogger(cenv.reportedUndefinedNames, inner) :> DiagnosticsLogger) +/// Recognises a recoverable `UndefinedName` failure (possibly wrapped). Used by the inherit-clause +/// type-checking path to mark a `(tycon, range)` pair as already-reported so later passes +/// (Phase 1F, Phase 2A) can skip re-resolving the same syntactic clause. See issue dotnet/fsharp#16432. +let private isUndefinedNameFailure (e: exn) = + let rec loop (e: exn) = + match e with + | UndefinedName _ -> true + | WrappedError(inner, _) -> loop inner + | _ -> false + loop e //------------------------------------------------------------------------- // Mutually recursive shapes @@ -1397,16 +1379,24 @@ module MutRecBindingChecking = // Phase2B: typecheck the argument to an 'inherits' call and build the new object expr for the inherit-call | Phase2AInherit (synBaseTy, arg, baseValOpt, m) -> - use _ = useInheritDedupLogger cenv + // If Phase 1D/1F already reported `UndefinedName` for this exact syntactic + // inherit clause, short-circuit: the user has seen the FS0039 once and we + // would only re-emit the same diagnostic by trying to re-resolve here. + // Non-`UndefinedName` failures (and the success path) still go through + // normal type-checking below, so IDE `ItemOccurrence.Use` sink events + // for valid types are preserved. See issue dotnet/fsharp#16432. let inheritsExpr, tpenv = - try - let baseTy, tpenv = TcType cenv NoNewTypars CheckCxs ItemOccurrence.Use WarnOnIWSAM.Yes envInstance tpenv synBaseTy - let baseTy = baseTy |> convertToTypeWithMetadataIfPossible g - let mTcNew = unionRanges synBaseTy.Range arg.Range - TcNewExpr cenv envInstance tpenv baseTy (Some synBaseTy.Range) true arg mTcNew - with RecoverableException e -> - errorRecovery e m + if cenv.inheritResolutionFailed.ContainsKey(struct (tcref.Stamp, synBaseTy.Range)) then mkUnit g m, tpenv + else + try + let baseTy, tpenv = TcType cenv NoNewTypars CheckCxs ItemOccurrence.Use WarnOnIWSAM.Yes envInstance tpenv synBaseTy + let baseTy = baseTy |> convertToTypeWithMetadataIfPossible g + let mTcNew = unionRanges synBaseTy.Range arg.Range + TcNewExpr cenv envInstance tpenv baseTy (Some synBaseTy.Range) true arg mTcNew + with RecoverableException e -> + errorRecovery e m + mkUnit g m, tpenv let envInstance = match baseValOpt with Some baseVal -> AddLocalVal g cenv.tcSink scopem baseVal envInstance | None -> envInstance let envNonRec = match baseValOpt with Some baseVal -> AddLocalVal g cenv.tcSink scopem baseVal envNonRec | None -> envNonRec let innerState = (tpenv, envInstance, envStatic, envNonRec, generalizedRecBinds, preGeneralizationRecBinds, uncheckedRecBindsTable) @@ -3349,9 +3339,30 @@ module EstablishTypeDefinitionCores = let kind = InferTyconKind g (kind, attrs, slotsigs, fields, inSig, isConcrete, m) let inherits = inherits |> List.map (fun (ty, m, _) -> (ty, m)) + // Resolve each inherit type. If a previous pass already failed with `UndefinedName` + // for this clause, skip re-resolution entirely (returns the recovery type used + // by `TcTypeAndRecover`). This avoids the duplicate FS0039 (#16432) without + // any diagnostic-sink trickery and prevents the duplicate work too. Failures + // that are NOT `UndefinedName` (e.g. constraint or type-provider errors that + // only surface in the `CheckCxs` second pass) are NOT marked, so they continue + // to flow through the normal recovery path. let inheritedTys = - use _ = useInheritDedupLogger cenv - fst (List.mapFold (mapFoldFst (TcTypeAndRecover cenv NoNewTypars checkConstraints ItemOccurrence.UseInType WarnOnIWSAM.No envinner)) tpenv inherits) + inherits + |> List.mapFold (fun tpenv (ty, m) -> + let key = struct (tcref.Stamp, ty.Range) + if cenv.inheritResolutionFailed.ContainsKey key then + (g.obj_ty_ambivalent, m), tpenv + else + try + let resolved, tpenv = TcType cenv NoNewTypars checkConstraints ItemOccurrence.UseInType WarnOnIWSAM.No envinner tpenv ty + (resolved, m), tpenv + with RecoverableException e -> + errorRecovery e ty.Range + if isUndefinedNameFailure e then + cenv.inheritResolutionFailed.TryAdd(key, ()) |> ignore + (g.obj_ty_ambivalent, m), tpenv + ) tpenv + |> fst let implementedTys, inheritedTys = match kind with | SynTypeDefnKind.Interface -> From e2bd7bf1b6ec832515d6157d4c9815b28821efe6 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Fri, 5 Jun 2026 13:01:20 +0200 Subject: [PATCH 09/10] Address verifier feedback for inherit FS0039 root-cause fix (#16432) - Extend isUndefinedNameFailure to also unwrap the constraint-solver wrappers (ErrorFromAddingTypeEquation, ErrorFromAddingConstraint, ErrorFromApplyingDefault) so the marker is set when UndefinedName surfaces from the CheckCxs second pass via a generic inherit-clause type-argument failure. - Remove a misplaced /// doc comment from 'type cenv = TcFileState'. - Collapse three duplicated InheritsDeclarations regression Fact methods into a single Theory + InlineData driving an assertSingleFS39 helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Compiler/Checking/CheckDeclarations.fs | 11 +++- .../InheritsDeclarations.fs | 51 ++++--------------- 2 files changed, 18 insertions(+), 44 deletions(-) diff --git a/src/Compiler/Checking/CheckDeclarations.fs b/src/Compiler/Checking/CheckDeclarations.fs index e0b04b04ff5..21364677753 100644 --- a/src/Compiler/Checking/CheckDeclarations.fs +++ b/src/Compiler/Checking/CheckDeclarations.fs @@ -48,18 +48,25 @@ open FSharp.Compiler.TypeRelations open FSharp.Compiler.TypeProviders #endif -/// Helper for marking inherit-clause failures so later passes can skip duplicate work. -/// See `isUndefinedNameFailure` below. type cenv = TcFileState /// Recognises a recoverable `UndefinedName` failure (possibly wrapped). Used by the inherit-clause /// type-checking path to mark a `(tycon, range)` pair as already-reported so later passes /// (Phase 1F, Phase 2A) can skip re-resolving the same syntactic clause. See issue dotnet/fsharp#16432. +/// +/// `UndefinedName` may arrive wrapped by `WrappedError` or by any of the constraint-solver +/// wrappers (`ErrorFromAddingTypeEquation`, `ErrorFromAddingConstraint`, `ErrorFromApplyingDefault`) +/// — these are common when the inherit clause carries type arguments and the failure surfaces +/// during the `CheckCxs` second pass. All such wrappers are unwrapped so the marker is set in +/// every case. let private isUndefinedNameFailure (e: exn) = let rec loop (e: exn) = match e with | UndefinedName _ -> true | WrappedError(inner, _) -> loop inner + | ErrorFromAddingTypeEquation(error = inner) -> loop inner + | ErrorFromAddingConstraint(error = inner) -> loop inner + | ErrorFromApplyingDefault(error = inner) -> loop inner | _ -> false loop e diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/ClassTypes/InheritsDeclarations/InheritsDeclarations.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/ClassTypes/InheritsDeclarations/InheritsDeclarations.fs index 03a40c24131..a309e10dae4 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/ClassTypes/InheritsDeclarations/InheritsDeclarations.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/ClassTypes/InheritsDeclarations/InheritsDeclarations.fs @@ -61,13 +61,9 @@ module InheritsDeclarations = // inheriting from an unknown type should emit FS0039 only once, // not three times (once per name-resolution site in CheckDeclarations). - [] - let ``Inherit nonexistent type reports single FS0039`` () = + let private assertSingleFS39 source = let result = - FSharp """ -type MyClass() = - inherit NonExistentBase() -""" + FSharp source |> typecheck let fs39 = @@ -79,6 +75,13 @@ type MyClass() = sprintf "Expected exactly 1 FS0039 but got %d. Diagnostics:\n%A" fs39.Length result.Output.Diagnostics ) + [] + [] + [()\n")>] + [] + let ``Inherit from undefined type reports single FS0039`` source = + assertSingleFS39 source + [] let ``Two different undefined names still report separately`` () = let result = @@ -98,24 +101,6 @@ type MyClass() = sprintf "Expected >= 2 FS0039, got %d. Diagnostics:\n%A" fs39.Length result.Output.Diagnostics ) - [] - let ``Inherit with generic nonexistent type single error`` () = - let result = - FSharp """ -type MyClass() = - inherit MissingGeneric() -""" - |> typecheck - - let fs39 = - result.Output.Diagnostics - |> List.filter (fun d -> d.Error = (Error 39)) - - Assert.True( - fs39.Length = 1, - sprintf "Expected exactly 1 FS0039 but got %d. Diagnostics:\n%A" fs39.Length result.Output.Diagnostics - ) - [] let ``Valid inherit produces no FS0039`` () = FSharp """ @@ -125,21 +110,3 @@ type MyClass() = """ |> typecheck |> shouldSucceed - - [] - let ``Interface inherit nonexistent single error`` () = - let result = - FSharp """ -type IMyInterface = - inherit INonExistent -""" - |> typecheck - - let fs39 = - result.Output.Diagnostics - |> List.filter (fun d -> d.Error = (Error 39)) - - Assert.True( - fs39.Length = 1, - sprintf "Expected exactly 1 FS0039 but got %d. Diagnostics:\n%A" fs39.Length result.Output.Diagnostics - ) From 73e5702737f9135d8f9c153529613b8637a3cee5 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Tue, 9 Jun 2026 13:06:58 +0200 Subject: [PATCH 10/10] Clean up FS0039 inherit dedup: drop bloat, strict tests, active pattern - Delete .tools/ralph/research-notes.md (agent scratchpad, not for repo). - Replace isUndefinedNameFailure helper + XML doc with active pattern (|UndefinedNameError|_|) co-located with the catch. - Extract tryResolveInheritType local in EstablishTypeDefinitionCores so the call site reads as a single mapFold. - Drop narrative block comments at both Phase2A and Phase 1D/F sites; the function name and pattern carry the intent. - Trim 5-line XML doc on inheritResolutionFailed to a one-liner. - Rewrite tests with strict withDiagnostics asserting exact code, range, and message for the full diagnostic set (no per-code filtering). --- .tools/ralph/research-notes.md | 216 ------------------ src/Compiler/Checking/CheckBasics.fs | 6 +- src/Compiler/Checking/CheckBasics.fsi | 6 +- src/Compiler/Checking/CheckDeclarations.fs | 72 ++---- .../InheritsDeclarations.fs | 70 +++--- 5 files changed, 60 insertions(+), 310 deletions(-) delete mode 100644 .tools/ralph/research-notes.md diff --git a/.tools/ralph/research-notes.md b/.tools/ralph/research-notes.md deleted file mode 100644 index fe0a54c4872..00000000000 --- a/.tools/ralph/research-notes.md +++ /dev/null @@ -1,216 +0,0 @@ -# Research Notes: dotnet/fsharp#16432 / PR #19862 - -## Summary - -For an `inherit T()` clause where `T` is undefined, FSC emits FS0039 ("The type -`T` is not defined") **three times** at the exact same `(line, column)` range. -The current PR (#19862) masks the duplicates with a `cenv`-scoped -`InheritDedupDiagnosticsLogger` keyed on `(idRange, idText)` of the -`UndefinedName` exception. Empirical tracing confirms the root cause: the -identical `SynType` node embedded in the inherit clause is type-checked -**three** times by three independent passes of `CheckDeclarations`: -Phase 1D (FirstPass) and Phase 1F (SecondPass) of -`TcTyconDefnCore_Phase1D_Phase1F_EstablishSuperTypesAndInterfaceTypes`, then -Phase 2A `Phase2AInherit` of `MutRecBindingChecking`. Each pass independently -invokes name resolution on the same `SynType`, and each independent failure -emits FS0039 through the diagnostics sink. - -## The three call sites (verified) - -| # | Phase | File:Line | Call | Occurrence | CheckCxs | env | -| - | ---------------------- | -------------------------------- | ---------------- | ----------- | ------------ | ------------ | -| 1 | Phase 1D (FirstPass) | `CheckDeclarations.fs:3354` | `TcTypeAndRecover` | `UseInType` | `NoCheckCxs` | `envinner` | -| 2 | Phase 1F (SecondPass) | `CheckDeclarations.fs:3354` | `TcTypeAndRecover` | `UseInType` | `CheckCxs` | `envinner` | -| 3 | Phase 2A | `CheckDeclarations.fs:1399` (inside `try`, call at line 1403) | `TcType` | `Use` | `CheckCxs` | `envInstance` | - -Drivers for sites #1 and #2 (same function, two passes), in the same file: - -```text -CheckDeclarations.fs:4180 (envMutRecPrelim, withAttrs) |> TcTyconDefnCore_Phase1D_Phase1F_EstablishSuperTypesAndInterfaceTypes cenv tpenv inSig FirstPass -CheckDeclarations.fs:4216 (envMutRecPrelim, withAttrs) |> TcTyconDefnCore_Phase1D_Phase1F_EstablishSuperTypesAndInterfaceTypes cenv tpenv inSig SecondPass -``` - -Each pass walks `SynTypeDefnSimpleRepr.General(_, inherits, ...)` and feeds -the inherit `SynType` to `TcTypeAndRecover`. The recovery point that actually -calls `errorRecovery` (i.e. emits the diagnostic) is -`TcTypeOrMeasureAndRecover` at `CheckExpressions.fs:5217`: - -```fsharp -and TcTypeOrMeasureAndRecover kindOpt (cenv: cenv) newOk checkConstraints occ iwsam env tpenv ty = - let g = cenv.g - try - TcTypeOrMeasure kindOpt cenv newOk checkConstraints occ iwsam env tpenv ty - with RecoverableException e -> - errorRecovery e ty.Range - ... - recoveryTy, tpenv -``` - -Site #3 uses raw `TcType` and recovers via its own outer `try/with -RecoverableException e -> errorRecovery e m; mkUnit g m, tpenv` at -`CheckDeclarations.fs:1407-1410`. All three sites ultimately funnel into -`errorRecovery`, which routes through the active `DiagnosticsLogger`. - -## Why each pass exists (with quotes from source) - -- **Phase 1D / Phase 1F** — both runs share one function whose header - documents the dual-pass design (`CheckDeclarations.fs:3322-3324`): - - > `// Third phase: check and publish the super types. Run twice, once before constraints are established` - > `// and once after` - - The first pass (`NoCheckCxs`) is needed so the *shape* of the super-type - hierarchy is published into the symbol table before user-supplied - constraints are checked. The second pass (`CheckCxs`) re-resolves the same - syntax now with full constraint checking enabled. - -- **Phase 2A** (`MutRecBindingChecking`, `CheckDeclarations.fs:1398-1399`): - - > `// Phase2B: typecheck the argument to an 'inherits' call and build the new object expr for the inherit-call` - > `| Phase2AInherit (synBaseTy, arg, baseValOpt, m) ->` - - This is the *expression* type-checking pass. It needs to resolve - `synBaseTy` again because at this point it builds the actual `TcNewExpr` - for the constructor call to the base type (the previous two passes only - computed `TType`s for the inheritance graph; they did not elaborate the - `inherit Base(args)` expression). It also uses `envInstance`, which is the - enriched environment containing the implicit-constructor args, not - `envinner`. - -## Evidence — failing test output with PR mechanism neutralised - -`useInheritDedupLogger` was changed locally to a pass-through: - -```fsharp -let private useInheritDedupLogger (cenv: cenv) = - ignore cenv - UseTransformedDiagnosticsLogger(fun inner -> inner) -``` - -Running: - -```bash -dotnet test --project tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj \ - -c Debug --no-build -- --filter-method "*Inherit nonexistent type reports single FS0039*" --output Detailed -``` - -Verbatim assertion failure (test source: `tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/ClassTypes/InheritsDeclarations/InheritsDeclarations.fs:77`): - -``` -Expected exactly 1 FS0039 but got 3. Diagnostics: -[{ Error = Error 39 - Range = { StartLine = 3 - StartColumn = 12 - EndLine = 3 - EndColumn = 27 } - NativeRange = (3,12--3,27) - Message = "The type 'NonExistentBase' is not defined." - SubCategory = "typecheck" }; - { Error = Error 39 - Range = { StartLine = 3 - StartColumn = 12 - EndLine = 3 - EndColumn = 27 } - NativeRange = (3,12--3,27) - Message = "The type 'NonExistentBase' is not defined." - SubCategory = "typecheck" }; - { Error = Error 39 - Range = { StartLine = 3 - StartColumn = 12 - EndLine = 3 - EndColumn = 27 } - NativeRange = (3,12--3,27) - Message = "The type 'NonExistentBase' is not defined." - SubCategory = "typecheck" }] -``` - -All three diagnostics carry **identical** `NativeRange = (3,12--3,27)` and -identical message, confirming they originate from re-resolving the *same* -syntactic node, not from three different syntactic occurrences. - -## Evidence — `eprintfn` trace - -Per-site `eprintfn` was added at sites #1/#2 (`CheckDeclarations.fs:3353`) -and #3 (`CheckDeclarations.fs:1400`). Test stdout captured: - -``` -[TRACE 16432] Phase1D/1F inherit-resolve pass=FirstPass tycon=MyClass -[TRACE 16432] Phase1D/1F inherit-resolve pass=SecondPass tycon=MyClass -[TRACE 16432] Phase2A inherit-resolve range=(3,12--3,27) -``` - -Exactly three resolutions for the one `inherit NonExistentBase()` clause; the -Phase 2A range `(3,12--3,27)` matches the range of every emitted FS0039, -confirming the three diagnostics are produced by these three sites and only -these three sites. - -## SynType identity across phases - -`CheckDeclarations.fs:4579-4583` collects inherits from member definitions -into the `SynTypeDefnSimpleRepr.General.inherits` list: - -```fsharp -let inherits = - members |> List.choose (function - | SynMemberDefn.Inherit (Some ty, idOpt, m, _) -> Some(ty, m, idOpt) - | SynMemberDefn.ImplicitInherit (ty, _, idOpt, m, _) -> Some(ty, m, idOpt) - | _ -> None) -``` - -The very same `ty: SynType` object — produced once by the parser — flows two -ways: - -1. Into the `SynTypeDefnSimpleRepr.General(_, inherits, ...)` list consumed - by Phase 1D/1F (`CheckDeclarations.fs:3348-3354`), where each pass - destructures `(ty, m, _)` and passes `ty` to `TcTypeAndRecover`. -2. Remains inside the original `SynMemberDefn.ImplicitInherit (ty, _, _, _, _)` - (resp. `SynMemberDefn.Inherit (Some ty, ...)`) which is what - `MutRecBindingChecking` later receives as `Phase2AInherit (synBaseTy, arg, - baseValOpt, m)` — i.e. **`synBaseTy` is the same `SynType` reference**. - -Therefore `synBaseTy.Range == ty.Range` is true *by construction*, not by -coincidence. The trace above provides empirical confirmation: the Phase 2A -range `(3,12--3,27)` equals the range of every reported diagnostic, which is -the parser-assigned range of that one `SynType` node. - -## Cache-key rationale - -Because the `SynType` node is shared across passes, the pair -`(tcref.Stamp, ty.Range)` — equivalently `(tcref.Stamp, synBaseTy.Range)` at -the Phase 2A site — **uniquely identifies a single inherit clause across all -three passes**. `tcref.Stamp` disambiguates two different tycons that happen -to share a source range (e.g. when the syntax came from a synthetic source), -and `ty.Range` distinguishes multiple inherits within the same tycon -(currently only legal once per class, but the key is robust to interface -implementations and to future relaxation). - -Notably, the PR's current key `(idRange, idText)` of the `UndefinedName` -exception works too because `idRange` for an inherit identifier coincides -with the embedded `SynType`'s identifier range — but it is keyed on the -diagnostic payload rather than on the syntactic node itself. The root-cause -key `(tcref.Stamp, ty.Range)` is preferable because it can short-circuit the -*resolution attempt* rather than just suppress the resulting diagnostic. - -## Recommended next step (root-cause fix) - -Replace the diagnostics-sink dedup with a per-`cenv` -`ConcurrentDictionary` (call it -`inheritResolutionFailed`). At each of the three call sites, when the -`TcType*` call raises a `RecoverableException` whose underlying exception is -an `UndefinedName`, add `(tcref.Stamp, ty.Range)` to the set *and* let the -first pass emit its diagnostic normally. On the subsequent two passes, before -invoking `TcTypeAndRecover` / `TcType`, look up the key; if present, -short-circuit to `g.obj_ty_ambivalent` / `mkUnit g m` directly without -calling name resolution at all. This avoids: - -- emitting duplicate FS0039s (the primary goal), -- doing redundant work in name resolution and the tc-sink for the two - later passes, and -- masking *unrelated* recoverable diagnostics (constraint failures, type - provider failures) that the current sink-level dedup would also silently - drop if they happened to share the `(idRange, idText)` key with a previously - reported `UndefinedName`. - -Only `UndefinedName` should mark the cache key; other recoverable failures -must still flow through the diagnostics sink so that constraint, IWSAM, and -type-provider diagnostics surface correctly on Phase 1F and Phase 2A. diff --git a/src/Compiler/Checking/CheckBasics.fs b/src/Compiler/Checking/CheckBasics.fs index a1308886066..83ce05f92b9 100644 --- a/src/Compiler/Checking/CheckBasics.fs +++ b/src/Compiler/Checking/CheckBasics.fs @@ -313,11 +313,7 @@ type TcFileState = argInfoCache: ConcurrentDictionary - /// Marks `(tycon.Stamp, synBaseTy.Range)` pairs for which the inherit-clause type - /// failed to resolve with an `UndefinedName` diagnostic in an earlier pass. Subsequent - /// passes (Phase 1F, Phase 2A) consult this marker to skip re-resolving the same - /// syntactic inherit clause, eliminating the redundant work and the duplicate FS0039. - /// See issue dotnet/fsharp#16432. + /// Inherit clauses whose type already failed UndefinedName; skip re-resolution to avoid duplicate FS0039. inheritResolutionFailed: ConcurrentDictionary // forward call diff --git a/src/Compiler/Checking/CheckBasics.fsi b/src/Compiler/Checking/CheckBasics.fsi index a98686a5ac8..c264cfa5a46 100644 --- a/src/Compiler/Checking/CheckBasics.fsi +++ b/src/Compiler/Checking/CheckBasics.fsi @@ -274,11 +274,7 @@ type TcFileState = /// we're always dealing with the same instance and the updates don't get lost argInfoCache: ConcurrentDictionary - /// Marks `(tycon.Stamp, synBaseTy.Range)` pairs for which the inherit-clause type - /// failed to resolve with an `UndefinedName` diagnostic in an earlier pass. Subsequent - /// passes (Phase 1F, Phase 2A) consult this marker to skip re-resolving the same - /// syntactic inherit clause, eliminating the redundant work and the duplicate FS0039. - /// See issue dotnet/fsharp#16432. + /// Inherit clauses whose type already failed UndefinedName; skip re-resolution to avoid duplicate FS0039. inheritResolutionFailed: ConcurrentDictionary // forward call diff --git a/src/Compiler/Checking/CheckDeclarations.fs b/src/Compiler/Checking/CheckDeclarations.fs index 21364677753..e00b420201e 100644 --- a/src/Compiler/Checking/CheckDeclarations.fs +++ b/src/Compiler/Checking/CheckDeclarations.fs @@ -50,25 +50,14 @@ open FSharp.Compiler.TypeProviders type cenv = TcFileState -/// Recognises a recoverable `UndefinedName` failure (possibly wrapped). Used by the inherit-clause -/// type-checking path to mark a `(tycon, range)` pair as already-reported so later passes -/// (Phase 1F, Phase 2A) can skip re-resolving the same syntactic clause. See issue dotnet/fsharp#16432. -/// -/// `UndefinedName` may arrive wrapped by `WrappedError` or by any of the constraint-solver -/// wrappers (`ErrorFromAddingTypeEquation`, `ErrorFromAddingConstraint`, `ErrorFromApplyingDefault`) -/// — these are common when the inherit clause carries type arguments and the failure surfaces -/// during the `CheckCxs` second pass. All such wrappers are unwrapped so the marker is set in -/// every case. -let private isUndefinedNameFailure (e: exn) = - let rec loop (e: exn) = - match e with - | UndefinedName _ -> true - | WrappedError(inner, _) -> loop inner - | ErrorFromAddingTypeEquation(error = inner) -> loop inner - | ErrorFromAddingConstraint(error = inner) -> loop inner - | ErrorFromApplyingDefault(error = inner) -> loop inner - | _ -> false - loop e +let rec (|UndefinedNameError|_|) (e: exn) = + match e with + | UndefinedName _ -> Some () + | WrappedError(inner, _) + | ErrorFromAddingTypeEquation(error = inner) + | ErrorFromAddingConstraint(error = inner) + | ErrorFromApplyingDefault(error = inner) -> (|UndefinedNameError|_|) inner + | _ -> None //------------------------------------------------------------------------- // Mutually recursive shapes @@ -1386,12 +1375,6 @@ module MutRecBindingChecking = // Phase2B: typecheck the argument to an 'inherits' call and build the new object expr for the inherit-call | Phase2AInherit (synBaseTy, arg, baseValOpt, m) -> - // If Phase 1D/1F already reported `UndefinedName` for this exact syntactic - // inherit clause, short-circuit: the user has seen the FS0039 once and we - // would only re-emit the same diagnostic by trying to re-resolve here. - // Non-`UndefinedName` failures (and the success path) still go through - // normal type-checking below, so IDE `ItemOccurrence.Use` sink events - // for valid types are preserved. See issue dotnet/fsharp#16432. let inheritsExpr, tpenv = if cenv.inheritResolutionFailed.ContainsKey(struct (tcref.Stamp, synBaseTy.Range)) then mkUnit g m, tpenv @@ -3346,30 +3329,23 @@ module EstablishTypeDefinitionCores = let kind = InferTyconKind g (kind, attrs, slotsigs, fields, inSig, isConcrete, m) let inherits = inherits |> List.map (fun (ty, m, _) -> (ty, m)) - // Resolve each inherit type. If a previous pass already failed with `UndefinedName` - // for this clause, skip re-resolution entirely (returns the recovery type used - // by `TcTypeAndRecover`). This avoids the duplicate FS0039 (#16432) without - // any diagnostic-sink trickery and prevents the duplicate work too. Failures - // that are NOT `UndefinedName` (e.g. constraint or type-provider errors that - // only surface in the `CheckCxs` second pass) are NOT marked, so they continue - // to flow through the normal recovery path. - let inheritedTys = - inherits - |> List.mapFold (fun tpenv (ty, m) -> - let key = struct (tcref.Stamp, ty.Range) - if cenv.inheritResolutionFailed.ContainsKey key then + let tryResolveInheritType tpenv (ty: SynType, m) = + let key = struct (tcref.Stamp, ty.Range) + if cenv.inheritResolutionFailed.ContainsKey key then + (g.obj_ty_ambivalent, m), tpenv + else + try + let resolved, tpenv = TcType cenv NoNewTypars checkConstraints ItemOccurrence.UseInType WarnOnIWSAM.No envinner tpenv ty + (resolved, m), tpenv + with + | RecoverableException (UndefinedNameError as e) -> + errorRecovery e ty.Range + cenv.inheritResolutionFailed.TryAdd(key, ()) |> ignore (g.obj_ty_ambivalent, m), tpenv - else - try - let resolved, tpenv = TcType cenv NoNewTypars checkConstraints ItemOccurrence.UseInType WarnOnIWSAM.No envinner tpenv ty - (resolved, m), tpenv - with RecoverableException e -> - errorRecovery e ty.Range - if isUndefinedNameFailure e then - cenv.inheritResolutionFailed.TryAdd(key, ()) |> ignore - (g.obj_ty_ambivalent, m), tpenv - ) tpenv - |> fst + | RecoverableException e -> + errorRecovery e ty.Range + (g.obj_ty_ambivalent, m), tpenv + let inheritedTys = inherits |> List.mapFold tryResolveInheritType tpenv |> fst let implementedTys, inheritedTys = match kind with | SynTypeDefnKind.Interface -> diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/ClassTypes/InheritsDeclarations/InheritsDeclarations.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/ClassTypes/InheritsDeclarations/InheritsDeclarations.fs index a309e10dae4..d9dc77816cc 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/ClassTypes/InheritsDeclarations/InheritsDeclarations.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/ObjectOrientedTypeDefinitions/ClassTypes/InheritsDeclarations/InheritsDeclarations.fs @@ -57,52 +57,50 @@ module InheritsDeclarations = |> compile |> shouldSucceed - // Regression tests for https://github.com/dotnet/fsharp/issues/16432: - // inheriting from an unknown type should emit FS0039 only once, - // not three times (once per name-resolution site in CheckDeclarations). - - let private assertSingleFS39 source = - let result = - FSharp source - |> typecheck - - let fs39 = - result.Output.Diagnostics - |> List.filter (fun d -> d.Error = (Error 39)) + [] + let ``Inherit from undefined non-generic type reports single FS0039`` () = + FSharp "type MyClass() =\n inherit NonExistentBase()\n" + |> typecheck + |> shouldFail + |> withDiagnostics [ + (Error 39, Line 2, Col 13, Line 2, Col 28, "The type 'NonExistentBase' is not defined.") + ] - Assert.True( - fs39.Length = 1, - sprintf "Expected exactly 1 FS0039 but got %d. Diagnostics:\n%A" fs39.Length result.Output.Diagnostics - ) + [] + let ``Inherit from undefined generic type reports single FS0039`` () = + FSharp "type MyClass() =\n inherit MissingGeneric()\n" + |> typecheck + |> shouldFail + |> withDiagnostics [ + (Error 39, Line 2, Col 13, Line 2, Col 27, "The type 'MissingGeneric' is not defined.") + ] - [] - [] - [()\n")>] - [] - let ``Inherit from undefined type reports single FS0039`` source = - assertSingleFS39 source + [] + let ``Interface inheriting undefined interface reports single FS0039`` () = + FSharp "type IMyInterface =\n inherit INonExistent\n" + |> typecheck + |> shouldFail + |> withDiagnostics [ + (Error 39, Line 2, Col 13, Line 2, Col 25, "The type 'INonExistent' is not defined.") + (Error 887, Line 2, Col 5, Line 2, Col 25, "The type 'obj' is not an interface type") + ] [] - let ``Two different undefined names still report separately`` () = - let result = - FSharp """ + let ``Undefined inherit and undefined value report independently`` () = + FSharp """ type MyClass() = inherit NonExistentBase() member _.X = undefinedValue """ - |> typecheck - - let fs39 = - result.Output.Diagnostics - |> List.filter (fun d -> d.Error = (Error 39)) - - Assert.True( - fs39.Length >= 2, - sprintf "Expected >= 2 FS0039, got %d. Diagnostics:\n%A" fs39.Length result.Output.Diagnostics - ) + |> typecheck + |> shouldFail + |> withDiagnostics [ + (Error 39, Line 3, Col 13, Line 3, Col 28, "The type 'NonExistentBase' is not defined.") + (Error 39, Line 4, Col 18, Line 4, Col 32, "The value or constructor 'undefinedValue' is not defined.") + ] [] - let ``Valid inherit produces no FS0039`` () = + let ``Valid inherit produces no diagnostics`` () = FSharp """ open System type MyClass() =