Package of array type reference marked implicit if component type is available on classpath (noclasspath)

[slarse]

It seems like this issue persists when running in classpath mode as well, but I'm running everything in noclasspath so I'll address this from that perspective. Anyway, array type references for which the component type is in fact available on the classpath always get their package references marked implicit, regardless of what the source code looks like. Here's a trivial example.

import java.util.*;

public class QualifiedArrayType {
    java.lang.Object[] builtinTypeArray;
    java.util.Date[] importedTypeArray;
}

The packages here are clearly explicit, but they are marked implicit. I believe this is due to a few issues that "work together".

First, when building an array type reference, the component type seems to typically be set here:

spoon/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java

Line 911 in e060bd2

arrayref.setComponentType(getTypeReference(binding.leafComponentType(), resolveGeneric));

That call to getTypeReference seems to always end up in this case, at which point the package is set to be implicit:

spoon/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java

Lines 775 to 779 in e060bd2

	} else {
	CtPackageReference packageReference = getPackageReference(binding.getPackage());
	packageReference.setImplicit(true);
	ref.setPackage(packageReference);
	}

That's the root of the problem. I don't understand why the reference is set to implicit here, but removing that causes the test case SpoonifierTest.testGeneratedSpoonifyCode to fail. So I'm assuming that it should be set implicit here.

I found another potential solution by calling JDTTreeBuilderHelper.handleImplicit on the array type reference after it has been created, with a tiny modification in handleImplicit. Putting up a PR once I've tried it out as a solution for my own problems.

[MartinWitt]

After a first look, i would say the SpoonifierTest.testGeneratedSpoonifyCode is created from the AST with all "bugs" and the test could be changed. SpoonifyVisitor creates a String allowing recreation of the AST including it's old state.

[slarse]

@MartinWitt That sounds reasonable. I just felt a bit uneasy simply removing the setImplicit there as I don't understand its purpose.

I did try removing it, and from initial testing it solves the most serious issues I encountered. But that could simply be that for my purposes, it's significantly better that a package that must be explicit is actually marked explicit, than one that should be implicit is accidentally marked explicit.

I'm running some more tests locally to see if other issues crop up from removing that statement. If anyone knows why the packageReference.setImplicit(true) statement sits where it does up there, I'd be happy to hear about it.