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

Field is ambiguous in annotation but fine in class or script body #259

Closed
AntonPanikov opened this issue Jan 25, 2017 · 5 comments
Closed

Comments

@AntonPanikov
Copy link

If you have two (or more) on demand static imports with the same name to import, the first one must be imported, but the second and others must be ignored as in JavaSpecs: http://docs.oracle.com/javase/specs/jls/se8/html/jls-7.html#jls-7.5.4

It is working perfectly fine for non static on demand imports and for static on demand imports if you use imported names in code. But as soon as you try to use name in annotation, groovy compiler complains about ambiguous field.

I am using groovy eclipse of version: Groovy-Eclipse Feature 2.9.2.xx-201701250356-e46

Here is a sample example:

package test

interface TestStrings {
    public static final String ONE = 'one'
    public static final String TWO = 'two'
    public static final String THREE = 'three'
}

interface TestStrings2 {
    public static final String ONE = 'one from two'
    public static final String TWO = 'two from two'
    public static final String THREE = 'three from two'
}
package test

@interface TestAnnotation {
    String[] value()
}
package test

import static test.TestStrings.*
import static test.TestStrings2.*

class Test {

    @TestAnnotation(value = [ONE, TWO, THREE])
    static main(args) {
        println "$ONE"
    }

}

Groovy eclipse will complain about ONE, TWO, THREE constants in annotation, but not in println. Code is running fine and printing 'one' as expected and if you change order of static imports:

import static test.TestStrings.*
import static test.TestStrings2.*

then println will print 'one from two' as expected. So there is no ambiguous field, because the same field is used and worked fine in code and groovy eclipse is not complaining about it, while in annotations it is failing.

At the same time non static import on demand is working fine for both: code and annotation:

package test

import test.*
import test.sub.*

class Test {

    @TestAnnotation(value = [TestStrings.ONE, TestStrings.TWO, TestStrings.THREE])
    static main(args) {
        println "$TestStrings.ONE"
    }

}

If you have the same TestString interface defined in package test.sub, then no errors for this case will be raised.

@eric-milles
Copy link
Member

eric-milles commented Jan 26, 2017

I do not agree with your interpretation of the rules. What it is saying is if you have this it is okay:

import static java.util.Collections.*
import static java.util.Collections.*

Also, these starts may pull in several occurrences of the name emptyList and that's okay too.

If you have

import static a.b.C.*
import static a.d.D.*

and they both pull in an E, you need to disambiguate with import static a.d.D.E

@AntonPanikov
Copy link
Author

As from specs:

Two or more static-import-on-demand declarations in the same compilation unit may name the same member; the effect is as if the member was imported exactly once.

Means that it is ok to name E member twice by it name "E", even from different packages. Members imported by it short name - this is a purpose of import - to refer member just by it name - "E" in this example. But the exactly the first one will be imported, others - ignored.

As in my example:

package test

import static test.TestStrings.*
import static test.TestStrings2.*

class Test {
    static main(args) {
        println "$ONE"
    }
}

Will compile without any errors now (with current version of plugin) and print constant from the first import. And if you change the order of imports, it will change imported constant and printed value. It is already supported and works as in specs.

The question is: as soon I want to use the same imported value in annotation, groovy eclipse will complain about ambiguity in annotation only, but will able to refer proper constant from method correctly. Why is it able to use first imported constant in println, but not in the annotation?

So in your example, this will work right now without any errors:

import static a.b.C.*  // E here
import static a.d.D.*  // E here

println E // E from a.b.C.E

but using E in annotation will rise an error.

@eric-milles eric-milles changed the title The filed is ambiguous in annotation but fine in code. Field is ambiguous in annotation but fine in class or script body Jan 26, 2017
@eric-milles
Copy link
Member

I'll try a couple things in Java and see where Groovy and Java differ. I may end up filing a bug with core Groovy and patching the println case to emit an error.

During my whole Java development career it has been a rule of thumb to avoid star imports because you can get into sticky messes such as this one. You are relying on the language and tool designers to get this right for you. If you disambiguate with another import, you regain complete control. I'd much rather take that option than to be vulnerable to a tool change or someone adding another member to the first class that takes over for the other members of the same name.

@AntonPanikov
Copy link
Author

I absolutely agree that this is a really bad practice and lead to confusion and even bugs in case of auto import reordering. I just found out this inconsistency in behavior for code and for annotation and bring it to you. Thanks you for your research. Unfortunately I found out that we relay on this in our project and I am not the only developer in my team, so it will take some time to improve such cases. Thanks.

@eric-milles
Copy link
Member

Thanks for bringing these issues up. Can you consider this closed? I will follow through with some investigations and a change somewhere. Also the alias issue should be fixed; could you recheck?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants