Skip to content

[generator] Remove IntDefinitionAttribute and the dead javadoc-to-mdoc tool - #12683

Draft
simonrozsival wants to merge 1 commit into
mainfrom
simonrozsival-compact-intdefinition-metadata
Draft

[generator] Remove IntDefinitionAttribute and the dead javadoc-to-mdoc tool#12683
simonrozsival wants to merge 1 commit into
mainfrom
simonrozsival-compact-intdefinition-metadata

Conversation

@simonrozsival

Copy link
Copy Markdown
Member

⚠️ Draft / experimental — seeking feedback, not ready to merge

This is an intentionally breaking, experimental change. It is not a drop-in fix — it removes a public API type and deletes a whole (dead) tool. Opening as a draft to get feedback on the approach, the compatibility analysis, and whether this is something the team wants to pursue, before investing in updating the committed API-compat reference/acceptable-breakages files.

Background

Android.Runtime.IntDefinitionAttribute is applied by the generator to every enum member it binds (11,243 applications on Mono.Android API-37 alone). Each application carries two strings: the managed member name (ConstantMember) and the originating Java field (JniField). Investigation showed this attribute exists solely to feed a legacy javadoc→mdoc enum-mapping importer (javadoc-to-mdoc.cs / Xamarin.Android.Tools.JavadocImporter), and that importer turns out to be dead code that isn't part of any build in this repo.

Evidence the attribute has no remaining in-tree consumer

  • BindingStudio does not read IntDefinitionAttribute at all — it consumes api.xml/map.csv/methodmap.csv (per the requester's own research).
  • The current, active docs pipeline (_RunMdoc/UpdateExternalDocumentation in src/Mono.Android/Mono.Android.targets) shells out to the external mdoc.exe/mdoc.dll NuGet tool via mdoc update/mdoc fx-bootstrap. It has nothing to do with this attribute or with javadoc-to-mdoc.cs.
  • Trimmed apps already strip every instance of IntDefinitionAttribute via ILLink.LinkAttributes.xml's RemoveAttributeInstances. So this change mainly benefits reference assemblies, NuGet package size, and non-trimmed (Debug/library) consumption — trimmed/linked apps see no runtime benefit since the attribute was already gone there.

Evidence javadoc-to-mdoc is dead code

  • grep -i javadoc Xamarin.Android.slnx Xamarin.Android.Build.Tasks.slnx Xamarin.Android-Tests.slnxzero matches. Neither src/Xamarin.Android.Tools.JavadocImporter/Xamarin.Android.Tools.JavadocImporter.csproj nor tools/javadoc2mdoc/javadoc2mdoc.csproj is referenced by any solution in this repo — they are not part of the build graph at all, not merely unused at runtime.
  • The MSBuild flag that would route to it, _UseLegacyJavadocImport, is never set to true anywhere in-repo. Its only consumer, the Xamarin.Android.Tasks.ImportJavaDoc task (invoked from Xamarin.Android.Bindings.Documentation.targets's BuildDocumentation target), was therefore already unreachable dead code before this PR — and it targets a tool executable (javadoc-to-mdoc.exe) that no project in this repo has built for some time anyway.
  • No CI workflow, Makefile target, or doc in this repo references the tool.

What this PR removes

IntDefinitionAttribute:

  • Deletes src/Mono.Android/Android.Runtime/IntDefinitionAttribute.cs (the type itself — not gated, not deprecated, fully removed).
  • Stops the generator (external/Java.Interop/.../EnumGenerator.cs) from emitting [IntDefinition]; removes the now-dead IntDefinitionAttr SourceWriter helper and its test support fixture; updates generator expected-output test baselines.
  • Removes the one hand-authored usage in the tree, Android.OS.VibrationEffectEffectType (the enum values are preserved — only the attribute applications are removed).
  • Removes the now-moot ILLink.LinkAttributes.xml entry for the type.
  • Removes all Android.Runtime.IntDefinitionAttribute* entries from every active API level's PublicAPI.Shipped.txt, and adds the corresponding *REMOVED* markers to each PublicAPI.Unshipped.txt (API-35, 36, 36.1, 37, 37.1, 37.2), matching this repo's convention for intentional public API removal.

javadoc-to-mdoc / JavadocImporter:

  • Deletes src/Xamarin.Android.Tools.JavadocImporter/ (14 files: the importer source, HTML/DTD entity resources, etc.) and tools/javadoc2mdoc/ (its console driver) entirely. No .csproj/.slnx edits were needed for this because — per the evidence above — neither project was referenced anywhere.
  • Removes the JavadocToMdocToolExe MSBuild property (Xamarin.Android.Bindings.Core.targets) and the dead ImportJavaDoc task invocation + its UsingTask declaration (Xamarin.Android.Bindings.Documentation.targets), plus the now-unused ImportJavaDoc.cs task class from Xamarin.Android.Build.Tasks.
  • Left untouched, deliberately: the JavaDoc/MDoc MSBuild tasks, _ExtractJavaDocJars/_GenerateJavaDocFromSourceJars targets, and JavaDocJar/JavaSourceJar item groups. These wrap the JDK's own javadoc tool and the external mdoc.exe — they are unrelated to the deleted javadoc-to-mdoc.cs tool and remain part of a separate (still _UseLegacyJavadocImport-gated, still unrelated) legacy binding-project doc-extraction path that is out of scope here. Also left untouched: the already-[Obsolete("Do not use.")] Xamarin.AndroidTools.MonoDroidSdk.JavaDocToMDocExe string constant (different assembly, already marked dead by a prior maintainer).
  • Confirmed via full-repo search that the only remaining hits for IntDefinitionAttribute/JavadocImporter/javadoc-to-mdoc are in bin/ build output (gitignored) and the external/android-api-docs submodule's already-published historical docs for older API levels (a separate repo/history, out of scope).

Measured size impact (API-37, before/after, generator rebuilt for each side)

Config Runtime Δ Ref Δ
Debug -1,399,296 bytes -1,399,296 bytes
Release -1,398,784 bytes -1,398,784 bytes

Metadata-level breakdown (via System.Reflection.Metadata, all four Debug/Release runtime/ref DLLs):

  • CustomAttribute table: exactly 11,243 IntDefinitionAttribute rows removed in every configuration (this matches the prior estimate exactly), plus a further 7-11 CA rows from attributes applied to the deleted type/members themselves.
  • #Blob heap: ~1,263,300 bytes smaller in every configuration — closely matching the prior estimate of 1,263,277 bytes (attribute constructor/property signatures and string blobs).
  • TypeDef: -1 (the attribute class). MethodDef: -5 (ctor + 2 property getters + 2 property setters).
  • The delta is identical between runtime and ref assemblies in each configuration, because CustomAttribute/blob-heap metadata is preserved even in IL-stripped reference assemblies — only method bodies differ between ref/runtime, so both reference assemblies and runtime assemblies benefit equally from this change (this is primarily a metadata-size win, not an IL-size win).

ApiCompat

Running a normal dotnet build (i.e. without DisableApiCompatibilityCheck) against the committed tests/api-compatibility/reference/net11.0/Mono.Android.dll contract (which itself contains 10,095 IntDefinitionAttribute usages) now reports:

Compat issues with assembly Mono.Android:
TypesMustExist : Type 'Android.Runtime.IntDefinitionAttribute' does not exist in the implementation but it does exist in the contract.
Total Issues: 1

This is the expected, intended breaking change — ApiCompat's TypesMustExist rule reports the missing type once rather than once per usage site. This PR deliberately does not update tests/api-compatibility/acceptable-breakages-vReference-net11.0.txt to suppress this, so CI will flag it for reviewers rather than silently pass. If this design is accepted, that file (and the reference dump) would need to be regenerated as a follow-up.

Compatibility risk for old/pre-compiled bindings

Any previously-compiled assembly (NuGet binding package, app, etc.) that references Android.Runtime.IntDefinitionAttribute will:

  • Continue to compile, load, and execute normally for all code paths that don't reflect over the attribute — the CLR resolves attribute-constructor metadata tokens lazily, only when something actually calls a reflection API like GetCustomAttributes()/GetCustomAttributesData() on a member decorated with it.
  • Fail specifically when reflection enumerates custom attributes on a member that carries IntDefinitionAttribute (standard CLR behavior for a missing metadata-referenced type). No in-tree code does this after this PR (see evidence above), so the risk is scoped to external tooling/reflection that specifically walks enum-member attributes looking for IntDefinitionAttribute — which, per the dead-tool evidence above, should not exist for any currently-active in-repo or documented workflow.

Verified unaffected

  • GeneratedEnumAttribute — untouched; it lives in a separate file with its own SourceWriter and CecilApiImporter code path, structurally unrelated to IntDefinitionAttribute.
  • Generated enums themselves — values, names, and GeneratedEnumAttribute applications are unchanged; only the extra [IntDefinition] application is gone.
  • generator-Tests: 489/489 passed after the removal (fixture/expected-output files updated to match the new no-attribute output).
  • dotnet build src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj (which pulls in Mono.Android as a project reference) builds with 0 errors using -p:DisableApiCompatibilityCheck=True; without that flag, exactly the single expected ApiCompat finding above.

What feedback/CI signal this draft is seeking

  1. Whether the maintainers agree IntDefinitionAttribute and javadoc-to-mdoc/JavadocImporter are truly dead and safe to remove outright (vs. a more conservative gate-off-by-default design that was also prototyped and can be resurrected if preferred).
  2. Whether CI's ApiCompat failure (the single TypesMustExist finding) is acceptable to formally suppress via the acceptable-breakages file, or whether this should instead ship as a genuinely breaking change in a major version with release-notes coverage.
  3. Any additional consumers (internal or external, e.g. NuGet packages, doc tooling, third-party bindings) that the search here may have missed.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

This is an experimental, intentionally breaking change proposed as a
draft PR for feedback, not a change intended to merge as-is.

IntDefinitionAttribute
-----------------------
`Android.Runtime.IntDefinitionAttribute` was applied by the generator to
every enum member (11,243 applications across Mono.Android API-37) to
record the originating Java field name/managed member name for the sole
purpose of feeding a legacy javadoc-to-mdoc enum-mapping importer. That
importer is not part of any build in this repo (see below), so the
attribute has no remaining in-tree consumer:

 * BindingStudio does not read it; it consumes api.xml/map.csv/methodmap.csv.
 * The modern docs pipeline (`_RunMdoc`/`UpdateExternalDocumentation` in
   Mono.Android.targets) shells out to the external `mdoc.exe`/`mdoc.dll`
   tool and never touches this attribute.
 * Trimmed apps already strip every instance of it via
   ILLink.LinkAttributes.xml's `RemoveAttributeInstances`, so this mainly
   benefits reference assemblies, NuGet package size, and non-trimmed
   (library/Debug) consumption.

Changes:
 * Delete `Android.Runtime.IntDefinitionAttribute` and its PublicAPI
   entries (marked `*REMOVED*` in every active API level's
   PublicAPI.Unshipped.txt, following repo convention).
 * Stop the generator (EnumGenerator.cs) from emitting `[IntDefinition]`;
   remove the now-dead `IntDefinitionAttr` SourceWriter, its support
   fixture, and update generator test baselines (489/489 generator-Tests
   pass).
 * Remove the one hand-authored usage (VibrationEffectEffectType),
   preserving the enum values themselves.
 * Remove the now-moot ILLink.LinkAttributes.xml entry for the type.

Measured impact (API-37, this machine, before/after, generator
rebuilt for each side):
 * Debug:   runtime -1,399,296 bytes, ref -1,399,296 bytes
 * Release: runtime -1,398,784 bytes, ref -1,398,784 bytes
 * CustomAttribute table: exactly 11,243 IntDefinitionAttribute rows
   removed in every configuration (plus ~7-11 additional CA rows from
   attributes on the deleted type/members themselves).
 * #Blob heap: ~1,263,300 bytes smaller in every configuration, closely
   matching the prior estimate of 1,263,277 bytes.
 * TypeDef -1, MethodDef -5 (the attribute class + its 5 members).
 * `dotnet build` ApiCompat against the committed
   tests/api-compatibility reference assembly (10,095 attribute usages
   in the recorded contract) now reports exactly the expected break:
   `TypesMustExist: Type 'Android.Runtime.IntDefinitionAttribute' does
   not exist in the implementation but it does exist in the contract`,
   Total Issues: 1. This is intentionally NOT suppressed via the
   acceptable-breakages file so CI surfaces it for review.
 * GeneratedEnumAttribute is untouched (separate file, separate
   SourceWriter/CecilApiImporter path) and generated enums are
   otherwise unaffected.

Old/pre-compiled bindings that reference the removed attribute type
will continue to compile, load, and run normally for all code that does
not reflect over it; standard CLR semantics mean only reflection calls
that enumerate custom attributes on members decorated with
IntDefinitionAttribute (e.g. `GetCustomAttributes()`) will fail once the
type is gone, since attribute type resolution happens lazily at the
point of reflection. This is the accepted breaking-change surface for
this draft.

javadoc-to-mdoc / JavadocImporter
-----------------------------------
`src/Xamarin.Android.Tools.JavadocImporter/` (javadoc-to-mdoc.cs and
support files) and `tools/javadoc2mdoc/` (its console driver) are not
referenced by any .slnx/.sln in this repo -- grepping
Xamarin.Android.slnx, Xamarin.Android.Build.Tasks.slnx, and
Xamarin.Android-Tests.slnx for "javadoc" returns zero matches, so
neither project is part of the current build graph. The MSBuild flag
that would route to it, `_UseLegacyJavadocImport`, is never set to
`true` anywhere in-repo, so its only consumer
(`Xamarin.Android.Tasks.ImportJavaDoc`, invoked from
Xamarin.Android.Bindings.Documentation.targets'
`BuildDocumentation` target) was already unreachable dead code before
this change. No CI workflow, Makefile target, or doc references the
tool either.

Changes:
 * Delete `src/Xamarin.Android.Tools.JavadocImporter/` and
   `tools/javadoc2mdoc/` entirely (16 files; no .csproj/.slnx changes
   needed since neither was referenced anywhere).
 * Remove the `JavadocToMdocToolExe` property
   (Xamarin.Android.Bindings.Core.targets) and the dead `ImportJavaDoc`
   MSBuild task invocation + UsingTask declaration
   (Xamarin.Android.Bindings.Documentation.targets), plus the now-unused
   `ImportJavaDoc.cs` task class in Xamarin.Android.Build.Tasks.
 * Left `JavaDoc`/`MDoc` tasks, `_ExtractJavaDocJars`,
   `_GenerateJavaDocFromSourceJars`, and the `JavaDocJar`/`JavaSourceJar`
   item groups untouched: these wrap the JDK's own `javadoc` tool and
   the external `mdoc.exe`, are unrelated to the deleted
   javadoc-to-mdoc.cs tool, and remain part of the (still-gated,
   unrelated) legacy binding-project doc-extraction infrastructure --
   out of scope for this change.
 * Left the already-`[Obsolete("Do not use.")]`
   `Xamarin.AndroidTools.MonoDroidSdk.JavaDocToMDocExe` string constant
   untouched (different assembly, already marked dead by a prior
   maintainer, no project dependency).
 * Confirmed via full-repo search that the only remaining references to
   IntDefinitionAttribute/JavadocImporter/javadoc-to-mdoc are in
   `bin/` build output (gitignored) and the `external/android-api-docs`
   submodule's already-published historical docs (out of scope, a
   separate repo/history).

Validated with:
 * `dotnet test external/Java.Interop/tests/generator-Tests/generator-Tests.csproj`
   -> 489/489 passed.
 * `dotnet build src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj`
   (which pulls in Mono.Android) -> 0 errors with
   `-p:DisableApiCompatibilityCheck=True`; without it, exactly the
   expected single ApiCompat TypesMustExist finding.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@simonrozsival simonrozsival added the do-not-merge PR should not be merged. label Sep 4, 2026
@simonrozsival simonrozsival changed the title [Experimental/DRAFT] Remove IntDefinitionAttribute and the dead javadoc-to-mdoc tool [Experimental] Remove IntDefinitionAttribute and the dead javadoc-to-mdoc tool Sep 4, 2026
@simonrozsival simonrozsival changed the title [Experimental] Remove IntDefinitionAttribute and the dead javadoc-to-mdoc tool [generator] Remove IntDefinitionAttribute and the dead javadoc-to-mdoc tool Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge PR should not be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant