[generator] Add support for <ns-replace> metadata. #939
Conversation
src/Java.Interop.Tools.Generator/Metadata/NamespaceTransform.cs
Outdated
Show resolved
Hide resolved
|
Good feedback, added support for |
|
|
||
| foreach (var xe in FixupDocument.XPathSelectElements ("/metadata/ns-replace")) { | ||
| if (NamespaceTransform.TryParse (xe, out var transform)) | ||
| list.Add (transform); |
There was a problem hiding this comment.
What should happen if there's a duplicate? Should we care? Emit a warning?
Particularly if we're thinking of adding Metadata.xml into NuGet packages for "reuse" by "downstream binding projects", the likelihood for duplicates will increase. It shouldn't be an error, but would a warning be useful? Or would it be too common for warnings?
There was a problem hiding this comment.
I think it's fine to ignore duplicates, we do the same for other metadata.
I'm not aware of any plan to add Metadata.xml into NuGet packages.
|
|
||
| public List<NamespaceTransform> GetNamespaceTransforms () | ||
| { | ||
| var list = new List<NamespaceTransform> (); |
There was a problem hiding this comment.
Should this be a List<T>, or should it be a HashSet<T>?
There was a problem hiding this comment.
Order is significant, so we need to stick with List.
| } | ||
| } | ||
|
|
||
| public List<NamespaceTransform> GetNamespaceTransforms () |
There was a problem hiding this comment.
Should this return an interface instead of a List<T> from a public method in a public class? Means we can't change the runtime type easily, and -- when taking "duplicates" into consideration -- a Set<T> of some form might be ideal?
There was a problem hiding this comment.
I don't think we consider this public API that we are guaranteeing API compatibility with, but changed anyways.
|
|
||
| namespace Java.Interop.Tools.Generator | ||
| { | ||
| public class NamespaceTransform |
There was a problem hiding this comment.
Rando aside: would this be a good use case for C#9 records? https://docs.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-9#record-types
There was a problem hiding this comment.
Doesn't look like you can override primary constructors (?!), so this isn't a good use case.
There was a problem hiding this comment.
However, you can still use record types without using primary constructors…
There was a problem hiding this comment.
Reading through the link, I don't see any benefits to using record, and we haven't bumped to C# 9 yet.
|
Commit message for review: Fixes: https://github.com/xamarin/java.interop/issues/727
Today, when `generator` creates a .NET `namespace` from a
Java `package`, it applies a simple PascalCase transformation to make
the namespace match established .NET naming standards. For example.
`package android.database` becomes `namespace Android.Database`.
However there are a few scenarios that this is not a good fit for:
1. Word phrases where upper-casing only the first letter is not
desirable, e.g. `package androidx` to `namespace AndroidX`.
2. When Java package names are longer than the desired C# namespaces;
`com.` is a common Java package prefix, but isn't common in C#:
`package com.google.android.material.animation` should become
`namespace Google.Android.Material.Animation`.
Both of these scenarios can require many repeated `metadata` lines
[to fix][0]:
<attr path="/api/package[@name='androidx.core.accessibilityservice']" name="managedName">AndroidX.Core.AccessibilityService</attr>
<attr path="/api/package[@name='androidx.core.app']" name="managedName">AndroidX.Core.App</attr>
…
Improve support for this scenario by adding a support for a new
`<ns-replace/>` element to `Metadata.xml` transform files.
(We may also add support for an `@(AndroidNamespaceReplacement)` item
group as suggested in xamarin/java.interop#727 in the future.)
<metadata>
<ns-replace source='Androidx' replacement='Xamarin.AndroidX' />
<ns-replace source='.Androidx.' replacement='Xamarin.AndroidX' />
<ns-replace source='Com' replacement='' />
<ns-replace source='Com.Google.' replacement='Google' />
<ns-replace source='.Compose' replacement='ComposeUI' />
</metadata>
The `//ns-replace/@source` attribute is a Java [PackageName][1] to
replace. Note: the Java PackageName grammar is:
> *PackageName*:
> - *Identifier*
> - *PackageName* `.` *Identifier*
The `//ns-replace` values *do not* override the [managedName][2]
attribute if already specified within metadata.
The `//ns-replace/@source` attribute:
* Specifies a "match"; when it matches, instances of `@source` are
replaced with `@replacement`.
* Is interpreted in a *case-insensitive* manner; `Androidx` matches
`androidx` and `AndroidX` and `ANDROIDX`.
* Only matches on full [Identifier][3]s and [PackageName][1]s;
<ns-replace source='Com' replacement='' />
Matches `Com.Google.Library`, but not `Common.Google.Library` or
`Google.Imaging.Dicom`.
* Multiple "dotted" Identifiers may be used, and all may match
*anywhere*, *in order*, in the Java package name.
<ns-replace source='Com.Google' replacement='Google' />
will match `com.google.library` and `example.com.google`, but
won't match `Common.Google` or `Com.Googles`.
* *May start with* a `.`, which means that `//ns-replace/@source`
is only matched at the *end* of a Java package name:
<ns-replace source='.Compose' replacement='ComposeUI' />
will match `com.google.androidx.compose`,
but not `com.google.androidx.compose.writer`.
* *May end with* a `.`, which means that `//ns-replace/@source` is
only matched at the *beginning* of the package name:
<ns-replace source='Androidx.' replacement='Xamarin.AndroidX' />
matches `androidx.core`, but not `com.square.okhttp.androidx`.
* May both begin and end with a `.`, which means that only exact
(case insensitive) Java package names are matched:
<ns-replace source='.Androidx.' replacement='Xamarin.AndroidX' />
matches `androidx`, but not `com.google.androidx.core`.
Additionally, duplicate `//ns-replace/@source` values are *ignored*.
`//ns-replace` elements are processed "in order" for a given
`Metadata.xml` file, but the ordering of `generator --fixup` files /
`@(TransformFile)`s is unspecified.
Multiple replacements may affect a single namespace:
<ns-replace source='Androidx' replacement='Xamarin.AndroidX' />
<ns-replace source='View' replacement='Views' />
changes `Androidx.View` to `Xamarin.AndroidX.Views`.
[0]: https://github.com/xamarin/AndroidX/blob/f97553ff428f9b6ea754f173567d220048245a16/source/androidx.core/core/Transforms/Metadata.Namespaces.xml#L10-L34
[1]: https://docs.oracle.com/javase/specs/jls/se8/html/jls-6.html#jls-6.5
[2]: https://docs.microsoft.com/en-us/xamarin/android/platform/binding-java-library/customizing-bindings/java-bindings-metadata#managedname
[3]: https://docs.oracle.com/javase/specs/jls/se8/html/jls-3.html#jls-Identifier |
Fixes: #727
Today, when
generatorcreates a .NETnamespacefrom a Javapackage, it applies a simple pascal case transformation to make the namespace match established .NET naming standards.For example:
package android.databasebecomesnamespace Android.Database.However there are a few scenarios that this is not a good fit for.
(1) Words where Pascal case is not desired:
androidx->AndroidX(2) Java package names are often longer than C# namespaces:
com.google.android.material.animation->Google.Android.Material.Animation.Both of these scenarios can lead to many repeated
metadatalines to fix:(Source)
Solution
To help these scenarios, we introduce a new metadata-specified item that would allow simple replacements. (Note that
generatoraccepts<ns-replace>inmetadatafiles, though we will likely provide the MSBuild item specified in #727 as the preferred way for users to specify replacements.)Examples:
Implementation Notes
These replacements will only be run for
<package>elements that do not specify a@managedNameattribute. If you use@managedNameyou are opting to provide the exact name, we will not process it further.Unlike unused metadata, these replacement will not raise a warning if they are unused.
Case Sensitivity
Replacements take place after the automatic Pascal case transform, but the compare is case-insensitive.
Thus, both of the following are equivalent:
Word Bounds
Replacements take place only on full words (namespace parts).
Thus,
Matches
Com.Google.Library, but notCommon.Google.LibraryorGoogle.Imaging.Dicom.Multiple full words can be used:
Word Position
The word part match can be constrained to the beginning or end of a namespace by appending a
.or prepending a., respectively.matches
Androidx.Core, but notSquare.OkHttp.Androidx.Similarly,
matches
Google.AndroidX.Compose, but notGoogle.Compose.Writer.Both prepending and appending a
.makes it an exact match.matches
Androidx, but notGoogle.Androidx.Core.Replacement Order
Replacements run in the order specified by the
metadatafile, however adding replacements to multiplemetadatafiles may result in an unintended order.Replacements are run sequentially, and multiple replacements may affect a single namespace.
changes
Androidx.ViewtoXamarin.AndroidX.Views.