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

review feat: finish the changing behaviour for noclasspath mode #2074

Merged
merged 6 commits into from
Jun 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 37 additions & 2 deletions src/main/java/spoon/Launcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@
import com.martiansoftware.jsap.JSAPException;
import com.martiansoftware.jsap.JSAPResult;
import com.martiansoftware.jsap.Switch;
import com.martiansoftware.jsap.stringparsers.EnumeratedStringParser;
import com.martiansoftware.jsap.stringparsers.FileStringParser;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.filefilter.IOFileFilter;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Level;
import org.apache.log4j.Logger;
import spoon.SpoonModelBuilder.InputType;
Expand Down Expand Up @@ -71,6 +73,10 @@
*/
public class Launcher implements SpoonAPI {

enum CLASSPATH_MODE {
NOCLASSPATH, FULLCLASSPATH
}

public static final String SPOONED_CLASSES = "spooned-classes";

public static final String OUTPUTDIR = "spooned";
Expand Down Expand Up @@ -352,11 +358,22 @@ protected static JSAP defineArgs() {
sw1.setDefault("false");
jsap.registerParameter(sw1);


opt2 = new FlaggedOption("cpmode");
opt2.setLongFlag(opt2.getID());
String acceptedValues = StringUtils.join(CLASSPATH_MODE.values(), "; ");
opt2.setStringParser(EnumeratedStringParser.getParser(acceptedValues));
msg = "Classpath mode to use in Spoon: " + acceptedValues;
opt2.setHelp(msg);
opt2.setRequired(true);
opt2.setDefault(CLASSPATH_MODE.NOCLASSPATH.name());
jsap.registerParameter(opt2);

// nobinding
sw1 = new Switch("noclasspath");
sw1.setShortFlag('x');
sw1.setLongFlag("noclasspath");
sw1.setHelp("Does not assume a full classpath");
sw1.setHelp("[DEPRECATED] Does not assume a full classpath (Please use --cpmode now, as the default behaviour has changed.)");
jsap.registerParameter(sw1);

// show GUI
Expand Down Expand Up @@ -427,7 +444,25 @@ protected void processArguments() {
environment.setComplianceLevel(jsapActualArgs.getInt("compliance"));
environment.setLevel(jsapActualArgs.getString("level"));
environment.setAutoImports(jsapActualArgs.getBoolean("imports"));
environment.setNoClasspath(jsapActualArgs.getBoolean("noclasspath"));

if (jsapActualArgs.getBoolean("noclasspath")) {
Launcher.LOGGER.warn("The usage of --noclasspath argument is now deprecated: noclasspath is now the default behaviour.");
} else {
Launcher.LOGGER.warn("Spoon is now using the 'no classpath mode' by default. If you want to ensure using Spoon in full classpath mode, please use the new flag: --cpmode fullclasspath.");
}

String cpmode = jsapActualArgs.getString("cpmode").toUpperCase();
CLASSPATH_MODE classpath_mode = CLASSPATH_MODE.valueOf(cpmode);
switch (classpath_mode) {
case NOCLASSPATH:
environment.setNoClasspath(true);
break;

case FULLCLASSPATH:
environment.setNoClasspath(false);
break;
}

environment.setPreserveLineNumbers(jsapActualArgs.getBoolean("lines"));
environment.setTabulationSize(jsapActualArgs.getInt("tabsize"));
environment.useTabulations(jsapActualArgs.getBoolean("tabs"));
Expand Down
15 changes: 12 additions & 3 deletions src/main/java/spoon/testing/utils/ModelUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ public Factory createFactory() {

/** Utility method for testing: creates the model of the given `classesToBuild` from src/test/java and returns the factory */
public static Factory build(Class<?>... classesToBuild) throws Exception {
Launcher launcher = new Launcher();
final Launcher launcher = new Launcher();
launcher.getEnvironment().setNoClasspath(false);
launcher.getEnvironment().setCommentEnabled(false);
SpoonModelBuilder comp = launcher.createCompiler();
for (Class<?> classToBuild : classesToBuild) {
Expand Down Expand Up @@ -101,9 +102,17 @@ public static Factory build(File... filesToBuild) {
return comp.getFactory();
}

/** Builds and returns the Spoon model of `` classToBuild */
public static <T> CtType<T> buildClass(Class<T> classToBuild) throws Exception {
return build(classToBuild).Type().get(classToBuild);
return buildClass(classToBuild, true);
}

/** Builds and returns the Spoon model of `` classToBuild */
public static <T> CtType<T> buildClass(Class<T> classToBuild, boolean ensureFullclasspath) throws Exception {
if (ensureFullclasspath) {
return build(classToBuild).Type().get(classToBuild);
} else {
return buildNoClasspath(classToBuild).Type().get(classToBuild);
}
}

/** checks that the file `outputDirectoryFile` can be parsed with Spoon , given a compliance level. */
Expand Down
5 changes: 5 additions & 0 deletions src/test/java/spoon/test/annotation/AnnotationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,7 @@ public void testUsageOfTypeAnnotationBeforeExceptionInSignatureOfMethod() throws
@Test
public void testUsageOfTypeAnnotationInReturnTypeInMethod() throws Exception {
final Launcher launcher = new Launcher();
launcher.getEnvironment().setNoClasspath(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need those changes? if it works in full classpath, it should work in noclasspath, correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it works but the printed result is not exactly the same, so instead of fixing the assert I decided to keep the old setup.

Here the obtained diff if we call the test with a noclasspath:

Return type with an type annotation must be well printed expected:
<[public [email protected]
]String m3() {
    re...> but was:<[@spoon.test.annotation.testclasses.TypeAnnotation
public java.lang.]String m3() {
    re...>

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean that we don't have the fully-qualified names in noclasspath or not?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it means that in noclasspath, the annotation seems not printed exactly at the same place: before or after the modifier of a field.

launcher.addInputResource("./src/test/java/spoon/test/annotation/testclasses/AnnotationsAppliedOnAnyTypeInAClass.java");
launcher.buildModel();
Factory factory = launcher.getFactory();
Expand All @@ -564,6 +565,7 @@ public void testUsageOfTypeAnnotationInReturnTypeInMethod() throws Exception {
@Test
public void testUsageOfTypeAnnotationOnParameterInMethod() throws Exception {
final Launcher launcher = new Launcher();
launcher.getEnvironment().setNoClasspath(false);
launcher.addInputResource("./src/test/java/spoon/test/annotation/testclasses/AnnotationsAppliedOnAnyTypeInAClass.java");
launcher.buildModel();
Factory factory = launcher.getFactory();
Expand All @@ -582,6 +584,7 @@ public void testUsageOfTypeAnnotationOnParameterInMethod() throws Exception {
@Test
public void testUsageOfTypeAnnotationOnLocalVariableInMethod() throws Exception {
final Launcher launcher = new Launcher();
launcher.getEnvironment().setNoClasspath(false);
launcher.addInputResource("./src/test/java/spoon/test/annotation/testclasses/AnnotationsAppliedOnAnyTypeInAClass.java");
launcher.buildModel();
Factory factory = launcher.getFactory();
Expand Down Expand Up @@ -766,6 +769,7 @@ public void testUsageOfParametersInTypeAnnotation() throws Exception {
@Test
public void testOutputGeneratedByTypeAnnotation() throws Exception {
final Launcher launcher = new Launcher();
launcher.getEnvironment().setNoClasspath(false);
launcher.addInputResource("./src/test/java/spoon/test/annotation/testclasses/AnnotationsAppliedOnAnyTypeInAClass.java");
launcher.buildModel();
// we only write to disk here
Expand Down Expand Up @@ -930,6 +934,7 @@ public void testGetAnnotationOuter() throws Exception {
@Test
public void testAbstractAllAnnotationProcessor() throws Exception {
Launcher spoon = new Launcher();
spoon.getEnvironment().setNoClasspath(false);
spoon.addInputResource("./src/test/java/spoon/test/annotation/testclasses/AnnotationsAppliedOnAnyTypeInAClass.java");
spoon.addInputResource("./src/test/java/spoon/test/annotation/testclasses/BasicAnnotation.java");
spoon.addInputResource("./src/test/java/spoon/test/annotation/testclasses/TypeAnnotation.java");
Expand Down
12 changes: 12 additions & 0 deletions src/test/java/spoon/test/api/APITest.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import spoon.Launcher;
import spoon.OutputType;
import spoon.SpoonAPI;
import spoon.compiler.Environment;
import spoon.compiler.InvalidClassPathException;
import spoon.reflect.code.CtIf;
import spoon.reflect.code.CtStatement;
Expand Down Expand Up @@ -578,4 +579,15 @@ public void testGetOneLinerMainClassFromCU() {
assertSame(l, mainType);
}

@Test
public void testLauncherDefaultValues() {
// contract: check default value for classpath and comments in Launcher

Launcher launcher = new Launcher();
Environment environment = launcher.getEnvironment();

assertTrue(environment.getNoClasspath());
assertTrue(environment.isCommentsEnabled());
}

}