From b424eeada6eded450310f06d0d08267055da2620 Mon Sep 17 00:00:00 2001 From: RamziBAHBA Date: Tue, 20 May 2025 16:58:44 +0200 Subject: [PATCH 1/3] Add LoopInvariantStatement rule check v1 --- .../python/PythonRuleRepository.java | 3 +- .../checks/LoopInvariantStatementCheck.java | 92 +++++++++++++++++++ .../LoopInvariantStatementCheckTest.java | 30 ++++++ .../checks/loopInvariantStatement.py | 9 ++ .../checks/loopInvariantStatementCompliant.py | 9 ++ .../rules/python/GCI204.html | 22 +++++ .../rules/python/GCI204.json | 11 +++ testLoop.sh | 6 ++ 8 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheck.java create mode 100644 src/test/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheckTest.java create mode 100644 src/test/resources/checks/loopInvariantStatement.py create mode 100644 src/test/resources/checks/loopInvariantStatementCompliant.py create mode 100644 src/test/resources/org/green-code-initiative/rules/python/GCI204.html create mode 100644 src/test/resources/org/green-code-initiative/rules/python/GCI204.json create mode 100644 testLoop.sh diff --git a/src/main/java/org/greencodeinitiative/creedengo/python/PythonRuleRepository.java b/src/main/java/org/greencodeinitiative/creedengo/python/PythonRuleRepository.java index c385979..df28be9 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/python/PythonRuleRepository.java +++ b/src/main/java/org/greencodeinitiative/creedengo/python/PythonRuleRepository.java @@ -40,7 +40,8 @@ public class PythonRuleRepository implements RulesDefinition, PythonCustomRuleRe AvoidFullSQLRequest.class, AvoidListComprehensionInIterations.class, DetectUnoptimizedImageFormat.class, - AvoidMultipleIfElseStatementCheck.class + AvoidMultipleIfElseStatementCheck.class, + LoopInvariantStatementCheck.class ); public static final String LANGUAGE = "py"; diff --git a/src/main/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheck.java b/src/main/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheck.java new file mode 100644 index 0000000..1c511d3 --- /dev/null +++ b/src/main/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheck.java @@ -0,0 +1,92 @@ +/* + * creedengo - Python language - Provides rules to reduce the environmental footprint of your Python programs + * Copyright © 2024 Green Code Initiative (https://green-code-initiative.org) + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.greencodeinitiative.creedengo.python.checks; + +import org.sonar.check.Rule; +import org.sonar.plugins.python.api.PythonSubscriptionCheck; +import org.sonar.plugins.python.api.SubscriptionContext; +import org.sonar.plugins.python.api.tree.ForStatement; +import org.sonar.plugins.python.api.tree.WhileStatement; +import org.sonar.plugins.python.api.tree.StatementList; +import org.sonar.plugins.python.api.tree.CallExpression; +import org.sonar.plugins.python.api.tree.Expression; +import org.sonar.plugins.python.api.tree.Tree; +import org.sonar.plugins.python.api.tree.AssignmentStatement; +import org.sonar.plugins.python.api.tree.Name; +import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey; + +import java.util.List; +import java.util.Set; + +@Rule(key = "GCI204") +@DeprecatedRuleKey(repositoryKey = "ecocode-python", ruleKey = "EC204") +public class LoopInvariantStatementCheck extends PythonSubscriptionCheck { + + private static final String MESSAGE_ERROR = "Loop invariant statement detected. Consider moving this computation outside the loop to improve performance."; + private static final Set POTENTIALLY_INVARIANT_FUNCTIONS = Set.of( + "len", "sum", "sorted", "list", "tuple", "set", "dict", "max", "min" + ); + + @Override + public void initialize(Context context) { + // Enregistrement pour les boucles for et while + context.registerSyntaxNodeConsumer(Tree.Kind.FOR_STMT, this::visitLoop); + context.registerSyntaxNodeConsumer(Tree.Kind.WHILE_STMT, this::visitLoop); + } + + private void visitLoop(SubscriptionContext ctx) { + // Obtenir le corps de la boucle, qu'il s'agisse d'une boucle for ou while + StatementList body = ctx.syntaxNode() instanceof ForStatement forStatement + ? forStatement.body() + : ((WhileStatement) ctx.syntaxNode()).body(); + + analyzeLoopBody(ctx, body); + } + + private void analyzeLoopBody(SubscriptionContext ctx, StatementList body) { + // Parcourir les instructions du corps de la boucle + body.statements().stream() + .filter(stmt -> stmt.is(Tree.Kind.ASSIGNMENT_STMT)) + .map(stmt -> (AssignmentStatement) stmt) + .forEach(assignment -> checkAssignmentForInvariants(ctx, assignment)); + } + + private void checkAssignmentForInvariants(SubscriptionContext ctx, AssignmentStatement assignment) { + // Vérifier si la partie droite de l'assignation contient un appel de fonction potentiellement invariant + Expression expression = assignment.assignedValue(); + checkExpressionForInvariants(ctx, assignment, expression); + } + + private void checkExpressionForInvariants(SubscriptionContext ctx, AssignmentStatement assignment, Expression expr) { + if (expr.is(Tree.Kind.CALL_EXPR)) { + CallExpression callExpr = (CallExpression) expr; + if (isPotentiallyInvariantCall(callExpr)) { + ctx.addIssue(assignment, MESSAGE_ERROR); + } + } + } + + private boolean isPotentiallyInvariantCall(CallExpression callExpr) { + if (!callExpr.callee().is(Tree.Kind.NAME)) { + return false; + } + + var functionName = ((Name) callExpr.callee()).name(); + return POTENTIALLY_INVARIANT_FUNCTIONS.contains(functionName); + } +} \ No newline at end of file diff --git a/src/test/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheckTest.java b/src/test/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheckTest.java new file mode 100644 index 0000000..a2d5737 --- /dev/null +++ b/src/test/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheckTest.java @@ -0,0 +1,30 @@ +/* + * creedengo - Python language - Provides rules to reduce the environmental footprint of your Python programs + * Copyright © 2024 Green Code Initiative (https://green-code-initiative.org) + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.greencodeinitiative.creedengo.python.checks; + +import org.junit.Test; +import org.sonar.python.checks.utils.PythonCheckVerifier; + +public class LoopInvariantStatementCheckTest { + + @Test + public void test() { + PythonCheckVerifier.verify("src/test/resources/checks/loopInvariantStatement.py", new LoopInvariantStatementCheck()); + PythonCheckVerifier.verifyNoIssue("src/test/resources/checks/loopInvariantStatementCompliant.py", new LoopInvariantStatementCheck()); + } +} diff --git a/src/test/resources/checks/loopInvariantStatement.py b/src/test/resources/checks/loopInvariantStatement.py new file mode 100644 index 0000000..a4bd8bd --- /dev/null +++ b/src/test/resources/checks/loopInvariantStatement.py @@ -0,0 +1,9 @@ +def process_data(data): + x = (1, 2, 3, 4) + + results = [] + for i in range(10_000): + n = len(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + results.append(n * i) + + return results \ No newline at end of file diff --git a/src/test/resources/checks/loopInvariantStatementCompliant.py b/src/test/resources/checks/loopInvariantStatementCompliant.py new file mode 100644 index 0000000..733a9cd --- /dev/null +++ b/src/test/resources/checks/loopInvariantStatementCompliant.py @@ -0,0 +1,9 @@ +def process_data(data): + x = (1, 2, 3, 4) + n = len(x) # Compliant {{Computed once outside the loop}} + + results = [] + for i in range(10_000): + results.append(n * i) + + return results \ No newline at end of file diff --git a/src/test/resources/org/green-code-initiative/rules/python/GCI204.html b/src/test/resources/org/green-code-initiative/rules/python/GCI204.html new file mode 100644 index 0000000..f018944 --- /dev/null +++ b/src/test/resources/org/green-code-initiative/rules/python/GCI204.html @@ -0,0 +1,22 @@ +

For a method having a single parameter, the types of its return value and its parameter should never be the same.

+ +

Noncompliant Code Example

+
+class MyClass {
+  int doSomething(int a) { // Noncompliant
+    return 42;
+  }
+}
+
+ +

Compliant Solution

+
+class MyClass {
+  int doSomething() { // Compliant
+    return 42;
+  }
+  long doSomething(int a) { // Compliant
+    return 42L;
+  }
+}
+
\ No newline at end of file diff --git a/src/test/resources/org/green-code-initiative/rules/python/GCI204.json b/src/test/resources/org/green-code-initiative/rules/python/GCI204.json new file mode 100644 index 0000000..667b2be --- /dev/null +++ b/src/test/resources/org/green-code-initiative/rules/python/GCI204.json @@ -0,0 +1,11 @@ +{ + "title": "Return type and parameter of a method should not be the same", + "type": "Bug", + "status": "ready", + "tags": [ + "bugs", + "gandalf", + "magic" + ], + "defaultSeverity": "Critical" +} \ No newline at end of file diff --git a/testLoop.sh b/testLoop.sh new file mode 100644 index 0000000..d135830 --- /dev/null +++ b/testLoop.sh @@ -0,0 +1,6 @@ +if [ $# -eq 0 ]; then + echo "Error missing parameter" + exit 1 +fi +TEST_CLASS=$1 +mvn clean test -Dtest=$TEST_CLASS -DtrimStack \ No newline at end of file From 94ab0c0fd3fa5ac4eb21fb385dd59211b97e61ec Mon Sep 17 00:00:00 2001 From: RamziBAHBA Date: Wed, 21 May 2025 13:35:08 +0200 Subject: [PATCH 2/3] Add LoopInvariantStatement rule check v2 --- .../checks/LoopInvariantStatementCheck.java | 185 +++++++++++++----- .../checks/loopInvariantStatement.py | 59 +++++- .../checks/loopInvariantStatementCompliant.py | 60 +++++- 3 files changed, 250 insertions(+), 54 deletions(-) diff --git a/src/main/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheck.java b/src/main/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheck.java index 1c511d3..da1a395 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheck.java +++ b/src/main/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheck.java @@ -1,92 +1,179 @@ -/* - * creedengo - Python language - Provides rules to reduce the environmental footprint of your Python programs - * Copyright © 2024 Green Code Initiative (https://green-code-initiative.org) - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ package org.greencodeinitiative.creedengo.python.checks; import org.sonar.check.Rule; import org.sonar.plugins.python.api.PythonSubscriptionCheck; import org.sonar.plugins.python.api.SubscriptionContext; -import org.sonar.plugins.python.api.tree.ForStatement; -import org.sonar.plugins.python.api.tree.WhileStatement; -import org.sonar.plugins.python.api.tree.StatementList; +import org.sonar.plugins.python.api.tree.AssignmentStatement; +import org.sonar.plugins.python.api.tree.BinaryExpression; import org.sonar.plugins.python.api.tree.CallExpression; import org.sonar.plugins.python.api.tree.Expression; -import org.sonar.plugins.python.api.tree.Tree; -import org.sonar.plugins.python.api.tree.AssignmentStatement; +import org.sonar.plugins.python.api.tree.ExpressionStatement; +import org.sonar.plugins.python.api.tree.ForStatement; +import org.sonar.plugins.python.api.tree.IfStatement; import org.sonar.plugins.python.api.tree.Name; +import org.sonar.plugins.python.api.tree.Statement; +import org.sonar.plugins.python.api.tree.StatementList; +import org.sonar.plugins.python.api.tree.Tree; +import org.sonar.plugins.python.api.tree.WhileStatement; import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey; -import java.util.List; +import java.util.HashSet; +import java.util.Objects; import java.util.Set; @Rule(key = "GCI204") @DeprecatedRuleKey(repositoryKey = "ecocode-python", ruleKey = "EC204") public class LoopInvariantStatementCheck extends PythonSubscriptionCheck { - private static final String MESSAGE_ERROR = "Loop invariant statement detected. Consider moving this computation outside the loop to improve performance."; + private static final String MESSAGE_ERROR = + "Loop invariant statement detected. Consider moving this computation outside the loop to improve performance."; private static final Set POTENTIALLY_INVARIANT_FUNCTIONS = Set.of( - "len", "sum", "sorted", "list", "tuple", "set", "dict", "max", "min" + "len", "sum", "sorted", "list", "tuple", "set", "dict", "max", "min", "abs" ); + /** + * A set used to store nodes of type {@link Tree} that have already been analyzed + * or reported during the loop invariant checking process. This ensures that each + * node is processed only once, avoiding redundant checks and duplicate reporting + * of issues. + */ + private final Set reportedNodes = new HashSet<>(); + + /** + * Initializes the context for the rule by registering syntax node consumers + * that specifically target "for" and "while" loop statements. + * + * @param context the context used to register and manage syntax node analysis + */ @Override public void initialize(Context context) { - // Enregistrement pour les boucles for et while context.registerSyntaxNodeConsumer(Tree.Kind.FOR_STMT, this::visitLoop); context.registerSyntaxNodeConsumer(Tree.Kind.WHILE_STMT, this::visitLoop); } + /** + * Processes and analyzes "for" and "while" loops to reset the set of reported nodes and analyze + * the loop body for potential invariant issues. + * + * @param ctx the subscription context managing the analysis and issue reporting + */ private void visitLoop(SubscriptionContext ctx) { - // Obtenir le corps de la boucle, qu'il s'agisse d'une boucle for ou while - StatementList body = ctx.syntaxNode() instanceof ForStatement forStatement - ? forStatement.body() - : ((WhileStatement) ctx.syntaxNode()).body(); + // Reset the set of reported nodes for each new top-level loop analysis + if (ctx.syntaxNode().parent() == null || !(ctx.syntaxNode().parent() instanceof StatementList) || + (ctx.syntaxNode().parent() instanceof StatementList && + (ctx.syntaxNode().parent().parent() == null || + !(ctx.syntaxNode().parent().parent() instanceof ForStatement || + ctx.syntaxNode().parent().parent() instanceof WhileStatement)))) { + reportedNodes.clear(); + } - analyzeLoopBody(ctx, body); + // Log the line number of the loop being analyzed + // Extract the current loop statement node + Statement stmt = (Statement) ctx.syntaxNode(); + StatementList body; + if (stmt instanceof ForStatement) { + body = ((ForStatement) stmt).body(); + } else { + body = ((WhileStatement) stmt).body(); + } + if (body != null) { + analyzeLoopBody(ctx, body); + } } + /** + * Analyzes the body of a loop to identify and report issues related to loop invariants. + * This method iterates through the statements in the provided loop body and performs + * specific checks depending on the type of statement encountered. Invariant-related issues + * are reported through the subscription context as necessary. + * + * @param ctx the subscription context managing the analysis and issue reporting + * @param body the list of statements representing the body of the loop to analyze + */ private void analyzeLoopBody(SubscriptionContext ctx, StatementList body) { - // Parcourir les instructions du corps de la boucle - body.statements().stream() - .filter(stmt -> stmt.is(Tree.Kind.ASSIGNMENT_STMT)) - .map(stmt -> (AssignmentStatement) stmt) - .forEach(assignment -> checkAssignmentForInvariants(ctx, assignment)); + // Log the start of the loop body analysis + for (Statement stmt : body.statements()) { + if (stmt instanceof AssignmentStatement) { + checkAssignmentForInvariants(ctx, (AssignmentStatement) stmt); + } else if (stmt instanceof IfStatement ifStmt) { + if (ifStmt.body() != null) { + analyzeLoopBody(ctx, ifStmt.body()); + } + if (ifStmt.elseBranch() != null && ifStmt.elseBranch() instanceof StatementList) { + analyzeLoopBody(ctx, (StatementList) Objects.requireNonNull(ifStmt.elseBranch())); + } + } else if (stmt instanceof ExpressionStatement) { + // Extract the first expression from the statement + Expression expr = ((ExpressionStatement) stmt).expressions().get(0); + if (expr instanceof CallExpression && isPotentiallyInvariantCall((CallExpression) expr) && + !reportedNodes.contains(expr)) { + // Report the issue and add the expression to the set of reported nodes + ctx.addIssue(expr, MESSAGE_ERROR); + reportedNodes.add(expr); + } + } else if (stmt instanceof ForStatement || stmt instanceof WhileStatement) { + // For nested loops, analyze the body of the loop + StatementList nestedBody; + if (stmt instanceof ForStatement) { + nestedBody = ((ForStatement) stmt).body(); + } else { + nestedBody = ((WhileStatement) stmt).body(); + } + if (nestedBody != null) { + analyzeLoopBody(ctx, nestedBody); + } + } + } } + /** + * Tracks and stores nodes corresponding to analyzed loop assignments. + * Used to ensure that each node is checked only once for invariants. + */ private void checkAssignmentForInvariants(SubscriptionContext ctx, AssignmentStatement assignment) { - // Vérifier si la partie droite de l'assignation contient un appel de fonction potentiellement invariant - Expression expression = assignment.assignedValue(); - checkExpressionForInvariants(ctx, assignment, expression); + // Log the line number of the current assignment being analyzed + // Get the expression assigned to the target variable + Expression valueExpr = assignment.assignedValue(); + checkExpressionForInvariants(ctx, valueExpr); } - private void checkExpressionForInvariants(SubscriptionContext ctx, AssignmentStatement assignment, Expression expr) { - if (expr.is(Tree.Kind.CALL_EXPR)) { - CallExpression callExpr = (CallExpression) expr; - if (isPotentiallyInvariantCall(callExpr)) { - ctx.addIssue(assignment, MESSAGE_ERROR); + /** + * Analyzes the provided expression to check for loop invariants, reporting issues if necessary. + * This method identifies potentially invariant function calls and recursively checks binary + * expressions for invariants in their operands. + * + * @param ctx the subscription context managing the analysis and issue reporting + * @param expr the expression to analyze for invariants + */ + private void checkExpressionForInvariants(SubscriptionContext ctx, Expression expr) { + if (expr instanceof CallExpression callExpr) { + if (isPotentiallyInvariantCall(callExpr) && !reportedNodes.contains(expr)) { + ctx.addIssue(expr, MESSAGE_ERROR); + reportedNodes.add(expr); } + } else if (expr instanceof BinaryExpression binaryExpr) { + // For binary expressions, recursively check both operands for invariants + checkExpressionForInvariants(ctx, binaryExpr.leftOperand()); + checkExpressionForInvariants(ctx, binaryExpr.rightOperand()); } } + /** + * Determines whether the provided function call is potentially invariant. + * A call is considered potentially invariant if its callee matches a predefined set of function names + * deemed to exhibit invariant behavior in the context of loop analysis. + * + * @param callExpr the function call expression to analyze + * @return true if the function call is deemed potentially invariant; false otherwise + */ private boolean isPotentiallyInvariantCall(CallExpression callExpr) { - if (!callExpr.callee().is(Tree.Kind.NAME)) { - return false; + // Log the line number of the function call being checked + Expression callee = callExpr.callee(); + if (callee instanceof Name) { + String name = ((Name) callee).name(); + // Return true if the function name matches a potentially invariant function + return POTENTIALLY_INVARIANT_FUNCTIONS.contains(name); } - - var functionName = ((Name) callExpr.callee()).name(); - return POTENTIALLY_INVARIANT_FUNCTIONS.contains(functionName); + return false; } } \ No newline at end of file diff --git a/src/test/resources/checks/loopInvariantStatement.py b/src/test/resources/checks/loopInvariantStatement.py index a4bd8bd..b0607be 100644 --- a/src/test/resources/checks/loopInvariantStatement.py +++ b/src/test/resources/checks/loopInvariantStatement.py @@ -1,9 +1,64 @@ -def process_data(data): +def non_compliant_len_inside_for(data): x = (1, 2, 3, 4) - results = [] for i in range(10_000): n = len(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} results.append(n * i) + return results + +def non_compliant_list_inside_for(): + d = {"a": 1, "b": 2, "c": 3} + results = [] + for i in range(1000): + keys = list(d.keys()) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + results.append(keys[i % len(keys)]) + return results +def non_compliant_len_outside_for_with_if(data): + x = (1, 2, 3, 4) + results = [] + for i in range(10_000): + if i % 2 == 0 : + n = len(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + results.append(n * i) + return results + +def non_compliant_sum_inside_double_for(data): + x = (1, 2, 3, 4) + results = [] + for i in range(10_000): + n = sum(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + results.append(n * i) + for j in range(10_000): + n = sum(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + results.append(n * j) + return results + +def non_compliant_max_inside_nested_for(data): + x = (1, 2, 3, 4) + results = [] + for i in range(10_000): + for j in range(10_000): + n = max(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + results.append(n + i + j) + return results + +def non_compliant_min_inside_while(data): + x = (1, 2, 3, 4) + results = [] + i = 0 + while i < 10_000: + n = min(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + results.append(n * i) + i += 1 + return results + +def non_compliant_two_abs_inside_for(data): + x = 25 + y = -66 + results = [] + for i in range(10_000): + n = abs(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + m = abs(y) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + results.append(n + m + i) return results \ No newline at end of file diff --git a/src/test/resources/checks/loopInvariantStatementCompliant.py b/src/test/resources/checks/loopInvariantStatementCompliant.py index 733a9cd..aa0169a 100644 --- a/src/test/resources/checks/loopInvariantStatementCompliant.py +++ b/src/test/resources/checks/loopInvariantStatementCompliant.py @@ -1,9 +1,63 @@ -def process_data(data): +def compliant_len_outside_for(data): x = (1, 2, 3, 4) n = len(x) # Compliant {{Computed once outside the loop}} - results = [] for i in range(10_000): results.append(n * i) - + return results + +def compliant_list_outside_for(): + d = {"a": 1, "b": 2, "c": 3} + keys = list(d.keys()) # Compliant {{Computed once outside the loop}} + results = [] + for i in range(1000): + results.append(keys[i % len(keys)]) + return results + +def compliant_len_outside_for_with_if(data): + x = (1, 2, 3, 4) + n = len(x) # Compliant {{Computed once outside the loop}} + results = [] + for i in range(10_000): + if i % 2 == 0 : + results.append(n * i) + return results + +def compliant_sum_outside_double_for(data): + x = (1, 2, 3, 4) + n = sum(x) # Compliant {{Computed once outside the loop}} + results = [] + for i in range(10_000): + results.append(n * i) + for j in range(10_000): + results.append(n * j) + return results + +def compliant_max_inside_nested_for(data): + x = (1, 2, 3, 4) + n = max(x) # Compliant {{Computed once outside the loop}} + results = [] + for i in range(10_000): + for j in range(10_000): + results.append(n + i + j) + return results + +def compliant_min_inside_while(data): + x = (1, 2, 3, 4) + n = min(x) # Compliant {{Computed once outside the loop}} + results = [] + i = 0 + while i < 10_000: + results.append(n * i) + i += 1 + return results + +def compliant_two_abs_inside_for(data): + x = 25 + y = -66 + n = abs(x) # Compliant {{Computed once outside the loop}} + m = abs(y) # Compliant {{Computed once outside the loop}} + results = [] + for i in range(10_000): + results.append(n + m + i) return results \ No newline at end of file From 169bf73ffe7b4f79435fc83cd3ad4b77e02f2118 Mon Sep 17 00:00:00 2001 From: RamziBAHBA Date: Wed, 21 May 2025 14:17:30 +0200 Subject: [PATCH 3/3] Add LoopInvariantStatement rule check v3 --- .../checks/LoopInvariantStatementCheck.java | 50 ++++++++++++++++--- .../checks/loopInvariantStatement.py | 43 ++++++++++++---- .../checks/loopInvariantStatementCompliant.py | 25 +++++++++- 3 files changed, 100 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheck.java b/src/main/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheck.java index da1a395..fb942a0 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheck.java +++ b/src/main/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheck.java @@ -1,3 +1,20 @@ +/* + * creedengo - Python language - Provides rules to reduce the environmental footprint of your Python programs + * Copyright © 2024 Green Code Initiative (https://green-code-initiative.org) + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ package org.greencodeinitiative.creedengo.python.checks; import org.sonar.check.Rule; @@ -25,8 +42,25 @@ @DeprecatedRuleKey(repositoryKey = "ecocode-python", ruleKey = "EC204") public class LoopInvariantStatementCheck extends PythonSubscriptionCheck { + /** + * The error message displayed when a loop invariant computation is detected + * within a loop. This indicates that certain operations within the loop body + * do not depend on the loop iteration and can be moved outside the loop for + * improved performance and efficiency. + */ private static final String MESSAGE_ERROR = - "Loop invariant statement detected. Consider moving this computation outside the loop to improve performance."; + "Loop invariant detected. Move outside loop for better performance."; + + /** + * A collection of function names that are considered potentially invariant + * within the context of loop analysis. These functions are deemed invariant + * because their outputs depend solely on their inputs and are not affected + * by changes in external or mutable state. + *

+ * This set is used to simplify the analysis of loop invariants by predefining + * a list of functions that are assumed to exhibit such behavior, thus avoiding + * redundant checks for invariance on these specific functions. + */ private static final Set POTENTIALLY_INVARIANT_FUNCTIONS = Set.of( "len", "sum", "sorted", "list", "tuple", "set", "dict", "max", "min", "abs" ); @@ -42,6 +76,7 @@ public class LoopInvariantStatementCheck extends PythonSubscriptionCheck { /** * Initializes the context for the rule by registering syntax node consumers * that specifically target "for" and "while" loop statements. + * This ensures that any loops in the code are analyzed for potential invariant issues. * * @param context the context used to register and manage syntax node analysis */ @@ -59,6 +94,7 @@ public void initialize(Context context) { */ private void visitLoop(SubscriptionContext ctx) { // Reset the set of reported nodes for each new top-level loop analysis + // Ensure that nested loops and parent-child relationships are handled correctly if (ctx.syntaxNode().parent() == null || !(ctx.syntaxNode().parent() instanceof StatementList) || (ctx.syntaxNode().parent() instanceof StatementList && (ctx.syntaxNode().parent().parent() == null || @@ -67,8 +103,7 @@ private void visitLoop(SubscriptionContext ctx) { reportedNodes.clear(); } - // Log the line number of the loop being analyzed - // Extract the current loop statement node + // Extract the current loop statement node to process its body Statement stmt = (Statement) ctx.syntaxNode(); StatementList body; if (stmt instanceof ForStatement) { @@ -91,7 +126,6 @@ private void visitLoop(SubscriptionContext ctx) { * @param body the list of statements representing the body of the loop to analyze */ private void analyzeLoopBody(SubscriptionContext ctx, StatementList body) { - // Log the start of the loop body analysis for (Statement stmt : body.statements()) { if (stmt instanceof AssignmentStatement) { checkAssignmentForInvariants(ctx, (AssignmentStatement) stmt); @@ -104,6 +138,7 @@ private void analyzeLoopBody(SubscriptionContext ctx, StatementList body) { } } else if (stmt instanceof ExpressionStatement) { // Extract the first expression from the statement + // Check if the expression is a function call that is potentially invariant Expression expr = ((ExpressionStatement) stmt).expressions().get(0); if (expr instanceof CallExpression && isPotentiallyInvariantCall((CallExpression) expr) && !reportedNodes.contains(expr)) { @@ -131,8 +166,8 @@ private void analyzeLoopBody(SubscriptionContext ctx, StatementList body) { * Used to ensure that each node is checked only once for invariants. */ private void checkAssignmentForInvariants(SubscriptionContext ctx, AssignmentStatement assignment) { - // Log the line number of the current assignment being analyzed // Get the expression assigned to the target variable + // Checks whether the assigned expression includes invariant calculations Expression valueExpr = assignment.assignedValue(); checkExpressionForInvariants(ctx, valueExpr); } @@ -153,6 +188,7 @@ private void checkExpressionForInvariants(SubscriptionContext ctx, Expression ex } } else if (expr instanceof BinaryExpression binaryExpr) { // For binary expressions, recursively check both operands for invariants + // This ensures that deeply nested expressions are fully analyzed checkExpressionForInvariants(ctx, binaryExpr.leftOperand()); checkExpressionForInvariants(ctx, binaryExpr.rightOperand()); } @@ -167,11 +203,11 @@ private void checkExpressionForInvariants(SubscriptionContext ctx, Expression ex * @return true if the function call is deemed potentially invariant; false otherwise */ private boolean isPotentiallyInvariantCall(CallExpression callExpr) { - // Log the line number of the function call being checked Expression callee = callExpr.callee(); if (callee instanceof Name) { String name = ((Name) callee).name(); - // Return true if the function name matches a potentially invariant function + // Return true if the function name matches a potentially invariant function. + // Function names are sourced from the predefined set to detect invariant behavior return POTENTIALLY_INVARIANT_FUNCTIONS.contains(name); } return false; diff --git a/src/test/resources/checks/loopInvariantStatement.py b/src/test/resources/checks/loopInvariantStatement.py index b0607be..495599b 100644 --- a/src/test/resources/checks/loopInvariantStatement.py +++ b/src/test/resources/checks/loopInvariantStatement.py @@ -2,7 +2,7 @@ def non_compliant_len_inside_for(data): x = (1, 2, 3, 4) results = [] for i in range(10_000): - n = len(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + n = len(x) # Noncompliant {{Loop invariant detected. Move outside loop for better performance.}} results.append(n * i) return results @@ -10,7 +10,7 @@ def non_compliant_list_inside_for(): d = {"a": 1, "b": 2, "c": 3} results = [] for i in range(1000): - keys = list(d.keys()) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + keys = list(d.keys()) # Noncompliant {{Loop invariant detected. Move outside loop for better performance.}} results.append(keys[i % len(keys)]) return results @@ -19,7 +19,7 @@ def non_compliant_len_outside_for_with_if(data): results = [] for i in range(10_000): if i % 2 == 0 : - n = len(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + n = len(x) # Noncompliant {{Loop invariant detected. Move outside loop for better performance.}} results.append(n * i) return results @@ -27,10 +27,10 @@ def non_compliant_sum_inside_double_for(data): x = (1, 2, 3, 4) results = [] for i in range(10_000): - n = sum(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + n = sum(x) # Noncompliant {{Loop invariant detected. Move outside loop for better performance.}} results.append(n * i) for j in range(10_000): - n = sum(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + n = sum(x) # Noncompliant {{Loop invariant detected. Move outside loop for better performance.}} results.append(n * j) return results @@ -39,7 +39,7 @@ def non_compliant_max_inside_nested_for(data): results = [] for i in range(10_000): for j in range(10_000): - n = max(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + n = max(x) # Noncompliant {{Loop invariant detected. Move outside loop for better performance.}} results.append(n + i + j) return results @@ -48,7 +48,7 @@ def non_compliant_min_inside_while(data): results = [] i = 0 while i < 10_000: - n = min(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + n = min(x) # Noncompliant {{Loop invariant detected. Move outside loop for better performance.}} results.append(n * i) i += 1 return results @@ -58,7 +58,30 @@ def non_compliant_two_abs_inside_for(data): y = -66 results = [] for i in range(10_000): - n = abs(x) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} - m = abs(y) # Noncompliant {{Loop invariant statement detected. Consider moving this computation outside the loop to improve performance.}} + n = abs(x) # Noncompliant {{Loop invariant detected. Move outside loop for better performance.}} + m = abs(y) # Noncompliant {{Loop invariant detected. Move outside loop for better performance.}} results.append(n + m + i) - return results \ No newline at end of file + return results + +def non_compliant_min_inside_nested_for_and_while(data): + x = (1, 2, 3, 4) + results = [] + for i in range(10_000): + j = 0 + while j < 10_000: + n = min(x) # Noncompliant {{Loop invariant detected. Move outside loop for better performance.}} + results.append(n + i + j) + j += 1 + return results + +# # Exemple fonction définie auparavant qui ne modifie pas la variable +# def calcul_len(a): +# return len(a) +# +# def non_compliant_len_method_inside_for(data): +# x = (1, 2, 3, 4) +# results = [] +# for i in range(10_000): +# n = calcul_len(x) # Noncompliant {{Loop invariant detected. Move outside loop for better performance.}} +# results.append(n * i) +# return results \ No newline at end of file diff --git a/src/test/resources/checks/loopInvariantStatementCompliant.py b/src/test/resources/checks/loopInvariantStatementCompliant.py index aa0169a..17857b3 100644 --- a/src/test/resources/checks/loopInvariantStatementCompliant.py +++ b/src/test/resources/checks/loopInvariantStatementCompliant.py @@ -60,4 +60,27 @@ def compliant_two_abs_inside_for(data): results = [] for i in range(10_000): results.append(n + m + i) - return results \ No newline at end of file + return results + +def compliant_min_inside_nested_for_and_while(data): + x = (1, 2, 3, 4) + n = min(x) # Compliant {{Computed once outside the loop}} + results = [] + for i in range(10_000): + j = 0 + while j < 10_000: + results.append(n + i + j) + j += 1 + return results + +# # Exemple fonction définie auparavant qui ne modifie pas la variable +# def calcul_len(a): +# return len(a) +# +# def compliant_len_method_outside_for(data): +# x = (1, 2, 3, 4) +# n = calcul_len(x) # Compliant {{Computed once outside the loop}} +# results = [] +# for i in range(10_000): +# results.append(n * i) +# return results \ No newline at end of file