Clash when importing inner class with the same name as package suffix

[WonderCsabo]

Importing inner classes can introduce not expected compilation problems

For example in AndroidAnnotations, the user can have a following annotated class.

package id.mypapp;
public class R {
  public static class id {
     public static final int myItem = 1;
  }
   public static class menu {
     public static final int menu = 2;
  }
}

import id.myapp.R;
// other imports

@OptionsMenu(R.menu.menu)
public class HelloAndroidActivity extends Activity {
    @OptionsItem(R.id.myItem)
    void myItemClicked() {

    }
}

This is generated:

import id.myapp.R.id;
// other imports

public final class HelloAndroidActivity_ extends HelloAndroidActivity {

    @Override
    public boolean onCreateOptionsMenu(Menu menu) {
        // compilation error:
        // id is a package name part here, but compiler thinks its
        // inner class id.myapp.R.id
        menuInflater.inflate(id.myapp.R.menu.mymenu, menu);
        return super.onCreateOptionsMenu(menu);
    }

    @Override
    public boolean onOptionsItemSelected(MenuItem item) {
        if (itemId_ == id.mymenuitem) { // id.myapp.R.id
            //
        }
        return super.onOptionsItemSelected(item);
    }
}

So the main problem is that jcodemodel imports id.myapp.R.id inner class, however when it references id.myapp.R.menu.mymenu field , the compiler will think the first part of it is not a package part, but the inner class id.myapp.R.id.

The most easier solution would be to never import inner classes or maybe add an option to not to do so. (BTW, in Android, it is conventional to only import the id.myapp.R class, and not its inner classes.)

[phax]

Oh dear lord - another very exciting bug :) Will look into it, but this seems to be a tricky one

[phax]

Can you point me to the code where you are doing the generation? I found an issue with JFieldVar where the type was not emitted. Maybe that helps?

[phax]

Can you modify https://github.com/phax/jcodemodel/blob/master/src/test/java/com/helger/jcodemodel/supplementary/issues/Issue30FuncTest.java in a way so that it comes close to your problem? Thanks

[WonderCsabo]

I also use direct classes instead of defined classes here:

AbstractJClass rClassId = cm.directClass("id.aa.R.id");
AbstractJClass rClassMenu = cm.directClass("id.aa.R.menu");

JFieldRef myItem = rClassId.staticRef("myItem");
JFieldRef myMenu = rClassMenu.staticRef("myMenu");

final JPackage aPkg2 = cm._package("id.aa");
final JDefinedClass aClassAct = aPkg2._class("HelloAndroidActivity_");
final JMethod aMethodCreate = aClassAct.method(JMod.PUBLIC, cm.BOOLEAN, "onCreateOptionsMenu");
aMethodCreate.body().add(JExpr.ref("menuInflater").invoke("inflate").arg(myMenu));
final JMethod aMethodSelected = aClassAct.method(JMod.PUBLIC, cm.BOOLEAN, "onOptionsItemSelected");
aMethodSelected.body()._if(JExpr.ref("itemId_").eq(myItem));

This is generated:

import id.myapp.R.id;
import id.myapp.R.menu;

public class HelloAndroidActivity_ {

    public boolean onCreateOptionsMenu() {
        menuInflater.inflate(menu.myMenu);
    }

    public boolean onOptionsItemSelected() {
        if (itemId_ == id.myItem) {
        }
    }
}

So there is one problem: codemodel does not know that R.id is an inner class actually, because i just use directClass on the FQCN of the inner class. I guess it thinks (correctly), that R.id is a top-level class. However, in your example, it does know that, but it still generates a bad code:

public class HelloAndroidActivity_ {

    public boolean onCreateOptionsMenu() {
        menuInflater.inflate(myMenu);
    }

    public boolean onOptionsItemSelected() {
        if (itemId_ == myItem) {
        }
    }
}

I could not reproduce my exact problem with modifying your test case. But i created an simple case in the AndroidAnnotations test suite, which reproduces the issue. If i publish that, you can debug into it and access the codemodel object, and you can easily debug through the generation. Is that OK for you?

[phax]

Sounds like a plan!

[WonderCsabo]

Here is the branch. Nothing special is needed to run the test, just import it into your IDE, and run the test with JUnit. However it is more safe to build the project on command line (mvn install) before importing into the IDE. We use IntelliJ, but for this test Eclipse should also work.

[phax]

Well I have no Android SDK installed and it is all together quite cumbersome to import this. Sorry. But I may found something helpful for you: pull and checkout final JDirectClass rClassId = cm.directClass ("id.aa.R")._class ("id"); - I added JDirectClass the possibility to specify nested classes. For me the output with the changed example:

    final JCodeModel cm = new JCodeModel ();

    final JDirectClass rClassId = cm.directClass ("id.aa.R")._class ("id");
    final JDirectClass rClassMenu = cm.directClass ("id.aa.R")._class ("menu");

    final JFieldRef myItem = rClassId.staticRef ("myItem");
    final JFieldRef myMenu = rClassMenu.staticRef ("myMenu");

    final JPackage aPkg2 = cm._package ("id.aa");
    final JDefinedClass aClassAct = aPkg2._class ("HelloAndroidActivity_");
    final JMethod aMethodCreate = aClassAct.method (JMod.PUBLIC, cm.BOOLEAN, "onCreateOptionsMenu");
    aMethodCreate.body ().add (JExpr.ref ("menuInflater").invoke ("inflate").arg (myMenu));
    final JMethod aMethodSelected = aClassAct.method (JMod.PUBLIC, cm.BOOLEAN, "onOptionsItemSelected");
    aMethodSelected.body ()._if (JExpr.ref ("itemId_").eq (myItem));

    cm.build (new SingleStreamCodeWriter (System.out));

looks like:

package id.aa;

public class HelloAndroidActivity_ {

    public boolean onCreateOptionsMenu() {
        menuInflater.inflate(id.aa.R.menu.myMenu);
    }

    public boolean onOptionsItemSelected() {
        if (itemId_ == id.aa.R.id.myItem) {
        }
    }
}

[WonderCsabo]

final JDirectClass rClassId = cm.directClass ("id.aa.R")._class ("id");

I am afraid the _class() method is only available in JDefinedClass. Of course, because you cannot add new inner classes to an unknown class which is not generated by you.

[phax]

No, its not. In the current HEAD revision I added a new inbetween class AbstractJClassContainer which is the super class for JDirectClass and JDefinedClass

[WonderCsabo]

Nice! I just build HEAD and changed our code based on your solution. It works well, the R.\ inner classes are not imported at all, so there cannot be such name clashes.

[WonderCsabo]

Hmm, i found a regression with your new code:

[ERROR] Stacktrace: java.lang.IllegalArgumentException: JClass name GenericFragmentArguments<S,P> contains illegal character <
[ERROR] at com.helger.jcodemodel.AbstractJClassContainer.<init>(AbstractJClassContainer.java:122)
[ERROR] at com.helger.jcodemodel.JDirectClass.<init>(JDirectClass.java:80)
[ERROR] at com.helger.jcodemodel.JCodeModel.directClass(JCodeModel.java:300)
[ERROR] at com.helger.jcodemodel.JCodeModel.directClass(JCodeModel.java:287)

This was not a problem before.

[phax]

Will look at it. I think I know the problem

Am 15. September 2015 10:02:19 MESZ, schrieb "Csaba Kozák" notifications@github.com:

Hmm, i found a regression with your new code:

[ERROR] Stacktrace: java.lang.IllegalArgumentException: JClass name
GenericFragmentArguments<S,P> contains illegal character <
[ERROR] at
com.helger.jcodemodel.AbstractJClassContainer.<init>(AbstractJClassContainer.java:122)
[ERROR] at
com.helger.jcodemodel.JDirectClass.<init>(JDirectClass.java:80)
[ERROR] at
com.helger.jcodemodel.JCodeModel.directClass(JCodeModel.java:300)
[ERROR] at
com.helger.jcodemodel.JCodeModel.directClass(JCodeModel.java:287)

This was not a problem before.

---

Reply to this email directly or view it on GitHub: https://github.com/phax/jcodemodel/issues/30#issuecomment-140314458

Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

[phax]

Updated version is on HEAD. I ensured that name(), fullName() and _package() work as expected

[WonderCsabo]

I am seeing multiple regressions:

• imports are generated, but in the code, still everything is referenced by FQCN:

import javax.xml.bind.annotation.XmlType;
import javax.xml.ws.Action;
import javax.xml.ws.WebServiceRef;
import javax.xml.ws.WebServiceRefs;
import javax.xml.ws.soap.Addressing;
import javax.xml.ws.soap.AddressingFeature.Responses;
import android.content.Context;
import dalvik.annotation.TestTargetClass;
import org.androidannotations.api.BackgroundExecutor;
import org.androidannotations.copyannotations.HasOtherAnnotations.Event;

@javax.xml.bind.annotation.XmlType
@dalvik.annotation.TestTargetClass(String.class)
@javax.xml.ws.WebServiceRefs({
    @javax.xml.ws.WebServiceRef(type = String.class)
})
public final class HasOtherAnnotations_

• JFieldVar usages seems to be wrong, they generate this: <ClassName>.<fieldName> for non-static fields:

OnViewChangedNotifier.replaceNotifier(ActivityNotInManifest_.onViewChangedNotifier_);

[phax]

Thanks for noting this. Both regresssions were fixed. Please re-check. Thanks

[WonderCsabo]

New regression:

Now classes which could be imported, and was imported before, are not imported now. Example:

import org.androidannotations.api.BackgroundExecutor;

@javax.xml.bind.annotation.XmlType
@dalvik.annotation.TestTargetClass(String.class)
@javax.xml.ws.WebServiceRefs({
    @javax.xml.ws.WebServiceRef(type = String.class)
})
public final class HasOtherAnnotations_

Another example:

package org.androidannotations.generation;

import org.androidannotations.api.builder.ActivityIntentBuilder;
import org.androidannotations.api.view.HasViews;
import org.androidannotations.api.view.OnViewChangedNotifier;

public final class WindowFeatureAppCompatActivity_
    extends org.androidannotations.generation.WindowFeatureAppCompatActivity
    implements HasViews
{

As you can see, interestingly some classes are imported, some classes are not.

---

Not really a regression, but it would be nice to import outer classes of inner classes, instead of only using the FQCN in the code all the time:

Currently:

this.getDrawable(org.androidannotations.generation.R.drawable.myDrawable)

Would be nice:

import org.androidannotations.generation.R;

this.getDrawable(R.drawable.myDrawable)

Maybe this is related to the regression.

[WonderCsabo]

I reduced the project, so it now only contains pure java modules. You should be able to import it into any IDE. Hopefully it will help you investigate regressions.

[phax]

Okay, now I can import the project and run the test. Both the regressions you found did not render the project uncompilable, but are simply suggestions for improvement. Did I get this right?

[WonderCsabo]

I would say the first is really a regression, because it killed the good import strategy of code model which was working before. :S

[WonderCsabo]

I think it is also a regression in another aspect: lots of projects' test suite will fail because of this, just like mine. This should not happen.

[phax]

I did some stuff and it worked for me on the example branch. It is still not error free, but I think this is because of missing Android dependencies (like android.support.v4.app.Fragment). So can you also please check. One problem I see is when using generics. In this case you should not use cm.directClass ("com.test.GenericFragmentArguments<S,P>") but instead use (JNarrowedClass) cm.parseType ("com.test.GenericFragmentArguments<S,P>") whilch results in a JNarrowedClass whichs erasure is a JDirectClass. hth

[WonderCsabo]

Great, this is a great enhancement! The test failures were because of me, i think.

Unfortunately i found a new regression:

JCodeModel cm = new JCodeModel();

JPackage aPkg1 = cm._package("id.myapp.activity");

JDefinedClass testClass = aPkg1._class("TestClass");

JDirectClass androidR = cm.directClass("android.R");
JDirectClass androidRId = androidR._class("id");
JDirectClass myR = cm.directClass("id.myapp.R");
JDirectClass myRId = myR._class("id");

JBlock constructorBody = testClass.constructor(JMod.PUBLIC).body();
JVar myInt = constructorBody.decl(cm.INT, "myInt", androidRId.staticRef("someId"));
JVar myInt2 = constructorBody.decl(cm.INT, "myInt2", myRId.staticRef("otherId"));

cm.build (new SingleStreamCodeWriter(System.out));

Generates:

package id.myapp.activity;

import android.R;
import id.myapp.R;

public class TestClass {

    public TestClass() {
        int myInt = R.id.someId;
        int myInt2 = R.id.otherId;
    }
}

Clash on the two R class!

[phax]

I got it, I tried it, but I failed. Maybe tomorrow is a better day :) In the meantime I fixed all the JavaDoc errors - also of relevance I guess ;-)

This one is tricky because the imports work on the outer class but the usage works on the direct type and I haven't figured out how this works together....

[WonderCsabo]

Thanks for your work so far, hope you can fix this. :)

[phax]

Hopefully it has now been fixed. At least the provided example works. Please also cross-check. Thanks

[WonderCsabo]

It seems the problem is fixed. I ran the whole AndroidAnnotations test suite, and everything seems to be fine.

[phax]

Release 2.7.11 is on its way to Maven Central...

[WonderCsabo]

Thanks! It is now on Maven Central. BTW, why do you deploy the test-sources artifact? I do not see much reason for it, but maybe i am wrong.

[phax]

I do it for all the products. No need for you to download it. Maybe it was a requirement to publish on Maven Central - can't remember exactly. This configuration is part of the parent POM I'm using for all my 80 repos :)

[WonderCsabo]

This is not a requirement of Central, i am sure. However, it does not really matter, i was just curious.