Skip to content

Commit

Permalink
[Java.Interop] Use PublicApiAnalyzers to ensure we do not break API (#…
Browse files Browse the repository at this point in the history
…1170)

Fixes: #1169

Context: dotnet/android@76ab8b2

dotnet/android@76ab8b2c mentions:

> Now that we are in the .NET `TargetFramework` world we also need
> to ensure we do not *add* any new API to a Target Framework once it
> has shipped.  A `TargetFramework` is essentially a contract that we
> cannot change.  (Imagine if you had different minor versions of .NET
> on your local machine and CI machine, what works on one should work
> on the other.)

This prevents an issue where a user on `.NET 8.0.300` uses a method
that isn't available to their coworker or CI on `.NET 8.0.100`.

This logical argument also applies to `Java.Interop.dll`.

Enable Microsoft's [PublicApiAnalyzers][0] for `Java.Interop.dll`.
([PublicApiAnalyzers documentation][1].)  This ensures that we don't
add new API once we've shipped `Java.Interop.dll` for a given
.NET version.

Update `build-tools/jnienv-gen` so that `JniEnvironment.g.cs` enables
nullable reference types.  This allows us to *avoid* disabling RS0041.

Co-authored-by: Jonathan Pryor <jonpryor@vt.edu>

[0]: https://github.com/dotnet/roslyn-analyzers/tree/ace28a1039d09626a94d3b0f8ce69e547ca2bcbf/src/PublicApiAnalyzers
[1]: https://github.com/dotnet/roslyn-analyzers/blob/ace28a1039d09626a94d3b0f8ce69e547ca2bcbf/src/PublicApiAnalyzers/PublicApiAnalyzers.Help.md
  • Loading branch information
jpobst authored Dec 1, 2023
1 parent 8b85462 commit 0f1efeb
Show file tree
Hide file tree
Showing 8 changed files with 2,017 additions and 1,090 deletions.
7 changes: 4 additions & 3 deletions build-tools/jnienv-gen/Generator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ static void GenerateFile (TextWriter o)
o.WriteLine ("// Generated file; DO NOT EDIT!");
o.WriteLine ("//");
o.WriteLine ("// To make changes, edit monodroid/tools/jnienv-gen-interop and rerun");
o.WriteLine ("#nullable enable");
o.WriteLine ();
o.WriteLine ("#if !FEATURE_JNIENVIRONMENT_SAFEHANDLES && !FEATURE_JNIENVIRONMENT_JI_INTPTRS && !FEATURE_JNIENVIRONMENT_JI_PINVOKES && !FEATURE_JNIENVIRONMENT_XA_INTPTRS && !FEATURE_JNIENVIRONMENT_JI_FUNCTION_POINTERS");
o.WriteLine ("#define FEATURE_JNIENVIRONMENT_JI_PINVOKES");
Expand Down Expand Up @@ -255,7 +256,7 @@ static void GenerateJniNativeInterfaceInvoker (TextWriter o, HandleStyle style)
if (e.Prebind)
o.WriteLine ("\t\tpublic readonly {0} {1};\n", d, e.Name);
else {
o.WriteLine ("\t\t{0} _{1};", d, e.Name);
o.WriteLine ("\t\t{0}? _{1};", d, e.Name);
o.WriteLine ("\t\tpublic {0} {1} {{", d, e.Name);
o.WriteLine ("\t\t\tget {");
o.WriteLine ("\t\t\t\tif (_{0} == null)\n\t\t\t\t\t{1}", e.Name, Initialize (e, "_", d));
Expand Down Expand Up @@ -551,7 +552,7 @@ static void RaiseException (TextWriter o, JniFunction entry, HandleStyle style)
return;

o.WriteLine ();
o.WriteLine ("\t\t\tException __e = JniEnvironment.GetExceptionForLastThrowable ({0});",
o.WriteLine ("\t\t\tException? __e = JniEnvironment.GetExceptionForLastThrowable ({0});",
(style == HandleStyle.JIIntPtrPinvokeWithErrors || style == HandleStyle.JIFunctionPtrWithErrors)
? "thrown"
: "");
Expand Down Expand Up @@ -1127,7 +1128,7 @@ public override string[] GetMarshalToManagedStatements (HandleStyle style, strin
case HandleStyle.JIFunctionPtrWithErrors:
return new[] {
string.Format ("if ({0} == IntPtr.Zero)", variable),
string.Format ("\treturn null;"),
string.Format ($"\tthrow new InvalidOperationException (\"Should not be reached; `{entry.Name}` should have thrown!\");"),
string.Format ("return new {0} ({1}, {2}, {3}, isStatic: {4});", type, entry.Parameters [1].Name, entry.Parameters [2].Name, variable, IsStatic ? "true" : "false"),
};
case HandleStyle.XAIntPtr:
Expand Down
15 changes: 4 additions & 11 deletions src/Java.Interop/Java.Interop.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,11 @@
<DependentUpon>JniPeerMembers.JniInstanceMethods_Invoke.tt</DependentUpon>
</Compile>
</ItemGroup>
<ProjectExtensions>
<MonoDevelop>
<Properties>
<Policies>
<VersionControlPolicy>
<CommitMessageStyle Indent="&#x9;" LineAlign="0" IncludeDirectoryPaths="True" />
</VersionControlPolicy>
</Policies>
</Properties>
</MonoDevelop>
</ProjectExtensions>
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.PublicApiAnalyzers" Version="3.3.4">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.SourceLink.GitHub" PrivateAssets="All" />
</ItemGroup>
<Import Project="Java.Interop.targets" />
Expand Down
4 changes: 4 additions & 0 deletions src/Java.Interop/Java.Interop/JavaObjectArray.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public class JavaObjectArray<T> : JavaArray<T>
{
internal static readonly ValueMarshaler Instance = new ValueMarshaler ();

[SuppressMessage ("ApiDesign", "RS0022:Constructor make noninheritable base class inheritable", Justification = "Existing public API")]
public JavaObjectArray (ref JniObjectReference handle, JniObjectReferenceOptions options)
: base (ref handle, options)
{
Expand All @@ -29,20 +30,23 @@ static JniObjectReference NewArray (int length)
}
}

[SuppressMessage ("ApiDesign", "RS0022:Constructor make noninheritable base class inheritable", Justification = "Existing public API")]
public unsafe JavaObjectArray (int length)
: this (ref *InvalidJniObjectReference, JniObjectReferenceOptions.None)
{
var peer = NewArray (CheckLength (length));
Construct (ref peer, JniObjectReferenceOptions.CopyAndDispose);
}

[SuppressMessage ("ApiDesign", "RS0022:Constructor make noninheritable base class inheritable", Justification = "Existing public API")]
public JavaObjectArray (IList<T> value)
: this (CheckLength (value))
{
for (int i = 0; i < value.Count; ++i)
SetElementAt (i, value [i]);
}

[SuppressMessage ("ApiDesign", "RS0022:Constructor make noninheritable base class inheritable", Justification = "Existing public API")]
public JavaObjectArray (IEnumerable<T> value)
: this (ToList (value))
{
Expand Down
2 changes: 1 addition & 1 deletion src/Java.Interop/Java.Interop/JniEnvironment.Types.cs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ public static void RegisterNatives (JniObjectReference type, JniNativeMethodRegi
}
#endif // DEBUG && NETCOREAPP

int r = _RegisterNatives (type, methods, numMethods);
int r = _RegisterNatives (type, methods ?? Array.Empty<JniNativeMethodRegistration>(), numMethods);

if (r != 0) {
throw new InvalidOperationException (
Expand Down
3 changes: 3 additions & 0 deletions src/Java.Interop/Java.Interop/JniValueMarshaler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public struct JniValueMarshalerState : IEquatable<JniValueMarshalerState> {
public IJavaPeerable? PeerableValue {get; private set;}
public object? Extra {get; private set;}

[SuppressMessage ("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", Justification = "Existing public API")]
public JniValueMarshalerState (JniArgumentValue jniArgumentValue, object? extra = null)
{
JniArgumentValue = jniArgumentValue;
Expand All @@ -57,6 +58,7 @@ public JniValueMarshalerState (JniArgumentValue jniArgumentValue, object? extra
Extra = extra;
}

[SuppressMessage ("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", Justification = "Existing public API")]
public JniValueMarshalerState (JniObjectReference referenceValue, object? extra = null)
{
JniArgumentValue = new JniArgumentValue (referenceValue);
Expand All @@ -65,6 +67,7 @@ public JniValueMarshalerState (JniObjectReference referenceValue, object? extra
Extra = extra;
}

[SuppressMessage ("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", Justification = "Existing public API")]
public JniValueMarshalerState (IJavaPeerable? peerableValue, object? extra = null)
{
PeerableValue = peerableValue;
Expand Down
Loading

0 comments on commit 0f1efeb

Please sign in to comment.