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

Improve AST functionality with excludeMain() #369

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,18 @@ public class UnwantedNodesAssert extends AbstractAssert<UnwantedNodesAssert, Pat
*/
private final LanguageLevel level;

/**
* Whether to enable the main method for the Java parser Default is set to false
sarpsahinalp marked this conversation as resolved.
Show resolved Hide resolved
*/
private final boolean excludeMainMethod;

private UnwantedNodesAssert(Path path, LanguageLevel level) {
this(path, level, false);
}

private UnwantedNodesAssert(Path path, LanguageLevel level, boolean excludeMainMethod) {
super(requireNonNull(path), UnwantedNodesAssert.class);
this.excludeMainMethod = excludeMainMethod;
this.level = level;
if (!Files.isDirectory(path)) {
fail("The source directory %s does not exist", path); //$NON-NLS-1$
Expand Down Expand Up @@ -90,7 +100,16 @@ public static UnwantedNodesAssert assertThatSourcesIn(Path directory) {
public UnwantedNodesAssert withinPackage(String packageName) {
Objects.requireNonNull(packageName, "The package name must not be null."); //$NON-NLS-1$
var newPath = actual.resolve(Path.of("", packageName.split("\\."))); //$NON-NLS-1$ //$NON-NLS-2$
return new UnwantedNodesAssert(newPath, level);
return new UnwantedNodesAssert(newPath, level, excludeMainMethod);
}

/**
* Configures if the main method should be excluded from the AST
*
* @return An unwanted node assertion object (for chaining)
*/
public UnwantedNodesAssert excludeMainMethod() {
return new UnwantedNodesAssert(actual, level, true);
}

/**
Expand All @@ -100,7 +119,7 @@ public UnwantedNodesAssert withinPackage(String packageName) {
* @return An unwanted node assertion object (for chaining)
*/
public UnwantedNodesAssert withLanguageLevel(LanguageLevel level) {
return new UnwantedNodesAssert(actual, level);
return new UnwantedNodesAssert(actual, level, excludeMainMethod);
}

/**
Expand All @@ -120,7 +139,7 @@ public UnwantedNodesAssert hasNo(Type type) {
}
StaticJavaParser.getParserConfiguration().setLanguageLevel(level);
Optional<String> errorMessage = UnwantedNode.getMessageForUnwantedNodesForAllFilesBelow(actual,
type.getNodeNameNodeMap());
type.getNodeNameNodeMap(), excludeMainMethod);
errorMessage.ifPresent(unwantedNodeMessageForAllJavaFiles -> failWithMessage(
localized("ast.method.has_no") + System.lineSeparator() + unwantedNodeMessageForAllJavaFiles)); //$NON-NLS-1$
return this;
Expand Down
23 changes: 18 additions & 5 deletions src/main/java/de/tum/in/test/api/ast/model/JavaFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

import com.github.javaparser.StaticJavaParser;
import com.github.javaparser.ast.CompilationUnit;
import com.github.javaparser.ast.Node;
import com.github.javaparser.ast.body.MethodDeclaration;

/**
* Stores all required information about a Java file to be analyzed
Expand All @@ -26,11 +28,22 @@ public class JavaFile {
private final Path javaFilePath;
private final CompilationUnit javaFileAST;

public JavaFile(Path javaFilePath, CompilationUnit javaFileAST) {
public JavaFile(Path javaFilePath, CompilationUnit javaFileAST, boolean excludeMainMethod) {
this.javaFilePath = javaFilePath;
if (excludeMainMethod) {
excludeMainMethod(javaFileAST);
}
this.javaFileAST = javaFileAST;
}

private static void excludeMainMethod(CompilationUnit javaFileAST) {
javaFileAST.findAll(MethodDeclaration.class).stream()
.filter(method -> method.isStatic() && method.getNameAsString().equals("main")
&& method.getParameters().size() == 1 && method.getType().isVoidType()
&& method.getParameter(0).getTypeAsString().equals("String[]"))
Strohgelaender marked this conversation as resolved.
Show resolved Hide resolved
.forEach(Node::remove);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We modify the AST here, it seems. Are we absolutely sure this has no unintended side effects?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or put differently: what does the removal do? Both according to the documentation and the actual implementation?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removal removes the node from the tree and it's child nodes. This seems to be working as intented in the documentation and the actual implementation.

}

public Path getJavaFilePath() {
return javaFilePath;
}
Expand All @@ -47,12 +60,12 @@ public CompilationUnit getJavaFileAST() {
* @return The information of the Java-file packed into a JavaFile object (null
* if the file is not a Java-file)
*/
public static JavaFile convertFromFile(Path pathOfFile) {
public static JavaFile convertFromFile(Path pathOfFile, Boolean excludeMainMethod) {
sarpsahinalp marked this conversation as resolved.
Show resolved Hide resolved
if (!JAVAFILEMATCHER.matches(pathOfFile.getFileName())) {
return null;
}
try {
return new JavaFile(pathOfFile, StaticJavaParser.parse(pathOfFile));
return new JavaFile(pathOfFile, StaticJavaParser.parse(pathOfFile), excludeMainMethod);
} catch (IOException e) {
LOG.error("Error reading Java file '{}'", pathOfFile.toAbsolutePath(), e); //$NON-NLS-1$
throw new AssertionError(localized("ast.method.convert_from_file", pathOfFile.toAbsolutePath()));
Expand All @@ -67,9 +80,9 @@ public static JavaFile convertFromFile(Path pathOfFile) {
* list if the directory does not exist or if none of the files in the
* directory or its subdirectories is a Java-file)
*/
public static List<JavaFile> readFromDirectory(Path pathOfDirectory) {
public static List<JavaFile> readFromDirectory(Path pathOfDirectory, boolean excludeMainMethod) {
try (Stream<Path> directoryContentStream = Files.walk(pathOfDirectory)) {
return directoryContentStream.map(JavaFile::convertFromFile).filter(Objects::nonNull)
return directoryContentStream.map(path -> convertFromFile(path, excludeMainMethod)).filter(Objects::nonNull)
.collect(Collectors.toList());
} catch (IOException e) {
LOG.error("Error reading Java files in '{}'", pathOfDirectory.toAbsolutePath(), e); //$NON-NLS-1$
Expand Down
16 changes: 8 additions & 8 deletions src/main/java/de/tum/in/test/api/ast/model/UnwantedNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ public static List<UnwantedNode> getUnwantedNodesInJavaFile(JavaFile javaFile,
* information (packed into UnwantedNode objects)
*/
public static Map<Path, List<UnwantedNode>> getUnwantedNodesForFileAt(Path pathOfJavaFile,
Map<String, Class<? extends Node>> nodesDefinedAsUnwanted) {
JavaFile javaFile = JavaFile.convertFromFile(pathOfJavaFile);
Map<String, Class<? extends Node>> nodesDefinedAsUnwanted, boolean excludeMainMethod) {
JavaFile javaFile = JavaFile.convertFromFile(pathOfJavaFile, excludeMainMethod);
if (javaFile == null) {
return Map.of();
}
Expand Down Expand Up @@ -113,9 +113,9 @@ public static String getFormattedFileString(Path filePath, Map<Path, List<Unwant
* @return Error message
*/
public static Optional<String> getMessageForUnwantedNodesForFileAt(Path pathOfJavaFile,
Map<String, Class<? extends Node>> nodeNameUnwantedNodeMap) {
Map<Path, List<UnwantedNode>> unwantedNodes = getUnwantedNodesForFileAt(pathOfJavaFile,
nodeNameUnwantedNodeMap);
Map<String, Class<? extends Node>> nodeNameUnwantedNodeMap, boolean excludeMainMethod) {
Map<Path, List<UnwantedNode>> unwantedNodes = getUnwantedNodesForFileAt(pathOfJavaFile, nodeNameUnwantedNodeMap,
excludeMainMethod);
if (unwantedNodes.isEmpty()) {
return Optional.empty();
}
Expand All @@ -132,11 +132,11 @@ public static Optional<String> getMessageForUnwantedNodesForFileAt(Path pathOfJa
* @return Error message
*/
public static Optional<String> getMessageForUnwantedNodesForAllFilesBelow(Path pathOfDirectory,
Map<String, Class<? extends Node>> nodeNameUnwantedNodeMap) {
return JavaFile.readFromDirectory(pathOfDirectory).stream()
Map<String, Class<? extends Node>> nodeNameUnwantedNodeMap, boolean excludeMainMethod) {
return JavaFile.readFromDirectory(pathOfDirectory, excludeMainMethod).stream()
.sorted(Comparator.comparing(JavaFile::getJavaFilePath))
.map(javaFile -> getMessageForUnwantedNodesForFileAt(javaFile.getJavaFilePath(),
nodeNameUnwantedNodeMap))
nodeNameUnwantedNodeMap, excludeMainMethod))
.filter(Optional::isPresent).map(Optional::get).map(message -> message + System.lineSeparator())
.reduce(String::concat).map(String::trim).map(message -> " " + message);
}
Expand Down
24 changes: 24 additions & 0 deletions src/test/java/de/tum/in/test/integration/AstAssertionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -638,4 +638,28 @@ void test_testLevelIsNull() {
"The 'level' is not set. Please use UnwantedNodesAssert.withLanguageLevel(LanguageLevel)."));
}
}

@Nested
@DisplayName("Exclude-Main-Test-Tests")
class ExcludeMainTestTests {

@TestTest
void test_testExcludeMain_Success() {
String testExcludeMain_Success = "testHasBelowNoLoopsOutsideMainMethod_Success";
tests.assertThatEvents().haveExactly(1, finishedSuccessfully(testExcludeMain_Success));
}

@TestTest
void test_testExcludeMain_Fail() {
String testExcludeMain_Fail = "testHasBelowNoLoopsOutsideMainMethod_Fail";
tests.assertThatEvents().haveExactly(1,
testFailedWith(testExcludeMain_Fail, AssertionError.class,
"Unwanted statement found:" + System.lineSeparator() + " - In "
+ Path.of("src", "test", "java", "de", "tum", "in", "test", "integration",
"testuser", "subject", "structural", "astTestFiles", "excludeMain", "yes",
"ClassWithLoopOutsideMainMethod.java")
+ ":" + System.lineSeparator() + " - For-Statement was found:"
+ System.lineSeparator() + " - Between line 6 (column 3) and line 8 (column 3)"));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -436,4 +436,22 @@ void testLevelIsNull() {
UnwantedNodesAssert.assertThatProjectSources().hasNo(LoopType.ANY);
}
}

@Nested
@DisplayName("Exclude-Main-Tests")
class ExcludeErrorTests {
@Test
void testHasBelowNoLoopsOutsideMainMethod_Success() {
UnwantedNodesAssert.assertThatProjectSources().withinPackage(BASE_PACKAGE + ".excludeMain.no")
.excludeMainMethod().withLanguageLevel(ParserConfiguration.LanguageLevel.JAVA_17)
.hasNo(LoopType.ANY);
}

@Test
void testHasBelowNoLoopsOutsideMainMethod_Fail() {
UnwantedNodesAssert.assertThatProjectSources().withinPackage(BASE_PACKAGE + ".excludeMain.yes")
.excludeMainMethod().withLanguageLevel(ParserConfiguration.LanguageLevel.JAVA_17)
.hasNo(LoopType.ANY);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package de.tum.in.test.integration.testuser.subject.structural.astTestFiles.excludeMain.no;

public class ClassWithNoLoopsOutsideMainMethod {

public void ifStatement() {
int x = 3;
if (x == 1) {
System.out.println("Hello");
} else if (x == 0) {
System.out.println("World");
} else {
System.out.println("!");
}
}

public void ifExpression() {
int x = 3;
System.out.println(x == 1 ? "Hello" : (x == 0 ? "World" : "!"));
}

public void switchStatement() {
String output;
switch (3) {
case 1:
output = "Hello";
break;
case 0:
output = "World";
break;
default:
output = "!";
break;
}
System.out.println(output);
}

public void assertStatement() {
assert 3 == 0;
}

public void throwStatement() throws Exception {
throw new Exception("This is a checked exception.");
}

public void catchStatement() {
try {
throwStatement();
} catch (Exception e) {
}
}

public static void main(String[] args) {
for (int i = 0; i < args.length; i++) {
int x = i;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package de.tum.in.test.integration.testuser.subject.structural.astTestFiles.excludeMain.yes;

public class ClassWithLoopOutsideMainMethod {

public void forLoop() {
for (int i = 0; i < 3; i++) {
System.out.println("Hello World");
}
}

public static void main(String[] args) {
if (args.length > 0) {
System.out.println("Hello World");
}
}
}
Loading