[typemap] Diagnose unsupported constructor shapes - #12567
Open
simonrozsival wants to merge 5 commits into
Open
Conversation
Detect ambiguous and unrepresentable Java constructor signatures before trimmable typemap generation emits partial output. Add localized XA4259-XA4262 diagnostics, llvm-ir parity fixtures, scanner and generator coverage, and a focused build regression test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Compile the fixture exclusion helper with its required namespaces and record llvm-ir's single-dimensional Java signature for rectangular managed arrays. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use effective Export and explicit registration signatures during constructor validation, validate implicit Export base forwarding, detect explicit signature collisions, and reject noncanonical super argument references. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Validate explicit constructor base calls by declared JNI signature, analyze non-public registered constructors, distinguish inferred parameterless fallbacks, and parse super argument references outside Java literals, comments, and qualified member access. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
27 tasks
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
Review tier: Lite
Findings: 1
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
 |
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Scanner/ConstructorDetectionTests.cs — 💡 suggestion — ScanPeer opens and reads both fixture assemblies from disk on every test invocation.… |
 |
src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs — ❌ error — Base-constructor compatibility for non-explicit constructors is currently checked via… |
What changed in this PR
Adds first-class constructor-shape validation to the trimmable typemap pipeline, surfacing new localized XA4259–XA4262 diagnostics and ensuring generation stops before emitting partial Java/type-map output when constructors are not representable or are ambiguous.
Changes:
- Introduces constructor diagnostics (collision, unsupported parameter shapes, missing compatible base ctor, invalid
SuperArgumentsString) and wires them into the generator/Build.Tasks logger with localized resources. - Expands test coverage with new “invalid constructor” fixture assembly and adds focused unit/integration/build tests to validate diagnostics and “no partial output” behavior.
- Documents new error codes (XA4259–XA4262) and adds them to the docs index/TOC.
| File | Description |
|---|---|
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/TestFixtures/StubAttributes.cs | Extends stub ExportAttribute to support ctor usage and SuperArgumentsString for scanner tests. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Scanner/ConstructorDetectionTests.cs | Adds targeted tests for new constructor diagnostics and representable/explicit cases. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj | Adds a new fixture project and copies its output alongside existing fixtures. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/InvalidConstructorFixtures/InvalidConstructors.cs | New fixture types that intentionally trigger (or avoid) constructor diagnostics. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/InvalidConstructorFixtures/InvalidConstructorFixtures.csproj | New fixture assembly project (unsafe enabled) used as scanner/generator input. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TrimmableTypeMapGeneratorTests.cs | Verifies generator logs coded errors and returns no partial outputs when ctor diagnostics exist. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/UserTypesFixture/UserTypesFixture.csproj | Enables unsafe to support new user fixture types using pointers/function pointers. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/UserTypesFixture/UserTypes.cs | Adds user fixture types mirroring ctor-collision/unrepresentable/super-args scenarios. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/ScannerRunner.cs | Adds legacy constructor extraction to support ctor parity tests. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/ScannerComparisonTests.cs | Excludes ctor-diagnostic fixtures from legacy↔new marshal-method parity comparisons. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/ConstructorParityTests.cs | New integration tests that pin legacy behavior around ctor collisions/unrepresentable ctors/super args. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/TrimmableTypeMapBuildTests.cs | Adds a build-level regression ensuring XA4259 fails before any partial typemap Java is written. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs | Implements new logger hooks for XA4259–XA4262 in the MSBuild task. |
| src/Xamarin.Android.Build.Tasks/Properties/Resources.resx | Adds localized strings for XA4259–XA4262. |
| src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs | Updates generated resource accessors for XA4259–XA4262. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs | Validates constructor diagnostics early and returns no generated outputs on failure. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs | Computes ctor diagnostics during scanning, including signature collapse, parameter-shape validation, base-ctor compatibility, and SuperArgumentsString validation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerInfo.cs | Adds ConstructorDiagnostics model plus diagnostic kind/info types. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/ITrimmableTypeMapLogger.cs | Extends logger interface with ctor diagnostic logging hooks. |
| Documentation/docs-mobile/TOC.yml | Adds entries for XA4259–XA4262 docs. |
| Documentation/docs-mobile/messages/xa4259.md | New documentation page for XA4259. |
| Documentation/docs-mobile/messages/xa4260.md | New documentation page for XA4260. |
| Documentation/docs-mobile/messages/xa4261.md | New documentation page for XA4261. |
| Documentation/docs-mobile/messages/xa4262.md | New documentation page for XA4262. |
| Documentation/docs-mobile/messages/index.md | Adds XA4259–XA4262 to the message index listing. |
Files not reviewed (1)
- src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs: Generated file
Comment on lines
+251
to
+257
| using var scanner = new JavaPeerScanner (); | ||
| var testAssemblyDir = Path.GetDirectoryName (typeof (ConstructorDetectionTests).Assembly.Location) | ||
| ?? throw new InvalidOperationException ("Cannot determine test assembly directory."); | ||
| using var fixtureReader = new PEReader (File.OpenRead (TestFixtureAssemblyPath)); | ||
| var invalidFixturePath = Path.Combine (testAssemblyDir, "InvalidConstructorFixtures.dll"); | ||
| using var invalidFixtureReader = new PEReader (File.OpenRead (invalidFixturePath)); | ||
| var fixtureMetadata = fixtureReader.GetMetadataReader (); |
Comment on lines
+2557
to
+2561
| string forwardedBaseSignature = exportInfo?.SuperArgumentsString == "" ? "()V" : jniSignature; | ||
| bool hasCompatibleBaseConstructor = hasExplicitRegistration | ||
| ? baseConstructors.Any (baseCtor => baseCtor.RegisterInfo.Signature == forwardedBaseSignature) | ||
| : baseConstructors.Any (baseCtor => | ||
| HaveIdenticalParameterTypes (methodDef, index, baseCtor.Method, baseCtor.Index, baseCtor.DeclaringType)); |
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
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.
Summary
SuperArgumentsStringexpressionsValidation
Part of #12561