diff --git a/src/main/java/analyzer/exercises/twofer/AvoidStringFormat.java b/src/main/java/analyzer/exercises/twofer/AvoidStringFormat.java deleted file mode 100644 index 8ccd1cd5..00000000 --- a/src/main/java/analyzer/exercises/twofer/AvoidStringFormat.java +++ /dev/null @@ -1,18 +0,0 @@ -package analyzer.exercises.twofer; - -import analyzer.Comment; - -/** - * @see Markdown Template - */ -class AvoidStringFormat extends Comment { - @Override - public String getKey() { - return "java.two-fer.avoid_string_format"; - } - - @Override - public Type getType() { - return Type.ACTIONABLE; - } -} diff --git a/src/main/java/analyzer/exercises/twofer/TwoferAnalyzer.java b/src/main/java/analyzer/exercises/twofer/TwoferAnalyzer.java index 2b8b7e96..d5ea990d 100644 --- a/src/main/java/analyzer/exercises/twofer/TwoferAnalyzer.java +++ b/src/main/java/analyzer/exercises/twofer/TwoferAnalyzer.java @@ -1,33 +1,86 @@ package analyzer.exercises.twofer; import analyzer.OutputCollector; + +import java.util.List; + +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.expr.StringLiteralExpr; +import com.github.javaparser.ast.stmt.IfStmt; +import com.github.javaparser.ast.stmt.ReturnStmt; +import com.github.javaparser.ast.stmt.Statement; +import com.github.javaparser.ast.visitor.VoidVisitorAdapter; + import analyzer.Analyzer; import analyzer.Solution; import analyzer.comments.AvoidHardCodedTestCases; +import analyzer.comments.PreferStringConcatenation; /** * The {@link TwoferAnalyzer} is the analyzer implementation for the {@code two-fer} practice exercise. * * @see The two-fer exercise on the Java track */ -public class TwoferAnalyzer implements Analyzer { +public class TwoferAnalyzer extends VoidVisitorAdapter implements Analyzer { @Override public void analyze(Solution solution, OutputCollector output) { - TwoferWalker walker = new TwoferWalker(); - - solution.getCompilationUnits().forEach(cu -> cu.walk(walker)); + for (CompilationUnit compilationUnit : solution.getCompilationUnits()) { + compilationUnit.accept(this, output); + } + } - if (walker.hasHardCodedTestCases) { + @Override + public void visit(MethodDeclaration node, OutputCollector output) { + if (hasHardCodedTestCases(node)) { output.addComment(new AvoidHardCodedTestCases()); - } else if (walker.usesFormat) { - output.addComment(new AvoidStringFormat()); - } else if (walker.returnCount > 1) { + return; + } + + if (hasIfStatement(node)) { + output.addComment(new UseTernaryOperator()); + } + + if (hasMoreThanOneReturnStatement(node)) { output.addComment(new UseOneReturn()); - } else { - if (walker.usesIfStatement) { - output.addComment(new UseTernaryOperator()); - } } + + if (callsFormatMethod(node)) { + output.addComment(new PreferStringConcatenation()); + } + } + + private static boolean hasHardCodedTestCases(MethodDeclaration node) { + List hardcodedStrings = node.findAll(StringLiteralExpr.class, + x -> x.getValue().contains("Alice") || x.getValue().contains("Bob")); + + return hardcodedStrings.size() > 1; + } + + private static boolean hasMoreThanOneReturnStatement(MethodDeclaration node) { + long returnStmtCount = node.getBody() + .map(body -> body.getStatements().stream().filter(TwoferAnalyzer::isReturnStatement).count()) + .orElse(0L); + + return returnStmtCount > 1; + } + + private static boolean callsFormatMethod(MethodDeclaration node) { + return !node.findAll(MethodCallExpr.class, x -> x.getNameAsString().contains("format")).isEmpty(); + } + + private static boolean hasIfStatement(MethodDeclaration node) { + return node.getBody().map(body -> body.getStatements().stream().anyMatch(TwoferAnalyzer::isIfStatement)) + .orElse(false); + } + + private static boolean isIfStatement(Statement statement) { + return !statement.findAll(IfStmt.class).isEmpty(); + } + + private static boolean isReturnStatement(Statement statement) { + return !statement.findAll(ReturnStmt.class).isEmpty(); } } diff --git a/src/main/java/analyzer/exercises/twofer/TwoferWalker.java b/src/main/java/analyzer/exercises/twofer/TwoferWalker.java deleted file mode 100644 index f5c8c943..00000000 --- a/src/main/java/analyzer/exercises/twofer/TwoferWalker.java +++ /dev/null @@ -1,33 +0,0 @@ -package analyzer.exercises.twofer; - -import com.github.javaparser.ast.Node; -import com.github.javaparser.ast.expr.MethodCallExpr; -import com.github.javaparser.ast.expr.StringLiteralExpr; -import com.github.javaparser.ast.stmt.*; - -import java.util.function.Consumer; - -class TwoferWalker implements Consumer { - boolean hasHardCodedTestCases; - boolean usesIfStatement; - boolean hasMethodCall; - boolean usesFormat; - int returnCount; - - @Override - public void accept(Node node) { - if (node instanceof StringLiteralExpr && !this.hasHardCodedTestCases) { - this.hasHardCodedTestCases = node.toString().contains("Alice") || node.toString().contains("Bob"); - } else if (node instanceof ReturnStmt) { - this.returnCount++; - } else if (node instanceof IfStmt) { - this.usesIfStatement = true; - } else if (node instanceof MethodCallExpr && !this.hasMethodCall) { - this.hasMethodCall = true; - - if (((MethodCallExpr) node).getName().toString().equals("format")) { - this.usesFormat = true; - } - } - } -} \ No newline at end of file diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.twofer.UsesMultipleReturns.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.twofer.UsesMultipleReturns.approved.txt index e7bd4728..a9a0c216 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.twofer.UsesMultipleReturns.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.twofer.UsesMultipleReturns.approved.txt @@ -1,5 +1,10 @@ { "comments": [ + { + "comment": "java.two-fer.use_ternary_operator", + "params": {}, + "type": "actionable" + }, { "comment": "java.two-fer.use_one_return", "params": {}, diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.twofer.UsesStringFormat.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.twofer.UsesStringFormat.approved.txt index 184c68eb..2b150682 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.twofer.UsesStringFormat.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.twofer.UsesStringFormat.approved.txt @@ -1,9 +1,9 @@ { "comments": [ { - "comment": "java.two-fer.avoid_string_format", + "comment": "java.general.prefer_string_concatenation", "params": {}, - "type": "actionable" + "type": "informative" }, { "comment": "java.general.feedback_request", diff --git a/tests/two-fer/non-optimal-solution/.meta/config.json b/tests/two-fer/hardcoding-tests/.meta/config.json similarity index 100% rename from tests/two-fer/non-optimal-solution/.meta/config.json rename to tests/two-fer/hardcoding-tests/.meta/config.json diff --git a/tests/two-fer/hardcoding-tests/expected_analysis.json b/tests/two-fer/hardcoding-tests/expected_analysis.json new file mode 100644 index 00000000..e5fab09b --- /dev/null +++ b/tests/two-fer/hardcoding-tests/expected_analysis.json @@ -0,0 +1,14 @@ +{ + "comments": [ + { + "comment": "java.general.avoid_hard_coded_test_cases", + "params": {}, + "type": "essential" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] +} \ No newline at end of file diff --git a/tests/two-fer/non-optimal-solution/expected_tags.json b/tests/two-fer/hardcoding-tests/expected_tags.json similarity index 100% rename from tests/two-fer/non-optimal-solution/expected_tags.json rename to tests/two-fer/hardcoding-tests/expected_tags.json diff --git a/tests/two-fer/hardcoding-tests/src/main/java/TwoFer.java b/tests/two-fer/hardcoding-tests/src/main/java/TwoFer.java new file mode 100644 index 00000000..99392584 --- /dev/null +++ b/tests/two-fer/hardcoding-tests/src/main/java/TwoFer.java @@ -0,0 +1,12 @@ +class Twofer { + public String twofer(String name) { + if (name == null) { + // fall through + } else if (name.equals("Alice")) { + return "One for Alice, one for me."; + } else if (name.equals("Bob")) { + return "One for Bob, one for me."; + } + return "One for you, one for me."; + } +} diff --git a/tests/two-fer/non-optimal-solution/src/main/java/Twofer.java b/tests/two-fer/non-optimal-solution/src/main/java/Twofer.java deleted file mode 100644 index 87484bee..00000000 --- a/tests/two-fer/non-optimal-solution/src/main/java/Twofer.java +++ /dev/null @@ -1,9 +0,0 @@ -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); - } -} diff --git a/tests/two-fer/using-if-statement/.meta/config.json b/tests/two-fer/using-if-statement/.meta/config.json new file mode 100644 index 00000000..e5238df8 --- /dev/null +++ b/tests/two-fer/using-if-statement/.meta/config.json @@ -0,0 +1,39 @@ +{ + "authors": [ + "Smarticles101" + ], + "contributors": [ + "FridaTveit", + "ikhadykin", + "jmrunkle", + "jssander", + "kytrinyx", + "lemoncurry", + "msomji", + "muzimuzhi", + "rdavid1099", + "sjwarner-bp", + "SleeplessByte", + "sshine", + "stkent", + "uzilan", + "Valkryst", + "ymoskovits" + ], + "files": { + "solution": [ + "src/main/java/Twofer.java" + ], + "test": [ + "src/test/java/TwoferTest.java" + ], + "example": [ + ".meta/src/reference/java/Twofer.java" + ], + "invalidator": [ + "build.gradle" + ] + }, + "blurb": "Create a sentence of the form \"One for X, one for me.\".", + "source_url": "https://github.com/exercism/problem-specifications/issues/757" +} \ No newline at end of file diff --git a/tests/two-fer/non-optimal-solution/expected_analysis.json b/tests/two-fer/using-if-statement/expected_analysis.json similarity index 78% rename from tests/two-fer/non-optimal-solution/expected_analysis.json rename to tests/two-fer/using-if-statement/expected_analysis.json index 184c68eb..a0a565af 100644 --- a/tests/two-fer/non-optimal-solution/expected_analysis.json +++ b/tests/two-fer/using-if-statement/expected_analysis.json @@ -1,7 +1,7 @@ { "comments": [ { - "comment": "java.two-fer.avoid_string_format", + "comment": "java.two-fer.use_ternary_operator", "params": {}, "type": "actionable" }, diff --git a/tests/two-fer/using-if-statement/expected_tags.json b/tests/two-fer/using-if-statement/expected_tags.json new file mode 100644 index 00000000..eb25b190 --- /dev/null +++ b/tests/two-fer/using-if-statement/expected_tags.json @@ -0,0 +1,3 @@ +{ + "tags": [] +} \ No newline at end of file diff --git a/tests/two-fer/using-if-statement/src/main/java/TwoFer.java b/tests/two-fer/using-if-statement/src/main/java/TwoFer.java new file mode 100644 index 00000000..18bad333 --- /dev/null +++ b/tests/two-fer/using-if-statement/src/main/java/TwoFer.java @@ -0,0 +1,7 @@ +class Twofer { + public String twofer(String rawName) { + String name = rawName; + if (name == null) name = "you"; + return "One for " + name + ", one for me."; + } +} diff --git a/tests/two-fer/using-multiple-returns/.meta/config.json b/tests/two-fer/using-multiple-returns/.meta/config.json new file mode 100644 index 00000000..e5238df8 --- /dev/null +++ b/tests/two-fer/using-multiple-returns/.meta/config.json @@ -0,0 +1,39 @@ +{ + "authors": [ + "Smarticles101" + ], + "contributors": [ + "FridaTveit", + "ikhadykin", + "jmrunkle", + "jssander", + "kytrinyx", + "lemoncurry", + "msomji", + "muzimuzhi", + "rdavid1099", + "sjwarner-bp", + "SleeplessByte", + "sshine", + "stkent", + "uzilan", + "Valkryst", + "ymoskovits" + ], + "files": { + "solution": [ + "src/main/java/Twofer.java" + ], + "test": [ + "src/test/java/TwoferTest.java" + ], + "example": [ + ".meta/src/reference/java/Twofer.java" + ], + "invalidator": [ + "build.gradle" + ] + }, + "blurb": "Create a sentence of the form \"One for X, one for me.\".", + "source_url": "https://github.com/exercism/problem-specifications/issues/757" +} \ No newline at end of file diff --git a/tests/two-fer/using-multiple-returns/expected_analysis.json b/tests/two-fer/using-multiple-returns/expected_analysis.json new file mode 100644 index 00000000..a9a0c216 --- /dev/null +++ b/tests/two-fer/using-multiple-returns/expected_analysis.json @@ -0,0 +1,19 @@ +{ + "comments": [ + { + "comment": "java.two-fer.use_ternary_operator", + "params": {}, + "type": "actionable" + }, + { + "comment": "java.two-fer.use_one_return", + "params": {}, + "type": "actionable" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] +} \ No newline at end of file diff --git a/tests/two-fer/using-multiple-returns/expected_tags.json b/tests/two-fer/using-multiple-returns/expected_tags.json new file mode 100644 index 00000000..eb25b190 --- /dev/null +++ b/tests/two-fer/using-multiple-returns/expected_tags.json @@ -0,0 +1,3 @@ +{ + "tags": [] +} \ No newline at end of file diff --git a/tests/two-fer/using-multiple-returns/src/main/java/TwoFer.java b/tests/two-fer/using-multiple-returns/src/main/java/TwoFer.java new file mode 100644 index 00000000..8ab36ef2 --- /dev/null +++ b/tests/two-fer/using-multiple-returns/src/main/java/TwoFer.java @@ -0,0 +1,6 @@ +class Twofer { + public String twofer(String name) { + if (name == null) return "One for you, one for me."; + return "One for " + name + ", one for me."; + } +} diff --git a/tests/two-fer/using-string-format/.meta/config.json b/tests/two-fer/using-string-format/.meta/config.json new file mode 100644 index 00000000..e5238df8 --- /dev/null +++ b/tests/two-fer/using-string-format/.meta/config.json @@ -0,0 +1,39 @@ +{ + "authors": [ + "Smarticles101" + ], + "contributors": [ + "FridaTveit", + "ikhadykin", + "jmrunkle", + "jssander", + "kytrinyx", + "lemoncurry", + "msomji", + "muzimuzhi", + "rdavid1099", + "sjwarner-bp", + "SleeplessByte", + "sshine", + "stkent", + "uzilan", + "Valkryst", + "ymoskovits" + ], + "files": { + "solution": [ + "src/main/java/Twofer.java" + ], + "test": [ + "src/test/java/TwoferTest.java" + ], + "example": [ + ".meta/src/reference/java/Twofer.java" + ], + "invalidator": [ + "build.gradle" + ] + }, + "blurb": "Create a sentence of the form \"One for X, one for me.\".", + "source_url": "https://github.com/exercism/problem-specifications/issues/757" +} \ No newline at end of file diff --git a/tests/two-fer/using-string-format/expected_analysis.json b/tests/two-fer/using-string-format/expected_analysis.json new file mode 100644 index 00000000..2b150682 --- /dev/null +++ b/tests/two-fer/using-string-format/expected_analysis.json @@ -0,0 +1,14 @@ +{ + "comments": [ + { + "comment": "java.general.prefer_string_concatenation", + "params": {}, + "type": "informative" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] +} \ No newline at end of file diff --git a/tests/two-fer/using-string-format/expected_tags.json b/tests/two-fer/using-string-format/expected_tags.json new file mode 100644 index 00000000..eb25b190 --- /dev/null +++ b/tests/two-fer/using-string-format/expected_tags.json @@ -0,0 +1,3 @@ +{ + "tags": [] +} \ No newline at end of file diff --git a/tests/two-fer/using-string-format/src/main/java/TwoFer.java b/tests/two-fer/using-string-format/src/main/java/TwoFer.java new file mode 100644 index 00000000..5ce32ef1 --- /dev/null +++ b/tests/two-fer/using-string-format/src/main/java/TwoFer.java @@ -0,0 +1,5 @@ +class Twofer { + public String twofer(String name) { + return String.format("One for %s, one for me.", name == null ? "you" : name); + } +}