From a0fbf407f8bbee19dcd052fa1b59c239783e0c8a Mon Sep 17 00:00:00 2001 From: Martin Wittlinger Date: Sun, 1 Oct 2023 22:58:34 +0200 Subject: [PATCH] feat: convert UnnecessaryImplements, UnnecessaryToString and IndexOfReplaceableByContains rules (#1120) --- code-transformation/build.gradle | 1 + .../analyzer/spoon/SpoonRules.java | 11 +- .../rules/IndexOfReplaceableByContains.java | 130 ++++++++++++++++++ .../spoon/rules/UnnecessaryImplements.java | 89 ++++++++++++ .../spoon/rules/UnnecessaryToString.java | 86 ++++++++++++ .../code_solver/transformations/BadSmell.java | 2 +- 6 files changed, 316 insertions(+), 3 deletions(-) create mode 100644 code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/rules/IndexOfReplaceableByContains.java create mode 100644 code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/rules/UnnecessaryImplements.java create mode 100644 code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/rules/UnnecessaryToString.java diff --git a/code-transformation/build.gradle b/code-transformation/build.gradle index 7fab43fb6..5b0333ed0 100644 --- a/code-transformation/build.gradle +++ b/code-transformation/build.gradle @@ -17,6 +17,7 @@ jar { dependencies { implementation group: 'org.eclipse.jgit', name: 'org.eclipse.jgit', version: '6.7.0.202309050840-r' implementation group: 'org.eclipse.jgit', name: 'org.eclipse.jgit.ssh.apache', version: '6.7.0.202309050840-r' + implementation project(path: ':matcher') testImplementation "com.google.truth:truth:1.1.5" implementation "com.github.docker-java:docker-java-core:+" implementation "com.github.docker-java:docker-java-transport-httpclient5:+" diff --git a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/SpoonRules.java b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/SpoonRules.java index 512920f4d..8b59e739b 100644 --- a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/SpoonRules.java +++ b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/SpoonRules.java @@ -7,12 +7,19 @@ import xyz.keksdose.spoon.code_solver.analyzer.AbstractRefactoring; import xyz.keksdose.spoon.code_solver.analyzer.AnalyzerRule; import xyz.keksdose.spoon.code_solver.analyzer.spoon.rules.AccessStaticViaInstance; +import xyz.keksdose.spoon.code_solver.analyzer.spoon.rules.IndexOfReplaceableByContains; +import xyz.keksdose.spoon.code_solver.analyzer.spoon.rules.UnnecessaryImplements; +import xyz.keksdose.spoon.code_solver.analyzer.spoon.rules.UnnecessaryToString; import xyz.keksdose.spoon.code_solver.transformations.BadSmell; /** Enum for all spoon based rules. */ public enum SpoonRules implements AnalyzerRule { - Access_Static_Via_Instance("AccessStaticViaInstance", AccessStaticViaInstance::new); - + ACCESS_STATIC_VIA_INSTANCE("AccessStaticViaInstance", AccessStaticViaInstance::new), + UNNECESSARY_TO_STRING("UnnecessaryToString", UnnecessaryToString::new), + UNNECESSARY_IMPLEMENTS("UnnecessaryImplements", UnnecessaryImplements::new), + INDEX_OF_REPLACEABLE_BY_CONTAINS( + "IndexOfReplaceableByContains", IndexOfReplaceableByContains::new), + ; private final RuleId ruleId; private final Function refactoring; diff --git a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/rules/IndexOfReplaceableByContains.java b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/rules/IndexOfReplaceableByContains.java new file mode 100644 index 000000000..782b3d467 --- /dev/null +++ b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/rules/IndexOfReplaceableByContains.java @@ -0,0 +1,130 @@ +package xyz.keksdose.spoon.code_solver.analyzer.spoon.rules; + +import io.github.martinwitt.laughing_train.domain.entity.AnalyzerResult; +import io.github.martinwitt.laughing_train.spoonutils.InvocationMatcher; +import java.util.ArrayList; +import java.util.List; +import spoon.reflect.code.*; +import spoon.reflect.declaration.CtElement; +import spoon.reflect.declaration.CtType; +import spoon.reflect.factory.Factory; +import spoon.reflect.reference.CtExecutableReference; +import spoon.reflect.visitor.filter.TypeFilter; +import xyz.keksdose.spoon.code_solver.analyzer.spoon.SpoonAnalyzerResult; +import xyz.keksdose.spoon.code_solver.analyzer.spoon.SpoonRefactoring; +import xyz.keksdose.spoon.code_solver.history.Change; +import xyz.keksdose.spoon.code_solver.history.ChangeListener; +import xyz.keksdose.spoon.code_solver.history.MarkdownString; +import xyz.keksdose.spoon.code_solver.transformations.BadSmell; + +public class IndexOfReplaceableByContains extends SpoonRefactoring { + + private final ThreadLocal matcher = new ThreadLocal(); + + private static final BadSmell BAD_SMELL = + new BadSmell( + MarkdownString.fromMarkdown("IndexOfReplaceableByContains"), + MarkdownString.fromMarkdown( + "The `indexOf` method returns -1 if the substring is not found. This can be replaced by the contains method.")); + + /** + * Creates a new refactoring with a given result. + * + * @param result the result of an analysis run. + */ + public IndexOfReplaceableByContains(AnalyzerResult result) { + super(result); + matcher.set(new InvocationMatcher("java.lang.String", "indexOf")); + } + + @SuppressWarnings({"rawtypes", "unchecked"}) + @Override + public void refactor(ChangeListener listener, CtType type) { + Factory factory = type.getFactory(); + for (ResultRecord indexMinusOnePair : getIndexMinusOnePairs(type)) { + if (toPosition(indexMinusOnePair.indexOfCall().getPosition()).equals(result.position())) { + CtExpression indexOfCall = indexMinusOnePair.indexOfCall(); + CtBinaryOperator parent = indexOfCall.getParent(CtBinaryOperator.class); + CtBinaryOperator oldParent = parent.clone(); + if (indexOfCall instanceof CtInvocation invocation) { + CtExecutableReference containsCalls = getContainsCalls(factory); + invocation.setExecutable(containsCalls); + parent.replace( + factory.createUnaryOperator().setKind(UnaryOperatorKind.NOT).setOperand(invocation)); + Change change = + new Change( + BAD_SMELL, + MarkdownString.fromMarkdown( + "Converted `%s` to index of invocation `%s`" + .formatted(oldParent, invocation)), + type, + result); + listener.setChanged(type, change); + } + } + } + } + + private static CtExecutableReference getContainsCalls(Factory factory) { + return factory + .createExecutableReference() + .setDeclaringType(factory.Type().createReference("java.lang.String")) + .setSimpleName("contains"); + } + + @Override + public List getHandledBadSmells() { + return List.of(BAD_SMELL); + } + + @Override + public List analyze(String sourceRoot, CtType ctType) { + List results = new ArrayList<>(); + List indexMinusOnePairs = getIndexMinusOnePairs(ctType); + for (ResultRecord indexMinusOnePair : indexMinusOnePairs) { + results.add( + SpoonAnalyzerResult.createResult( + BAD_SMELL.getName().asText(), + ctType, + "The indexOf call %s can be replaced by contains." + .formatted(indexMinusOnePair.indexOfCall), + "The indexOf call `%s` can be replaced by contains." + .formatted(indexMinusOnePair.indexOfCall), + indexMinusOnePair.indexOfCall, + sourceRoot)); + } + return results; + } + + private boolean isIndexOfCall(CtExpression expression) { + if (expression instanceof CtInvocation invocation) { + return matcher.get().matches(invocation); + } + return false; + } + + private boolean isMinusOne(CtExpression expression) { + if (expression instanceof CtUnaryOperator operator + && operator.getKind().equals(UnaryOperatorKind.NEG)) { + return operator.getOperand() instanceof CtLiteral literal && literal.getValue().equals(1); + } + return false; + } + + record ResultRecord(CtExpression indexOfCall, CtExpression minusOne) {} + + private List getIndexMinusOnePairs(CtElement clazz) { + List resultRecords = new ArrayList<>(); + List> list = clazz.getElements(new TypeFilter<>(CtBinaryOperator.class)); + for (CtBinaryOperator ctBinaryOperator : list) { + CtExpression rightHandOperand = ctBinaryOperator.getRightHandOperand(); + CtExpression leftHandOperand = ctBinaryOperator.getLeftHandOperand(); + if (isIndexOfCall(leftHandOperand) && isMinusOne(rightHandOperand)) { + resultRecords.add(new ResultRecord(leftHandOperand, rightHandOperand)); + } else if (isIndexOfCall(rightHandOperand) && isMinusOne(leftHandOperand)) { + resultRecords.add(new ResultRecord(rightHandOperand, leftHandOperand)); + } + } + return resultRecords; + } +} diff --git a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/rules/UnnecessaryImplements.java b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/rules/UnnecessaryImplements.java new file mode 100644 index 000000000..b2f08135b --- /dev/null +++ b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/rules/UnnecessaryImplements.java @@ -0,0 +1,89 @@ +package xyz.keksdose.spoon.code_solver.analyzer.spoon.rules; + +import io.github.martinwitt.laughing_train.domain.entity.AnalyzerResult; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import spoon.reflect.declaration.CtType; +import spoon.reflect.declaration.CtTypeInformation; +import spoon.reflect.reference.CtTypeReference; +import xyz.keksdose.spoon.code_solver.analyzer.spoon.SpoonAnalyzerResult; +import xyz.keksdose.spoon.code_solver.analyzer.spoon.SpoonRefactoring; +import xyz.keksdose.spoon.code_solver.history.Change; +import xyz.keksdose.spoon.code_solver.history.ChangeListener; +import xyz.keksdose.spoon.code_solver.history.MarkdownString; +import xyz.keksdose.spoon.code_solver.transformations.BadSmell; + +public class UnnecessaryImplements extends SpoonRefactoring { + + private static final BadSmell BAD_SMELL = + new BadSmell( + MarkdownString.fromRaw("UnnecessaryImplements"), + MarkdownString.fromRaw( + "This class has 1 or more interfaces which are already implemented.")); + + /** + * Creates a new refactoring with a given result. + * + * @param result the result of an analysis run. + */ + public UnnecessaryImplements(AnalyzerResult result) { + super(result); + } + + @Override + public void refactor(ChangeListener listener, CtType type) { + // TODO: Check if the equals real works here + for (CtTypeReference unnecessaryImplement : getUnnecessaryImplements(type)) { + if (toPosition(unnecessaryImplement.getPosition()).equals(result.position())) { + type.removeSuperInterface(unnecessaryImplement); + listener.setChanged( + type, + new Change( + BAD_SMELL, + MarkdownString.fromMarkdown( + "Deleted unnecessary implement `%s`".formatted(unnecessaryImplement)), + type, + result)); + } + } + } + + @Override + public List getHandledBadSmells() { + return List.of(BAD_SMELL); + } + + @Override + public List analyze(String sourceRoot, CtType ctType) { + List results = new ArrayList<>(); + for (CtTypeReference ctTypeReference : getUnnecessaryImplements(ctType)) { + results.add( + SpoonAnalyzerResult.createResult( + BAD_SMELL.getName().asText(), + ctType, + "The implement declaration %s is unnecessary.".formatted(ctTypeReference), + "The implement declaration `%s` is unnecessary.".formatted(ctTypeReference), + ctTypeReference, + sourceRoot)); + } + return results; + } + + private Set> getUnnecessaryImplements(CtTypeInformation ctType) { + Set> result = new HashSet<>(); + Set> superInterfaces = ctType.getSuperInterfaces(); + for (CtTypeReference ctTypeReference : superInterfaces) { + for (CtTypeReference needed : superInterfaces) { + if (ctTypeReference.equals(needed)) { + continue; + } + if (ctTypeReference.isSubtypeOf(needed)) { + result.add(ctTypeReference); + } + } + } + return result; + } +} diff --git a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/rules/UnnecessaryToString.java b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/rules/UnnecessaryToString.java new file mode 100644 index 000000000..28e217d9a --- /dev/null +++ b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/analyzer/spoon/rules/UnnecessaryToString.java @@ -0,0 +1,86 @@ +package xyz.keksdose.spoon.code_solver.analyzer.spoon.rules; + +import io.github.martinwitt.laughing_train.domain.entity.AnalyzerResult; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import spoon.reflect.code.CtInvocation; +import spoon.reflect.declaration.CtType; +import spoon.reflect.visitor.Filter; +import xyz.keksdose.spoon.code_solver.analyzer.spoon.SpoonAnalyzerResult; +import xyz.keksdose.spoon.code_solver.analyzer.spoon.SpoonRefactoring; +import xyz.keksdose.spoon.code_solver.history.Change; +import xyz.keksdose.spoon.code_solver.history.ChangeListener; +import xyz.keksdose.spoon.code_solver.history.MarkdownString; +import xyz.keksdose.spoon.code_solver.transformations.BadSmell; + +public class UnnecessaryToString extends SpoonRefactoring { + + private static final BadSmell BAD_SMELL = + new BadSmell( + MarkdownString.fromRaw("UnnecessaryToString"), + MarkdownString.fromRaw("Unnecessary toString() call on String object.")); + + /** + * Creates a new refactoring with a given result. + * + * @param result the result of an analysis run. + */ + public UnnecessaryToString(AnalyzerResult result) { + super(result); + } + + @Override + public void refactor(ChangeListener listener, CtType type) { + if (!isSameType(type, Path.of(result.filePath()))) { + return; + } + List> elements = type.getElements(new UnnecessaryToStringFilter()); + for (CtInvocation invocation : elements) { + if (toPosition(invocation.getPosition()).equals(result.position())) { + invocation.delete(); + listener.setChanged( + type, + new Change( + BAD_SMELL, + MarkdownString.fromMarkdown( + "Deleted unnecessary toString call `%s`".formatted(invocation)), + type, + result)); + } + } + } + + @Override + public List getHandledBadSmells() { + return List.of(BAD_SMELL); + } + + @Override + public List analyze(String sourceRoot, CtType ctType) { + List results = new ArrayList<>(); + List> elements = ctType.getElements(new UnnecessaryToStringFilter()); + for (CtInvocation invocation : elements) { + results.add( + SpoonAnalyzerResult.createResult( + BAD_SMELL.getName().asText(), + ctType, + "The toString call %s is unnecessary.".formatted(invocation), + "The toString call `%s` is unnecessary.".formatted(invocation), + invocation, + sourceRoot)); + } + return results; + } + + private static final class UnnecessaryToStringFilter implements Filter> { + + @Override + public boolean matches(CtInvocation element) { + return element.getTarget() != null + && element.getTarget().getType() != null + && element.getTarget().getType().getSimpleName().equals("String") + && element.getExecutable().getSimpleName().equals("toString"); + } + } +} diff --git a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/transformations/BadSmell.java b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/transformations/BadSmell.java index 8b91b2c61..26b83180d 100644 --- a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/transformations/BadSmell.java +++ b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/transformations/BadSmell.java @@ -21,7 +21,7 @@ public BadSmell() { this.links = List.of(); } - public BadSmell(MarkdownString description, MarkdownString name) { + public BadSmell(MarkdownString name, MarkdownString description) { this.description = description; this.name = name; this.links = List.of();