Revive NRefactory conversion tests and cover all of C# standard chapter 10#3912
Merged
Conversation
The block of tests inherited from NRefactory's ConversionsTest was commented out because it depended on ResolverTestBase (full AST + CSharpResolver). Rewrite all of them against CSharpConversions directly: hand-built MethodGroupResolveResults over fixture types compiled into the test assembly (extension methods injected via the internal extensionMethods field), DefaultTypeParameter instances with cross-referencing constraints, and metadata-backed fixture interfaces for the ExpansiveInheritance termination test. Two deviations from the NRefactory originals: MethodGroupConversion_RefArgumentObjectVsDynamic now expects a valid conversion, because object/dynamic mismatch in a ref parameter is an identity conversion and current csc accepts the assignment (verified); PreferUserDefinedConversionOverReferenceConversion resolved an invocation, so it is recast as an OverloadResolution test over FakeMethod candidates. Assisted-by: Claude:claude-fable-5:Claude Code
Convert the commented-out block in ExplicitConversionsTest the same way as the implicit ConversionTest block: type-parameter casts via DefaultTypeParameter with cross-referencing constraints, user-defined operators as fixture types in the test assembly, constant sources via ConstantResolveResult. The rr.Input asserts of the originals were resolver artifacts and are dropped; UseDefinedExplicitConversion_Lifted instead exercises the ResolveResult-based ExplicitConversion entry point. Also extend PreferAmbiguousConversionOverReferenceConversion with the overload-resolution half of the original NRefactory test (the ambiguous conversion must not prevent M(BB) from being chosen over M(object)), which the first revival pass had reduced to the conversion classification alone. Assisted-by: Claude:claude-fable-5:Claude Code
Audit the conversion test suites against the conversions chapter of the draft-v8 C# standard and add tests for every rule that had none: exhaustive implicit/explicit numeric conversion matrix, tuple/ValueTuple and nullable-annotation identity, interpolated-string / throw-expression / tuple-literal conversions, the boxing rule set including variance-based boxing and unboxing, delegate-to-System.Delegate and IReadOnlyList<T> reference conversions, type-parameter variance and effective-base-class casts, generic method groups (inference, explicit type arguments, no inference from the return type), the anonymous-function compatibility checks CSharpConversions performs itself (via a LambdaResolveResult test double), standard-conversion exclusion of user-defined operators, and operators declared in base classes of the source type. Two rules are implementation gaps rather than test gaps and get ignored placeholder tests naming the gap: default literal conversions (10.2.16) are an explicit TODO in CSharpConversions, and switch expressions (10.2.18) have no ResolveResult representation. The full section-by-section map is in Analyses/ILSpy/2026-07-24_conversions-spec-coverage.md. Assisted-by: Claude:claude-fable-5:Claude Code
Dumping the implicit/explicit classification of all pairs from a 120-type universe and diffing it against what csc actually compiles surfaced two divergences (16 affected pairs), both proven by a compiling-and-running snippet and captured here as ignored known-bug tests: 1. Nullable conversions derived from tuple conversions are missing: csc accepts "(long, object)? a = t;" for t of type (int, string) as well as the lifted and explicit forms, CSharpConversions returns None. The ECMA spec's 10.6.1 does not list tuple conversions as liftable, so this is a case of Roslyn exceeding the spec. 2. An explicit user-defined conversion to a nullable target is rejected when the operator result additionally needs an explicit numeric conversion: csc accepts "(int?)new ImplicitToLong()" (operator to long, then explicit long -> int?), CSharpConversions returns None. The same sweep confirmed the SByte..Decimal TypeCode range in ImplicitEnumerationConversion is correct: csc accepts zero constants of any numeric type (0.0, 0f, 0m) for enum conversion and rejects '\0', matching the implementation exactly. 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.
Revives the NRefactory-era conversion tests that were commented out in
ConversionTests.csandExplicitConversionTest.cs("TODO: we should probably revive these tests somehow") and extends the suites so that every rule of the conversions chapter of the draft-v8 C# standard has a test case.MethodGroupResolveResults over fixture types compiled into the test assembly (extension methods injected through the internalextensionMethodsfield), type-parameter conversions viaDefaultTypeParameterwith cross-referencing constraints, user-defined operators as metadata-backed fixture types, and constant expressions viaConstantResolveResult.MethodGroupConversion_RefArgumentObjectVsDynamicnow expects a valid conversion: object/dynamic mismatch in arefposition is an identity conversion and current csc accepts the assignment.CSharpConversionsperforms itself.[Ignore]d placeholder tests: default literal conversions (10.2.16, an explicit TODO inCSharpConversions) and switch expression conversions (10.2.18, no ResolveResult representation).CSharpConversionsbugs, captured as[Ignore]d known-bug tests: nullable conversions derived from tuple conversions are missing entirely, and explicit user-defined conversions to a nullable target are rejected when the operator result additionally needs an explicit numeric conversion.This pull request was authored by an AI agent (Claude Code) operated by @siegfriedpammer.
🤖 Generated with Claude Code