diff --git a/pom.xml b/pom.xml index 1dc2e42..95ea4ae 100644 --- a/pom.xml +++ b/pom.xml @@ -126,7 +126,7 @@ com.github.javaparser javaparser-core - 3.2.4 + 3.24.4 com.opencsv @@ -162,4 +162,4 @@ - \ No newline at end of file + diff --git a/src/main/java/testsmell/SmellFactory.java b/src/main/java/testsmell/SmellFactory.java new file mode 100644 index 0000000..b7f51fd --- /dev/null +++ b/src/main/java/testsmell/SmellFactory.java @@ -0,0 +1,8 @@ +package testsmell; + +import thresholds.Thresholds; + +@FunctionalInterface +public interface SmellFactory { + AbstractSmell createInstance(Thresholds thresholds); +} diff --git a/src/main/java/testsmell/TestSmellDetector.java b/src/main/java/testsmell/TestSmellDetector.java index 12665c8..851fa4b 100644 --- a/src/main/java/testsmell/TestSmellDetector.java +++ b/src/main/java/testsmell/TestSmellDetector.java @@ -1,6 +1,5 @@ package testsmell; -import com.github.javaparser.JavaParser; import com.github.javaparser.ast.CompilationUnit; import com.github.javaparser.ast.body.TypeDeclaration; import org.apache.commons.lang3.StringUtils; @@ -16,7 +15,7 @@ public class TestSmellDetector { - private List testSmells; + private List testSmells; private Thresholds thresholds; /** @@ -32,30 +31,30 @@ public TestSmellDetector(Thresholds thresholds) { private void initializeSmells() { testSmells = new ArrayList<>(); - testSmells.add(new AssertionRoulette(thresholds)); - testSmells.add(new ConditionalTestLogic(thresholds)); - testSmells.add(new ConstructorInitialization(thresholds)); - testSmells.add(new DefaultTest(thresholds)); - testSmells.add(new EmptyTest(thresholds)); - testSmells.add(new ExceptionCatchingThrowing(thresholds)); - testSmells.add(new GeneralFixture(thresholds)); - testSmells.add(new MysteryGuest(thresholds)); - testSmells.add(new PrintStatement(thresholds)); - testSmells.add(new RedundantAssertion(thresholds)); - testSmells.add(new SensitiveEquality(thresholds)); - testSmells.add(new VerboseTest(thresholds)); - testSmells.add(new SleepyTest(thresholds)); - testSmells.add(new EagerTest(thresholds)); - testSmells.add(new LazyTest(thresholds)); - testSmells.add(new DuplicateAssert(thresholds)); - testSmells.add(new UnknownTest(thresholds)); - testSmells.add(new IgnoredTest(thresholds)); - testSmells.add(new ResourceOptimism(thresholds)); - testSmells.add(new MagicNumberTest(thresholds)); - testSmells.add(new DependentTest(thresholds)); + testSmells.add(AssertionRoulette::new); + testSmells.add(ConditionalTestLogic::new); + testSmells.add(ConstructorInitialization::new); + testSmells.add(DefaultTest::new); + testSmells.add(EmptyTest::new); + testSmells.add(ExceptionCatchingThrowing::new); + testSmells.add(GeneralFixture::new); + testSmells.add(MysteryGuest::new); + testSmells.add(PrintStatement::new); + testSmells.add(RedundantAssertion::new); + testSmells.add(SensitiveEquality::new); + testSmells.add(VerboseTest::new); + testSmells.add(SleepyTest::new); + testSmells.add(EagerTest::new); + testSmells.add(LazyTest::new); + testSmells.add(DuplicateAssert::new); + testSmells.add(UnknownTest::new); + testSmells.add(IgnoredTest::new); + testSmells.add(ResourceOptimism::new); + testSmells.add(MagicNumberTest::new); + testSmells.add(DependentTest::new); } - public void setTestSmells(List testSmells) { + public void setTestSmells(List testSmells) { this.testSmells = testSmells; } @@ -65,7 +64,10 @@ public void setTestSmells(List testSmells) { * @return list of smell names */ public List getTestSmellNames() { - return testSmells.stream().map(AbstractSmell::getSmellName).collect(Collectors.toList()); + return testSmells.stream() + .map(factory -> factory.createInstance(thresholds)) + .map(AbstractSmell::getSmellName) + .collect(Collectors.toList()); } /** @@ -73,24 +75,24 @@ public List getTestSmellNames() { * test smells */ public TestFile detectSmells(TestFile testFile) throws IOException { - initializeSmells(); CompilationUnit testFileCompilationUnit = null; CompilationUnit productionFileCompilationUnit = null; FileInputStream testFileInputStream, productionFileInputStream; if (!StringUtils.isEmpty(testFile.getTestFilePath())) { testFileInputStream = new FileInputStream(testFile.getTestFilePath()); - testFileCompilationUnit = JavaParser.parse(testFileInputStream); + testFileCompilationUnit = Util.parseJava(testFileInputStream); TypeDeclaration typeDeclaration = testFileCompilationUnit.getTypes().get(0); testFile.setNumberOfTestMethods(typeDeclaration.getMethods().size()); } if (!StringUtils.isEmpty(testFile.getProductionFilePath())) { productionFileInputStream = new FileInputStream(testFile.getProductionFilePath()); - productionFileCompilationUnit = JavaParser.parse(productionFileInputStream); + productionFileCompilationUnit = Util.parseJava(productionFileInputStream); } - for (AbstractSmell smell : testSmells) { + for (SmellFactory smellFactory : testSmells) { + AbstractSmell smell = smellFactory.createInstance(thresholds); try { smell.runAnalysis(testFileCompilationUnit, productionFileCompilationUnit, testFile.getTestFileNameWithoutExtension(), diff --git a/src/main/java/testsmell/Util.java b/src/main/java/testsmell/Util.java index b1830c7..6e94455 100644 --- a/src/main/java/testsmell/Util.java +++ b/src/main/java/testsmell/Util.java @@ -1,7 +1,16 @@ package testsmell; +import java.io.ByteArrayInputStream; +import java.io.InputStream; + +import com.github.javaparser.JavaParser; +import com.github.javaparser.ParserConfiguration; +import com.github.javaparser.StaticJavaParser; import com.github.javaparser.ast.Modifier; import com.github.javaparser.ast.body.MethodDeclaration; +import com.github.javaparser.ParseProblemException; +import com.github.javaparser.ParseResult; +import com.github.javaparser.ast.CompilationUnit; public class Util { @@ -12,7 +21,7 @@ public static boolean isValidTestMethod(MethodDeclaration n) { //only analyze methods that either have a @test annotation (Junit 4) or the method name starts with 'test' if (n.getAnnotationByName("Test").isPresent() || n.getNameAsString().toLowerCase().startsWith("test")) { //must be a public method - if (n.getModifiers().contains(Modifier.PUBLIC)) { + if (n.getModifiers().stream().anyMatch(m -> m.getKeyword() == Modifier.Keyword.PUBLIC)) { valid = true; } } @@ -28,7 +37,7 @@ public static boolean isValidSetupMethod(MethodDeclaration n) { //only analyze methods that either have a @Before annotation (Junit 4) or the method name is 'setUp' if (n.getAnnotationByName("Before").isPresent() || n.getNameAsString().equals("setUp")) { //must be a public method - if (n.getModifiers().contains(Modifier.PUBLIC)) { + if (n.getModifiers().stream().anyMatch(m -> m.getKeyword() == Modifier.Keyword.PUBLIC)) { valid = true; } } @@ -54,4 +63,26 @@ public static boolean isNumber(String str) { } return false; } + + /** + * Replicates the old {@link JavaParser#parse(InputStream)} behavior without {@link StaticJavaParser}, + * since other projects may mess with the static configuration. + */ + public static CompilationUnit parseJava(InputStream code) { + JavaParser parser = new JavaParser(); + parser.getParserConfiguration() + .setLanguageLevel(ParserConfiguration.LanguageLevel.BLEEDING_EDGE) // use latest supported java version + .setAttributeComments(false); // exclude comments + + ParseResult parseResult = parser.parse(code); + if (parseResult.isSuccessful() && parseResult.getResult().isPresent()) { + return parseResult.getResult().get(); + } else { + throw new ParseProblemException(parseResult.getProblems()); + } + } + + public static CompilationUnit parseJava(String code) { + return parseJava(new ByteArrayInputStream(code.getBytes())); + } } diff --git a/src/main/java/testsmell/smell/ConditionalTestLogic.java b/src/main/java/testsmell/smell/ConditionalTestLogic.java index fd02180..63b5f95 100644 --- a/src/main/java/testsmell/smell/ConditionalTestLogic.java +++ b/src/main/java/testsmell/smell/ConditionalTestLogic.java @@ -121,7 +121,7 @@ public void visit(ForStmt n, Void arg) { } @Override - public void visit(ForeachStmt n, Void arg) { + public void visit(ForEachStmt n, Void arg) { super.visit(n, arg); if (currentMethod != null) { foreachCount++; diff --git a/src/main/java/testsmell/smell/DependentTest.java b/src/main/java/testsmell/smell/DependentTest.java index 36af513..2b57120 100644 --- a/src/main/java/testsmell/smell/DependentTest.java +++ b/src/main/java/testsmell/smell/DependentTest.java @@ -11,6 +11,8 @@ import java.io.FileNotFoundException; import java.util.ArrayList; import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; public class DependentTest extends AbstractSmell { @@ -38,8 +40,14 @@ public void runAnalysis(CompilationUnit testFileCompilationUnit, CompilationUnit classVisitor = new DependentTest.ClassVisitor(); classVisitor.visit(testFileCompilationUnit, null); + Set testMethodNames = testMethods.stream() + .map(method -> method.getMethodDeclaration().getNameAsString()) + .collect(Collectors.toSet()); + for (TestMethod testMethod : testMethods) { - if (testMethod.getCalledMethods().stream().anyMatch(x -> x.getName().equals(testMethods.stream().map(z -> z.getMethodDeclaration().getNameAsString())))) { + boolean match = testMethod.getCalledMethods() + .stream().anyMatch(method -> testMethodNames.contains(method.getName())); + if (match) { smellyElementsSet.add(new testsmell.TestMethod(testMethod.getMethodDeclaration().getNameAsString())); } } diff --git a/src/main/java/testsmell/smell/EagerTest.java b/src/main/java/testsmell/smell/EagerTest.java index a782d85..7235cd1 100644 --- a/src/main/java/testsmell/smell/EagerTest.java +++ b/src/main/java/testsmell/smell/EagerTest.java @@ -122,7 +122,7 @@ public void visit(MethodDeclaration n, Void arg) { } } else { //collect a list of all public/protected members of the production class for (Modifier modifier : n.getModifiers()) { - if (modifier.name().toLowerCase().equals("public") || modifier.name().toLowerCase().equals("protected")) { + if (modifier.getKeyword() == Modifier.Keyword.PUBLIC || modifier.getKeyword() == Modifier.Keyword.PROTECTED) { productionMethods.add(n); } } @@ -220,4 +220,4 @@ public void visit(VariableDeclarator n, Void arg) { super.visit(n, arg); } } -} \ No newline at end of file +} diff --git a/src/main/java/testsmell/smell/IgnoredTest.java b/src/main/java/testsmell/smell/IgnoredTest.java index ee588ab..78dedfc 100644 --- a/src/main/java/testsmell/smell/IgnoredTest.java +++ b/src/main/java/testsmell/smell/IgnoredTest.java @@ -78,7 +78,7 @@ public void visit(MethodDeclaration n, Void arg) { //JUnit 3 //check if test method is not public if (n.getNameAsString().toLowerCase().startsWith("test")) { - if (!n.getModifiers().contains(Modifier.PUBLIC)) { + if (n.getModifiers().stream().noneMatch(m -> m.getKeyword() == Modifier.Keyword.PUBLIC)) { testMethod = new TestMethod(n.getNameAsString()); testMethod.setSmell(true); smellyElementsSet.add(testMethod); diff --git a/src/main/java/testsmell/smell/LazyTest.java b/src/main/java/testsmell/smell/LazyTest.java index e3a2303..3c0c968 100644 --- a/src/main/java/testsmell/smell/LazyTest.java +++ b/src/main/java/testsmell/smell/LazyTest.java @@ -138,7 +138,7 @@ public void visit(MethodDeclaration n, Void arg) { } } else { //collect a list of all public/protected members of the production class for (Modifier modifier : n.getModifiers()) { - if (modifier.name().toLowerCase().equals("public") || modifier.name().toLowerCase().equals("protected")) { + if (modifier.getKeyword() == Modifier.Keyword.PUBLIC || modifier.getKeyword() == Modifier.Keyword.PROTECTED) { productionMethods.add(n); } } diff --git a/src/main/java/testsmell/smell/UnknownTest.java b/src/main/java/testsmell/smell/UnknownTest.java index e7223e4..b9b6ed8 100644 --- a/src/main/java/testsmell/smell/UnknownTest.java +++ b/src/main/java/testsmell/smell/UnknownTest.java @@ -1,6 +1,7 @@ package testsmell.smell; import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.Node; import com.github.javaparser.ast.NodeList; import com.github.javaparser.ast.body.MethodDeclaration; import com.github.javaparser.ast.expr.AnnotationExpr; @@ -55,14 +56,14 @@ public void visit(MethodDeclaration n, Void arg) { if (Util.isValidTestMethod(n)) { Optional assertAnnotation = n.getAnnotationByName("Test"); if (assertAnnotation.isPresent()) { - for (int i = 0; i < assertAnnotation.get().getNodeLists().size(); i++) { - NodeList c = assertAnnotation.get().getNodeLists().get(i); - for (int j = 0; j < c.size(); j++) - if (c.get(j) instanceof MemberValuePair) { - if (((MemberValuePair) c.get(j)).getName().equals("expected") && ((MemberValuePair) c.get(j)).getValue().toString().contains("Exception")) - ; + for (Node node : assertAnnotation.get().getChildNodes()) { + if (node instanceof MemberValuePair) { + MemberValuePair pair = (MemberValuePair) node; + if (pair.getName().getIdentifier().equals("expected") + && pair.getValue().toString().contains("Exception")) { hasExceptionAnnotation = true; } + } } } currentMethod = n; diff --git a/src/test/kotlin/testsmell/TestAssertionsDetection.kt b/src/test/kotlin/testsmell/TestAssertionsDetection.kt index 18dcede..d0a3395 100644 --- a/src/test/kotlin/testsmell/TestAssertionsDetection.kt +++ b/src/test/kotlin/testsmell/TestAssertionsDetection.kt @@ -1,6 +1,6 @@ package testsmell -import com.github.javaparser.JavaParser +import com.github.javaparser.StaticJavaParser import com.github.javaparser.ast.CompilationUnit import org.junit.jupiter.api.Assertions import org.junit.jupiter.api.BeforeEach @@ -19,8 +19,8 @@ class TestAssertionsDetection { @BeforeEach fun setup() { - testCompilationUnit = JavaParser.parse(simpleTest) - productionCompilationUnit = JavaParser.parse(simpleClass) + testCompilationUnit = StaticJavaParser.parse(simpleTest) + productionCompilationUnit = StaticJavaParser.parse(simpleClass) testFile = mock(TestFile::class.java) Mockito.`when`(testFile.testFileNameWithoutExtension).thenReturn("fake/path") Mockito.`when`(testFile.productionFileNameWithoutExtension).thenReturn("fake/path") @@ -83,4 +83,4 @@ class TestAssertionsDetection { } } """.trimIndent() -} \ No newline at end of file +} diff --git a/src/test/kotlin/testsmell/TestDetectionCorrectness.kt b/src/test/kotlin/testsmell/TestDetectionCorrectness.kt index e44e60a..6e3def3 100644 --- a/src/test/kotlin/testsmell/TestDetectionCorrectness.kt +++ b/src/test/kotlin/testsmell/TestDetectionCorrectness.kt @@ -1,6 +1,5 @@ package testsmell -import com.github.javaparser.JavaParser import com.github.javaparser.ast.CompilationUnit import org.junit.jupiter.api.Assertions import org.junit.jupiter.api.BeforeEach @@ -10,6 +9,7 @@ import org.mockito.Mockito.`when` import org.mockito.Mockito.mock import testsmell.smell.AssertionRoulette import testsmell.smell.EagerTest +import testsmell.Util; import thresholds.DefaultThresholds import thresholds.SpadiniThresholds @@ -24,8 +24,8 @@ class TestDetectionCorrectness { @BeforeEach fun setup() { - testCompilationUnit = JavaParser.parse(fractionTest) - productionCompilationUnit = JavaParser.parse(fractionSource) + testCompilationUnit = Util.parseJava(fractionTest) + productionCompilationUnit = Util.parseJava(fractionSource) testFile = mock(TestFile::class.java) `when`(testFile.testFileNameWithoutExtension).thenReturn("fake/path") `when`(testFile.productionFileNameWithoutExtension).thenReturn("fake/path") @@ -2111,7 +2111,7 @@ class TestDetectionCorrectness { f = Fraction.getFraction(-1, 1, Integer.MAX_VALUE); assertEquals("-2147483648/2147483647", f.toString()); } - + @Test public void testToProperString() { Fraction f; @@ -2149,4 +2149,4 @@ class TestDetectionCorrectness { } } """.trimIndent() -} \ No newline at end of file +}