diff --git a/src/main/java/analyzer/AnalyzerRoot.java b/src/main/java/analyzer/AnalyzerRoot.java index 1ff8a654..57a37914 100644 --- a/src/main/java/analyzer/AnalyzerRoot.java +++ b/src/main/java/analyzer/AnalyzerRoot.java @@ -1,5 +1,7 @@ package analyzer; +import analyzer.comments.FeedbackRequest; +import analyzer.exercises.GlobalAnalyzer; import analyzer.exercises.hamming.HammingAnalyzer; import analyzer.exercises.twofer.TwoferAnalyzer; import com.github.javaparser.ast.CompilationUnit; @@ -11,9 +13,15 @@ public class AnalyzerRoot { public static Analysis analyze(String slug, List compilationUnits) { var analysis = new Analysis(); + for (Analyzer analyzer : createAnalyzers(slug)) { analyzer.analyze(compilationUnits, analysis); } + + if (!analysis.getComments().isEmpty()) { + analysis.addComment(new FeedbackRequest()); + } + return analysis; } @@ -25,6 +33,8 @@ private static List createAnalyzers(String slug) { case "two-fer" -> analyzers.add(new TwoferAnalyzer()); } + analyzers.add(new GlobalAnalyzer()); + return List.copyOf(analyzers); } } diff --git a/src/main/java/analyzer/comments/AvoidPrintStatements.java b/src/main/java/analyzer/comments/AvoidPrintStatements.java new file mode 100644 index 00000000..638ce14b --- /dev/null +++ b/src/main/java/analyzer/comments/AvoidPrintStatements.java @@ -0,0 +1,19 @@ +package analyzer.comments; + +import analyzer.Comment; +import analyzer.CommentType; + +/** + * @see Markdown Template + */ +public class AvoidPrintStatements extends Comment { + @Override + public String getKey() { + return "java.general.avoid_print_statements"; + } + + @Override + public CommentType getType() { + return CommentType.INFORMATIVE; + } +} diff --git a/src/main/java/analyzer/comments/DoNotUseMainMethod.java b/src/main/java/analyzer/comments/DoNotUseMainMethod.java new file mode 100644 index 00000000..4c738550 --- /dev/null +++ b/src/main/java/analyzer/comments/DoNotUseMainMethod.java @@ -0,0 +1,19 @@ +package analyzer.comments; + +import analyzer.Comment; +import analyzer.CommentType; + +/** + * @see Markdown Template + */ +public class DoNotUseMainMethod extends Comment { + @Override + public String getKey() { + return "java.general.do_not_use_main_method"; + } + + @Override + public CommentType getType() { + return CommentType.ESSENTIAL; + } +} diff --git a/src/main/java/analyzer/comments/FeedbackRequest.java b/src/main/java/analyzer/comments/FeedbackRequest.java new file mode 100644 index 00000000..86549172 --- /dev/null +++ b/src/main/java/analyzer/comments/FeedbackRequest.java @@ -0,0 +1,13 @@ +package analyzer.comments; + +import analyzer.Comment; + +/** + * @see Markdown Template + */ +public class FeedbackRequest extends Comment { + @Override + public String getKey() { + return "java.general.feedback_request"; + } +} diff --git a/src/main/java/analyzer/exercises/GlobalAnalyzer.java b/src/main/java/analyzer/exercises/GlobalAnalyzer.java new file mode 100644 index 00000000..b35c2da1 --- /dev/null +++ b/src/main/java/analyzer/exercises/GlobalAnalyzer.java @@ -0,0 +1,52 @@ +package analyzer.exercises; + +import analyzer.Analysis; +import analyzer.Analyzer; +import analyzer.comments.AvoidPrintStatements; +import analyzer.comments.DoNotUseMainMethod; +import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.body.MethodDeclaration; +import com.github.javaparser.ast.expr.MethodCallExpr; +import com.github.javaparser.ast.visitor.VoidVisitorAdapter; + +import java.util.List; + +public class GlobalAnalyzer extends VoidVisitorAdapter implements Analyzer { + private Analysis analysis; + + @Override + public void analyze(List compilationUnits, Analysis analysis) { + this.analysis = analysis; + for (CompilationUnit compilationUnit : compilationUnits) { + compilationUnit.accept(this, null); + } + } + + @Override + public void visit(MethodDeclaration n, Void arg) { + if (isMainMethod(n)) { + analysis.addComment(new DoNotUseMainMethod()); + } + + super.visit(n, arg); + } + + @Override + public void visit(MethodCallExpr n, Void arg) { + if (isPrintStatement(n)) { + analysis.addComment(new AvoidPrintStatements()); + } + + super.visit(n, arg); + } + + private static boolean isMainMethod(MethodDeclaration node) { + return node.getNameAsString().equals("main") && node.isStatic() && node.getType().isVoidType(); + } + + private static boolean isPrintStatement(MethodCallExpr node) { + return node.getScope() + .map(scope -> scope.toString().matches("System\\.(?:out|err)")) + .orElse(false); + } +} diff --git a/src/main/java/analyzer/exercises/twofer/TwoferWalker.java b/src/main/java/analyzer/exercises/twofer/TwoferWalker.java index 4f636b70..0be43980 100644 --- a/src/main/java/analyzer/exercises/twofer/TwoferWalker.java +++ b/src/main/java/analyzer/exercises/twofer/TwoferWalker.java @@ -23,8 +23,8 @@ class TwoferWalker implements Consumer { public void accept(Node node) { if (node instanceof ClassOrInterfaceDeclaration) { this.hasClassTwofer = ((ClassOrInterfaceDeclaration) node).getName().toString().equals("Twofer"); - } else if (node instanceof MethodDeclaration) { - this.hasMethodTwofer = ((MethodDeclaration) node).getName().toString().equals("twofer"); + } else if (node instanceof MethodDeclaration methodDeclaration && methodDeclaration.getNameAsString().equals("twofer")) { + this.hasMethodTwofer = true; } else if (node instanceof StringLiteralExpr && !this.hasHardCodedTestCases) { this.hasHardCodedTestCases = node.toString().contains("Alice") || node.toString().contains("Bob"); } else if (node instanceof ReturnStmt) { diff --git a/src/test/java/analyzer/ExerciseAnalyzerTest.java b/src/test/java/analyzer/AnalyzerTest.java similarity index 55% rename from src/test/java/analyzer/ExerciseAnalyzerTest.java rename to src/test/java/analyzer/AnalyzerTest.java index 72fcb6e3..899653f6 100644 --- a/src/test/java/analyzer/ExerciseAnalyzerTest.java +++ b/src/test/java/analyzer/AnalyzerTest.java @@ -1,15 +1,23 @@ package analyzer; import com.github.javaparser.StaticJavaParser; +import com.github.javaparser.ast.CompilationUnit; import java.util.List; -public abstract class ExerciseAnalyzerTest { +public abstract class AnalyzerTest { protected abstract Analyzer getAnalyzer(); protected Analysis analyzeResourceFile(String resourceFileName) { var resource = getClass().getResourceAsStream(resourceFileName); - var compilationUnit = StaticJavaParser.parse(resource); + return analyze(StaticJavaParser.parse(resource)); + } + + protected Analysis analyzeString(String javaCode) { + return analyze(StaticJavaParser.parse(javaCode)); + } + + private Analysis analyze(CompilationUnit compilationUnit) { var analysis = new Analysis(); getAnalyzer().analyze(List.of(compilationUnit), analysis); return analysis; diff --git a/src/test/java/analyzer/exercises/GlobalAnalyzerTest.java b/src/test/java/analyzer/exercises/GlobalAnalyzerTest.java new file mode 100644 index 00000000..33885058 --- /dev/null +++ b/src/test/java/analyzer/exercises/GlobalAnalyzerTest.java @@ -0,0 +1,79 @@ +package analyzer.exercises; + +import analyzer.Analyzer; +import analyzer.AnalyzerTest; +import analyzer.comments.AvoidPrintStatements; +import analyzer.comments.DoNotUseMainMethod; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; + +public class GlobalAnalyzerTest extends AnalyzerTest { + @Override + protected Analyzer getAnalyzer() { + return new GlobalAnalyzer(); + } + + @MethodSource + @ParameterizedTest + public void solutionsWithMainMethod(String solution) { + var actual = analyzeString(solution); + assertThat(actual.getComments()).contains(new DoNotUseMainMethod()); + } + + private static Stream solutionsWithMainMethod() { + return Stream.of( + """ + class Solution { + public static void main(String... args) {} + }""", + """ + class Solution { + public static void main(String ...args) {} + }""", + """ + class Solution { + public static void main(String[] args) {} + }""" + ); + } + + @MethodSource + @ParameterizedTest + public void solutionsWithPrintStatements(String solution) { + var actual = analyzeString(solution); + assertThat(actual.getComments()).contains(new AvoidPrintStatements()); + } + + private static Stream solutionsWithPrintStatements() { + return Stream.of( + """ + class Solution { + void method() { + System.out.println("Printing line to stdout"); + } + }""", + """ + class Solution { + void method() { + System.err.println("Printing line to stderr"); + } + }""", + """ + class Solution { + void method() { + System.out.print("Printing to stdout"); + } + }""", + """ + class Solution { + void method() { + System.err.print("Printing to stderr"); + } + }""" + ); + } +} diff --git a/src/test/java/analyzer/exercises/hamming/HammingAnalyzerTest.java b/src/test/java/analyzer/exercises/hamming/HammingAnalyzerTest.java index eb71d057..91bd901f 100644 --- a/src/test/java/analyzer/exercises/hamming/HammingAnalyzerTest.java +++ b/src/test/java/analyzer/exercises/hamming/HammingAnalyzerTest.java @@ -2,7 +2,7 @@ import analyzer.Analyzer; import analyzer.Comment; -import analyzer.ExerciseAnalyzerTest; +import analyzer.AnalyzerTest; import analyzer.comments.ConstructorTooLong; import analyzer.comments.MethodTooLong; import analyzer.comments.UseProperClassName; @@ -15,7 +15,7 @@ import static org.assertj.core.api.Assertions.assertThat; -public class HammingAnalyzerTest extends ExerciseAnalyzerTest { +public class HammingAnalyzerTest extends AnalyzerTest { @Override protected Analyzer getAnalyzer() { return new HammingAnalyzer(); diff --git a/src/test/java/analyzer/exercises/twofer/TwoferAnalyzerTest.java b/src/test/java/analyzer/exercises/twofer/TwoferAnalyzerTest.java index 58e5f026..594c8096 100644 --- a/src/test/java/analyzer/exercises/twofer/TwoferAnalyzerTest.java +++ b/src/test/java/analyzer/exercises/twofer/TwoferAnalyzerTest.java @@ -2,7 +2,7 @@ import analyzer.Analyzer; import analyzer.Comment; -import analyzer.ExerciseAnalyzerTest; +import analyzer.AnalyzerTest; import analyzer.comments.AvoidHardCodedTestCases; import analyzer.comments.UseProperClassName; import analyzer.comments.UseProperMethodName; @@ -14,7 +14,7 @@ import static org.assertj.core.api.Assertions.assertThat; -public class TwoferAnalyzerTest extends ExerciseAnalyzerTest { +public class TwoferAnalyzerTest extends AnalyzerTest { @Override protected Analyzer getAnalyzer() { diff --git a/tests/hamming/expected_analysis.json b/tests/hamming/expected_analysis.json index 66c00133..7e7cbaa6 100644 --- a/tests/hamming/expected_analysis.json +++ b/tests/hamming/expected_analysis.json @@ -6,5 +6,9 @@ { "comment": "java.hamming.should_use_string_is_empty", "type": "informative" + }, + { + "comment": "java.general.feedback_request", + "type": "informative" } ]} \ No newline at end of file diff --git a/tests/hello-world/README.md b/tests/hello-world/README.md new file mode 100644 index 00000000..d4fede7b --- /dev/null +++ b/tests/hello-world/README.md @@ -0,0 +1 @@ +This test is designed to receive no comments at all from the analyzer. diff --git a/tests/hello-world/expected_analysis.json b/tests/hello-world/expected_analysis.json new file mode 100644 index 00000000..9e26dfee --- /dev/null +++ b/tests/hello-world/expected_analysis.json @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/tests/hello-world/expected_tags.json b/tests/hello-world/expected_tags.json new file mode 100644 index 00000000..9e26dfee --- /dev/null +++ b/tests/hello-world/expected_tags.json @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/tests/hello-world/src/main/java/Greeter.java b/tests/hello-world/src/main/java/Greeter.java new file mode 100644 index 00000000..1f5a5e00 --- /dev/null +++ b/tests/hello-world/src/main/java/Greeter.java @@ -0,0 +1,5 @@ +class Greeter { + String getGreeting() { + return "Hello, World!"; + } +} diff --git a/tests/two-fer/README.md b/tests/two-fer/README.md new file mode 100644 index 00000000..918dc2c4 --- /dev/null +++ b/tests/two-fer/README.md @@ -0,0 +1,4 @@ +This test is designed to receive comments from multiple analyzers: + +- The global analyzer should complain about the use of a `main` method and print statements +- The `two-fer` analyzer should complain about multiple return statements. diff --git a/tests/two-fer/expected_analysis.json b/tests/two-fer/expected_analysis.json new file mode 100644 index 00000000..63d1d831 --- /dev/null +++ b/tests/two-fer/expected_analysis.json @@ -0,0 +1,18 @@ +{"comments": [ + { + "comment": "java.general.do_not_use_main_method", + "type": "essential" + }, + { + "comment": "java.two-fer.avoid_string_format", + "type": "actionable" + }, + { + "comment": "java.general.avoid_print_statements", + "type": "informative" + }, + { + "comment": "java.general.feedback_request", + "type": "informative" + } +]} \ No newline at end of file diff --git a/tests/two-fer/expected_tags.json b/tests/two-fer/expected_tags.json new file mode 100644 index 00000000..9e26dfee --- /dev/null +++ b/tests/two-fer/expected_tags.json @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/tests/two-fer/src/main/java/Twofer.java b/tests/two-fer/src/main/java/Twofer.java new file mode 100644 index 00000000..c780a4f8 --- /dev/null +++ b/tests/two-fer/src/main/java/Twofer.java @@ -0,0 +1,15 @@ +class Twofer { + public String twofer(String name) { + if (name == null) { + return "Two for you, two for me."; + } + + return String.format("Two for %s, two for me.", name); + } + + public static void main(String[] args) { + TwoFer twoFer = new TwoFer(); + System.out.println(twoFer.twofer(null)); + System.out.println(twoFer.twofer("John")); + } +} diff --git a/tests/unknown-exercise/README.md b/tests/unknown-exercise/README.md new file mode 100644 index 00000000..2609a95e --- /dev/null +++ b/tests/unknown-exercise/README.md @@ -0,0 +1,2 @@ +This test is designed to analyze a solution for which no exercise analyzer is implemented. +It should still receive feedback from the global analyzer. diff --git a/tests/unknown-exercise/expected_analysis.json b/tests/unknown-exercise/expected_analysis.json index 9e26dfee..91c83c29 100644 --- a/tests/unknown-exercise/expected_analysis.json +++ b/tests/unknown-exercise/expected_analysis.json @@ -1 +1,14 @@ -{} \ No newline at end of file +{"comments": [ + { + "comment": "java.general.do_not_use_main_method", + "type": "essential" + }, + { + "comment": "java.general.avoid_print_statements", + "type": "informative" + }, + { + "comment": "java.general.feedback_request", + "type": "informative" + } +]} \ No newline at end of file diff --git a/tests/unknown-exercise/src/main/java/UnknownExercise.java b/tests/unknown-exercise/src/main/java/UnknownExercise.java index 9e6a0ca4..75afaef6 100644 --- a/tests/unknown-exercise/src/main/java/UnknownExercise.java +++ b/tests/unknown-exercise/src/main/java/UnknownExercise.java @@ -2,4 +2,9 @@ class UnknownExercise { int calculate() { return 42; } + + public static void main(String[] args) { + var exercise = new UnknownExercise(); + System.out.println(exercise.calculate()); + } }