Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XML syntax warnings when using JavaSourceJar #850

Closed
cncb-gh opened this issue Jun 13, 2021 · 18 comments · Fixed by #1036
Closed

XML syntax warnings when using JavaSourceJar #850

cncb-gh opened this issue Jun 13, 2021 · 18 comments · Fixed by #1036
Assignees
Labels
bug Component does not function as intended javadoc Issues with consuming Java documentation formats

Comments

@cncb-gh
Copy link

cncb-gh commented Jun 13, 2021

Error Message or Issue

After upgrading to VS 16.10 (from 16.4), I get several general XML syntax errors when trying to compile the https://github.com/Baseflow/ExoPlayerXamarin binding library:

Severity Code Description Project File Line Suppression State Error (1 24): Syntax error, expected: #PCDATA, <tt>, <TT>, <i>, <I>, {@code, {@docRoot}, {@inheritDoc}, {@link, {@linkplain, {@literal, {@value}, {@value, UnknownHtmlElementStart, <p>, <P>, <pre>, <PRE>, @author, @apiSince, @deprecated, @deprecatedSince, @exception, @param, @return, @see, @serialData, @serialField, @since, @throws, @[unknown], @version ExoPlayer.Common C:\Craigs Files\Apps\ExoPlayerXamarin-develop\ExoPlayer.Common\BINDINGSGENERATOR 1

Version Information

Other Helpful Info

@jpobst jpobst transferred this issue from dotnet/android Jun 14, 2021
@jpobst jpobst removed their assignment Jun 14, 2021
@jpobst
Copy link
Contributor

jpobst commented Jun 14, 2021

I think the only current workaround for this is to remove the <JavaSourceJar> files from your .csproj files.

<JavaSourceJar Include="JavaDocs\*.jar" />

@jpobst jpobst added bug Component does not function as intended generator Issues binding a Java library (generator, class-parse, etc.) labels Jun 14, 2021
@jonpryor
Copy link
Member

@cncb-gh: How are you observing this error? It looks like you have $(TreatWarningsAsErrors) disabled:

https://github.com/Baseflow/ExoPlayerXamarin/blob/5214b7158fc6611f602ffc0f0122429a3ae49b6d/Directory.Build.props#L22

We should only be emitting a warning for this, and probably not even a warning… (We should fix that.)

@cncb-gh
Copy link
Author

cncb-gh commented Jun 14, 2021

I don' t know - they are showing up as errors in the Build Output as well as the Error List. Is there another setting in VS that might affect this?

I tried removing the source jar files and now get invalid token errors in some generated files. It is placing '?' in some methods like so (notice "void?"):

[Register ("writeToParcel", "(Landroid/os/Parcel;I)V", "GetWriteToParcel_Landroid_os_Parcel_IHandler")] public abstract void? WriteToParcel (global::Android.OS.Parcel? dest, [global::Android.Runtime.GeneratedEnum] global::Android.OS.ParcelableWriteFlags flags);

@jpobst
Copy link
Contributor

jpobst commented Jun 15, 2021

Looks like this is the full output message that is being emitted:

## Unable to translate remarks for com/google/android/exoplayer2/Player:

A media player interface defining traditional high-level functionality, such as the ability to

play, pause, seek and query properties of the currently playing media.

<p>

Some important properties of media players that implement this interface are:

<ul>

    <li>They can provide a {@link Timeline} representing the structure of the media being played,

    which can be obtained by calling {@link #getCurrentTimeline()}.</li>

    <li>They can provide a {@link TrackGroupArray} defining the currently available tracks,

    which can be obtained by calling {@link #getCurrentTrackGroups()}.</li>

    <li>They contain a number of renderers, each of which is able to render tracks of a single

    type (e.g. audio, video or text). The number of renderers and their respective track types

    can be obtained by calling {@link #getRendererCount()} and {@link #getRendererType(int)}.

    </li>

    <li>They can provide a {@link TrackSelectionArray} defining which of the currently available

    tracks are selected to be rendered by each renderer. This can be obtained by calling

    {@link #getCurrentTrackSelections()}}.</li>

</ul>

Error (31:41): Syntax error, expected: #PCDATA, <tt>, <TT>, <i>, <I>, {@code, {@docRoot}, {@inheritDoc}, {@link, {@linkplain, {@literal, {@value}, {@value, UnknownHtmlElementStart, </tt>, </TT>, </i>, </I>, </p>, </P>, <p>, <P>, <pre>, <PRE>, @author, @apiSince, @deprecated, @deprecatedSince, @exception, @param, @return, @see, @serialData, @serialField, @since, @throws, @[unknown], @version
    {@link #getCurrentTrackSelections()}}.</li>
                                        ^

This part:

Error (31:41): Syntax error, expected: ......

Is close enough to the MSBuild error format that MSBuild seems to be recognizing it as an actual error, which is not intended.

image

I think if we change this line to something else we can get this back to an informational message as intended:

// JavadocInfo.cs: 189
writer.WriteLine ($"{m.Level} {m.Location}: {m.Message}");

@jpobst
Copy link
Contributor

jpobst commented Jun 15, 2021

I have a PR that fixes MSBuild interpreting these error messages as real errors:
#851

I believe @pjcollins will be looking at improving the Javadoc parsing in the future to prevent the messages from happening in the first place.

(See also #843)

@cncb-gh
Copy link
Author

cncb-gh commented Jun 15, 2021

Thanks. In the meantime, do you know how I can fix the "invalid token" error when removing the JavaDoc references?

@jpobst
Copy link
Contributor

jpobst commented Jun 15, 2021

Did you remove them from all of your projects? I had tried removing them from all the projects, and did not get the invalid token errors you mentioned.

jonpryor pushed a commit that referenced this issue Jun 15, 2021
Context: #850

When a parsing issue is hit when importing Javadoc info, an "error"
is printed out like this:

	Error (31:41): Syntax error, expected: #PCDATA, <tt>, <TT>, <i>, <I>, {@code, {@docroot}, {@inheritdoc}, {@link, {@linkplain, {@literal, {@value}, {@value, UnknownHtmlElementStart, </tt>, </TT>, </i>, </I>, </p>, </P>, <p>, <P>, <pre>, <PRE>, @author, @apiSince, @deprecated, @deprecatedSince, @exception, @param, @return, @see, @Serialdata, @serialField, @SInCE, @throws, @[unknown], @Version
	    {@link #getCurrentTrackSelections()}}.</li>

This is intended to be informational, but this output format triggers
the MSBuild error parsing regex, and is interpreted as an actual
error, causing the build to fail.

Avoid the error by prepending `JavadocImport-` to the
`LogMessage.Level` enum value, so that MSBuild doesn't interpret the
string as an error and the build can successfully complete:

	JavadocImport-Error (31:41): Syntax error, expected: #PCDATA, <tt>, <TT>, <i>, <I>, {@code, {@docroot}, {@inheritdoc}, {@link, {@linkplain, {@literal, {@value}, {@value, UnknownHtmlElementStart, </tt>, </TT>, </i>, </I>, </p>, </P>, <p>, <P>, <pre>, <PRE>, @author, @apiSince, @deprecated, @deprecatedSince, @exception, @param, @return, @see, @Serialdata, @serialField, @SInCE, @throws, @[unknown], @Version
	    {@link #getCurrentTrackSelections()}}.</li>
jonpryor pushed a commit that referenced this issue Jun 15, 2021
Context: #850

When a parsing issue is hit when importing Javadoc info, an "error"
is printed out like this:

	Error (31:41): Syntax error, expected: #PCDATA, <tt>, <TT>, <i>, <I>, {@code, {@docroot}, {@inheritdoc}, {@link, {@linkplain, {@literal, {@value}, {@value, UnknownHtmlElementStart, </tt>, </TT>, </i>, </I>, </p>, </P>, <p>, <P>, <pre>, <PRE>, @author, @apiSince, @deprecated, @deprecatedSince, @exception, @param, @return, @see, @Serialdata, @serialField, @SInCE, @throws, @[unknown], @Version
	    {@link #getCurrentTrackSelections()}}.</li>

This is intended to be informational, but this output format triggers
the MSBuild error parsing regex, and is interpreted as an actual
error, causing the build to fail.

Avoid the error by prepending `JavadocImport-` to the
`LogMessage.Level` enum value, so that MSBuild doesn't interpret the
string as an error and the build can successfully complete:

	JavadocImport-Error (31:41): Syntax error, expected: #PCDATA, <tt>, <TT>, <i>, <I>, {@code, {@docroot}, {@inheritdoc}, {@link, {@linkplain, {@literal, {@value}, {@value, UnknownHtmlElementStart, </tt>, </TT>, </i>, </I>, </p>, </P>, <p>, <P>, <pre>, <PRE>, @author, @apiSince, @deprecated, @deprecatedSince, @exception, @param, @return, @see, @Serialdata, @serialField, @SInCE, @throws, @[unknown], @Version
	    {@link #getCurrentTrackSelections()}}.</li>
jonpryor added a commit to dotnet/android that referenced this issue Jun 16, 2021
Context: dotnet/java-interop#850

Changes: http://github.com/xamarin/Java.Interop/compare/a5ed8919fb2ec894cb8144e51ae7c29b4811ee2a...5e3164733cf85db304c250999dcfa3c69d240301

  * dotnet/java-interop@5e316473: [generator] Avoid 'error (…):' construct in diagnostic messages (#851)
  * dotnet/java-interop@ab687cab: [build] Bump to Mono with MSBuild 16.10 (#848)
  * dotnet/java-interop@ce869c76: [generator] Gracefully handle BindingGeneratorException. (#845)
@cncb-gh
Copy link
Author

cncb-gh commented Jun 16, 2021

Did you remove them from all of your projects? I had tried removing them from all the projects, and did not get the invalid token errors you mentioned.

I removed them from all projects but it still adds a "void?" to WriteToParcel() for me.

@cncb-gh
Copy link
Author

cncb-gh commented Jun 22, 2021

I can't figure out how to avoid the void? generation. The following in metadata.xml do not work. Thanks for any help.

<remove-node path="/api/package[@name='com.google.android.exoplayer2.metadata']/interface[@name='Metadata.Entry']/method[@name='writeToParcel' and count(parameter)=2 and parameter[1][@type='android.os.Parcel'] and parameter[2][@type='int']]"></remove-node>

<attr name="return" path="/api/package[@name='com.google.android.exoplayer2.metadata']/interface[@name='Metadata.EntryInvoker']/method[@name='writeToParcel' and count(parameter)=2 and parameter[1][@type='android.os.Parcel'] and parameter[2][@type='int']]">void</attr>

@jpobst
Copy link
Contributor

jpobst commented Jun 22, 2021

You could try something like:

<attr name="return-not-null" path="/api/package[@name='com.google.android.exoplayer2.metadata']/interface[@name='Metadata.EntryInvoker']/method[@name='writeToParcel' and count(parameter)=2 and parameter[1][@type='android.os.Parcel'] and parameter[2][@type='int']]">true</attr>

@cncb-gh
Copy link
Author

cncb-gh commented Jun 23, 2021

Unfortunately, this didn't work for me either. The only thing that works is removing the whole Metadata class with remove-node. Thanks for your help.

@cncb-gh
Copy link
Author

cncb-gh commented Jun 24, 2021

Somehow even though it builds successfully, several interfaces and classes are missing in the output. I think I tracked it down to the "Player" interface never being generated. I get this warning but it does not tell me anything specific:

1>obj\Release\monoandroid10.0\api.xml(3, 6) : warning BG8C01: For type 'Com.Google.Android.Exoplayer2.BasePlayer', base interface 'com.google.android.exoplayer2.Player' is invalid.

Player Interface:
https://github.com/google/ExoPlayer/blob/release-v2/library/common/src/main/java/com/google/android/exoplayer2/Player.java

BasePlayer:
https://github.com/google/ExoPlayer/blob/release-v2/library/common/src/main/java/com/google/android/exoplayer2/BasePlayer.java

I would appreciate any more help you are able to give. Thanks for your time.

@jpobst
Copy link
Contributor

jpobst commented Jun 24, 2021

That's not one I see very often. It means it found the interface com.google.android.exoplayer2.Player, but there is some issue with it that will prevent it from getting generated.

Hopefully there is another message somewhere in the diagnostic output that tells what is wrong with the com.google.android.exoplayer2.Player interface.

@cncb-gh
Copy link
Author

cncb-gh commented Jun 24, 2021

I turned on "Diagnostic" build output and unfortunately don't see anything that tells what is wrong with the interface. Just the one warning for BasePlayer about the invalid interface and several invalid parameter and return type warnings where the Player interface is attempted to be used. Here is the build output:
Output-Build.txt

@jpobst
Copy link
Contributor

jpobst commented Jun 24, 2021

It's in your log:

1>        C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Xamarin\Android\Xamarin.Android.Bindings.Core.targets(53,5): message BG0000: obj\Release\monoandroid10.0\api.xml(2220, 10): warning BG8801: Invalid parameter type 'com.google.android.exoplayer2.Player.Listener' for member 'Com.Google.Android.Exoplayer2.IPlayer.RemoveListener (Com.Google.Android.Exoplayer2.IPlayerListener)'.
1>        obj\Release\monoandroid10.0\api.xml(2030, 6) : warning BG8503: Invalidating 'Com.Google.Android.Exoplayer2.IPlayer' and all its nested types because some of its methods were invalid.

You will need to follow the trail to figure out the root issue causing the types to be invalidated.

That is:

  • IPlayer is invalidated because IPlayerListener is invalid
  • Search for why IPlayerListener is invalid
  • Repeat as needed

@cncb-gh
Copy link
Author

cncb-gh commented Jun 24, 2021

I traced it back to me removing the Metadata class to avoid the 'void?' token error. I have tried both your suggestion as well as

<attr name="return-not-null" path="/api/package[@name='com.google.android.exoplayer2.metadata']/interface[@name='Metadata.Entry']/method[@name='writeToParcel' and count(parameter)=2 and parameter[1][@type='android.os.Parcel'] and parameter[2][@type='int']]">true</attr>

but it always generates a 'void?' (it seems that IEntryInvoker is automatically generated by the binder for IEntry because it is not defined in Java). Any other ideas on how to solve this?

@jpobst
Copy link
Contributor

jpobst commented Jun 25, 2021

The easiest way to solve it would be to turn off <Nullable> for that assembly.

@cncb-gh
Copy link
Author

cncb-gh commented Jun 30, 2021

<Nullable> worked but that uncovered a number of other errors including several runtime Java.Lang.AbstractMethodError exceptions. These seem to mostly have come from a deprecated interface? I'm sure there is a better way to do it, but I had to slowly remove these methods one-at-a-time until the errors stopped. But, I finally have something running successfully so thank you!

jonpryor pushed a commit that referenced this issue Jul 12, 2021
Context: #850 (comment)

When a user binds a Java library that contains a Java interface which
implements another Java interface:

	// Java; android.jar
	public interface /* android.view. */ Menu {…}

	// Java; androidx.core.core.jar
	public interface /* androidx.core.internal.view. */ SupportMenu implements android.view.Menu {…}

then when we create the binding for the "leaf" interface:

	// C# binding for androidx.core.core
	namespace AndroidX.Core.Internal.View {
	    public interface ISupportMenu : Android.Views.IMenu {…}

	    internal partial class ISupportMenuInvoker : Java.Lang.Object, ISupportMenu {
	        // …
	    }
	}

The generated `*Invoker` type implements all methods for all
implemented interfaces, e.g. methods from both `IMenu` and
`ISupportMenu`.

When:

 1. The base interface (e.g. `IMenu`) comes from a referenced
    assembly, *and*

 2. `$(Nullable)`=Enable when binding the derived interface

then we interpret the return types on imported interface methods as
*always* being of a nullable type, even for types such as `void`:

	// C# binding for androidx.core.core with $(Nullable)=Enable
	partial class ISupportMenuInvoker : Java.Lang.Object, ISupportMenu {

	    public unsafe void? Clear () {…}
	}

This results in errors from the C# compiler:

	Error CS1519: Invalid token '?' in class, record, struct, or interface member declaration
	Error CS1520: Method must have a return type
	Error CS0535: 'ISupportMenuInvoker' does not implement interface member 'IMenu.Clear()'

The culprit is twofold:

  - In `CecilApiImporter`, we set `Method.ManagedReturn` to
    `System.Void` instead of `void`.

  - In `CodeGenerationOptions`, we only check for `void` to omit the
    null operator, not `System.Void`.

Note this also happens for all primitive types like
`System.Int32`/`int`.

This commit fixes both aspects:

  - Change `Method.ManagedReturn` to contain primitive types instead
    of fully qualified types.

  - Update `CodeGenerationOptions.GetNullable()` to correctly handle
    fully qualified types if one slips through.

With this change, all of AndroidX/GooglePlayServices/FaceBook/MLKit
can be successfully compiled with `<Nullable>enable</Nullable>`.
jpobst added a commit that referenced this issue Sep 30, 2021
Context: #850 (comment)

When a user binds a Java library that contains a Java interface which
implements another Java interface:

	// Java; android.jar
	public interface /* android.view. */ Menu {…}

	// Java; androidx.core.core.jar
	public interface /* androidx.core.internal.view. */ SupportMenu implements android.view.Menu {…}

then when we create the binding for the "leaf" interface:

	// C# binding for androidx.core.core
	namespace AndroidX.Core.Internal.View {
	    public interface ISupportMenu : Android.Views.IMenu {…}

	    internal partial class ISupportMenuInvoker : Java.Lang.Object, ISupportMenu {
	        // …
	    }
	}

The generated `*Invoker` type implements all methods for all
implemented interfaces, e.g. methods from both `IMenu` and
`ISupportMenu`.

When:

 1. The base interface (e.g. `IMenu`) comes from a referenced
    assembly, *and*

 2. `$(Nullable)`=Enable when binding the derived interface

then we interpret the return types on imported interface methods as
*always* being of a nullable type, even for types such as `void`:

	// C# binding for androidx.core.core with $(Nullable)=Enable
	partial class ISupportMenuInvoker : Java.Lang.Object, ISupportMenu {

	    public unsafe void? Clear () {…}
	}

This results in errors from the C# compiler:

	Error CS1519: Invalid token '?' in class, record, struct, or interface member declaration
	Error CS1520: Method must have a return type
	Error CS0535: 'ISupportMenuInvoker' does not implement interface member 'IMenu.Clear()'

The culprit is twofold:

  - In `CecilApiImporter`, we set `Method.ManagedReturn` to
    `System.Void` instead of `void`.

  - In `CodeGenerationOptions`, we only check for `void` to omit the
    null operator, not `System.Void`.

Note this also happens for all primitive types like
`System.Int32`/`int`.

This commit fixes both aspects:

  - Change `Method.ManagedReturn` to contain primitive types instead
    of fully qualified types.

  - Update `CodeGenerationOptions.GetNullable()` to correctly handle
    fully qualified types if one slips through.

With this change, all of AndroidX/GooglePlayServices/FaceBook/MLKit
can be successfully compiled with `<Nullable>enable</Nullable>`.
@jpobst jpobst changed the title Several XML syntax errors when trying to bind in VS 16.10 XML syntax warnings when using JavaSourceJar Oct 21, 2021
@jpobst jpobst added javadoc Issues with consuming Java documentation formats and removed generator Issues binding a Java library (generator, class-parse, etc.) labels Jul 19, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Component does not function as intended javadoc Issues with consuming Java documentation formats
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants