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

feat: Add processor fixing sonar rule S4065_ThreadLocalWithInitial #706

Merged
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
3 changes: 2 additions & 1 deletion src/main/java/sorald/Processors.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,14 @@ private Processors() {}
put("S3032", GetClassLoaderProcessor.class);
put("S3067", SynchronizationOnGetClassProcessor.class);
put("S3984", UnusedThrowableProcessor.class);
put("S4065", ThreadLocalWithInitial.class);
put("S4973", CompareStringsBoxedTypesWithEqualsProcessor.class);
}
};

// GENERATED FIELD
public static final String RULE_DESCRIPTIONS =
"S1068: Unused \"private\" fields should be removed\nS1118: Utility classes should not have public constructors\n\t(incomplete: Only handles implicit public constructor)\nS1132: Strings literals should be placed on the left side when checking for equality\nS1155: Collection.isEmpty() should be used to test for emptiness\nS1217: \"Thread.run()\" should not be called directly\nS1444: \"public static\" fields should be constant\n\t(incomplete: does not fix variable naming)\nS1481: Unused local variables should be removed\nS1596: \"Collections.EMPTY_LIST\", \"EMPTY_MAP\", and \"EMPTY_SET\" should not be used\nS1656: Variables should not be self-assigned\nS1854: Unused assignments should be removed\nS1860: Synchronization should not be based on Strings or boxed primitives\nS1948: Fields in a \"Serializable\" class should either be transient or serializable\nS2057: Every class implementing Serializable should declare a static final serialVersionUID.\n\t(incomplete: This processor does not address the case where the class already has a serialVersionUID with a non long type.)\nS2095: Resources should be closed\nS2097: \"equals(Object obj)\" should test argument type\nS2111: \"BigDecimal(double)\" should not be used\nS2116: \"hashCode\" and \"toString\" should not be called on array instances\nS2142: \"InterruptedException\" should not be ignored\nS2164: Math should not be performed on floats\nS2167: \"compareTo\" should not return \"Integer.MIN_VALUE\"\nS2184: Math operands should be cast before assignment\nS2204: \".equals()\" should not be used to test the values of \"Atomic\" classes\nS2225: \"toString()\" and \"clone()\" methods should not return null\n\t(incomplete: does not fix null returning clone())\nS2272: \"Iterator.next()\" methods should throw \"NoSuchElementException\"\nS2755: XML parsers should not be vulnerable to XXE attacks\n\t(incomplete: This processor is a WIP and currently supports a subset of rule 2755. See Sorald\'s documentation for details.)\nS3032: JEE applications should not \"getClassLoader\"\nS3067: \"getClass\" should not be used for synchronization\nS3984: Exception should not be created without being thrown\nS4973: Strings and Boxed types should be compared using \"equals()\"";
"S1068: Unused \"private\" fields should be removed\nS1118: Utility classes should not have public constructors\n\t(incomplete: Only handles implicit public constructor)\nS1132: Strings literals should be placed on the left side when checking for equality\nS1155: Collection.isEmpty() should be used to test for emptiness\nS1217: \"Thread.run()\" should not be called directly\nS1444: \"public static\" fields should be constant\n\t(incomplete: does not fix variable naming)\nS1481: Unused local variables should be removed\nS1596: \"Collections.EMPTY_LIST\", \"EMPTY_MAP\", and \"EMPTY_SET\" should not be used\nS1656: Variables should not be self-assigned\nS1854: Unused assignments should be removed\nS1860: Synchronization should not be based on Strings or boxed primitives\nS1948: Fields in a \"Serializable\" class should either be transient or serializable\nS2057: Every class implementing Serializable should declare a static final serialVersionUID.\n\t(incomplete: This processor does not address the case where the class already has a serialVersionUID with a non long type.)\nS2095: Resources should be closed\nS2097: \"equals(Object obj)\" should test argument type\nS2111: \"BigDecimal(double)\" should not be used\nS2116: \"hashCode\" and \"toString\" should not be called on array instances\nS2142: \"InterruptedException\" should not be ignored\nS2164: Math should not be performed on floats\nS2167: \"compareTo\" should not return \"Integer.MIN_VALUE\"\nS2184: Math operands should be cast before assignment\nS2204: \".equals()\" should not be used to test the values of \"Atomic\" classes\nS2225: \"toString()\" and \"clone()\" methods should not return null\n\t(incomplete: does not fix null returning clone())\nS2272: \"Iterator.next()\" methods should throw \"NoSuchElementException\"\nS2755: XML parsers should not be vulnerable to XXE attacks\n\t(incomplete: This processor is a WIP and currently supports a subset of rule 2755. See Sorald\'s documentation for details.)\nS3032: JEE applications should not \"getClassLoader\"\nS3067: \"getClass\" should not be used for synchronization\nS3984: Exception should not be created without being thrown\nS4065: \"ThreadLocal.withInitial\" should be preferred\nS4973: Strings and Boxed types should be compared using \"equals()\"";

public static Class<? extends SoraldAbstractProcessor<?>> getProcessor(String key) {
return RULE_KEY_TO_PROCESSOR.get(key);
Expand Down
71 changes: 71 additions & 0 deletions src/main/java/sorald/processor/ThreadLocalWithInitial.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package sorald.processor;

import java.util.List;
import java.util.function.Supplier;
import sorald.annotations.ProcessorAnnotation;
import spoon.reflect.code.CtExpression;
import spoon.reflect.code.CtInvocation;
import spoon.reflect.code.CtLambda;
import spoon.reflect.code.CtNewClass;
import spoon.reflect.code.CtReturn;
import spoon.reflect.code.CtStatement;
import spoon.reflect.code.CtTypeAccess;
import spoon.reflect.declaration.CtClass;
import spoon.reflect.reference.CtExecutableReference;
import spoon.reflect.reference.CtTypeReference;

@ProcessorAnnotation(key = "S4065", description = "\"ThreadLocal.withInitial\" should be preferred")
public class ThreadLocalWithInitial extends SoraldAbstractProcessor<CtNewClass<?>> {

@Override
protected void repairInternal(CtNewClass<?> newClass) {
CtClass<?> innerClass = newClass.getAnonymousClass();
CtExecutableReference<?> initialValueMethod = findInitialValueMethod(innerClass);
CtLambda<?> lambda = createSupplier(initialValueMethod);
CtInvocation<?> invocation = createInitialMethod(newClass, lambda);
invocation.setArguments(List.of(lambda));
newClass.replace(invocation);
}

private CtInvocation<?> createInitialMethod(CtNewClass<?> threadLocal, CtLambda<?> lambda) {
CtTypeAccess<Object> target = getFactory().createTypeAccess(createThreadLocalRef());
CtExecutableReference<?> executableReference =
getFactory()
.Executable()
.createReference(
threadLocal.getType(),
true,
threadLocal.getType(),
"withInitial",
List.of(lambda.getType()));
return getFactory().createInvocation(target, executableReference);
}

private CtTypeReference<Object> createThreadLocalRef() {
return getFactory().createCtTypeReference(ThreadLocal.class);
}

private CtLambda<?> createSupplier(CtExecutableReference<?> initialValueMethod) {
CtLambda<?> lambda = getFactory().createLambda();
if (initialValueMethod.getDeclaration().getBody().getStatements().size() == 1) {
lambda.setExpression(
getReturnStatement(
initialValueMethod.getDeclaration().getBody().getStatement(0)));
} else {
lambda.setBody(initialValueMethod.getDeclaration().getBody());
}
lambda.setType(getFactory().createCtTypeReference(Supplier.class));
return lambda;
}

private <T> CtExpression<T> getReturnStatement(CtStatement statement) {
return ((CtReturn<T>) statement).getReturnedExpression();
}

private CtExecutableReference<?> findInitialValueMethod(CtClass<?> innerClass) {
return innerClass.getDeclaredExecutables().stream()
.filter(v -> v.getSimpleName().equals("initialValue"))
.findFirst()
.get();
}
}
10 changes: 10 additions & 0 deletions src/main/java/sorald/processor/ThreadLocalWithInitial.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Sorald fixes the violations of this rule by replacing an anonymous `ThreadLocal` class overriding `initalValue` with an invocation of `ThreadLocal.withInitial(Supplier)`.
Example:
```diff
- ThreadLocal<String> myThreadLocal = new ThreadLocal<String>() {
- @Override
- protected String initialValue() {
- return "Hello";
- }
+ Threadlocal<String> myThreadLocal = ThreadLocal.withInitial(() -> "Hello");
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

public class ThreadLocalInitialWithExactlyOneReturnStatement {

public void bar() {
ThreadLocal<String> threadLocal = new ThreadLocal<String>() { // Noncompliant
@Override
protected String initialValue() {
return "hello";
}
};
threadLocal.set("42");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

public class ThreadLocalInitialWithExactlyOneReturnStatement {

public void bar() {
ThreadLocal<String> threadLocal = ThreadLocal.withInitial(() -> "hello");
threadLocal.set("42");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@

public class ThreadLocalInitialWithMultipleStatements {

public void bar() {
ThreadLocal<String> threadLocal = new ThreadLocal<String>() { // Noncompliant
@Override
protected String initialValue() {
String s = "hello";
return s+"42";
}
};
threadLocal.set("42");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

public class ThreadLocalInitialWithMultipleStatements {

public void bar() {
ThreadLocal<String> threadLocal = ThreadLocal.withInitial(() -> {String s = "hello"; return s+"42";});
threadLocal.set("42");
}
}