diff --git a/CHANGELOG.md b/CHANGELOG.md index a5c8c92c..83168201 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -116,6 +116,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Update ecocode-rules-specifications to 1.4.6 +## [unreleased] + +### Changed + +- #193 GCI27 - reduce false positives for System.arraycopy suggestions inside conditional and try/catch blocks + +## [2.1.2] - 2026-05-20 + [unreleased](https://github.com/green-code-initiative/creedengo-java/compare/2.1.2...HEAD) [2.1.2](https://github.com/green-code-initiative/creedengo-java/compare/2.1.1...2.1.2) [2.1.1](https://github.com/green-code-initiative/creedengo-java/compare/2.1.0...2.1.1) diff --git a/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/GCI27/ArrayCopyCheck.java b/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/GCI27/ArrayCopyCheck.java index 7d075c44..b80a1a65 100644 --- a/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/GCI27/ArrayCopyCheck.java +++ b/src/it/test-projects/creedengo-java-plugin-test-project/src/main/java/org/greencodeinitiative/creedengo/java/checks/GCI27/ArrayCopyCheck.java @@ -70,14 +70,14 @@ public void copyWithForLoop() { } // Copy with nested conditions - for (int i = 0; i < len; i++) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (int i = 0; i < len; i++) { if(i + 2 < len) { dest[i] = src[i + 2]; } } // Copy with nested ELSE conditions - for (int i = 0; i < len; i++) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (int i = 0; i < len; i++) { if(i + 2 >= len) { i++; } else { @@ -86,7 +86,7 @@ public void copyWithForLoop() { } // Copy with more nested conditions - for (int i = 0; i < len; i++) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (int i = 0; i < len; i++) { if(i + 2 < len) { if(dest != null) { if(src != null) { @@ -99,7 +99,7 @@ public void copyWithForLoop() { } // Copy nested by try/catch - for (int i = 0; i < len; i++) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (int i = 0; i < len; i++) { try { dest[i] = src[i]; } catch (RuntimeException e) { @@ -108,7 +108,7 @@ public void copyWithForLoop() { } // Copy nested by try/catch and if - for (int i = 0; i < len; i++) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (int i = 0; i < len; i++) { try { if(dest != null) { dest[i] = src[i]; @@ -119,7 +119,7 @@ public void copyWithForLoop() { } // Copy nested by try/catch in catch - for (int i = 0; i < len; i++) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (int i = 0; i < len; i++) { try { dest.toString(); } catch (RuntimeException e) { @@ -130,7 +130,7 @@ public void copyWithForLoop() { } // Copy nested by try/catch in finally - for (int i = 0; i < len; i++) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (int i = 0; i < len; i++) { try { dest.toString(); } catch (RuntimeException e) { @@ -159,7 +159,7 @@ public void copyWithForEachLoop() { // Copy with nested conditions by foreach i = -1; - for (boolean b : src) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (boolean b : src) { if(b) { dest[++i] = b; } @@ -167,7 +167,7 @@ public void copyWithForEachLoop() { // Copy with nested ELSE conditions by foreach i = -1; - for (boolean b : src) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (boolean b : src) { if(i + 2 >= len) { i++; } else { @@ -177,7 +177,7 @@ public void copyWithForEachLoop() { // Copy with more nested conditions i = -1; - for (boolean b : src) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (boolean b : src) { if(i + 2 < len) { if(dest != null) { if(src != null) { @@ -191,7 +191,7 @@ public void copyWithForEachLoop() { // Copy nested by try/catch i = -1; - for (boolean b : src) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (boolean b : src) { try { dest[++i] = b; } catch (RuntimeException e) { @@ -201,7 +201,7 @@ public void copyWithForEachLoop() { // Copy nested by try/catch and if i = -1; - for (boolean b : src) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (boolean b : src) { try { if(dest != null) { dest[++i] = b; @@ -213,7 +213,7 @@ public void copyWithForEachLoop() { // Copy nested by try/catch in catch i = -1; - for (boolean b : src) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (boolean b : src) { try { dest.toString(); } catch (RuntimeException e) { @@ -225,7 +225,7 @@ public void copyWithForEachLoop() { // Copy nested by try/catch in finally i = -1; - for (boolean b : src) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (boolean b : src) { try { dest.toString(); } catch (RuntimeException e) { @@ -250,7 +250,7 @@ public void copyWithForEachLoop() { // Copy with nested conditions i = 0; - for (boolean b : src) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (boolean b : src) { if(b) { dest[i] = src[i]; } @@ -259,7 +259,7 @@ public void copyWithForEachLoop() { // Copy with nested ELSE conditions i = 0; - for (boolean b : src) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (boolean b : src) { if(i + 2 >= len) { i++; } else { @@ -270,7 +270,7 @@ public void copyWithForEachLoop() { // Copy with more nested conditions i = 0; - for (boolean b : src) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (boolean b : src) { if(i + 2 < len) { if(dest != null) { if(src != null) { @@ -285,7 +285,7 @@ public void copyWithForEachLoop() { // Copy nested by try/catch i = 0; - for (boolean b : src) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (boolean b : src) { try { dest[i] = src[i]; } catch (RuntimeException e) { @@ -296,7 +296,7 @@ public void copyWithForEachLoop() { // Copy nested by try/catch and if i = 0; - for (boolean b : src) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (boolean b : src) { try { if(dest != null) { dest[i] = src[i]; @@ -309,7 +309,7 @@ public void copyWithForEachLoop() { // Copy nested by try/catch in catch i = 0; - for (boolean b : src) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (boolean b : src) { try { dest.toString(); } catch (RuntimeException e) { @@ -322,7 +322,7 @@ public void copyWithForEachLoop() { // Copy nested by try/catch in finally i = 0; - for (boolean b : src) { // Noncompliant {{Use System.arraycopy to copy arrays}} + for (boolean b : src) { try { dest.toString(); } catch (RuntimeException e) { @@ -355,7 +355,7 @@ public void copyWithWhileLoop() { // Copy with nested conditions i = 0; - while (i < len) { // Noncompliant {{Use System.arraycopy to copy arrays}} + while (i < len) { if(i + 2 < len) { dest[i] = src[i + 2]; } @@ -364,7 +364,7 @@ public void copyWithWhileLoop() { // Copy with nested ELSE conditions i = 0; - while (i < len) { // Noncompliant {{Use System.arraycopy to copy arrays}} + while (i < len) { if(i + 2 >= len) { i++; } else { @@ -375,7 +375,7 @@ public void copyWithWhileLoop() { // Copy with more nested conditions i = 0; - while (i < len) { // Noncompliant {{Use System.arraycopy to copy arrays}} + while (i < len) { if(i + 2 < len) { if(dest != null) { if(src != null) { @@ -390,7 +390,7 @@ public void copyWithWhileLoop() { // Copy nested by try/catch and if i = 0; - while (i < len) { // Noncompliant {{Use System.arraycopy to copy arrays}} + while (i < len) { try { if(dest != null) { dest[i] = src[i]; @@ -403,7 +403,7 @@ public void copyWithWhileLoop() { // Copy nested by try/catch in catch i = 0; - while (i < len) { // Noncompliant {{Use System.arraycopy to copy arrays}} + while (i < len) { try { dest.toString(); } catch (RuntimeException e) { @@ -436,7 +436,7 @@ public void copyWithDoWhileLoop() { // Copy with nested conditions i = 0; - do { // Noncompliant {{Use System.arraycopy to copy arrays}} + do { if(i + 2 < len) { dest[i] = src[i + 2]; } @@ -445,7 +445,7 @@ public void copyWithDoWhileLoop() { // Copy with nested ELSE conditions i = 0; - do { // Noncompliant {{Use System.arraycopy to copy arrays}} + do { if(i + 2 >= len) { i++; } else { @@ -456,7 +456,7 @@ public void copyWithDoWhileLoop() { // Copy with more nested conditions i = 0; - do { // Noncompliant {{Use System.arraycopy to copy arrays}} + do { if(i + 2 < len) { if(dest != null) { if(src != null) { @@ -471,7 +471,7 @@ public void copyWithDoWhileLoop() { // Copy nested by try/catch and if i = 0; - do { // Noncompliant {{Use System.arraycopy to copy arrays}} + do { try { if(dest != null) { dest[i] = src[i]; @@ -484,7 +484,7 @@ public void copyWithDoWhileLoop() { // Copy nested by try/catch in catch i = 0; - do { // Noncompliant {{Use System.arraycopy to copy arrays}} + do { try { dest.toString(); } catch (RuntimeException e) { diff --git a/src/main/java/org/greencodeinitiative/creedengo/java/checks/ArrayCopyCheck.java b/src/main/java/org/greencodeinitiative/creedengo/java/checks/ArrayCopyCheck.java index bc4d569e..b379962d 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/java/checks/ArrayCopyCheck.java +++ b/src/main/java/org/greencodeinitiative/creedengo/java/checks/ArrayCopyCheck.java @@ -28,18 +28,15 @@ import org.sonar.plugins.java.api.tree.ArrayAccessExpressionTree; import org.sonar.plugins.java.api.tree.AssignmentExpressionTree; import org.sonar.plugins.java.api.tree.BlockTree; -import org.sonar.plugins.java.api.tree.CatchTree; import org.sonar.plugins.java.api.tree.DoWhileStatementTree; import org.sonar.plugins.java.api.tree.ExpressionStatementTree; import org.sonar.plugins.java.api.tree.ExpressionTree; import org.sonar.plugins.java.api.tree.ForEachStatement; import org.sonar.plugins.java.api.tree.ForStatementTree; import org.sonar.plugins.java.api.tree.IdentifierTree; -import org.sonar.plugins.java.api.tree.IfStatementTree; import org.sonar.plugins.java.api.tree.StatementTree; import org.sonar.plugins.java.api.tree.Tree; import org.sonar.plugins.java.api.tree.Tree.Kind; -import org.sonar.plugins.java.api.tree.TryStatementTree; import org.sonar.plugins.java.api.tree.VariableTree; import org.sonar.plugins.java.api.tree.WhileStatementTree; import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey; @@ -180,61 +177,27 @@ private List getBlocsOfCode(final Tree tree) { } else if (tree instanceof DoWhileStatementTree) { DoWhileStatementTree castedDoWhileTree = (DoWhileStatementTree) tree; addBloc(blocs, castedDoWhileTree.statement()); - } else if (tree instanceof IfStatementTree) { - IfStatementTree castedIfTree = (IfStatementTree) tree; - addBloc(blocs, castedIfTree.thenStatement()); - addBloc(blocs, castedIfTree.elseStatement()); - } else if (tree instanceof TryStatementTree) { - final TryStatementTree tryTree = (TryStatementTree) tree; - addBloc(blocs, tryTree.block()); - addBloc(blocs, extractCatchBlocks(tryTree)); - addBloc(blocs, tryTree.finallyBlock()); } return blocs; } /** - * Assignments extraction from block of code. + * Extract direct assignments from a block of code. * - * @param tree - * @param block - * @return + * @param tree current tree + * @param bloc current block + * @return list of assignments */ private List extractAssignments(final Tree tree, final Bloc bloc) { final BlockTree block = bloc.getBlockTree(); - // Prepare useful predicates - final Predicate blocksPredicate = statement -> statement.is(Kind.IF_STATEMENT) - || statement.is(Kind.TRY_STATEMENT); final Predicate assignmentPredicate = statement -> statement.is(Kind.EXPRESSION_STATEMENT) && ((ExpressionStatementTree) statement).expression().is(Kind.ASSIGNMENT); - // Filter expressions to find assignments - final List result = block.body().stream().filter(assignmentPredicate) + return block.body().stream() + .filter(assignmentPredicate) .map(assign -> (AssignmentExpressionTree) ((ExpressionStatementTree) assign).expression()) .collect(Collectors.toList()); - - // Recursive loop for nested blocks, add nested assignments to results - final List ifStatements = block.body().stream().filter(blocksPredicate) - .collect(Collectors.toList()); - for (final StatementTree ifstatement : ifStatements) { - final List blocs = getBlocsOfCode(ifstatement); - for (final Bloc b : blocs) { - result.addAll(extractAssignments(tree, b)); - } - } - return result; - } - - /** - * Extract all blocks of code from try/catch statement - * - * @param tryTree - * @return Array of StatementTree - */ - private BlockTree[] extractCatchBlocks(final TryStatementTree tryTree) { - final List catches = tryTree.catches(); - return catches.stream().map(CatchTree::block).collect(Collectors.toList()).toArray(new BlockTree[0]); } /**