Skip to content

Commit

Permalink
feat: Add processor fixing sonar rule S4065_ThreadLocalWithInitial (#706
Browse files Browse the repository at this point in the history
)

* Add processor fixing sonar rule S4065_ThreadLocalWithInitial

* apply requested changes

* fix formatting

* apply changes

* rename files and remove check

* fix method name
  • Loading branch information
MartinWitt authored Jan 31, 2022
1 parent eaf0d03 commit c11c1bb
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 1 deletion.
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");
}
}

0 comments on commit c11c1bb

Please sign in to comment.