Add global analyzer to catch common mistakes (#85)

* Add global analyzer to catch common mistakes

* Expand golden test scenarios

* Add feedback request if any comments

* Add golden test that receives no feedback

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<CompilationUnit> 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<Analyzer> createAnalyzers(String slug) {
             case "two-fer" -> analyzers.add(new TwoferAnalyzer());
         }
 
+        analyzers.add(new GlobalAnalyzer());
+
         return List.copyOf(analyzers);
     }
 }

src/main/java/analyzer/comments/AvoidPrintStatements.java

@@ -0,0 +1,19 @@
+package analyzer.comments;
+
+import analyzer.Comment;
+import analyzer.CommentType;
+
+/**
+ * @see <a href="https://github.com/exercism/website-copy/blob/main/analyzer-comments/java/general/avoid_print_statements.md">Markdown Template</a>
+ */
+public class AvoidPrintStatements extends Comment {
+    @Override
+    public String getKey() {
+        return "java.general.avoid_print_statements";
+    }
+
+    @Override
+    public CommentType getType() {
+        return CommentType.INFORMATIVE;
+    }
+}

src/main/java/analyzer/comments/DoNotUseMainMethod.java

@@ -0,0 +1,19 @@
+package analyzer.comments;
+
+import analyzer.Comment;
+import analyzer.CommentType;
+
+/**
+ * @see <a href="https://github.com/exercism/website-copy/blob/main/analyzer-comments/java/general/do_not_use_main_method.md">Markdown Template</a>
+ */
+public class DoNotUseMainMethod extends Comment {
+    @Override
+    public String getKey() {
+        return "java.general.do_not_use_main_method";
+    }
+
+    @Override
+    public CommentType getType() {
+        return CommentType.ESSENTIAL;
+    }
+}

src/main/java/analyzer/comments/FeedbackRequest.java

@@ -0,0 +1,13 @@
+package analyzer.comments;
+
+import analyzer.Comment;
+
+/**
+ * @see <a href="https://github.com/exercism/website-copy/blob/main/analyzer-comments/java/general/feedback_request.md">Markdown Template</a>
+ */
+public class FeedbackRequest extends Comment {
+    @Override
+    public String getKey() {
+        return "java.general.feedback_request";
+    }
+}

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<Void> implements Analyzer {
+    private Analysis analysis;
+
+    @Override
+    public void analyze(List<CompilationUnit> 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);
+    }
+}

src/main/java/analyzer/exercises/twofer/TwoferWalker.java

@@ -23,8 +23,8 @@ class TwoferWalker implements Consumer<Node> {
     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) {

src/test/java/analyzer/ExerciseAnalyzerTest.java → 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;

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<String> 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<String> 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");
+                            }
+                        }"""
+        );
+    }
+}

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();

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() {

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"
   }
 ]}

tests/hello-world/README.md

@@ -0,0 +1 @@
+This test is designed to receive no comments at all from the analyzer.

tests/hello-world/expected_analysis.json

@@ -0,0 +1 @@
+{}

tests/hello-world/expected_tags.json

@@ -0,0 +1 @@
+{}

tests/hello-world/src/main/java/Greeter.java

@@ -0,0 +1,5 @@
+class Greeter {
+    String getGreeting() {
+        return "Hello, World!";
+    }
+}

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.

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"
+  }
+]}

tests/two-fer/expected_tags.json

@@ -0,0 +1 @@
+{}

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"));
+    }
+}

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.

tests/unknown-exercise/expected_analysis.json

@@ -1 +1,14 @@
-{}
+{"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"
+  }
+]}

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());
+	}
 }