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

IOORE importing type with name ending in a period #835

Closed
jpobst opened this issue May 11, 2021 · 6 comments · Fixed by #837 or dotnet/android#5945
Closed

IOORE importing type with name ending in a period #835

jpobst opened this issue May 11, 2021 · 6 comments · Fixed by #837 or dotnet/android#5945
Labels
bug Component does not function as intended generator Issues binding a Java library (generator, class-parse, etc.)

Comments

@jpobst
Copy link
Contributor

jpobst commented May 11, 2021

Context: com.google.firebase.firestore.model.Document

The following code:

  private static final Comparator<Document> KEY_COMPARATOR =
      (left, right) -> left.getKey().compareTo(right.getKey());

  /** A document comparator that returns document by key and key only. */
  public static Comparator<Document> keyComparator() {
    return KEY_COMPARATOR;
  }

is being parsed out as:

<class abstract="false" deprecated="not deprecated" extends="java.lang.Object" extends-generic-aware="java.lang.Object" jni-extends="Ljava/lang/Object;" final="false" name="Document." static="false" visibility="public" jni-signature="Lcom/google/firebase/firestore/model/Document$;"></class>

The name='Document.' causes an IndexOutOfRangeException building the model from the XML:

   at System.String.get_Chars(Int32 index)
   at MonoDroid.Generation.XmlApiImporter.CreateGenBaseSupport(XElement pkg, XElement elem, Boolean isInterface) in C:\code\xamarin-android-backport\external\Java.Interop\tools\generator\Java.Interop.Tools.Generator.Importers\XmlApiImporter.cs:line 290
   at MonoDroid.Generation.XmlApiImporter.CreateClass(XElement pkg, XElement elem, CodeGenerationOptions options) in C:\code\xamarin-android-backport\external\Java.Interop\tools\generator\Java.Interop.Tools.Generator.Importers\XmlApiImporter.cs:line 110
   at MonoDroid.Generation.XmlApiImporter.ParsePackage(XElement ns, CodeGenerationOptions options) in C:\code\xamarin-android-backport\external\Java.Interop\tools\generator\Java.Interop.Tools.Generator.Importers\XmlApiImporter.cs:line 66
   at MonoDroid.Generation.XmlApiImporter.Parse(XDocument doc, CodeGenerationOptions options) in C:\code\xamarin-android-backport\external\Java.Interop\tools\generator\Java.Interop.Tools.Generator.Importers\XmlApiImporter.cs:line 35
@jpobst jpobst added bug Component does not function as intended generator Issues binding a Java library (generator, class-parse, etc.) labels May 11, 2021
@jpobst
Copy link
Contributor Author

jpobst commented May 11, 2021

Notes:

This is an issue caused by a change between firebase-firestore 22.1.1 and 22.1.2.

Java source code: https://github.com/firebase/firebase-android-sdk/blob/master/firebase-firestore/src/main/java/com/google/firebase/firestore/model/Document.java

Java library: https://maven.google.com/web/index.html?q=firestore#com.google.firebase:firebase-firestore:22.1.2

Decompilation:
image

class-parse

    <class
      abstract="false"
      deprecated="not deprecated"
      jni-extends="Ljava/lang/Object;"
      extends="java.lang.Object"
      extends-generic-aware="java.lang.Object"
      final="false"
      name="Document."
      jni-signature="Lcom/google/firebase/firestore/model/Document$;"
      source-file-name="Document.java"
      static="false"
      visibility="public" />

adjusted

    <class abstract="false" deprecated="not deprecated" extends="java.lang.Object" 
extends-generic-aware="java.lang.Object" jni-extends="Ljava/lang/Object;" final="false" 
name="Document." static="false" visibility="public" 
jni-signature="Lcom/google/firebase/firestore/model/Document$;"></class>

@AmrAlSayed0
Copy link

This would also help with this issue #789 (comment).

@jonpryor
Copy link
Member

I'm wondering about how specifically this is happening, as I cannot repro it:

import java.util.Comparator;

class Example implements Comparable<Example> {
    public int compareTo(Example a) {return 0;}

    static final Comparator<Example> COMPARATOR = (left, right) -> left.compareTo(right);

    public static Comparator<Example> keyComparator() { return COMPARATOR; }
}

Using both JDK 1.8 and JDK 11's javac, I do not get an Example$.class file as output. Instead, Example.COMPARATOR is implemented in terms of InvokeDynamic & a static method:

$ javap -l -constants -c -private -s Example.class
…
  private static int lambda$static$0(Example, Example);
    descriptor: (LExample;LExample;)I
    Code:
       0: aload_0
       1: aload_1
       2: invokevirtual #4                  // Method compareTo:(LExample;)I
       5: ireturn
    LineNumberTable:
      line 6: 0

  static {};
    descriptor: ()V
    Code:
       0: invokedynamic #5,  0              // InvokeDynamic #0:compare:()Ljava/util/Comparator;
       5: putstatic     #2                  // Field COMPARATOR:Ljava/util/Comparator;
       8: return
    LineNumberTable:
      line 6: 0

The lambda$static$0 method is from the lambda expression.

(My Kotlin toolchain is screwed up, so I can't currently try to see what Kotlin would do in a similar scenario.)

I thus don't understand where Document. is coming from. I certainly don't understand why Document. is public, of all things.

@jonpryor
Copy link
Member

Doubly odd is that Document.class doesn't even mention Document$.class:

$ javap -constants -p -l -c -verbose -private -s  Document.class| grep 'Document\$'
  #28 = Utf8               com/google/firebase/firestore/model/Document$$Lambda$1
  #29 = Class              #28            // com/google/firebase/firestore/model/Document$$Lambda$1
  #33 = Methodref          #29.#32        // com/google/firebase/firestore/model/Document$$Lambda$1.lambdaFactory$:()Ljava/util/Comparator;
         0: invokestatic  #33                 // Method com/google/firebase/firestore/model/Document$$Lambda$1.lambdaFactory$:()Ljava/util/Comparator;
# note: no mention of `Document$`,
# except as part of a larger `Document$$Lambda$1`

Document$$Lambda$1.class does mention Document$, as an invokestatic target:

  public int compare(java.lang.Object, java.lang.Object);
    descriptor: (Ljava/lang/Object;Ljava/lang/Object;)I
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=2, locals=3, args_size=3
         0: aload_1
         1: checkcast     #14                 // class com/google/firebase/firestore/model/Document
         4: aload_2
         5: checkcast     #14                 // class com/google/firebase/firestore/model/Document
         8: invokestatic  #20                 // Method com/google/firebase/firestore/model/Document$.lambda$static$0:(Lcom/google/firebase/firestore/model/Document;Lcom/google/firebase/firestore/model/Document;)I
        11: ireturn

I have to wonder if there's some "Java bytecode post processing" going on…

@jonpryor
Copy link
Member

Meanwhile, @jpobst mentioned:

My current thought is to skip types with no effective "name" in XmlApiImporter in generator with a warning or a debug message.

We already skip types with "obfuscated" names:

https://github.com/xamarin/java.interop/blob/12e670a8560f69581d3a3adf0a9d91e8ce8c9afa/tools/generator/Java.Interop.Tools.Generator.Importers/XmlApiImporter.cs#L63-L72

https://github.com/xamarin/java.interop/blob/12e670a8560f69581d3a3adf0a9d91e8ce8c9afa/tools/generator/Java.Interop.Tools.Generator.Importers/XmlApiImporter.cs#L490-L507

so perhaps the "easy" fix is to extend IsObfuscatedName() so that Document. is treated as "obfuscated"?

@jpobst
Copy link
Contributor Author

jpobst commented May 12, 2021

I have to wonder if there's some "Java bytecode post processing" going on…

Yeah, I couldn't repro it either, and thought this might be the case.

jonpryor pushed a commit that referenced this issue May 12, 2021
Fixes: #835

Context: https://github.com/firebase/firebase-android-sdk/blob/1fba4a70395c20dcc858b22ee8247acbed7cf8d3/firebase-firestore/src/main/java/com/google/firebase/firestore/model/Document.java
Context: https://maven.google.com/web/index.html?q=firestore#com.google.firebase:firebase-firestore:22.1.2

Somehow Google has compiled `firebase-firestore.aar` to contain a
`Document$.class` with a "non-sensical" *empty* inner class name:

	<class
	  abstract="false"
	  deprecated="not deprecated"
	  jni-extends="Ljava/lang/Object;"
	  extends="java.lang.Object"
	  extends-generic-aware="java.lang.Object"
	  final="false"
	  name="Document."
	  jni-signature="Lcom/google/firebase/firestore/model/Document$;"
	  source-file-name="Document.java"
	  static="false"
	  visibility="public"
	/>

The presence of a `//class[@name='Document.']` element causes an
`IndexOutOfRangeException` for `generator` when it tries to parse
`Document.` into a parent class name and nested class name.

	System.IndexOutOfRangeException: Index was outside the bounds of the array.
	  at MonoDroid.Generation.XmlApiImporter.CreateGenBaseSupport (System.Xml.Linq.XElement pkg, System.Xml.Linq.XElement elem, System.Boolean isInterface)
	  at MonoDroid.Generation.XmlApiImporter.CreateClass (System.Xml.Linq.XElement pkg, System.Xml.Linq.XElement elem, 
	  at MonoDroid.Generation.Parser.ParsePackage (System.Xml.Linq.XElement ns, System.Predicate`1[T] p)
	  at MonoDroid.Generation.Parser.ParsePackage (System.Xml.Linq.XElement ns)
	  at MonoDroid.Generation.Parser.Parse (System.Xml.Linq.XDocument doc, System.Collections.Generic.IEnumerable`1[T] fixups, 
	  at MonoDroid.Generation.Parser.Parse (System.String filename, System.Collections.Generic.IEnumerable`1[T] fixups, 
	  at Xamarin.Android.Binder.CodeGenerator.Run (Xamarin.Android.Binder.CodeGeneratorOptions options, 
	  at Xamarin.Android.Binder.CodeGenerator.Run (Xamarin.Android.Binder.CodeGeneratorOptions options)
	  at Xamarin.Android.Binder.CodeGenerator.Main (System.String[] args)

We cannot reproduce *how* to get a `Document$.class` file, so we're
currently assuming this is some sort of bytecode manipulation.
As such, we are going to have `generator` ignore types with an empty
name and types that end in a period, which would indicate a nested
type with an empty name.
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue May 20, 2021
Fixes: dotnet/java-interop#835
Fixes: dotnet#5921

Context: dotnet#5894

Changes: http://github.com/xamarin/Java.Interop/compare/12e670a8560f69581d3a3adf0a9d91e8ce8c9afa...2573dc8c84fd4eb68e75bcae73912c26f4942356

  * dotnet/java-interop@2573dc8c: [Java.Interop.Tools.*] IMetadataResolver not TypeDefinitionCache (dotnet#842)
  * dotnet/java-interop@412e974b: Revert "[generator] Disable [SupportedOSPlatform] until .NET 5/6. (dotnet#781)" (dotnet#841)
  * dotnet/java-interop@23baf0bc: [Java.Interop] Fix NRT warnings introduced by targeting 'net6.0' (dotnet#840)
  * dotnet/java-interop@131c1496: [generator] Fix NRE from return type not consistently set (dotnet#834)
  * dotnet/java-interop@100fffc1: [generator] Ensure "global::" is prepended to generic return casts. (dotnet#838)
  * dotnet/java-interop@9b89e90e: [generator] Ignore types without names (dotnet#837)
  * dotnet/java-interop@0e01fb5d: [Java.Interop.Tools.JavaSource] Merge @return block values (dotnet#836)
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue May 20, 2021
Fixes: dotnet/java-interop#835
Fixes: dotnet#5921

Context: dotnet#5894

Changes: http://github.com/xamarin/Java.Interop/compare/12e670a8560f69581d3a3adf0a9d91e8ce8c9afa...2573dc8c84fd4eb68e75bcae73912c26f4942356

  * dotnet/java-interop@2573dc8c: [Java.Interop.Tools.*] IMetadataResolver not TypeDefinitionCache (dotnet#842)
  * dotnet/java-interop@412e974b: Revert "[generator] Disable [SupportedOSPlatform] until .NET 5/6. (dotnet#781)" (dotnet#841)
  * dotnet/java-interop@23baf0bc: [Java.Interop] Fix NRT warnings introduced by targeting 'net6.0' (dotnet#840)
  * dotnet/java-interop@131c1496: [generator] Fix NRE from return type not consistently set (dotnet#834)
  * dotnet/java-interop@100fffc1: [generator] Ensure "global::" is prepended to generic return casts. (dotnet#838)
  * dotnet/java-interop@9b89e90e: [generator] Ignore types without names (dotnet#837)
  * dotnet/java-interop@0e01fb5d: [Java.Interop.Tools.JavaSource] Merge @return block values (dotnet#836)
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue May 21, 2021
Fixes: dotnet/java-interop#835
Fixes: dotnet#5921

Context: dotnet#5894

Changes: http://github.com/xamarin/Java.Interop/compare/12e670a8560f69581d3a3adf0a9d91e8ce8c9afa...a5ed8919fb2ec894cb8144e51ae7c29b4811ee2a

  * dotnet/java-interop@a5ed8919: [Java.Interop.Tools.Cecil] Fix the xamarin-android build
  * dotnet/java-interop@2573dc8c: [Java.Interop.Tools.*] IMetadataResolver not TypeDefinitionCache (dotnet#842)
  * dotnet/java-interop@412e974b: Revert "[generator] Disable [SupportedOSPlatform] until .NET 5/6. (dotnet#781)" (dotnet#841)
  * dotnet/java-interop@23baf0bc: [Java.Interop] Fix NRT warnings introduced by targeting 'net6.0' (dotnet#840)
  * dotnet/java-interop@131c1496: [generator] Fix NRE from return type not consistently set (dotnet#834)
  * dotnet/java-interop@100fffc1: [generator] Ensure "global::" is prepended to generic return casts. (dotnet#838)
  * dotnet/java-interop@9b89e90e: [generator] Ignore types without names (dotnet#837)
  * dotnet/java-interop@0e01fb5d: [Java.Interop.Tools.JavaSource] Merge @return block values (dotnet#836)
jonpryor added a commit to dotnet/android that referenced this issue May 23, 2021
Fixes: dotnet/java-interop#835
Fixes: #5921

Context: #5894

Changes: http://github.com/xamarin/Java.Interop/compare/12e670a8560f69581d3a3adf0a9d91e8ce8c9afa...a5ed8919fb2ec894cb8144e51ae7c29b4811ee2a

  * dotnet/java-interop@a5ed8919: [Java.Interop.Tools.Cecil] Fix the xamarin-android build
  * dotnet/java-interop@2573dc8c: [Java.Interop.Tools.*] IMetadataResolver not TypeDefinitionCache (#842)
  * dotnet/java-interop@412e974b: Revert "[generator] Disable [SupportedOSPlatform] until .NET 5/6. (#781)" (#841)
  * dotnet/java-interop@23baf0bc: [Java.Interop] Fix NRT warnings introduced by targeting 'net6.0' (#840)
  * dotnet/java-interop@131c1496: [generator] Fix NRE from return type not consistently set (#834)
  * dotnet/java-interop@100fffc1: [generator] Ensure "global::" is prepended to generic return casts. (#838)
  * dotnet/java-interop@9b89e90e: [generator] Ignore types without names (#837)
  * dotnet/java-interop@0e01fb5d: [Java.Interop.Tools.JavaSource] Merge @return block values (#836)
@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 generator Issues binding a Java library (generator, class-parse, etc.)
Projects
None yet
3 participants