From 01161d986cdae365d61a46e832e05d71661581bd Mon Sep 17 00:00:00 2001 From: manumafe98 Date: Wed, 21 Feb 2024 10:46:39 -0300 Subject: [PATCH] Apllying suggestions Renaming comment of string format to prefer_string_concatenation Adding another helper method callsMethod Making comment about substrings general Adding another tests case for when substring is not being used in both methods Making the analyzer to return only the hardcode and substring method if those trigger, so we do not overload the student of comments --- .../loglevels/AvoidUsingStringFormat.java | 19 ---------------- .../loglevels/LogLevelsAnalyzer.java | 22 ++++++++++--------- .../loglevels/UseSubstringMethod.java | 18 --------------- .../analyzer/AnalyzerIntegrationTest.java | 1 + ...loglevels.HardCodingLogLevels.approved.txt | 8 ------- ...s.NotUsingSubstringOnLogLevel.approved.txt | 5 +---- ...ls.NotUsingSubstringOnMessage.approved.txt | 5 +---- ...t.loglevels.UsingStringFormat.approved.txt | 2 +- .../NotUsingSubstringOnMessage.java | 3 +-- .../expected_analysis.json | 2 +- 10 files changed, 18 insertions(+), 67 deletions(-) delete mode 100644 src/main/java/analyzer/exercises/loglevels/AvoidUsingStringFormat.java diff --git a/src/main/java/analyzer/exercises/loglevels/AvoidUsingStringFormat.java b/src/main/java/analyzer/exercises/loglevels/AvoidUsingStringFormat.java deleted file mode 100644 index e346b1b9..00000000 --- a/src/main/java/analyzer/exercises/loglevels/AvoidUsingStringFormat.java +++ /dev/null @@ -1,19 +0,0 @@ -package analyzer.exercises.loglevels; - -import analyzer.Comment; - -/** - * @see Markdown Template - */ -class AvoidUsingStringFormat extends Comment { - - @Override - public String getKey() { - return "java.log-levels.avoid_using_string_format"; - } - - @Override - public Type getType() { - return Type.INFORMATIVE; - } -} diff --git a/src/main/java/analyzer/exercises/loglevels/LogLevelsAnalyzer.java b/src/main/java/analyzer/exercises/loglevels/LogLevelsAnalyzer.java index 789467cf..7e93e3be 100644 --- a/src/main/java/analyzer/exercises/loglevels/LogLevelsAnalyzer.java +++ b/src/main/java/analyzer/exercises/loglevels/LogLevelsAnalyzer.java @@ -42,6 +42,12 @@ public void analyze(Solution solution, OutputCollector output) { public void visit(MethodDeclaration node, OutputCollector output) { if (containsHarcodedString(node)) { output.addComment(new DoNotHardcodeLogLevels()); + return; + } + + if (!node.getNameAsString().equals(REFORMAT) && doesNotCallMethod(node, SUBSTRING)) { + output.addComment(new UseSubstringMethod()); + return; } if (node.getNameAsString().equals(REFORMAT) && doesNotCallMethod(node, MESSAGE)) { @@ -52,16 +58,8 @@ public void visit(MethodDeclaration node, OutputCollector output) { output.addComment(new ReuseCode(REFORMAT, LOG_LEVEL)); } - if (node.getNameAsString().equals(MESSAGE) && doesNotCallMethod(node, SUBSTRING)) { - output.addComment(new UseSubstringMethod(MESSAGE, SUBSTRING)); - } - - if (node.getNameAsString().equals(LOG_LEVEL) && doesNotCallMethod(node, SUBSTRING)) { - output.addComment(new UseSubstringMethod(LOG_LEVEL, SUBSTRING)); - } - - if (node.getNameAsString().equals(REFORMAT) && !doesNotCallMethod(node, FORMAT)) { - output.addComment(new AvoidUsingStringFormat()); + if (node.getNameAsString().equals(REFORMAT) && callsMethod(node, FORMAT)) { + output.addComment(new PreferStringConcatenation()); } super.visit(node, output); @@ -78,4 +76,8 @@ private static boolean containsHarcodedString(MethodDeclaration node) { private static boolean doesNotCallMethod(MethodDeclaration node, String otherMethodName) { return node.findAll(MethodCallExpr.class, x -> x.getNameAsString().contains(otherMethodName)).isEmpty(); } + + private static boolean callsMethod(MethodDeclaration node, String otherMethodName) { + return !node.findAll(MethodCallExpr.class, x -> x.getNameAsString().contains(otherMethodName)).isEmpty(); + } } diff --git a/src/main/java/analyzer/exercises/loglevels/UseSubstringMethod.java b/src/main/java/analyzer/exercises/loglevels/UseSubstringMethod.java index 9abad723..c7388bf9 100644 --- a/src/main/java/analyzer/exercises/loglevels/UseSubstringMethod.java +++ b/src/main/java/analyzer/exercises/loglevels/UseSubstringMethod.java @@ -2,34 +2,16 @@ import analyzer.Comment; -import java.util.Map; - /** * @see Markdown Template */ class UseSubstringMethod extends Comment { - private final String callingMethod; - private final String methodToCall; - - - UseSubstringMethod(String callingMethod, String methodToCall) { - this.callingMethod = callingMethod; - this.methodToCall = methodToCall; - } @Override public String getKey() { return "java.log-levels.use_substring_method"; } - @Override - public Map getParameters() { - return Map.of( - "callingMethod", this.callingMethod, - "methodToCall", this.methodToCall - ); - } - @Override public Type getType() { return Type.ACTIONABLE; diff --git a/src/test/java/analyzer/AnalyzerIntegrationTest.java b/src/test/java/analyzer/AnalyzerIntegrationTest.java index 3c65cd31..d81f363b 100644 --- a/src/test/java/analyzer/AnalyzerIntegrationTest.java +++ b/src/test/java/analyzer/AnalyzerIntegrationTest.java @@ -139,6 +139,7 @@ void needforspeed(String scenario) throws IOException { "NoReuseOfBothMethods", "NotUsingSubstringOnLogLevel", "NotUsingSubstringOnMessage", + "NotUsingSubstringOnBothMethods", "UsingStringFormat" }) void loglevels(String scenario) throws IOException { diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.HardCodingLogLevels.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.HardCodingLogLevels.approved.txt index c89f0beb..a78ee576 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.HardCodingLogLevels.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.HardCodingLogLevels.approved.txt @@ -5,14 +5,6 @@ "params": {}, "type": "essential" }, - { - "comment": "java.log-levels.use_substring_method", - "params": { - "callingMethod": "message", - "methodToCall": "substring" - }, - "type": "actionable" - }, { "comment": "java.general.feedback_request", "params": {}, diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.NotUsingSubstringOnLogLevel.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.NotUsingSubstringOnLogLevel.approved.txt index d266bd5d..4874f89c 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.NotUsingSubstringOnLogLevel.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.NotUsingSubstringOnLogLevel.approved.txt @@ -2,10 +2,7 @@ "comments": [ { "comment": "java.log-levels.use_substring_method", - "params": { - "callingMethod": "logLevel", - "methodToCall": "substring" - }, + "params": {}, "type": "actionable" }, { diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.NotUsingSubstringOnMessage.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.NotUsingSubstringOnMessage.approved.txt index ff235c4f..4874f89c 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.NotUsingSubstringOnMessage.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.NotUsingSubstringOnMessage.approved.txt @@ -2,10 +2,7 @@ "comments": [ { "comment": "java.log-levels.use_substring_method", - "params": { - "callingMethod": "message", - "methodToCall": "substring" - }, + "params": {}, "type": "actionable" }, { diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.UsingStringFormat.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.UsingStringFormat.approved.txt index 24409217..19e8bdef 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.UsingStringFormat.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.loglevels.UsingStringFormat.approved.txt @@ -1,7 +1,7 @@ { "comments": [ { - "comment": "java.log-levels.avoid_using_string_format", + "comment": "java.log-levels.prefer_string_concatenation", "params": {}, "type": "informative" }, diff --git a/src/test/resources/scenarios/log-levels/NotUsingSubstringOnMessage.java b/src/test/resources/scenarios/log-levels/NotUsingSubstringOnMessage.java index a3c49ed7..79ae75cc 100644 --- a/src/test/resources/scenarios/log-levels/NotUsingSubstringOnMessage.java +++ b/src/test/resources/scenarios/log-levels/NotUsingSubstringOnMessage.java @@ -2,8 +2,7 @@ public class LogLevels { public static String message(String logLine) { - return logLine.split("]: ")[1] - .trim(); + return logLine.split("]: ")[1].trim(); } public static String logLevel(String logLine) { diff --git a/tests/log-levels/using-string-format/expected_analysis.json b/tests/log-levels/using-string-format/expected_analysis.json index 39065925..265a2966 100644 --- a/tests/log-levels/using-string-format/expected_analysis.json +++ b/tests/log-levels/using-string-format/expected_analysis.json @@ -1,7 +1,7 @@ { "comments": [ { - "comment": "java.log-levels.avoid_using_string_format", + "comment": "java.log-levels.prefer_string_concatenation", "params": {}, "type": "informative" },