review test(import): compare the computed imports with human imports

[tdurieux]

No description provided.

[surli]

I started to have a look on this one and I've several questions regarding the usage of ScannerImport and the contract we imposed. For example, first false assumptions detected by the test are the following ones:

spoon.support.reflect.code.CtForImpl
	spoon.reflect.ModelElementContainerDefaultCapacities.FOR_INIT_STATEMENTS_CONTAINER_DEFAULT_CAPACITY missing
	spoon.reflect.ModelElementContainerDefaultCapacities.FOR_UPDATE_STATEMENTS_CONTAINER_DEFAULT_CAPACITY missing
	spoon.reflect.ModelElementContainerDefaultCapacities unused

Those errors appeared because we took as a contract to always put an import preferably on the class type, instead of putting a static import on the field or the executable. Here we imported the class, and we did not imported the two fields. Then, in fact the test detected that we don't use inside Spoon the contract we put on ImportScanner.

So do we change our contract inside the ImportScanner or do we apply the contract on Spoon?

[monperrus]

I propose to refine this test, to test for "equivalent import" import Foo = import static Foo.FIELD

[surli]

propose to refine this test, to test for "equivalent import" import Foo = import static Foo.FIELD

OK I'll do that

[surli]

Ok now I have another issue: some imports are directly related to the javadoc: for example in ProblemFixer

/**
 * This interface defines problem fixers. Problem fixers can be provided when a
 * problem is reported to the environment. The user can then chose what fixer to
 * use.
 *
 * @see Environment#report(Processor, org.apache.log4j.Level, CtElement, String,
 * ProblemFixer[])
 */

leads to the import of spoon.compiler.Environment which is not used inside the class.

[pvojtechovsky]

Will it work also in cases when AST is refactored and some classes will be removed? Then it will probably produce unused imports, will not?

[pvojtechovsky]

return read only collection?

[surli]

I propose that we change this test to report a warning on the logs instead of a failure to be able to merge it, and to open an issue about that problem.

I won't make improvement on how the imports are managed in Spoon in the next weeks, and it does not really help to keep this PR open IMHO.

[surli]

As proposed above I changed the test to only check if no import is missing: for the unused imports, I only output a warning. For me the PR is ready.