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: feature Parameter remove refactoring #1317

Merged
merged 3 commits into from
May 30, 2017

Conversation

pvojtechovsky
Copy link
Collaborator

Implements a "method parameter remove refactoring function". It expects a CtParameter as input - it is the parameter, which has to be removed by this refactoring.

The refactoring function:

in preparation phase:

  • searches for all executables (methods and lambdas), which overrides/are overridden by the executable whose parameter has to be removed
  • searches for all invocations to all found executables

in validation phase:

  • reports an issue (stops refactoring) if to be removed parameter from any found executable is used
  • reports an issue (stops refactoring) if to be removed argument from any found invocation is not a readonly literal (is not safe to remove it).

in refactoring phase

  • removes appropriate argument from all found invocations
  • removes appropriate parameter from all found executables

@spoon-bot
Copy link
Collaborator

Revapi Analysis results

Old API: fr.inria.gforge.spoon:spoon-core:jar:5.7.0-20170521.070955-114

New API: fr.inria.gforge.spoon:spoon-core:jar:5.7.0-SNAPSHOT

Detected changes: 2.

Change 1

Name Element
Old method void spoon.support.visitor.SubInheritanceHierarchyResolver::forEachSubTypeInPackage(spoon.reflect.visitor.chain.CtConsumer)
New method <T extends spoon.reflect.declaration.CtType<?>> void spoon.support.visitor.SubInheritanceHierarchyResolver::forEachSubTypeInPackage(spoon.reflect.visitor.chain.CtConsumer)
Code java.generics.formalTypeParameterChanged
Description The formal type parameter changed from 'T extends spoon.reflect.declaration.CtTypeInformation' to 'T extends spoon.reflect.declaration.CtType<?>'.
Breaking source: breaking,

Change 2

Name Element
Old parameter void spoon.support.visitor.SubInheritanceHierarchyResolver::forEachSubTypeInPackage(===spoon.reflect.visitor.chain.CtConsumer===)
New parameter <T extends spoon.reflect.declaration.CtType<?>> void spoon.support.visitor.SubInheritanceHierarchyResolver::forEachSubTypeInPackage(===spoon.reflect.visitor.chain.CtConsumer===)
Code java.method.parameterTypeParameterChanged
Description The type parameter of method parameter changed. The original type was 'spoon.reflect.visitor.chain.CtConsumer'while the new type is 'spoon.reflect.visitor.chain.CtConsumer<T extends spoon.reflect.declaration.CtType<?>>'.
Breaking source: potentially_breaking,

@pvojtechovsky pvojtechovsky force-pushed the ParameterRemoveRefactoring branch 3 times, most recently from 82f4d75 to a36d248 Compare May 29, 2017 18:24
@pvojtechovsky pvojtechovsky changed the title WiP: feature Parameter remove refactoring review: feature Parameter remove refactoring May 29, 2017
CtMethod<?> methodTypeA_method1 = typeA.getMethodsByName("method1").get(0);
CtParameterRemoveRefactoring refactor = new CtParameterRemoveRefactoring().setTarget(methodTypeA_method1.getParameters().get(0));
refactor.setTarget(methodTypeA_method1.getParameters().get(0));
Copy link
Collaborator

Choose a reason for hiding this comment

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

already done at previous line

@@ -29,36 +29,46 @@
import java.io.FileNotFoundException;

public final class ModelUtils {

public static Launcher lastLauncher;
Copy link
Collaborator

Choose a reason for hiding this comment

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

stateful :-( do you really need this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I need it because I do not know how to do it better...
Have a look at testCtParameterRemoveRefactoring

	@Test
	public void testCtParameterRemoveRefactoring() {
		String testPackagePath = "spoon/test/refactoring/parameter/testclasses";
		Factory factory = ModelUtils.build(new File("./src/test/java/"+testPackagePath));
		Launcher launcher = ModelUtils.getLastLauncher();
		
		CtType<?> typeA = factory.Class().get(TypeA.class);
...
//I need launcher to print model to java. Is there nicer way?
		launcher.setSourceOutputDirectory(new File("./target/spooned/"));
		launcher.getModelBuilder().generateProcessedSourceFiles(OutputType.CLASSES);
		ModelUtils.canBeBuilt("./target/spooned/"+testPackagePath, 8);

@monperrus
Copy link
Collaborator

Simply use a launcher directly

       @Test
	public void testCtParameterRemoveRefactoring() {
                Laucher spoon = new Launcher();
		spoon.addInputRessource("spoon/test/refactoring/parameter/testclasses");
		CtType<?> typeA = spoon.getFactory().factory.Class().get(TypeA.class);
...
//I need launcher to print model to java. Is there nicer way?
		launcher.setSourceOutputDirectory(new File("./target/spooned/"));
		launcher.getModelBuilder().generateProcessedSourceFiles(OutputType.CLASSES);
		ModelUtils.canBeBuilt("./target/spooned/"+testPackagePath, 8);

@pvojtechovsky
Copy link
Collaborator Author

The tests was modified to be compatible with not modified ModelUtils. The change in ModelUtils was rolled back.

return false;
}
return getTargetInvocations().contains(invocation);
/* CtExecutableReference<?> execRef = invocation.getExecutable();
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove commented code?

@monperrus monperrus merged commit b6e524c into INRIA:master May 30, 2017
@pvojtechovsky pvojtechovsky deleted the ParameterRemoveRefactoring branch May 30, 2017 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants