From 28c9d57166331c59fcde302a361754cff2634f45 Mon Sep 17 00:00:00 2001 From: Christoph Knoedlseder Date: Fri, 4 Oct 2024 14:53:53 +0200 Subject: [PATCH] add swingutilities perfume --- .../perfume/ThreadSafeSwingDetector.java | 85 +++++++++++++++++++ .../data/perfumes/Thread_safe_swing.json | 9 ++ ...tector.java => AssertAllDetectorTest.java} | 15 ++-- .../ParameterizedTestDetectorTest.java | 16 ++-- .../ThreadSafeSwingDetectorTest.java | 56 ++++++++++++ .../swing/InvokeLaterInvokeAndWait.java | 24 ++++++ 6 files changed, 189 insertions(+), 16 deletions(-) create mode 100644 src/main/java/de/jsilbereisen/perfumator/engine/detector/perfume/ThreadSafeSwingDetector.java create mode 100644 src/main/resources/de/jsilbereisen/perfumator/data/perfumes/Thread_safe_swing.json rename src/test/java/detectors/{AssertAllDetector.java => AssertAllDetectorTest.java} (75%) create mode 100644 src/test/java/detectors/ThreadSafeSwingDetectorTest.java create mode 100644 src/test/resources/detectors/swing/InvokeLaterInvokeAndWait.java diff --git a/src/main/java/de/jsilbereisen/perfumator/engine/detector/perfume/ThreadSafeSwingDetector.java b/src/main/java/de/jsilbereisen/perfumator/engine/detector/perfume/ThreadSafeSwingDetector.java new file mode 100644 index 0000000..0f6037c --- /dev/null +++ b/src/main/java/de/jsilbereisen/perfumator/engine/detector/perfume/ThreadSafeSwingDetector.java @@ -0,0 +1,85 @@ +package de.jsilbereisen.perfumator.engine.detector.perfume; + +import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.ImportDeclaration; +import com.github.javaparser.ast.expr.MethodCallExpr; +import com.github.javaparser.symbolsolver.javaparsermodel.JavaParserFacade; +import de.jsilbereisen.perfumator.engine.detector.Detector; +import de.jsilbereisen.perfumator.engine.visitor.MethodCallByNameVisitor; +import de.jsilbereisen.perfumator.model.DetectedInstance; +import de.jsilbereisen.perfumator.model.perfume.Perfume; +import lombok.EqualsAndHashCode; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.*; + +@EqualsAndHashCode +public class ThreadSafeSwingDetector implements Detector { + + private Perfume perfume; + + private JavaParserFacade analysisContext; + + private final static String SWING_UTILITIES = "SwingUtilities"; + private final static String INVOKE_LATER = "invokeLater"; + private final static String INVOKE_AND_WAIT = "invokeAndWait"; + private final static Set RELEVANT_METHOD_NAMES = Set.of(INVOKE_LATER, INVOKE_AND_WAIT); + private final static Map RELEVANT_IMPORTS = + Map.of(INVOKE_LATER, "javax.swing.SwingUtilities.invokeLater", + INVOKE_AND_WAIT, "javax.swing.SwingUtilities.invokeAndWait"); + + @Override + public @NotNull List> detect(@NotNull CompilationUnit astRoot) { + List> detectedInstances = new ArrayList<>(); + Set staticImports = getStaticImports(astRoot); + List methodCalls = getInvokeLaterInvokeAndWaitMethodCalls(astRoot, staticImports); + methodCalls.forEach(callExpr -> detectedInstances.add(DetectedInstance.from(callExpr, perfume, astRoot))); + return detectedInstances; + } + + @Override + public void setConcreteDetectable(@NotNull Perfume concreteDetectable) { + this.perfume = concreteDetectable; + } + + @Override + public void setAnalysisContext(@Nullable JavaParserFacade analysisContext) { + this.analysisContext = analysisContext; + } + + private Set getStaticImports(@NotNull CompilationUnit astRoot) { + Set importedAnnotations = new HashSet<>(); + + for (ImportDeclaration importDeclaration : astRoot.getImports()) { + for (Map.Entry annotationToImport : RELEVANT_IMPORTS.entrySet()) { + if (importDeclaration.getNameAsString().equals(annotationToImport.getValue())) { + importedAnnotations.add(annotationToImport.getKey()); + } + } + } + return importedAnnotations; + } + + private List getInvokeLaterInvokeAndWaitMethodCalls(@NotNull CompilationUnit astRoot, + Set imports) { + MethodCallByNameVisitor methodCallByNameVisitor = new MethodCallByNameVisitor(); + astRoot.accept(methodCallByNameVisitor, null); + List relevantMethodCallExpressions = new ArrayList<>(); + for (MethodCallExpr methodCallExpr : methodCallByNameVisitor.getMethodCalls()) { + if (RELEVANT_METHOD_NAMES.contains(methodCallExpr.getNameAsString())) { + if (isPartOfSwingUtilities(methodCallExpr)) { + relevantMethodCallExpressions.add(methodCallExpr); + } else if (imports.contains(methodCallExpr.getNameAsString())) { + relevantMethodCallExpressions.add(methodCallExpr); + } + } + } + return relevantMethodCallExpressions; + } + + private boolean isPartOfSwingUtilities(MethodCallExpr methodCallExpr) { + var scope = methodCallExpr.getScope(); + return scope.map(expression -> expression.toString().equals(SWING_UTILITIES)).orElse(false); + } +} diff --git a/src/main/resources/de/jsilbereisen/perfumator/data/perfumes/Thread_safe_swing.json b/src/main/resources/de/jsilbereisen/perfumator/data/perfumes/Thread_safe_swing.json new file mode 100644 index 0000000..77b395b --- /dev/null +++ b/src/main/resources/de/jsilbereisen/perfumator/data/perfumes/Thread_safe_swing.json @@ -0,0 +1,9 @@ +{ + "name": "Thread safe Swing", + "description": "Java Swing application make use of the event dispatch thread. This thread must handle the initialization of and update operations on the GUI components, as well as event handling. Event handlers We as programmers are responsible that GUI components are created and updated in the event dispatch thread. We can achieve this by placing the related code inside a Runnable that we pass as an argument to either the \"SwingUtilities.invokeLater\" or the \"SwingUtilities.invokeAndWait\" method.", + "detectorClassSimpleName": "ThreadSafeSwingDetector", + "i18nBaseBundleName": "threadSafeSwing", + "sources": ["Programmierung II at Uni Passau", "https://docs.oracle.com/javase/tutorial/uiswing/concurrency/dispatch.html"], + "relatedPattern": "BUG", + "additionalInformation": null +} \ No newline at end of file diff --git a/src/test/java/detectors/AssertAllDetector.java b/src/test/java/detectors/AssertAllDetectorTest.java similarity index 75% rename from src/test/java/detectors/AssertAllDetector.java rename to src/test/java/detectors/AssertAllDetectorTest.java index bf82e36..910369b 100644 --- a/src/test/java/detectors/AssertAllDetector.java +++ b/src/test/java/detectors/AssertAllDetectorTest.java @@ -2,7 +2,6 @@ import com.github.javaparser.ast.CompilationUnit; import de.jsilbereisen.perfumator.engine.detector.Detector; -import de.jsilbereisen.perfumator.engine.detector.perfume.ParameterizedTestDetector; import de.jsilbereisen.perfumator.model.CodeRange; import de.jsilbereisen.perfumator.model.DetectedInstance; import de.jsilbereisen.perfumator.model.perfume.Perfume; @@ -15,9 +14,9 @@ import static org.assertj.core.api.Assertions.assertThat; -public class AssertAllDetector extends AbstractDetectorTest { - - private static final Path TEST_FILE = DEFAULT_DETECTOR_TEST_FILES_DIR.resolve("ParameterizedTests.java"); +class AssertAllDetectorTest extends AbstractDetectorTest { + + private static final Path TEST_FILE = DEFAULT_DETECTOR_TEST_FILES_DIR.resolve("AssertAllPerfume.java"); private static Perfume perfume; @@ -28,9 +27,9 @@ public class AssertAllDetector extends AbstractDetectorTest { @BeforeAll static void init() { perfume = new Perfume(); - perfume.setName("Parameterized Test"); + perfume.setName("Assert All"); - detector = new ParameterizedTestDetector(); + detector = new de.jsilbereisen.perfumator.engine.detector.perfume.AssertAllDetector(); detector.setConcreteDetectable(perfume); ast = parseAstForFile(TEST_FILE); @@ -45,7 +44,7 @@ void detect() { DetectedInstance detection = detections.get(0); assertThat(detection.getDetectable()).isEqualTo(perfume); - assertThat(detection.getTypeName()).isEqualTo("ParameterizedTests"); - assertThat(detection.getCodeRanges()).containsExactly(CodeRange.of(21, 5, 25, 5)); + assertThat(detection.getTypeName()).isEqualTo("AssertAllPerfume"); + assertThat(detection.getCodeRanges()).containsExactly(CodeRange.of(16, 9, 21, 9)); } } diff --git a/src/test/java/detectors/ParameterizedTestDetectorTest.java b/src/test/java/detectors/ParameterizedTestDetectorTest.java index e194d7a..ced340c 100644 --- a/src/test/java/detectors/ParameterizedTestDetectorTest.java +++ b/src/test/java/detectors/ParameterizedTestDetectorTest.java @@ -2,7 +2,7 @@ import com.github.javaparser.ast.CompilationUnit; import de.jsilbereisen.perfumator.engine.detector.Detector; -import de.jsilbereisen.perfumator.engine.detector.perfume.AssertAllDetector; +import de.jsilbereisen.perfumator.engine.detector.perfume.ParameterizedTestDetector; import de.jsilbereisen.perfumator.model.CodeRange; import de.jsilbereisen.perfumator.model.DetectedInstance; import de.jsilbereisen.perfumator.model.perfume.Perfume; @@ -15,9 +15,9 @@ import static org.assertj.core.api.Assertions.assertThat; -class ParameterizedTestDetectorTest extends AbstractDetectorTest { - - private static final Path TEST_FILE = DEFAULT_DETECTOR_TEST_FILES_DIR.resolve("AssertAllPerfume.java"); +public class ParameterizedTestDetectorTest extends AbstractDetectorTest { + + private static final Path TEST_FILE = DEFAULT_DETECTOR_TEST_FILES_DIR.resolve("ParameterizedTests.java"); private static Perfume perfume; @@ -28,9 +28,9 @@ class ParameterizedTestDetectorTest extends AbstractDetectorTest { @BeforeAll static void init() { perfume = new Perfume(); - perfume.setName("Assert All"); + perfume.setName("Parameterized Test"); - detector = new AssertAllDetector(); + detector = new ParameterizedTestDetector(); detector.setConcreteDetectable(perfume); ast = parseAstForFile(TEST_FILE); @@ -45,7 +45,7 @@ void detect() { DetectedInstance detection = detections.get(0); assertThat(detection.getDetectable()).isEqualTo(perfume); - assertThat(detection.getTypeName()).isEqualTo("AssertAllPerfume"); - assertThat(detection.getCodeRanges()).containsExactly(CodeRange.of(16, 9, 21, 9)); + assertThat(detection.getTypeName()).isEqualTo("ParameterizedTests"); + assertThat(detection.getCodeRanges()).containsExactly(CodeRange.of(22, 5, 26, 5)); } } diff --git a/src/test/java/detectors/ThreadSafeSwingDetectorTest.java b/src/test/java/detectors/ThreadSafeSwingDetectorTest.java new file mode 100644 index 0000000..d3c60a6 --- /dev/null +++ b/src/test/java/detectors/ThreadSafeSwingDetectorTest.java @@ -0,0 +1,56 @@ +package detectors; + +import com.github.javaparser.ast.CompilationUnit; +import de.jsilbereisen.perfumator.engine.detector.Detector; +import de.jsilbereisen.perfumator.engine.detector.perfume.ThreadSafeSwingDetector; +import de.jsilbereisen.perfumator.model.CodeRange; +import de.jsilbereisen.perfumator.model.DetectedInstance; +import de.jsilbereisen.perfumator.model.perfume.Perfume; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import test.AbstractDetectorTest; + +import java.nio.file.Path; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +public class ThreadSafeSwingDetectorTest extends AbstractDetectorTest { + + private static final Path TEST_FILE = + DEFAULT_DETECTOR_TEST_FILES_DIR.resolve("swing").resolve("InvokeLaterInvokeAndWait.java"); + + private static Perfume perfume; + + private static Detector detector; + + private static CompilationUnit ast; + + @BeforeAll + static void init() { + perfume = new Perfume(); + perfume.setName("Thread safe Swing"); + + detector = new ThreadSafeSwingDetector(); + detector.setConcreteDetectable(perfume); + + ast = parseAstForFile(TEST_FILE); + } + + @Test + void detect() { + List> detections = detector.detect(ast); + + assertThat(detections).hasSize(2); + + DetectedInstance detection = detections.get(0); + assertThat(detection.getDetectable()).isEqualTo(perfume); + assertThat(detection.getTypeName()).isEqualTo("InvokeLaterInvokeAndWait"); + assertThat(detection.getCodeRanges()).containsExactly(CodeRange.of(9, 9, 11, 10)); + + detection = detections.get(1); + assertThat(detection.getDetectable()).isEqualTo(perfume); + assertThat(detection.getTypeName()).isEqualTo("InvokeLaterInvokeAndWait"); + assertThat(detection.getCodeRanges()).containsExactly(CodeRange.of(13, 9, 15, 10)); + } +} diff --git a/src/test/resources/detectors/swing/InvokeLaterInvokeAndWait.java b/src/test/resources/detectors/swing/InvokeLaterInvokeAndWait.java new file mode 100644 index 0000000..7228113 --- /dev/null +++ b/src/test/resources/detectors/swing/InvokeLaterInvokeAndWait.java @@ -0,0 +1,24 @@ +package de.jsilbereisen.test; + +import javax.swing.SwingUtilities; +import static javax.swing.SwingUtilities.invokeAndWait; + +public class InvokeLaterInvokeAndWait { + + public static void main(String[] args) { + SwingUtilities.invokeLater(new Runnable() { + // perfume + }); + + invokeAndWait(() -> { + // perfume + }); + + // no perfume + invokeLater(); + } + + public static void invokeLater() { + // no perfume, not the library method we are looking for + } +} \ No newline at end of file