diff --git a/CHANGELOG.md b/CHANGELOG.md index d83178ed..c9ebbb57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - [#119](https://github.com/green-code-initiative/creedengo-java/issues/119) GCI94 - reduce false positives: rule no longer flags `orElse()` when argument is a constant, literal, static field or null; detection extended to Optional variables (semantic type check) and to computed arguments nested inside concatenation, ternary or object instantiation +- [#122](https://github.com/green-code-initiative/creedengo-java/issues/122) resolve bug : final keyword is not recognized when using instanceof pattern - [#69](https://github.com/green-code-initiative/creedengo-java/issues/69) correction of NullPointer in GCI79 rule + technical refactoring of GCI79 - update integration tests system to use the new component "creedengo-integration-test" - compatibility updates for SonarQube up to 26.2.0 diff --git a/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCIRulesIT.java b/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCIRulesIT.java index 86c5f5fc..98b61e3b 100644 --- a/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCIRulesIT.java +++ b/src/it/java/org/greencodeinitiative/creedengo/java/integration/tests/GCIRulesIT.java @@ -426,8 +426,8 @@ void testGCI82() { String filePath = "src/main/java/org/greencodeinitiative/creedengo/java/checks/GCI82/MakeNonReassignedVariablesConstants.java"; String ruleId = "creedengo-java:GCI82"; String ruleMsg = "The variable is never reassigned and can be 'final'"; - int[] startLines = new int[]{9, 14, 15, 20, 26, 29, 48, 75, 108, 121, 146}; - int[] endLines = new int[]{9, 14, 15, 20, 26, 29, 48, 75, 108, 121, 146}; + int[] startLines = new int[]{9, 14, 15, 20, 26, 29, 48, 75, 108, 121, 136, 179}; + int[] endLines = new int[]{9, 14, 15, 20, 26, 29, 48, 75, 108, 121, 136, 179}; checkIssuesForFile(filePath, ruleId, ruleMsg, startLines, endLines); } diff --git a/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/GCI82/MakeNonReassignedVariablesConstants.java b/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/GCI82/MakeNonReassignedVariablesConstants.java index dc192cdd..33888bfe 100644 --- a/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/GCI82/MakeNonReassignedVariablesConstants.java +++ b/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/GCI82/MakeNonReassignedVariablesConstants.java @@ -129,6 +129,39 @@ void reassignedInConstructor(){ o = new notReassignedInConstructorNotFinal(this.varDefinedInClassNotReassignedInConstructor); } + + public String nonReasignedVariableWithPatternInstanceOfShouldBeNonCompliant() { + final Object o = "NON-COMPLIANT"; + + if (o instanceof String var) { // Noncompliant {{The variable is never reassigned and can be 'final'}} + return var; + } + + return ""; + } + + + public String nonReasignedVariableWithPatternInstanceOfWithFinalShouldBeCompliant() { + final Object o = "COMPLIANT"; + + if (o instanceof final String var) { // Compliant : here final keyword should be recognized and not trigger the rule + return var; + } + + return ""; + } + + public String reasignedVariableWithPatternInstanceOfShouldBeCompliant() { + final Object o = "COMPLIANT"; + + if (o instanceof String var) { // Compliant : Variable is reassigned + var = "REASSIGN"; + return var; + } + + return ""; + } + } class reassignedInConstructor{ diff --git a/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java b/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java index 495bc4e2..2d0a0e1f 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java +++ b/src/main/java/org/greencodeinitiative/creedengo/java/checks/MakeNonReassignedVariablesConstants.java @@ -32,6 +32,7 @@ public void visitNode(@Nonnull Tree tree) { LOGGER.debug(" => isNotReassigned = {}", isNotReassigned(variableTree)); LOGGER.debug(" => isPassedAsNonFinalParameter = {}", isPassedAsNonFinalParameter(variableTree)); } + if (isNotFinalAndNotStatic(variableTree) && isNotReassigned(variableTree)) { reportIssue(tree, MESSAGE_RULE); } else { @@ -112,20 +113,7 @@ private static boolean parentIsKind(Tree tree, Kind... orKind) { } private static boolean isNotFinalAndNotStatic(VariableTree variableTree) { - return hasNoneOf(variableTree.modifiers(), Modifier.FINAL, Modifier.STATIC); - } - - private static boolean hasNoneOf(ModifiersTree modifiersTree, Modifier... unexpectedModifiers) { - return !hasAnyOf(modifiersTree, unexpectedModifiers); - } - - private static boolean hasAnyOf(ModifiersTree modifiersTree, Modifier... expectedModifiers) { - for(Modifier expectedModifier : expectedModifiers) { - if (hasModifier(modifiersTree, expectedModifier)) { - return true; - } - } - return false; + return !variableTree.symbol().isFinal() && !variableTree.symbol().isStatic(); // use symbol instead of modifiers since in case of type_pattern modifiers is always empty } public static boolean hasModifier(ModifiersTree modifiersTree, Modifier expectedModifier) {