Bug 15885 - jar2xml skips class when a missing type is found on a method parameter
Summary: jar2xml skips class when a missing type is found on a method parameter
Status: VERIFIED FIXED
Alias: None
Product: Android
Classification: Xamarin
Component: Bindings ()
Version: 4.10.1
Hardware: PC Mac OS
: Normal normal
Target Milestone: 7.1 (C9)
Assignee: Greg Munn
URL:
Depends on:
Blocks:
 
Reported: 2013-11-01 17:07 UTC by Bill Holmes
Modified: 2016-12-28 14:15 UTC (History)
5 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Scratch.Bxc15885.zip (7.33 KB, application/zip)
2014-01-09 11:18 UTC, Jonathan Pryor
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
VERIFIED FIXED

Comment 1 Bill Holmes 2013-11-01 17:10:15 UTC
Correction

GridLayout contains two methods (createAccessibilityNodeInfo and
onInitializeAccessibilityNodeInfo) that have a parameter type of
AccessibilityNodeInfo.
Comment 3 Bill Holmes 2013-11-04 23:00:20 UTC
I do not believe that it is a regression.  This case is difficult to test on previous versions because there are attributes in use that did not exist.  

I feel that we can postpone.
Comment 5 Radek Doulik 2013-11-20 15:08:21 UTC
This probably means you are behind http proxy. When we were designing the custom attribute we decided that we will not handle this problem and only report it in error message.

It might be of course something else. Bill, could you confirm that it is due to proxy?

To overcome the issue, you can download the package manually as described in the error XA5208: Download failed. Please download
https://dl-ssl.google.com/android/repository/support_r18.zip and put it to the
/svn/components/XamAndroidSupportV7GridLayout/source/binding/bin/Debug/18
directory.
Comment 7 Jonathan Pryor 2013-12-06 09:40:55 UTC
Yet another variation of this bug:

http://mono-for-android.1047100.n5.nabble.com/Java-Interface-type-to-c-binding-td5713630.html#a5713634

Build output:

http://mono-for-android.1047100.n5.nabble.com/file/n5713634/DiagonosticBuild.txt

> JARTOXML : warning J2XA005: missing class error was raised while reflecting setOnRefreshListener [public void com.costum.android.widget.PullToRefreshListView.setOnRefreshListener(com.costum.android.widget.PullToRefreshListView$OnRefreshListener)] : com/android/widget/R

What's particularly "funny" about this one is that the com.android.widget.R type (BADLY named!) is a generated type which shouldn't exist at binding time anyway.

What's doubly funny is that the warning message contains everything we'd actually need to bind the method: the name, the return type, and the parameters!
Comment 8 Atsushi Eno 2013-12-11 09:05:17 UTC
I'm not behind any proxy.
Comment 9 Atsushi Eno 2013-12-11 09:11:56 UTC
As we discussed on IRC the project on comment #7 is suspicious. No one could reproduce the issue. So it should be regarded as irrelevant.
Comment 10 Atsushi Eno 2014-01-09 06:52:40 UTC
With the latest monodroid, I could build those projects, so some issue in the downloader is gone.

So, back to the original issue.

> GridLayout contains two methods (createAccessibilityNodeInfo and
> onInitializeAccessibilityNodeInfo) that have a parameter type of
> OnLayoutChangeListener.  
> 
> SearchView contains two methods (addOnLayoutChangeListener and
> removeOnLayoutChangeListener) that have a parameter type of
> OnLayoutChangeListener.

Those members do not exist in those *declaring* Java class.
https://developer.android.com/reference/android/support/v7/widget/GridLayout.html
https://developer.android.com/reference/android/support/v7/widget/SearchView.html

Those members exist in the base classes (e.g. Android.Views.View).

And we don't add extraneous override methods on wherever Java overrides the method (because we are not Java and we invoke the overriden method by JNI nature).

By the way, you should not directly add support-v4.jar to the project but add reference to Android.Support.v4.dll instead (because otherwise you'll be adding a bunch of extraneous binding types with possibly inconsistent mappings between this and Mono.Android.Support.v4.dll).
So far you should be able to add reference to that pre-installed dll. By the time we switch v4.dll totally to components, you'll have to "resolve dependencies on other components" issue.
Comment 11 Jonathan Pryor 2014-01-09 11:18:42 UTC
Created attachment 5806 [details]
Scratch.Bxc15885.zip

Attached is an example project to illustrate the problem.

The attached project contains two types, package-private Internal.java and Public.java:

  package com.example;

  public class Public {
    public Object doSomething (Public p)
    {
      // Internal i = new Internal();
      Object i = new Object ();
      return i;
    }
  
    public Internal doSomethingElse (Internal i)
    {
      return null;
    }
  }

Build the attached project:

  xbuild
> 	:  warning J2XA006: missing class error was raised while reflecting com.example.Public : com/example/Internal

Actual results: Public is not bound.

Expected/desired results: Public has no "missing" base class, no "missing" implemented interfaces, and contains two public members with types that can certainly be found (Public.<init>, the default constructor, and Public.doSomething(), which only uses java.lang.Object and com.example.Public.

The only "wrinkle" is that it _also_ contains a public method which references a type that can't be found (it's not included into the .jar).

Yet because com.example.Internal is missing, the ENTIRE com.example.Public type is SKIPPED and UNBOUND.

THIS is the bug. jar2xml should be able to generate the following:

    <class name="Public">
        <constructor name="Public" />
        <method name="doSomething" returns="java.lang.Object">
            <parameter name="p0" type="com.example.Public"/>
        </method>
    </class>
Comment 12 Jonathan Pryor 2014-01-09 11:21:39 UTC
Unrelated: javap is able to handle the type Just Fine, even though a type is missing:

$ javap -classpath bin/Debug/classes.jar com.example.Public
Compiled from "Public.java"
public class com.example.Public extends java.lang.Object{
    public com.example.Public();
    public java.lang.Object doSomething(com.example.Public);
    public com.example.Internal doSomethingElse(com.example.Internal);
}

It's a pity javap output is tied to the "host" JDK...
Comment 13 Atsushi Eno 2014-01-10 07:29:55 UTC
Well, everyone had WRONG assumption that "when a method causes ClassLoader error it must be still possible to still resurrect any other methods". That was WRONG. It was Class.getDeclaredMethods() that caused the error. (Did you notice that it happens while it tries to enumerate ALL the methods?) This thus results in that it could not retrieve any methods AT ALL.

I'll add another stack trace dumping in jar2xml to explicitly show you from which standard Java class method failed. But that's what Java does, not jar2xml. You cannot recover from that state as you cannot even get the "list of methods".
Comment 14 Jonathan Pryor 2014-01-10 09:12:23 UTC
RESOLVED: KILL JAR2XML WITH FIRE AND REPLACE WITH JAVAP, DAMN THE CONSEQUENCES.

...but wait a minute. I thought jar2xml used asm-debug-all.jar so that it would interpret the Java bytecode ~directly, _without_ going through java reflection.

So why are we using Class.getDeclaredMethods()?
Comment 15 Atsushi Eno 2014-01-10 11:31:13 UTC
asm is used to supplement whatever Java reflection cannot resolve (for example constant value retrieval), not the opposite.

It's like cecil, you cannot easily resolve interfaces, base types etc. I quickly dumped the idea of rewriting everything in asm because of that. Implementing another reflection layer by ourselves doesn't seem very reliable approach, especially how much we could gain from that after all.
Comment 16 Jonathan Pryor 2014-01-10 14:20:47 UTC
Reopened to track the eventual jar2xml rewrite.
Comment 17 Peter Collins 2015-12-02 18:31:06 UTC
@JonP any chance this 'rewrite' mentioned in Comment #16 is class-parse? Bumping milestone to match the current planned schedule for making class-parse the default option.
Comment 18 Jonathan Pryor 2015-12-07 22:41:47 UTC
@PeterC: Yes, the Comment #16 rewrite is in fact class-parse.
Comment 19 Jonathan Pryor 2016-06-27 19:06:43 UTC
This doesn't work with C7.

This does work with C8 by setting the $(AndroidClassParser) MSBuild property to `class-parse`:

    xbuild /p:AndroidClassParser=class-parse

What's needed now is a way to change the $(AndroidClassParser) value from within the IDE.
Comment 20 dean.ellis 2016-11-11 11:25:43 UTC
Greg based on @jonp's comments in Command 19. This is just waiting for the IDE bits. Which I think are being worked on...
Comment 21 Greg Munn 2016-11-11 14:35:09 UTC
There's already a feature request for C9 for the UI for this but I've not been able to get to it. Please close this bug as resolved and, if needed for tracking, open a bug/enh for the UI since they are really separate issues.
Comment 22 Peter Collins 2016-11-11 14:54:05 UTC
We can use the trello card to track an IDE toggle. Marking as resolved per Comment #19.
Comment 23 Naqeeb 2016-12-28 14:15:38 UTC
I have checked this issue with latest C9 build and master build and observed that it is working fine. Here is the screencast for the same: https://www.screencast.com/t/YPBIRBNP

Environment info: https://gist.github.com/NaqeebAnsari/a40bfc803e034ee6c18b36cdec0847e7

Hence closing this issue.