diff --git a/src/main/java/org/greencodeinitiative/creedengo/python/PythonRuleRepository.java b/src/main/java/org/greencodeinitiative/creedengo/python/PythonRuleRepository.java index 96517f4..b1f35e1 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/python/PythonRuleRepository.java +++ b/src/main/java/org/greencodeinitiative/creedengo/python/PythonRuleRepository.java @@ -41,6 +41,7 @@ public class PythonRuleRepository implements RulesDefinition, PythonCustomRuleRe AvoidListComprehensionInIterations.class, DetectUnoptimizedImageFormat.class, AvoidMultipleIfElseStatementCheck.class, + LoopInvariantStatementCheck.class, PandasRequireUsecolsArgument.class, OptimizeSquareComputation.class, AvoidSqrtInLoop.class, 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..fb942a0 --- /dev/null +++ b/src/main/java/org/greencodeinitiative/creedengo/python/checks/LoopInvariantStatementCheck.java @@ -0,0 +1,215 @@ +/* + * 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.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.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.HashSet; +import java.util.Objects; +import java.util.Set; + +@Rule(key = "GCI204") +@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 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" + ); + + /** + * 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. + * 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 + */ + @Override + public void initialize(Context context) { + 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) { + // 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 || + !(ctx.syntaxNode().parent().parent() instanceof ForStatement || + ctx.syntaxNode().parent().parent() instanceof WhileStatement)))) { + reportedNodes.clear(); + } + + // Extract the current loop statement node to process its body + 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) { + 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 + // 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)) { + // 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) { + // Get the expression assigned to the target variable + // Checks whether the assigned expression includes invariant calculations + Expression valueExpr = assignment.assignedValue(); + checkExpressionForInvariants(ctx, valueExpr); + } + + /** + * 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 + // This ensures that deeply nested expressions are fully analyzed + 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) { + Expression callee = callExpr.callee(); + if (callee instanceof Name) { + String name = ((Name) callee).name(); + // 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; + } +} \ 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..495599b --- /dev/null +++ b/src/test/resources/checks/loopInvariantStatement.py @@ -0,0 +1,87 @@ +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 detected. Move outside loop for better 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 detected. Move outside loop for better 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 detected. Move outside loop for better 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 detected. Move outside loop for better performance.}} + results.append(n * i) + for j in range(10_000): + n = sum(x) # Noncompliant {{Loop invariant detected. Move outside loop for better 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 detected. Move outside loop for better 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 detected. Move outside loop for better 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 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 + +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 new file mode 100644 index 0000000..17857b3 --- /dev/null +++ b/src/test/resources/checks/loopInvariantStatementCompliant.py @@ -0,0 +1,86 @@ +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 + +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 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