[proposal] check class name against keywords and java.lang

[guiguilechat]

Keywords

see #70 for the keywords

java.lang

Issue

This is an issue because this package is already imported and as such not referenced to.

Typically if I create a my.package.Object class and then I create a method

@Override
boolean equals(java.lang.Object other) 

The object other actually refers to the class I am crating, and not the java.lang.Object class.
This issue literally happened to me when compiling against a openapi web service that produces "Object" responses.

Detection

This issue is very complicated : since the java reflection works bottom-up, that is the classes know their package but the package doesn't know its classes, it's difficult to find if the class exists without a java.lang.class.forName("java.lang."+sName), with trycatch around. This expensive call requires to cache the result in a hasmap<String, Boolean>

here is my code

private static final HashMap<String, Boolean> resolvedJavaLangNames = new HashMap<>();

     /**
 * check if a name already represents a class in the java.lang package. eg
 * String should return true, but LeetH4X0R should not.
 * 
 * @param className
 *          name to test
 * @return the existence of such a class in the java.lang (cached)
 */
public static boolean isJavaLangClass(String className) {
	if (resolvedJavaLangNames.containsKey(className)) {
		return resolvedJavaLangNames.get(className);
	}
	boolean isJavaLang = true;
	try {
		Class.forName("java.lang." + className);
	} catch (Exception e) {
		isJavaLang = false;
	}
	resolvedJavaLangNames.put(className, isJavaLang);
	return isJavaLang;
}

Looking at how fast it is to use JCodeModel, I consider the use of synchronization to be worthless (I usually don't and thus would have synced over the hashmap in the method)

It should check for specific exception (security exception means I can't tell)

Reaction

In the case the class C is in java.lang already, two possibilities :

• internally mark the class C as javaLangMisleading, so that all call to a java.lang.C are specifically written as such
• throw an exception (bad - unless in the option)

IMO both should be available. mark the class C as misleading , AND allow users to avoid this with a compilation option.

Especially, I'm not sure how to make the misleading part, since the writing of the class names may be done without the knowledge of where that name is used.

[glelouet]

in the specific case of refering to java.lang.Object it's an issue, not an enhancement.

[phax]

Makes total sense. Thanks for pointing this out. Part of 3.3.0 release

[glelouet]

I think it's a bit more complex than that (Maybe I'm wrong).

In some cases people may want to create eg an Object class to represent something. That is, nothing in the java language prevents from creating my.pck.Object .
The issue is that ATM if I create an Object class and I refer to java.lang.Object inside it, JCodeModel will translate it into "Object" (ignoring the java.lang package) because this package is already imported, but the java compiler will translate this "Object" into "my.pck.Object", because that's what the Object class refer to in this context.

The ideal solution would be to ensure the java.lang package is not removed when the class refereed to has the same name as a class declared in the current class or its subclasses.

example if I want to create

MyClass{

public static class Object{}

public java.lang.Object get(){ return null;}

}

Using JCodemodel, I can't, because JCodemodel will translate to public Object get() which the java compiler will translate to public MyClass.Object get()

Ensuring the JCodemodel does not do it may require a lot of additional modifications, so in a first and simpler pass I advise to allow the user to specifically prevent the usage of a class name that is already present in java.lang.

I think you misunderstood the part "reaction" that I wrote. I specifically wrote that throwing an exception (or returning false) is a bad behaviour unless specifically required by the user. The optimal solution is to make JCodeModel refer to not remove the java.lang. prefix when the class name is overriden by a local class.

[glelouet]

I will make more of an example of what should compile correctly, gimme some more time.

[glelouet]

This is valid java code, that should be produceable by jcm.

package jcodemodel;


public class MyClass {

	public static class Object {
	}

	public void call(java.lang.Object obj) {
	}

	public void call(Object obj) {
	}

	public static class Number {
	}

	public void call(java.lang.Number nb) {
	}

	public void call(Number nb) {
	}


}

[phax]

It does it, but the other way around:

package jcodemodel;

public class MyClass {

    public void call(Object obj) {
    }

    public void call(MyClass.Object obj) {
    }

    public void call(Number obj) {
    }

    public void call(MyClass.Number obj) {
    }

    public static class Number {
    }

    public static class Object {
    }
}

That is semantically equivalent I think ok....

[glelouet]

No because the java compiler will refuse to compile it. Number and Myclass.Number are equivalent in those cases, therefore you have two methods with the same signature.

That's why I gave the link to the memory compiler, because it shows that the compilation of the generated classes fails.

[glelouet]

here is the code that shows this :

@Test
	public void testBadPackage() throws JClassAlreadyExistsException, ClassNotFoundException {
		JCodeModel cm = new JCodeModel();
		JDefinedClass myclass = cm._class(JMod.PUBLIC, "mypck.MyClass");
		JDefinedClass myObject = myclass._class(JMod.PUBLIC | JMod.STATIC, "Object");
		myclass.method(JMod.PUBLIC, cm.VOID, "call").param(Object.class, "obj");
		myclass.method(JMod.PUBLIC, cm.VOID, "call").param(myObject, "obj");

		DynamicClassLoader dcl = DynamicClassLoader.generate(cm);
		dcl.findClass(myclass.fullName());
	}

Here is the output result :

compile diagnostic /mypck/MyClass.java:8: error: method call(mypck.MyClass.Object) is already defined in class mypck.MyClass
public void call(MyClass.Object obj) {

If you remove the last call to findClass you still have the output but it does not fail because it does not care if the class is badly defined.

failure :

FAILED: testBadPackage
java.lang.ClassFormatError: Truncated class file

[phax]

Ah okay - got it. I will check the dynamic compilation thingy separate...
New version creates:

package jcodemodel;

public class MyClass {

    public void call(java.lang.Object obj) {
    }

    public void call(MyClass.Object obj) {
    }

    public void call(java.lang.Number obj) {
    }

    public void call(MyClass.Number obj) {
    }

    public void call(Byte obj) {
    }

    public void call(Long obj) {
    }

    public static class Number {
    }

    public static class Object {
    }
}

[glelouet]

Note that if you can fix that quickly, there is no point failing when the user requests to create an Object class anymore.
I just thought that making it work correctly could request too much time.

Ideally you want the previous result to replace the Myclass.Number by Number, because then MyClass.number is the highest priority in the resolution.