Revive TypeInferenceTests from NRefactory and cover the C# standard inference rules - #3915
Merged
Merged
Conversation
The direct unit tests for TypeInference were lost when the NRefactory sources were replaced by the NuGet package (e88120c); since then the class had no dedicated coverage and ConversionTests still pointed to a test that no longer existed. Ported to the current type system API and NUnit constraint asserts. The two tests NRefactory ignored on .NET 4.5 now pin the covariant IReadOnlyList<T> results, since the test compilation uses the 4.5-era reference mscorlib; the common-subtype list test gains the ReadOnlyCollectionBuilder<T> candidates contributed by System.Core, which the NRefactory compilation did not reference. Also includes the seven tests that only exist in upstream icsharpcode/NRefactory (async lambdas, NullablePick, CoContraPick, bug 9300, user-defined-conversion bounds). Upstream wrote them against its source-based resolver harness, which this repo does not have, so they are reexpressed as direct InferTypeArguments calls using mock lambdas and helper types declared in the test assembly. Upstream's InferFromImplicitAsyncLambda was missing its [Test] attribute and never actually ran; here it does. Assisted-by: Claude:claude-fable-5:Claude Code
Walks the inference algorithm of the standard (draft-v11, 12.6.3) and adds a test per rule that the revived NRefactory suite did not already exercise: exact inference for ref parameters and its non-applicability of the base-type walk, explicit lambda parameter types, exact/upper bound inference through arrays, nullables and variance nesting, the unique-base-type restriction, value-type elements forcing exact inference, conflicting exact bounds, and best common type. Two rules are pinned as ignored tests because the implementation does not follow the standard yet: a value argument to an 'in' parameter infers no bound (12.6.3.7 wants a lower-bound inference), and tuple literals are not inferred elementwise (12.6.3.7/12.6.3.8). Both tests assert the csc-verified result and should go green when the rules are implemented. Nullable unwrapping in exact and upper-bound inferences needs no dedicated code path in this implementation: T? is represented as the constructed type Nullable<T>, so the constructed-type case already produces the elementwise exact inference the standard asks for; the new tests pin that equivalence. Function-pointer inference rules and the explicit-return-type inference of 12.6.3.15 remain untested: the former needs a MetadataModule to construct FunctionPointerType, the latter is not representable in LambdaResolveResult. Assisted-by: Claude:claude-fable-5:Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The direct unit tests for
TypeInferencewere lost when the NRefactory sources were replaced by the NuGet package (e88120c); since then the class had no dedicated coverage, andConversionTestsstill referencesTypeInferenceTests.IEnumerableCovarianceWithDynamicin a comment. This PR restores that suite and extends it.Commit 1 revives all 26 tests from upstream icsharpcode/NRefactory (the vendored snapshot only had 19), ported to the current type system API:
[Ignore]d on .NET 4.5 now pin the covariantIReadOnlyList<T>results, since the test compilation uses the 4.5-era reference mscorlib.InferTypeArgumentscalls with mock lambdas and helper types declared in the test assembly.InferFromImplicitAsyncLambdais missing its[Test]attribute and never actually ran there; here it does (and passes).Commit 2 walks the inference algorithm of the C# standard (draft-v11, 12.6.3) and adds a test per rule not already exercised: exact inference for
refparameters, explicit lambda parameter types, exact/upper-bound inference through arrays, nullables and variance nesting, the unique-base-type restriction, value-type elements forcing exact inference, conflicting exact bounds, and best common type.Two rules are pinned as
[Ignore]d tests asserting csc-verified behavior the implementation does not follow yet:in Tparameter infers no bound at all (12.6.3.7 requires a lower-bound inference);M<T>((T, T))called with(1, 2L)must inferT = long, but currently produces conflicting exact bounds.Test results: 44 passed, 2 ignored (the pins above); no product code is changed.
🤖 This PR was prepared by an AI agent (Claude Code) operated by @siegfriedpammer.