From 81b673371f53f7689ddb59e13761597f65abee4a Mon Sep 17 00:00:00 2001 From: manumafe98 Date: Mon, 4 Mar 2024 20:23:42 -0300 Subject: [PATCH 1/7] Adding Secrets Analyzer with Comments --- .../exercises/secrets/ImplementOperator.java | 36 ++++++++ .../exercises/secrets/PreferBitwiseNot.java | 19 +++++ .../exercises/secrets/SecretsAnalyzer.java | 83 +++++++++++++++++++ 3 files changed, 138 insertions(+) create mode 100644 src/main/java/analyzer/exercises/secrets/ImplementOperator.java create mode 100644 src/main/java/analyzer/exercises/secrets/PreferBitwiseNot.java create mode 100644 src/main/java/analyzer/exercises/secrets/SecretsAnalyzer.java diff --git a/src/main/java/analyzer/exercises/secrets/ImplementOperator.java b/src/main/java/analyzer/exercises/secrets/ImplementOperator.java new file mode 100644 index 00000000..a630de77 --- /dev/null +++ b/src/main/java/analyzer/exercises/secrets/ImplementOperator.java @@ -0,0 +1,36 @@ +package analyzer.exercises.secrets; + +import analyzer.Comment; + +import java.util.Map; + +/** + * @see Markdown Template + */ +class ImplementOperator extends Comment { + private final String operatorToUse; + private final String calledMethod; + + public ImplementOperator(String operatorToUse, String calledMethod) { + this.operatorToUse = operatorToUse; + this.calledMethod = calledMethod; + } + + @Override + public String getKey() { + return "java.secrets.implement_operator"; + } + + @Override + public Map getParameters() { + return Map.of( + "operatorToUse", this.operatorToUse, + "calledMethod", this.calledMethod + ); + } + + @Override + public Type getType() { + return Type.ESSENTIAL; + } +} diff --git a/src/main/java/analyzer/exercises/secrets/PreferBitwiseNot.java b/src/main/java/analyzer/exercises/secrets/PreferBitwiseNot.java new file mode 100644 index 00000000..709b3724 --- /dev/null +++ b/src/main/java/analyzer/exercises/secrets/PreferBitwiseNot.java @@ -0,0 +1,19 @@ +package analyzer.exercises.secrets; + +import analyzer.Comment; + +/** + * @see Markdown Template + */ +class PreferBitwiseNot extends Comment { + + @Override + public String getKey() { + return "java.secrets.prefer_bitwise_not"; + } + + @Override + public Type getType() { + return Type.INFORMATIVE; + } +} diff --git a/src/main/java/analyzer/exercises/secrets/SecretsAnalyzer.java b/src/main/java/analyzer/exercises/secrets/SecretsAnalyzer.java new file mode 100644 index 00000000..f1ac2616 --- /dev/null +++ b/src/main/java/analyzer/exercises/secrets/SecretsAnalyzer.java @@ -0,0 +1,83 @@ +package analyzer.exercises.secrets; + +import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.body.MethodDeclaration; +import com.github.javaparser.ast.expr.BinaryExpr; +import com.github.javaparser.ast.expr.UnaryExpr; +import com.github.javaparser.ast.visitor.VoidVisitorAdapter; + +import analyzer.Analyzer; +import analyzer.OutputCollector; +import analyzer.Solution; +import analyzer.comments.ExemplarSolution; + +/** + * The {@link SecretsAnalyzer} is the analyzer implementation for the {@code secrets} practice exercise. + * It extends from the {@link VoidVisitorAdapter} and uses the visitor pattern to traverse each compilation unit. + * + * @see The secrets exercise on the Java track + */ +public class SecretsAnalyzer extends VoidVisitorAdapter implements Analyzer { + private static final String EXERCISE_NAME = "Secrets"; + private static final String SHIFT_BACK = "shiftBack"; + private static final String SET_BITS = "setBits"; + private static final String FLIP_BITS = "flipBits"; + private static final String CLEAR_BITS = "clearBits"; + + @Override + public void analyze(Solution solution, OutputCollector output) { + for (CompilationUnit compilationUnit : solution.getCompilationUnits()) { + compilationUnit.accept(this, output); + } + + if (output.getComments().isEmpty()) { + output.addComment(new ExemplarSolution(EXERCISE_NAME)); + } + } + + @Override + public void visit(MethodDeclaration node, OutputCollector output) { + + if (node.getNameAsString().equals(SHIFT_BACK) && doesNotImplementUnsignedRightShift(node)) { + output.addComment(new ImplementOperator(">>>", SHIFT_BACK)); + } + + if (node.getNameAsString().equals(SET_BITS) && doesNotImplementBitwiseOr(node)) { + output.addComment(new ImplementOperator("|", SET_BITS)); + } + + if (node.getNameAsString().equals(FLIP_BITS) && doesNotImplementBitwiseXor(node)) { + output.addComment(new ImplementOperator("^", FLIP_BITS)); + } + + if (node.getNameAsString().equals(CLEAR_BITS) && doesNotImplementBitwiseAnd(node)) { + output.addComment(new ImplementOperator("&", CLEAR_BITS)); + } + + if (node.getNameAsString().equals(CLEAR_BITS) && doesNotImplementBitwiseNot(node)) { + output.addComment(new PreferBitwiseNot()); + } + + super.visit(node, output); + } + + private static boolean doesNotImplementBitwiseAnd(MethodDeclaration node) { + return node.findAll(BinaryExpr.class, x -> x.getOperator() == BinaryExpr.Operator.BINARY_AND).isEmpty(); + } + + private static boolean doesNotImplementBitwiseOr(MethodDeclaration node) { + return node.findAll(BinaryExpr.class, x -> x.getOperator() == BinaryExpr.Operator.BINARY_OR).isEmpty(); + } + + private static boolean doesNotImplementBitwiseXor(MethodDeclaration node) { + return node.findAll(BinaryExpr.class, x -> x.getOperator() == BinaryExpr.Operator.XOR).isEmpty(); + } + + private static boolean doesNotImplementBitwiseNot(MethodDeclaration node) { + return node.findAll(UnaryExpr.class, x -> x.getOperator() == UnaryExpr.Operator.BITWISE_COMPLEMENT).isEmpty(); + } + + private static boolean doesNotImplementUnsignedRightShift(MethodDeclaration node) { + return node.findAll(BinaryExpr.class, x -> x.getOperator() == BinaryExpr.Operator.UNSIGNED_RIGHT_SHIFT).isEmpty(); + } +} From a525a403af5bf5f73768fdf2d7106b9f1d39fc07 Mon Sep 17 00:00:00 2001 From: manumafe98 Date: Mon, 4 Mar 2024 20:54:09 -0300 Subject: [PATCH 2/7] Adding analyzer tests --- src/main/java/analyzer/AnalyzerRoot.java | 2 ++ .../analyzer/AnalyzerIntegrationTest.java | 17 +++++++++++++++++ ...Test.secrets.ExemplarSolution.approved.txt | 11 +++++++++++ ...st.secrets.NotUsingBitwiseAnd.approved.txt | 17 +++++++++++++++++ ...st.secrets.NotUsingBitwiseNot.approved.txt | 14 ++++++++++++++ ...est.secrets.NotUsingBitwiseOr.approved.txt | 17 +++++++++++++++++ ...st.secrets.NotUsingBitwiseXor.approved.txt | 17 +++++++++++++++++ ...ts.NotUsingUnsignedRightShift.approved.txt | 17 +++++++++++++++++ .../scenarios/secrets/ExemplarSolution.java | 19 +++++++++++++++++++ .../scenarios/secrets/NotUsingBitwiseAnd.java | 19 +++++++++++++++++++ .../scenarios/secrets/NotUsingBitwiseNot.java | 17 +++++++++++++++++ .../scenarios/secrets/NotUsingBitwiseOr.java | 19 +++++++++++++++++++ .../scenarios/secrets/NotUsingBitwiseXor.java | 19 +++++++++++++++++++ .../secrets/NotUsingUnsignedRightShift.java | 19 +++++++++++++++++++ 14 files changed, 224 insertions(+) create mode 100644 src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.ExemplarSolution.approved.txt create mode 100644 src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseAnd.approved.txt create mode 100644 src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseNot.approved.txt create mode 100644 src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseOr.approved.txt create mode 100644 src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseXor.approved.txt create mode 100644 src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingUnsignedRightShift.approved.txt create mode 100644 src/test/resources/scenarios/secrets/ExemplarSolution.java create mode 100644 src/test/resources/scenarios/secrets/NotUsingBitwiseAnd.java create mode 100644 src/test/resources/scenarios/secrets/NotUsingBitwiseNot.java create mode 100644 src/test/resources/scenarios/secrets/NotUsingBitwiseOr.java create mode 100644 src/test/resources/scenarios/secrets/NotUsingBitwiseXor.java create mode 100644 src/test/resources/scenarios/secrets/NotUsingUnsignedRightShift.java diff --git a/src/main/java/analyzer/AnalyzerRoot.java b/src/main/java/analyzer/AnalyzerRoot.java index ecb0815e..265d2729 100644 --- a/src/main/java/analyzer/AnalyzerRoot.java +++ b/src/main/java/analyzer/AnalyzerRoot.java @@ -8,6 +8,7 @@ import analyzer.exercises.leap.LeapAnalyzer; import analyzer.exercises.loglevels.LogLevelsAnalyzer; import analyzer.exercises.needforspeed.NeedForSpeedAnalyzer; +import analyzer.exercises.secrets.SecretsAnalyzer; import analyzer.exercises.twofer.TwoferAnalyzer; import java.util.ArrayList; @@ -55,6 +56,7 @@ private static List createAnalyzers(String slug) { case "log-levels" -> analyzers.add(new LogLevelsAnalyzer()); case "need-for-speed" -> analyzers.add(new NeedForSpeedAnalyzer()); case "two-fer" -> analyzers.add(new TwoferAnalyzer()); + case "secrets" -> analyzers.add(new SecretsAnalyzer()); } return List.copyOf(analyzers); diff --git a/src/test/java/analyzer/AnalyzerIntegrationTest.java b/src/test/java/analyzer/AnalyzerIntegrationTest.java index d81f363b..36f37f39 100644 --- a/src/test/java/analyzer/AnalyzerIntegrationTest.java +++ b/src/test/java/analyzer/AnalyzerIntegrationTest.java @@ -149,4 +149,21 @@ void loglevels(String scenario) throws IOException { Approvals.verify(serialize(output.analysis()), Approvals.NAMES.withParameters(scenario)); } + + @ParameterizedTest + @ValueSource(strings = { + "ExemplarSolution", + "NotUsingBitwiseAnd", + "NotUsingBitwiseNot", + "NotUsingBitwiseOr", + "NotUsingBitwiseXor", + "NotUsingUnsignedRightShift" + }) + void secrets(String scenario) throws IOException { + var path = Path.of("secrets", scenario + ".java"); + var solution = new SolutionFromFiles("secrets", SCENARIOS.resolve(path)); + var output = AnalyzerRoot.analyze(solution); + + Approvals.verify(serialize(output.analysis()), Approvals.NAMES.withParameters(scenario)); + } } diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.ExemplarSolution.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.ExemplarSolution.approved.txt new file mode 100644 index 00000000..a9ba9d58 --- /dev/null +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.ExemplarSolution.approved.txt @@ -0,0 +1,11 @@ +{ + "comments": [ + { + "comment": "java.general.exemplar", + "params": { + "exerciseName": "Secrets" + }, + "type": "celebratory" + } + ] +} \ No newline at end of file diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseAnd.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseAnd.approved.txt new file mode 100644 index 00000000..9e5ee387 --- /dev/null +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseAnd.approved.txt @@ -0,0 +1,17 @@ +{ + "comments": [ + { + "comment": "java.secrets.implement_operator", + "params": { + "operatorToUse": "&", + "calledMethod": "clearBits" + }, + "type": "essential" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] +} \ No newline at end of file diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseNot.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseNot.approved.txt new file mode 100644 index 00000000..2c07fb67 --- /dev/null +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseNot.approved.txt @@ -0,0 +1,14 @@ +{ + "comments": [ + { + "comment": "java.secrets.prefer_bitwise_not", + "params": {}, + "type": "informative" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] +} \ No newline at end of file diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseOr.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseOr.approved.txt new file mode 100644 index 00000000..368ce415 --- /dev/null +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseOr.approved.txt @@ -0,0 +1,17 @@ +{ + "comments": [ + { + "comment": "java.secrets.implement_operator", + "params": { + "operatorToUse": "|", + "calledMethod": "setBits" + }, + "type": "essential" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] +} \ No newline at end of file diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseXor.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseXor.approved.txt new file mode 100644 index 00000000..80bc9238 --- /dev/null +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseXor.approved.txt @@ -0,0 +1,17 @@ +{ + "comments": [ + { + "comment": "java.secrets.implement_operator", + "params": { + "operatorToUse": "^", + "calledMethod": "flipBits" + }, + "type": "essential" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] +} \ No newline at end of file diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingUnsignedRightShift.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingUnsignedRightShift.approved.txt new file mode 100644 index 00000000..d0f4cb75 --- /dev/null +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingUnsignedRightShift.approved.txt @@ -0,0 +1,17 @@ +{ + "comments": [ + { + "comment": "java.secrets.implement_operator", + "params": { + "operatorToUse": ">>>", + "calledMethod": "shiftBack" + }, + "type": "essential" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] +} \ No newline at end of file diff --git a/src/test/resources/scenarios/secrets/ExemplarSolution.java b/src/test/resources/scenarios/secrets/ExemplarSolution.java new file mode 100644 index 00000000..15311aa5 --- /dev/null +++ b/src/test/resources/scenarios/secrets/ExemplarSolution.java @@ -0,0 +1,19 @@ +package scenarios.secrets; + +public class Secrets { + public static int shiftBack(int value, int amount) { + return value >>> amount; + } + + public static int setBits(int value, int mask) { + return value | mask; + } + + public static int flipBits(int value, int mask) { + return value ^ mask; + } + + public static int clearBits(int value, int mask) { + return value & ~mask; + } +} diff --git a/src/test/resources/scenarios/secrets/NotUsingBitwiseAnd.java b/src/test/resources/scenarios/secrets/NotUsingBitwiseAnd.java new file mode 100644 index 00000000..64a34035 --- /dev/null +++ b/src/test/resources/scenarios/secrets/NotUsingBitwiseAnd.java @@ -0,0 +1,19 @@ +package scenarios.secrets; + +public class Secrets { + public static int shiftBack(int value, int amount) { + return value >>> amount; + } + + public static int setBits(int value, int mask) { + return value | mask; + } + + public static int flipBits(int value, int mask) { + return value ^ mask; + } + + public static int clearBits(int value, int mask) { + return ~(~value | mask); + } +} diff --git a/src/test/resources/scenarios/secrets/NotUsingBitwiseNot.java b/src/test/resources/scenarios/secrets/NotUsingBitwiseNot.java new file mode 100644 index 00000000..45bb8a5c --- /dev/null +++ b/src/test/resources/scenarios/secrets/NotUsingBitwiseNot.java @@ -0,0 +1,17 @@ +package scenarios.secrets; + +public class Secrets { + public static int shiftBack(int value, int amount) { + return value >>> amount; + } + public static int setBits(int value, int mask) { + return value | mask; + } + public static int flipBits(int value, int mask) { + return value ^ mask; + } + public static int clearBits(int value, int mask) { + int and = value & mask; + return and ^ value; + } +} diff --git a/src/test/resources/scenarios/secrets/NotUsingBitwiseOr.java b/src/test/resources/scenarios/secrets/NotUsingBitwiseOr.java new file mode 100644 index 00000000..25879a6c --- /dev/null +++ b/src/test/resources/scenarios/secrets/NotUsingBitwiseOr.java @@ -0,0 +1,19 @@ +package scenarios.secrets; + +public class Secrets { + public static int shiftBack(int value, int amount) { + return value >>> amount; + } + + public static int setBits(int value, int mask) { + return value + mask - (value & mask); + } + + public static int flipBits(int value, int mask) { + return value ^ mask; + } + + public static int clearBits(int value, int mask) { + return value & ~mask; + } +} diff --git a/src/test/resources/scenarios/secrets/NotUsingBitwiseXor.java b/src/test/resources/scenarios/secrets/NotUsingBitwiseXor.java new file mode 100644 index 00000000..b71207c5 --- /dev/null +++ b/src/test/resources/scenarios/secrets/NotUsingBitwiseXor.java @@ -0,0 +1,19 @@ +package scenarios.secrets; + +public class Secrets { + public static int shiftBack(int value, int amount) { + return value >>> amount; + } + + public static int setBits(int value, int mask) { + return value | mask; + } + + public static int flipBits(int value, int mask) { + return (value & ~mask) | (~value & mask); + } + + public static int clearBits(int value, int mask) { + return value & ~mask; + } +} diff --git a/src/test/resources/scenarios/secrets/NotUsingUnsignedRightShift.java b/src/test/resources/scenarios/secrets/NotUsingUnsignedRightShift.java new file mode 100644 index 00000000..5a7cc41c --- /dev/null +++ b/src/test/resources/scenarios/secrets/NotUsingUnsignedRightShift.java @@ -0,0 +1,19 @@ +package scenarios.secrets; + +public class Secrets { + public static int shiftBack(int value, int amount) { + return (value >> amount) & ~(Integer.MIN_VALUE >> (amount - 1)); + } + + public static int setBits(int value, int mask) { + return value | mask; + } + + public static int flipBits(int value, int mask) { + return value ^ mask; + } + + public static int clearBits(int value, int mask) { + return value & ~mask; + } +} From 14b904290b18b69b03ccc5a76e57664e935059cb Mon Sep 17 00:00:00 2001 From: manumafe98 Date: Mon, 4 Mar 2024 21:08:17 -0300 Subject: [PATCH 3/7] Adding smoke tests --- ...st.secrets.NotUsingBitwiseAnd.approved.txt | 4 ++-- ...est.secrets.NotUsingBitwiseOr.approved.txt | 4 ++-- ...st.secrets.NotUsingBitwiseXor.approved.txt | 4 ++-- ...ts.NotUsingUnsignedRightShift.approved.txt | 4 ++-- .../exemplar-solution/.meta/config.json | 24 +++++++++++++++++++ .../exemplar-solution/expected_analysis.json | 11 +++++++++ .../exemplar-solution/expected_tags.json | 3 +++ .../src/main/java/Secrets.java | 17 +++++++++++++ .../no-bitwise-not-used/.meta/config.json | 24 +++++++++++++++++++ .../expected_analysis.json | 14 +++++++++++ .../no-bitwise-not-used/expected_tags.json | 3 +++ .../src/main/java/Secrets.java | 15 ++++++++++++ .../no-bitwise-or-used/.meta/config.json | 24 +++++++++++++++++++ .../no-bitwise-or-used/expected_analysis.json | 17 +++++++++++++ .../no-bitwise-or-used/expected_tags.json | 3 +++ .../src/main/java/Secrets.java | 17 +++++++++++++ .../.meta/config.json | 24 +++++++++++++++++++ .../expected_analysis.json | 17 +++++++++++++ .../expected_tags.json | 3 +++ .../src/main/java/Secrets.java | 17 +++++++++++++ 20 files changed, 241 insertions(+), 8 deletions(-) create mode 100644 tests/secrets/exemplar-solution/.meta/config.json create mode 100644 tests/secrets/exemplar-solution/expected_analysis.json create mode 100644 tests/secrets/exemplar-solution/expected_tags.json create mode 100644 tests/secrets/exemplar-solution/src/main/java/Secrets.java create mode 100644 tests/secrets/no-bitwise-not-used/.meta/config.json create mode 100644 tests/secrets/no-bitwise-not-used/expected_analysis.json create mode 100644 tests/secrets/no-bitwise-not-used/expected_tags.json create mode 100644 tests/secrets/no-bitwise-not-used/src/main/java/Secrets.java create mode 100644 tests/secrets/no-bitwise-or-used/.meta/config.json create mode 100644 tests/secrets/no-bitwise-or-used/expected_analysis.json create mode 100644 tests/secrets/no-bitwise-or-used/expected_tags.json create mode 100644 tests/secrets/no-bitwise-or-used/src/main/java/Secrets.java create mode 100644 tests/secrets/no-unsigned-right-shift-used/.meta/config.json create mode 100644 tests/secrets/no-unsigned-right-shift-used/expected_analysis.json create mode 100644 tests/secrets/no-unsigned-right-shift-used/expected_tags.json create mode 100644 tests/secrets/no-unsigned-right-shift-used/src/main/java/Secrets.java diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseAnd.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseAnd.approved.txt index 9e5ee387..3d5dbfbf 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseAnd.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseAnd.approved.txt @@ -3,8 +3,8 @@ { "comment": "java.secrets.implement_operator", "params": { - "operatorToUse": "&", - "calledMethod": "clearBits" + "calledMethod": "clearBits", + "operatorToUse": "\u0026" }, "type": "essential" }, diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseOr.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseOr.approved.txt index 368ce415..f0a60b76 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseOr.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseOr.approved.txt @@ -3,8 +3,8 @@ { "comment": "java.secrets.implement_operator", "params": { - "operatorToUse": "|", - "calledMethod": "setBits" + "calledMethod": "setBits", + "operatorToUse": "|" }, "type": "essential" }, diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseXor.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseXor.approved.txt index 80bc9238..60bbf5be 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseXor.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseXor.approved.txt @@ -3,8 +3,8 @@ { "comment": "java.secrets.implement_operator", "params": { - "operatorToUse": "^", - "calledMethod": "flipBits" + "calledMethod": "flipBits", + "operatorToUse": "^" }, "type": "essential" }, diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingUnsignedRightShift.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingUnsignedRightShift.approved.txt index d0f4cb75..0322c920 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingUnsignedRightShift.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingUnsignedRightShift.approved.txt @@ -3,8 +3,8 @@ { "comment": "java.secrets.implement_operator", "params": { - "operatorToUse": ">>>", - "calledMethod": "shiftBack" + "calledMethod": "shiftBack", + "operatorToUse": "\u003e\u003e\u003e" }, "type": "essential" }, diff --git a/tests/secrets/exemplar-solution/.meta/config.json b/tests/secrets/exemplar-solution/.meta/config.json new file mode 100644 index 00000000..689a8774 --- /dev/null +++ b/tests/secrets/exemplar-solution/.meta/config.json @@ -0,0 +1,24 @@ +{ + "authors": [ + "kahgoh" + ], + "files": { + "solution": [ + "src/main/java/Secrets.java" + ], + "test": [ + "src/test/java/SecretsTest.java" + ], + "exemplar": [ + ".meta/src/reference/java/Secrets.java" + ], + "invalidator": [ + "build.gradle" + ] + }, + "forked_from": [ + "crystal/secrets" + ], + "icon": "secrets", + "blurb": "Learn about bit manipulation by writing a program to decrypt a message." +} \ No newline at end of file diff --git a/tests/secrets/exemplar-solution/expected_analysis.json b/tests/secrets/exemplar-solution/expected_analysis.json new file mode 100644 index 00000000..3a83a329 --- /dev/null +++ b/tests/secrets/exemplar-solution/expected_analysis.json @@ -0,0 +1,11 @@ +{ + "comments": [ + { + "comment": "java.general.exemplar", + "params": { + "exerciseName": "Secrets" + }, + "type": "celebratory" + } + ] +} \ No newline at end of file diff --git a/tests/secrets/exemplar-solution/expected_tags.json b/tests/secrets/exemplar-solution/expected_tags.json new file mode 100644 index 00000000..eb25b190 --- /dev/null +++ b/tests/secrets/exemplar-solution/expected_tags.json @@ -0,0 +1,3 @@ +{ + "tags": [] +} \ No newline at end of file diff --git a/tests/secrets/exemplar-solution/src/main/java/Secrets.java b/tests/secrets/exemplar-solution/src/main/java/Secrets.java new file mode 100644 index 00000000..9ef0a868 --- /dev/null +++ b/tests/secrets/exemplar-solution/src/main/java/Secrets.java @@ -0,0 +1,17 @@ +class Secrets { + public static int shiftBack(int value, int amount) { + return value >>> amount; + } + + public static int setBits(int value, int mask) { + return value | mask; + } + + public static int flipBits(int value, int mask) { + return value ^ mask; + } + + public static int clearBits(int value, int mask) { + return value & ~mask; + } +} diff --git a/tests/secrets/no-bitwise-not-used/.meta/config.json b/tests/secrets/no-bitwise-not-used/.meta/config.json new file mode 100644 index 00000000..689a8774 --- /dev/null +++ b/tests/secrets/no-bitwise-not-used/.meta/config.json @@ -0,0 +1,24 @@ +{ + "authors": [ + "kahgoh" + ], + "files": { + "solution": [ + "src/main/java/Secrets.java" + ], + "test": [ + "src/test/java/SecretsTest.java" + ], + "exemplar": [ + ".meta/src/reference/java/Secrets.java" + ], + "invalidator": [ + "build.gradle" + ] + }, + "forked_from": [ + "crystal/secrets" + ], + "icon": "secrets", + "blurb": "Learn about bit manipulation by writing a program to decrypt a message." +} \ No newline at end of file diff --git a/tests/secrets/no-bitwise-not-used/expected_analysis.json b/tests/secrets/no-bitwise-not-used/expected_analysis.json new file mode 100644 index 00000000..61bb17f7 --- /dev/null +++ b/tests/secrets/no-bitwise-not-used/expected_analysis.json @@ -0,0 +1,14 @@ +{ + "comments": [ + { + "comment": "java.secrets.prefer_bitwise_not", + "params": {}, + "type": "informative" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] +} \ No newline at end of file diff --git a/tests/secrets/no-bitwise-not-used/expected_tags.json b/tests/secrets/no-bitwise-not-used/expected_tags.json new file mode 100644 index 00000000..eb25b190 --- /dev/null +++ b/tests/secrets/no-bitwise-not-used/expected_tags.json @@ -0,0 +1,3 @@ +{ + "tags": [] +} \ No newline at end of file diff --git a/tests/secrets/no-bitwise-not-used/src/main/java/Secrets.java b/tests/secrets/no-bitwise-not-used/src/main/java/Secrets.java new file mode 100644 index 00000000..6575f7cb --- /dev/null +++ b/tests/secrets/no-bitwise-not-used/src/main/java/Secrets.java @@ -0,0 +1,15 @@ +class Secrets { + public static int shiftBack(int value, int amount) { + return value >>> amount; + } + public static int setBits(int value, int mask) { + return value | mask; + } + public static int flipBits(int value, int mask) { + return value ^ mask; + } + public static int clearBits(int value, int mask) { + int and = value & mask; + return and ^ value; + } +} diff --git a/tests/secrets/no-bitwise-or-used/.meta/config.json b/tests/secrets/no-bitwise-or-used/.meta/config.json new file mode 100644 index 00000000..689a8774 --- /dev/null +++ b/tests/secrets/no-bitwise-or-used/.meta/config.json @@ -0,0 +1,24 @@ +{ + "authors": [ + "kahgoh" + ], + "files": { + "solution": [ + "src/main/java/Secrets.java" + ], + "test": [ + "src/test/java/SecretsTest.java" + ], + "exemplar": [ + ".meta/src/reference/java/Secrets.java" + ], + "invalidator": [ + "build.gradle" + ] + }, + "forked_from": [ + "crystal/secrets" + ], + "icon": "secrets", + "blurb": "Learn about bit manipulation by writing a program to decrypt a message." +} \ No newline at end of file diff --git a/tests/secrets/no-bitwise-or-used/expected_analysis.json b/tests/secrets/no-bitwise-or-used/expected_analysis.json new file mode 100644 index 00000000..196712c6 --- /dev/null +++ b/tests/secrets/no-bitwise-or-used/expected_analysis.json @@ -0,0 +1,17 @@ +{ + "comments": [ + { + "comment": "java.secrets.implement_operator", + "params": { + "calledMethod": "setBits", + "operatorToUse": "|" + }, + "type": "essential" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] +} \ No newline at end of file diff --git a/tests/secrets/no-bitwise-or-used/expected_tags.json b/tests/secrets/no-bitwise-or-used/expected_tags.json new file mode 100644 index 00000000..eb25b190 --- /dev/null +++ b/tests/secrets/no-bitwise-or-used/expected_tags.json @@ -0,0 +1,3 @@ +{ + "tags": [] +} \ No newline at end of file diff --git a/tests/secrets/no-bitwise-or-used/src/main/java/Secrets.java b/tests/secrets/no-bitwise-or-used/src/main/java/Secrets.java new file mode 100644 index 00000000..84cef5fb --- /dev/null +++ b/tests/secrets/no-bitwise-or-used/src/main/java/Secrets.java @@ -0,0 +1,17 @@ +class Secrets { + public static int shiftBack(int value, int amount) { + return value >>> amount; + } + + public static int setBits(int value, int mask) { + return value + mask - (value & mask); + } + + public static int flipBits(int value, int mask) { + return value ^ mask; + } + + public static int clearBits(int value, int mask) { + return value & ~mask; + } +} diff --git a/tests/secrets/no-unsigned-right-shift-used/.meta/config.json b/tests/secrets/no-unsigned-right-shift-used/.meta/config.json new file mode 100644 index 00000000..689a8774 --- /dev/null +++ b/tests/secrets/no-unsigned-right-shift-used/.meta/config.json @@ -0,0 +1,24 @@ +{ + "authors": [ + "kahgoh" + ], + "files": { + "solution": [ + "src/main/java/Secrets.java" + ], + "test": [ + "src/test/java/SecretsTest.java" + ], + "exemplar": [ + ".meta/src/reference/java/Secrets.java" + ], + "invalidator": [ + "build.gradle" + ] + }, + "forked_from": [ + "crystal/secrets" + ], + "icon": "secrets", + "blurb": "Learn about bit manipulation by writing a program to decrypt a message." +} \ No newline at end of file diff --git a/tests/secrets/no-unsigned-right-shift-used/expected_analysis.json b/tests/secrets/no-unsigned-right-shift-used/expected_analysis.json new file mode 100644 index 00000000..dce41a94 --- /dev/null +++ b/tests/secrets/no-unsigned-right-shift-used/expected_analysis.json @@ -0,0 +1,17 @@ +{ + "comments": [ + { + "comment": "java.secrets.implement_operator", + "params": { + "calledMethod": "shiftBack", + "operatorToUse": "\u003e\u003e\u003e" + }, + "type": "essential" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] +} \ No newline at end of file diff --git a/tests/secrets/no-unsigned-right-shift-used/expected_tags.json b/tests/secrets/no-unsigned-right-shift-used/expected_tags.json new file mode 100644 index 00000000..eb25b190 --- /dev/null +++ b/tests/secrets/no-unsigned-right-shift-used/expected_tags.json @@ -0,0 +1,3 @@ +{ + "tags": [] +} \ No newline at end of file diff --git a/tests/secrets/no-unsigned-right-shift-used/src/main/java/Secrets.java b/tests/secrets/no-unsigned-right-shift-used/src/main/java/Secrets.java new file mode 100644 index 00000000..6b4bf528 --- /dev/null +++ b/tests/secrets/no-unsigned-right-shift-used/src/main/java/Secrets.java @@ -0,0 +1,17 @@ +class Secrets { + public static int shiftBack(int value, int amount) { + return (value >> amount) & ~(Integer.MIN_VALUE >> (amount - 1)); + } + + public static int setBits(int value, int mask) { + return value | mask; + } + + public static int flipBits(int value, int mask) { + return value ^ mask; + } + + public static int clearBits(int value, int mask) { + return value & ~mask; + } +} From 63b9311ec86ceb3dddd0dbfe5bcdc3da9e34fe80 Mon Sep 17 00:00:00 2001 From: manumafe98 Date: Mon, 4 Mar 2024 21:13:17 -0300 Subject: [PATCH 4/7] Fixing smoke tests indenting and prefer bitwise not comment javadoc --- .../exercises/secrets/PreferBitwiseNot.java | 2 +- .../expected_analysis.json | 24 ++++++++-------- .../no-bitwise-or-used/expected_analysis.json | 28 +++++++++---------- .../expected_analysis.json | 28 +++++++++---------- 4 files changed, 41 insertions(+), 41 deletions(-) diff --git a/src/main/java/analyzer/exercises/secrets/PreferBitwiseNot.java b/src/main/java/analyzer/exercises/secrets/PreferBitwiseNot.java index 709b3724..3a6eb380 100644 --- a/src/main/java/analyzer/exercises/secrets/PreferBitwiseNot.java +++ b/src/main/java/analyzer/exercises/secrets/PreferBitwiseNot.java @@ -3,7 +3,7 @@ import analyzer.Comment; /** - * @see Markdown Template + * @see Markdown Template */ class PreferBitwiseNot extends Comment { diff --git a/tests/secrets/no-bitwise-not-used/expected_analysis.json b/tests/secrets/no-bitwise-not-used/expected_analysis.json index 61bb17f7..5f92589b 100644 --- a/tests/secrets/no-bitwise-not-used/expected_analysis.json +++ b/tests/secrets/no-bitwise-not-used/expected_analysis.json @@ -1,14 +1,14 @@ { - "comments": [ - { - "comment": "java.secrets.prefer_bitwise_not", - "params": {}, - "type": "informative" - }, - { - "comment": "java.general.feedback_request", - "params": {}, - "type": "informative" - } - ] + "comments": [ + { + "comment": "java.secrets.prefer_bitwise_not", + "params": {}, + "type": "informative" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] } \ No newline at end of file diff --git a/tests/secrets/no-bitwise-or-used/expected_analysis.json b/tests/secrets/no-bitwise-or-used/expected_analysis.json index 196712c6..700594b3 100644 --- a/tests/secrets/no-bitwise-or-used/expected_analysis.json +++ b/tests/secrets/no-bitwise-or-used/expected_analysis.json @@ -1,17 +1,17 @@ { - "comments": [ - { - "comment": "java.secrets.implement_operator", - "params": { - "calledMethod": "setBits", - "operatorToUse": "|" - }, - "type": "essential" + "comments": [ + { + "comment": "java.secrets.implement_operator", + "params": { + "calledMethod": "setBits", + "operatorToUse": "|" }, - { - "comment": "java.general.feedback_request", - "params": {}, - "type": "informative" - } - ] + "type": "essential" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] } \ No newline at end of file diff --git a/tests/secrets/no-unsigned-right-shift-used/expected_analysis.json b/tests/secrets/no-unsigned-right-shift-used/expected_analysis.json index dce41a94..b9753917 100644 --- a/tests/secrets/no-unsigned-right-shift-used/expected_analysis.json +++ b/tests/secrets/no-unsigned-right-shift-used/expected_analysis.json @@ -1,17 +1,17 @@ { - "comments": [ - { - "comment": "java.secrets.implement_operator", - "params": { - "calledMethod": "shiftBack", - "operatorToUse": "\u003e\u003e\u003e" - }, - "type": "essential" + "comments": [ + { + "comment": "java.secrets.implement_operator", + "params": { + "calledMethod": "shiftBack", + "operatorToUse": "\u003e\u003e\u003e" }, - { - "comment": "java.general.feedback_request", - "params": {}, - "type": "informative" - } - ] + "type": "essential" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] } \ No newline at end of file From 9bbc36a64d0ecef8ce046053d702eca8c71e1ed9 Mon Sep 17 00:00:00 2001 From: manumafe98 Date: Tue, 5 Mar 2024 10:57:06 -0300 Subject: [PATCH 5/7] Applying suggestions Udating Analyzer root to run the analyzer on alphabetical order Changing implement operator comment to use bitwise operator Updating the analyzer to only return 1 essential comment at a time Generalizing the method that checks the usage of the operator Adding extra comment to check for conditional logic Updating preferBitwiseNot to trigger if the student used bitwise and --- src/main/java/analyzer/AnalyzerRoot.java | 2 +- .../secrets/AvoidConditionalLogic.java | 19 +++++++ .../exercises/secrets/SecretsAnalyzer.java | 52 +++++++++++-------- ...tOperator.java => UseBitwiseOperator.java} | 8 +-- .../analyzer/AnalyzerIntegrationTest.java | 3 +- ...st.secrets.NotUsingBitwiseAnd.approved.txt | 2 +- ...est.secrets.NotUsingBitwiseOr.approved.txt | 2 +- ...st.secrets.NotUsingBitwiseXor.approved.txt | 2 +- ...ts.NotUsingUnsignedRightShift.approved.txt | 2 +- ...Test.secrets.UsingIfStatement.approved.txt | 14 +++++ .../scenarios/secrets/UsingIfStatement.java | 23 ++++++++ .../no-bitwise-or-used/expected_analysis.json | 2 +- .../expected_analysis.json | 2 +- 13 files changed, 100 insertions(+), 33 deletions(-) create mode 100644 src/main/java/analyzer/exercises/secrets/AvoidConditionalLogic.java rename src/main/java/analyzer/exercises/secrets/{ImplementOperator.java => UseBitwiseOperator.java} (72%) create mode 100644 src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.UsingIfStatement.approved.txt create mode 100644 src/test/resources/scenarios/secrets/UsingIfStatement.java diff --git a/src/main/java/analyzer/AnalyzerRoot.java b/src/main/java/analyzer/AnalyzerRoot.java index 265d2729..483a61e9 100644 --- a/src/main/java/analyzer/AnalyzerRoot.java +++ b/src/main/java/analyzer/AnalyzerRoot.java @@ -55,8 +55,8 @@ private static List createAnalyzers(String slug) { case "leap" -> analyzers.add(new LeapAnalyzer()); case "log-levels" -> analyzers.add(new LogLevelsAnalyzer()); case "need-for-speed" -> analyzers.add(new NeedForSpeedAnalyzer()); - case "two-fer" -> analyzers.add(new TwoferAnalyzer()); case "secrets" -> analyzers.add(new SecretsAnalyzer()); + case "two-fer" -> analyzers.add(new TwoferAnalyzer()); } return List.copyOf(analyzers); diff --git a/src/main/java/analyzer/exercises/secrets/AvoidConditionalLogic.java b/src/main/java/analyzer/exercises/secrets/AvoidConditionalLogic.java new file mode 100644 index 00000000..d38dcf0e --- /dev/null +++ b/src/main/java/analyzer/exercises/secrets/AvoidConditionalLogic.java @@ -0,0 +1,19 @@ +package analyzer.exercises.secrets; + +import analyzer.Comment; + +/** + * @see Markdown Template + */ +class AvoidConditionalLogic extends Comment { + + @Override + public String getKey() { + return "java.secrets.avoid_conditional_logic"; + } + + @Override + public Type getType() { + return Type.ACTIONABLE; + } +} diff --git a/src/main/java/analyzer/exercises/secrets/SecretsAnalyzer.java b/src/main/java/analyzer/exercises/secrets/SecretsAnalyzer.java index f1ac2616..34227c34 100644 --- a/src/main/java/analyzer/exercises/secrets/SecretsAnalyzer.java +++ b/src/main/java/analyzer/exercises/secrets/SecretsAnalyzer.java @@ -3,7 +3,10 @@ import com.github.javaparser.ast.CompilationUnit; import com.github.javaparser.ast.body.MethodDeclaration; import com.github.javaparser.ast.expr.BinaryExpr; +import com.github.javaparser.ast.expr.ConditionalExpr; import com.github.javaparser.ast.expr.UnaryExpr; +import com.github.javaparser.ast.stmt.IfStmt; +import com.github.javaparser.ast.stmt.Statement; import com.github.javaparser.ast.visitor.VoidVisitorAdapter; import analyzer.Analyzer; @@ -38,46 +41,53 @@ public void analyze(Solution solution, OutputCollector output) { @Override public void visit(MethodDeclaration node, OutputCollector output) { - if (node.getNameAsString().equals(SHIFT_BACK) && doesNotImplementUnsignedRightShift(node)) { - output.addComment(new ImplementOperator(">>>", SHIFT_BACK)); + if (node.getNameAsString().equals(SHIFT_BACK) && doesNotUseOperator(node, BinaryExpr.Operator.UNSIGNED_RIGHT_SHIFT)) { + output.addComment(new UseBitwiseOperator(">>>", SHIFT_BACK)); + return; } - if (node.getNameAsString().equals(SET_BITS) && doesNotImplementBitwiseOr(node)) { - output.addComment(new ImplementOperator("|", SET_BITS)); + if (node.getNameAsString().equals(SET_BITS) && doesNotUseOperator(node, BinaryExpr.Operator.BINARY_OR)) { + output.addComment(new UseBitwiseOperator("|", SET_BITS)); + return; } - if (node.getNameAsString().equals(FLIP_BITS) && doesNotImplementBitwiseXor(node)) { - output.addComment(new ImplementOperator("^", FLIP_BITS)); + if (node.getNameAsString().equals(FLIP_BITS) && doesNotUseOperator(node, BinaryExpr.Operator.XOR)) { + output.addComment(new UseBitwiseOperator("^", FLIP_BITS)); + return; } - if (node.getNameAsString().equals(CLEAR_BITS) && doesNotImplementBitwiseAnd(node)) { - output.addComment(new ImplementOperator("&", CLEAR_BITS)); + if (node.getNameAsString().equals(CLEAR_BITS) && doesNotUseOperator(node, BinaryExpr.Operator.BINARY_AND)) { + output.addComment(new UseBitwiseOperator("&", CLEAR_BITS)); + return; } - if (node.getNameAsString().equals(CLEAR_BITS) && doesNotImplementBitwiseNot(node)) { + if (node.getNameAsString().equals(CLEAR_BITS) && !doesNotUseOperator(node, BinaryExpr.Operator.BINARY_AND) && doesNotImplementBitwiseNot(node)) { output.addComment(new PreferBitwiseNot()); } - super.visit(node, output); - } - - private static boolean doesNotImplementBitwiseAnd(MethodDeclaration node) { - return node.findAll(BinaryExpr.class, x -> x.getOperator() == BinaryExpr.Operator.BINARY_AND).isEmpty(); - } + if (hasConditional(node)) { + output.addComment(new AvoidConditionalLogic()); + } - private static boolean doesNotImplementBitwiseOr(MethodDeclaration node) { - return node.findAll(BinaryExpr.class, x -> x.getOperator() == BinaryExpr.Operator.BINARY_OR).isEmpty(); + super.visit(node, output); } - private static boolean doesNotImplementBitwiseXor(MethodDeclaration node) { - return node.findAll(BinaryExpr.class, x -> x.getOperator() == BinaryExpr.Operator.XOR).isEmpty(); + private static boolean doesNotUseOperator(MethodDeclaration node, BinaryExpr.Operator operator) { + return node.findAll(BinaryExpr.class, x -> x.getOperator() == operator).isEmpty(); } private static boolean doesNotImplementBitwiseNot(MethodDeclaration node) { return node.findAll(UnaryExpr.class, x -> x.getOperator() == UnaryExpr.Operator.BITWISE_COMPLEMENT).isEmpty(); } - private static boolean doesNotImplementUnsignedRightShift(MethodDeclaration node) { - return node.findAll(BinaryExpr.class, x -> x.getOperator() == BinaryExpr.Operator.UNSIGNED_RIGHT_SHIFT).isEmpty(); + private static boolean hasConditional(MethodDeclaration node) { + return node.getBody() + .map(body -> body.getStatements().stream() + .anyMatch(SecretsAnalyzer::isConditionalExpresion)) + .orElse(false); + } + + private static boolean isConditionalExpresion(Statement statement) { + return !statement.findAll(IfStmt.class).isEmpty() || !statement.findAll(ConditionalExpr.class).isEmpty(); } } diff --git a/src/main/java/analyzer/exercises/secrets/ImplementOperator.java b/src/main/java/analyzer/exercises/secrets/UseBitwiseOperator.java similarity index 72% rename from src/main/java/analyzer/exercises/secrets/ImplementOperator.java rename to src/main/java/analyzer/exercises/secrets/UseBitwiseOperator.java index a630de77..b671368c 100644 --- a/src/main/java/analyzer/exercises/secrets/ImplementOperator.java +++ b/src/main/java/analyzer/exercises/secrets/UseBitwiseOperator.java @@ -5,20 +5,20 @@ import java.util.Map; /** - * @see Markdown Template + * @see Markdown Template */ -class ImplementOperator extends Comment { +class UseBitwiseOperator extends Comment { private final String operatorToUse; private final String calledMethod; - public ImplementOperator(String operatorToUse, String calledMethod) { + public UseBitwiseOperator(String operatorToUse, String calledMethod) { this.operatorToUse = operatorToUse; this.calledMethod = calledMethod; } @Override public String getKey() { - return "java.secrets.implement_operator"; + return "java.secrets.use_bitwise_operator"; } @Override diff --git a/src/test/java/analyzer/AnalyzerIntegrationTest.java b/src/test/java/analyzer/AnalyzerIntegrationTest.java index 36f37f39..df5eb504 100644 --- a/src/test/java/analyzer/AnalyzerIntegrationTest.java +++ b/src/test/java/analyzer/AnalyzerIntegrationTest.java @@ -157,7 +157,8 @@ void loglevels(String scenario) throws IOException { "NotUsingBitwiseNot", "NotUsingBitwiseOr", "NotUsingBitwiseXor", - "NotUsingUnsignedRightShift" + "NotUsingUnsignedRightShift", + "UsingIfStatement" }) void secrets(String scenario) throws IOException { var path = Path.of("secrets", scenario + ".java"); diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseAnd.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseAnd.approved.txt index 3d5dbfbf..bb3ea384 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseAnd.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseAnd.approved.txt @@ -1,7 +1,7 @@ { "comments": [ { - "comment": "java.secrets.implement_operator", + "comment": "java.secrets.use_bitwise_operator", "params": { "calledMethod": "clearBits", "operatorToUse": "\u0026" diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseOr.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseOr.approved.txt index f0a60b76..af5409ce 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseOr.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseOr.approved.txt @@ -1,7 +1,7 @@ { "comments": [ { - "comment": "java.secrets.implement_operator", + "comment": "java.secrets.use_bitwise_operator", "params": { "calledMethod": "setBits", "operatorToUse": "|" diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseXor.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseXor.approved.txt index 60bbf5be..e7ec96d9 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseXor.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingBitwiseXor.approved.txt @@ -1,7 +1,7 @@ { "comments": [ { - "comment": "java.secrets.implement_operator", + "comment": "java.secrets.use_bitwise_operator", "params": { "calledMethod": "flipBits", "operatorToUse": "^" diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingUnsignedRightShift.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingUnsignedRightShift.approved.txt index 0322c920..342bc873 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingUnsignedRightShift.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingUnsignedRightShift.approved.txt @@ -1,7 +1,7 @@ { "comments": [ { - "comment": "java.secrets.implement_operator", + "comment": "java.secrets.use_bitwise_operator", "params": { "calledMethod": "shiftBack", "operatorToUse": "\u003e\u003e\u003e" diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.UsingIfStatement.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.UsingIfStatement.approved.txt new file mode 100644 index 00000000..984335f7 --- /dev/null +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.UsingIfStatement.approved.txt @@ -0,0 +1,14 @@ +{ + "comments": [ + { + "comment": "java.secrets.avoid_conditional_logic", + "params": {}, + "type": "actionable" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] +} \ No newline at end of file diff --git a/src/test/resources/scenarios/secrets/UsingIfStatement.java b/src/test/resources/scenarios/secrets/UsingIfStatement.java new file mode 100644 index 00000000..be84e6be --- /dev/null +++ b/src/test/resources/scenarios/secrets/UsingIfStatement.java @@ -0,0 +1,23 @@ +package scenarios.secrets; + +public class Secrets { + public static int shiftBack(int value, int amount) { + if (value < 0) { + return value >>> amount; + } + + return value >> amount; + } + + public static int setBits(int value, int mask) { + return value | mask; + } + + public static int flipBits(int value, int mask) { + return value ^ mask; + } + + public static int clearBits(int value, int mask) { + return value & ~mask; + } +} \ No newline at end of file diff --git a/tests/secrets/no-bitwise-or-used/expected_analysis.json b/tests/secrets/no-bitwise-or-used/expected_analysis.json index 700594b3..1d93914f 100644 --- a/tests/secrets/no-bitwise-or-used/expected_analysis.json +++ b/tests/secrets/no-bitwise-or-used/expected_analysis.json @@ -1,7 +1,7 @@ { "comments": [ { - "comment": "java.secrets.implement_operator", + "comment": "java.secrets.use_bitwise_operator", "params": { "calledMethod": "setBits", "operatorToUse": "|" diff --git a/tests/secrets/no-unsigned-right-shift-used/expected_analysis.json b/tests/secrets/no-unsigned-right-shift-used/expected_analysis.json index b9753917..127a672d 100644 --- a/tests/secrets/no-unsigned-right-shift-used/expected_analysis.json +++ b/tests/secrets/no-unsigned-right-shift-used/expected_analysis.json @@ -1,7 +1,7 @@ { "comments": [ { - "comment": "java.secrets.implement_operator", + "comment": "java.secrets.use_bitwise_operator", "params": { "calledMethod": "shiftBack", "operatorToUse": "\u003e\u003e\u003e" From 3c3686df29afa51c2c6be5efa911be8123d79cac Mon Sep 17 00:00:00 2001 From: manumafe98 Date: Thu, 7 Mar 2024 11:03:02 -0300 Subject: [PATCH 6/7] Adding extra scenario to match all essential comments --- .../analyzer/AnalyzerIntegrationTest.java | 3 +- ...singAnyOfTheExpectedOperators.approved.txt | 41 +++++++++++++++++++ .../NotUsingAnyOfTheExpectedOperators.java | 19 +++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingAnyOfTheExpectedOperators.approved.txt create mode 100644 src/test/resources/scenarios/secrets/NotUsingAnyOfTheExpectedOperators.java diff --git a/src/test/java/analyzer/AnalyzerIntegrationTest.java b/src/test/java/analyzer/AnalyzerIntegrationTest.java index df5eb504..7534284d 100644 --- a/src/test/java/analyzer/AnalyzerIntegrationTest.java +++ b/src/test/java/analyzer/AnalyzerIntegrationTest.java @@ -158,7 +158,8 @@ void loglevels(String scenario) throws IOException { "NotUsingBitwiseOr", "NotUsingBitwiseXor", "NotUsingUnsignedRightShift", - "UsingIfStatement" + "UsingIfStatement", + "NotUsingAnyOfTheExpectedOperators" }) void secrets(String scenario) throws IOException { var path = Path.of("secrets", scenario + ".java"); diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingAnyOfTheExpectedOperators.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingAnyOfTheExpectedOperators.approved.txt new file mode 100644 index 00000000..e25ede3e --- /dev/null +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingAnyOfTheExpectedOperators.approved.txt @@ -0,0 +1,41 @@ +{ + "comments": [ + { + "comment": "java.secrets.use_bitwise_operator", + "params": { + "calledMethod": "shiftBack", + "operatorToUse": "\u003e\u003e\u003e" + }, + "type": "essential" + }, + { + "comment": "java.secrets.use_bitwise_operator", + "params": { + "calledMethod": "setBits", + "operatorToUse": "|" + }, + "type": "essential" + }, + { + "comment": "java.secrets.use_bitwise_operator", + "params": { + "calledMethod": "flipBits", + "operatorToUse": "^" + }, + "type": "essential" + }, + { + "comment": "java.secrets.use_bitwise_operator", + "params": { + "calledMethod": "clearBits", + "operatorToUse": "\u0026" + }, + "type": "essential" + }, + { + "comment": "java.general.feedback_request", + "params": {}, + "type": "informative" + } + ] +} \ No newline at end of file diff --git a/src/test/resources/scenarios/secrets/NotUsingAnyOfTheExpectedOperators.java b/src/test/resources/scenarios/secrets/NotUsingAnyOfTheExpectedOperators.java new file mode 100644 index 00000000..07172761 --- /dev/null +++ b/src/test/resources/scenarios/secrets/NotUsingAnyOfTheExpectedOperators.java @@ -0,0 +1,19 @@ +package scenarios.secrets; + +public class Secrets { + public static int shiftBack(int value, int amount) { + return (value >> amount) & ~(Integer.MIN_VALUE >> (amount - 1)); + } + + public static int setBits(int value, int mask) { + return value + mask - (value & mask); + } + + public static int flipBits(int value, int mask) { + return (value & ~mask) | (~value & mask); + } + + public static int clearBits(int value, int mask) { + return ~(~value | mask); + } +} From 48ca2281fad0887669504281eed7d7b93d9175ed Mon Sep 17 00:00:00 2001 From: manumafe98 Date: Fri, 8 Mar 2024 10:47:10 -0300 Subject: [PATCH 7/7] Updting visit analyzer method to only add one essential commit --- .../exercises/secrets/SecretsAnalyzer.java | 21 ++++++++-------- ...singAnyOfTheExpectedOperators.approved.txt | 24 ------------------- 2 files changed, 11 insertions(+), 34 deletions(-) diff --git a/src/main/java/analyzer/exercises/secrets/SecretsAnalyzer.java b/src/main/java/analyzer/exercises/secrets/SecretsAnalyzer.java index 34227c34..63da8b1f 100644 --- a/src/main/java/analyzer/exercises/secrets/SecretsAnalyzer.java +++ b/src/main/java/analyzer/exercises/secrets/SecretsAnalyzer.java @@ -26,6 +26,7 @@ public class SecretsAnalyzer extends VoidVisitorAdapter impleme private static final String SET_BITS = "setBits"; private static final String FLIP_BITS = "flipBits"; private static final String CLEAR_BITS = "clearBits"; + private boolean essentialCommentAdded = false; @Override public void analyze(Solution solution, OutputCollector output) { @@ -41,31 +42,31 @@ public void analyze(Solution solution, OutputCollector output) { @Override public void visit(MethodDeclaration node, OutputCollector output) { - if (node.getNameAsString().equals(SHIFT_BACK) && doesNotUseOperator(node, BinaryExpr.Operator.UNSIGNED_RIGHT_SHIFT)) { + if (!essentialCommentAdded && node.getNameAsString().equals(SHIFT_BACK) && doesNotUseOperator(node, BinaryExpr.Operator.UNSIGNED_RIGHT_SHIFT)) { output.addComment(new UseBitwiseOperator(">>>", SHIFT_BACK)); - return; + essentialCommentAdded = true; } - if (node.getNameAsString().equals(SET_BITS) && doesNotUseOperator(node, BinaryExpr.Operator.BINARY_OR)) { + if (!essentialCommentAdded && node.getNameAsString().equals(SET_BITS) && doesNotUseOperator(node, BinaryExpr.Operator.BINARY_OR)) { output.addComment(new UseBitwiseOperator("|", SET_BITS)); - return; + essentialCommentAdded = true; } - if (node.getNameAsString().equals(FLIP_BITS) && doesNotUseOperator(node, BinaryExpr.Operator.XOR)) { + if (!essentialCommentAdded && node.getNameAsString().equals(FLIP_BITS) && doesNotUseOperator(node, BinaryExpr.Operator.XOR)) { output.addComment(new UseBitwiseOperator("^", FLIP_BITS)); - return; + essentialCommentAdded = true; } - if (node.getNameAsString().equals(CLEAR_BITS) && doesNotUseOperator(node, BinaryExpr.Operator.BINARY_AND)) { + if (!essentialCommentAdded && node.getNameAsString().equals(CLEAR_BITS) && doesNotUseOperator(node, BinaryExpr.Operator.BINARY_AND)) { output.addComment(new UseBitwiseOperator("&", CLEAR_BITS)); - return; + essentialCommentAdded = true; } - if (node.getNameAsString().equals(CLEAR_BITS) && !doesNotUseOperator(node, BinaryExpr.Operator.BINARY_AND) && doesNotImplementBitwiseNot(node)) { + if (!essentialCommentAdded && node.getNameAsString().equals(CLEAR_BITS) && !doesNotUseOperator(node, BinaryExpr.Operator.BINARY_AND) && doesNotImplementBitwiseNot(node)) { output.addComment(new PreferBitwiseNot()); } - if (hasConditional(node)) { + if (!essentialCommentAdded && hasConditional(node)) { output.addComment(new AvoidConditionalLogic()); } diff --git a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingAnyOfTheExpectedOperators.approved.txt b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingAnyOfTheExpectedOperators.approved.txt index e25ede3e..342bc873 100644 --- a/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingAnyOfTheExpectedOperators.approved.txt +++ b/src/test/resources/analyzer/AnalyzerIntegrationTest.secrets.NotUsingAnyOfTheExpectedOperators.approved.txt @@ -8,30 +8,6 @@ }, "type": "essential" }, - { - "comment": "java.secrets.use_bitwise_operator", - "params": { - "calledMethod": "setBits", - "operatorToUse": "|" - }, - "type": "essential" - }, - { - "comment": "java.secrets.use_bitwise_operator", - "params": { - "calledMethod": "flipBits", - "operatorToUse": "^" - }, - "type": "essential" - }, - { - "comment": "java.secrets.use_bitwise_operator", - "params": { - "calledMethod": "clearBits", - "operatorToUse": "\u0026" - }, - "type": "essential" - }, { "comment": "java.general.feedback_request", "params": {},