Skip to content

Commit

Permalink
feat: convert UnnecessaryImplements, UnnecessaryToString and IndexOfR…
Browse files Browse the repository at this point in the history
…eplaceableByContains rules (#1120)
  • Loading branch information
MartinWitt authored Oct 1, 2023
1 parent 20e8577 commit a0fbf40
Show file tree
Hide file tree
Showing 6 changed files with 316 additions and 3 deletions.
1 change: 1 addition & 0 deletions code-transformation/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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:+"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<AnalyzerResult, AbstractRefactoring> refactoring;

Expand Down
Original file line number Diff line number Diff line change
@@ -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<InvocationMatcher> matcher = new ThreadLocal<InvocationMatcher>();

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<BadSmell> getHandledBadSmells() {
return List.of(BAD_SMELL);
}

@Override
public List<SpoonAnalyzerResult> analyze(String sourceRoot, CtType<?> ctType) {
List<SpoonAnalyzerResult> results = new ArrayList<>();
List<ResultRecord> 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<ResultRecord> getIndexMinusOnePairs(CtElement clazz) {
List<ResultRecord> resultRecords = new ArrayList<>();
List<CtBinaryOperator<?>> 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;
}
}
Original file line number Diff line number Diff line change
@@ -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<BadSmell> getHandledBadSmells() {
return List.of(BAD_SMELL);
}

@Override
public List<SpoonAnalyzerResult> analyze(String sourceRoot, CtType<?> ctType) {
List<SpoonAnalyzerResult> 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<CtTypeReference<?>> getUnnecessaryImplements(CtTypeInformation ctType) {
Set<CtTypeReference<?>> result = new HashSet<>();
Set<CtTypeReference<?>> 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;
}
}
Original file line number Diff line number Diff line change
@@ -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<CtInvocation<?>> 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<BadSmell> getHandledBadSmells() {
return List.of(BAD_SMELL);
}

@Override
public List<SpoonAnalyzerResult> analyze(String sourceRoot, CtType<?> ctType) {
List<SpoonAnalyzerResult> results = new ArrayList<>();
List<CtInvocation<?>> 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<CtInvocation<?>> {

@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");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit a0fbf40

Please sign in to comment.