Imports sometimes incorrect when inheriting nested type of same name about javapoet HOT 12 CLOSED

In the dagger variant of JavaWriter, there's a concept of an originating element, which holds context about a generated type's parent.

That said, there was a bug in the dagger implementation, as this context was ignored for enclosed types. Thanks to @JakeWharton for pointing out this issue is very similar.

Anyway, here's the fix to dagger's JavaWriter and tests to go along with it.
google/dagger#99

from javapoet.

We have originating element as well but at a much more granular level.
While that may be the fix for Dagger, you cannot assume anything about the
relationship of the originating element to the generated type since it's a
completely separate domain.
On Jan 3, 2015 4:09 PM, "Adrian Cole" [email protected] wrote:

In the dagger variant of JavaWriter, there's a concept of an originating
element, which holds context about a generated type's parent.

That said, there was a bug in the dagger implementation, as this context
was ignored for enclosed types. Thanks to @JakeWharton
https://github.com/JakeWharton for pointing out this issue is very
similar.

Anyway, here's the fix to dagger's JavaWriter and tests to go along with
it.
google/dagger#99 google/dagger#99

—
Reply to this email directly or view it on GitHub
#77 (comment).

from javapoet.

I see what you mean: Originating Element is not the right concept to stick
to, even if it happened to have the type data I needed.
http://docs.oracle.com/javase/6/docs/api/javax/annotation/processing/Filer.html

Guess I know what I'm also doing tomorrow :P

from javapoet.

This needs some thought, but I think we have two choices:

• A concept similar to the explicit imports where you can inform the TypeWriter of additional type structure. In this example you would iterate the type hierarchy of the element used to specify the superclass to inform nested types.
• Make ClassName aware of nested types. I don't know how feasible this is, I'm just shooting from the hip here. This way when you create the ClassName from the TypeMirror you'd get all nested types as well.

I haven't thought about either of these at all. Just spitballing.

from javapoet.

Right now we also explicitly import from java.lang because we don't know if the local package overrides those imports. Which means that 99% of the time, we're generating unnecessary imports.

I like the idea of using originating elements to detect the names of the classes in the same package. Detecting inheritance will be much more difficult; particularly because we make no requirement that the subclass is generated after the superclass.

from javapoet.

Punting to 3.1 because it's difficult to do.

from javapoet.

I just ran into this issue in my JavaWriter -> JavaPoet migration for Dagger 2. @adriancole had a proposal for Dagger's JavaWriter in google/dagger#99, would that be workable here?

from javapoet.

I'm running into this again with Dagger's experimental ahead-of-time subcomponents mode. Nothing particularly new, just different codegen tickling this bug again.

The fix in many cases is simple, but it's unfortunate that we can't provide a better error to users besides generating code that doesn't compile.

from javapoet.

One possibility would be to add a method to JavaFile that specifies simple names that should always be fully-qualified. They would never be imported and, in the case of java.lang, they would never be considered implicitly imported. So if you wrote javaFileBuilder.alwaysQualify("Optional") then every reference to java.util.Optional would be spelled as java.util.Optional even if JavaPoet is unaware of any other Optional that might be around. Likewise javaFileBuilder.skipJavaLangImports(true).alwaysQualify("Thread") would cause references to java.lang.Thread to be spelled as java.lang.Thread even though other java.lang references are unqualified.

Callers would then at least have the option of scanning the supertypes of a generated class to see what nested types there are, and adding those to the alwaysQualify list. That would fix the issue I outlined in the issue description. We could also have a convenience method that does exactly this, say avoidClashesWithNestedClasses(TypeElement).

from javapoet.

@eamonnmcmanus how would nested types such as java.util.Map.Entry work in that case?

from javapoet.

The answer depends on what JavaPoet does today with nested classes. Does it import java.util.Map.Entry and then write Entry in the generated code? Or does it import java.util.Map and write Map.Entry in the code? The way to cause it to write the full java.util.Map.Entry would be alwaysQualify("Entry") in the first case and alwaysQualify("Map") in the second.

from javapoet.

I've opened a PR with the proposed implementation above in #734

from javapoet.