WiP refactor: Add import in types

[surli]

This PR is a change of paradigm regarding imports in Spoon.
Currently CtImports are attached to CompilationUnits and are computed twice: when building the model first, and then when pretty-printing the model.

This model is badly design if we want to take into account the imports when transforming the model: for example, renaming all CtReference won't remove the old imports even if they're not anymore pointing to something. See the unit test in #2083.

In order to fix this design, I propose the following changes:

• the imports are attached primarly to CtTypes and CtPackage (because of imports in package-info.java)
• they are visitable through the types as for any concept of the model
• we can still gather the imports of a CU by gathering all imports of its types

WDYT?

[surli]

@pvojtechovsky could you have a look on this branch on CtGenerationTest I don't understand why it says that CtRole.IMPORT is not settable from CtClassImpl...

[pvojtechovsky]

When I read your concept, it looked nice, but when reading the code I probably see one problem...
I have not read your implementation fully, so I might be wrong...

Imagine you have a Compilation unit with 2 Types. The first type is import holder so it gets all imports from the compilation unit... and the second class gets no imports? What if first type is just empty marker interface which needs no import at all, but the second type is full of code which needs imports? Or is there any way how to detect which import belongs to which type? If we cannot detect that, then what is advantage comparing previous concept with imports in compilation unit - which seems to be more correct from that point of view ... WDYT?

for example, renaming all CtReference won't remove the old imports even if they're not anymore pointing to something.

why it cannot access compilation unit and remove reference there?

[pvojtechovsky]

CtGenerationTest I don't understand why it says that CtRole.IMPORT is not settable from CtClassImpl...

It is again egg/chicken problem. RoleHandlersGenerator is using Pattern which uses RoleHandlers to generate RoleHandlers, but there is missing RoleHandler for new property CtTypeImpl#imports.

How to solve it? The spoon code from snapshot (with origin model and appropriate RoleHandlers) should be used to generate new RoleHandlers for your PR. I just do not know how to reach that ...

[surli]

It is again egg/chicken problem.

Thanks for the fix @pvojtechovsky

why it cannot access compilation unit and remove reference there?

The visitor of the model are never visiting the CUs: so in order to patch that we would need in several places of Spoon to recompute again the imports possibly after each transformation to avoid bugs when executing processors...

Imagine you have a Compilation unit with 2 Types. The first type is import holder so it gets all imports from the compilation unit... and the second class gets no imports? What if first type is just empty marker interface which needs no import at all, but the second type is full of code which needs imports? Or is there any way how to detect which import belongs to which type? If we cannot detect that, then what is advantage comparing previous concept with imports in compilation unit - which seems to be more correct from that point of view ... WDYT?

If you have a CU with 2 types, the already existing imports are attached by default to the main type of the CU. So when first calling JDTImportBuilder, yes the second type get no imports. But immediately after I call the ImportScanner, which should check that the imports are OK regarding the content of the type: then the second type will got the proper imports.

One advantage of that, is if you decide to output the code using types and not existing CU: with the current behaviour you loose all information regarding the imports, as they are stored in the CU and not the types.
With the new behaviour, you'll be able to manage both output: for CU you only got all imports from all types of the CUs, and for types you got imports of the specific type.

And more importantly it allows the visitor to process the imports like any other concept of Spoon.

[spoon-bot]

API changes: 6 (Detected by Revapi)

Old API: fr.inria.gforge.spoon:spoon-core:jar:6.3.0-20180619.225448-155 / New API: fr.inria.gforge.spoon:spoon-core:jar:6.3.0-SNAPSHOT

	Method was added to an interface.
Old	none
New	method ImportScanner#computeImports(CtPackage)
Breaking	binary: non_breaking

	The return type changed covarianly from 'java.util.Collection<spoon.reflect.declaration.CtImport>' to 'java.util.Set<spoon.reflect.declaration.CtImport>'.
Old	method DefaultJavaPrettyPrinter#computeImports(CtType)
New	method DefaultJavaPrettyPrinter#computeImports(CtType)
Breaking	binary: breaking,

	Method was removed.
Old	method ImportScanner#getAllImports()
New	none
Breaking	binary: breaking,

	The return type changed covarianly from 'java.util.Collection<spoon.reflect.declaration.CtImport>' to 'java.util.Set<spoon.reflect.declaration.CtImport>'.
Old	method CompilationUnit#getImports()
New	method CompilationUnit#getImports()
Breaking	binary: breaking,

	Method was removed.
Old	method CompilationUnit#setImports(Collection)
New	none
Breaking	binary: breaking,

	The type of the parameter changed from 'spoon.reflect.declaration.CtElement' to 'spoon.reflect.declaration.CtType'.
Old	parameter ImportScanner#computeImports(===CtElement===)
New	parameter ImportScanner#computeImports(===CtType===)
Breaking	binary: breaking,

[surli]

I'm closing this one for now. I won't investigate in this direction right now, as I fixed the import bug in #2083