diff --git a/pom.xml b/pom.xml index 9ba470af..6222fe10 100644 --- a/pom.xml +++ b/pom.xml @@ -80,7 +80,7 @@ 2.21.3 - 3.0.0 + rules-gci22-remove-unnecessary-method-calls-SNAPSHOT https://repo1.maven.org/maven2 diff --git a/src/it/java/org/greencodeinitiative/creedengo/python/integration/tests/GCIRulesIT.java b/src/it/java/org/greencodeinitiative/creedengo/python/integration/tests/GCIRulesIT.java index 1f8167e8..c00a02c7 100644 --- a/src/it/java/org/greencodeinitiative/creedengo/python/integration/tests/GCIRulesIT.java +++ b/src/it/java/org/greencodeinitiative/creedengo/python/integration/tests/GCIRulesIT.java @@ -559,4 +559,17 @@ void testGCI404() { checkIssuesForFile(filePath, ruleId, ruleMsg, startLines, endLines, SEVERITY, TYPE, EFFORT_15MIN); } + @Test + void testGCI22() { + String filePath = "src/GCI22/avoidUseOfMethodForBasicOperations.py"; + String ruleId = "creedengo-python:GCI22"; + String ruleMsg = "Avoid using methods for simple basic operations."; + int[] startLines = new int[]{9, 10, 11, 12, 13, 14, 17, 18, 19, 20, 21, 22, 26, 30, 35, 36, 41, 42, 44, 45, 46, 47, 48, 51, 52, 57, 58, 59, 65, 70, 73, 76, 79, 82, 85 + }; + int[] endLines = new int[]{9, 10, 11, 12, 13, 14, 17, 18, 19, 20, 21, 22, 26, 30, 35, 36, 41, 42, 44, 45, 46, 47, 48, 51, 52, 57, 58, 59, 65, 70, 73, 76, 79, 82, 85 + }; + + checkIssuesForFile(filePath, ruleId, ruleMsg, startLines, endLines, SEVERITY, TYPE, EFFORT_5MIN); + } + } diff --git a/src/it/test-projects/creedengo-python-plugin-test-project/src/GCI22/avoidUseOfMethodForBasicOperations.py b/src/it/test-projects/creedengo-python-plugin-test-project/src/GCI22/avoidUseOfMethodForBasicOperations.py new file mode 100644 index 00000000..551f749e --- /dev/null +++ b/src/it/test-projects/creedengo-python-plugin-test-project/src/GCI22/avoidUseOfMethodForBasicOperations.py @@ -0,0 +1,160 @@ +a = 10 +b = 3 +x = 5 +y = 8 +n1 = 0b1010 +n2 = 0b1100 + +#Arithmetic operations: +result_add = a.__add__(b) # Noncompliant {{Avoid using methods for simple basic operations.}} +result_sub = a.__sub__(b) # Noncompliant {{Avoid using methods for simple basic operations.}} +result_mul = a.__mul__(b) # Noncompliant {{Avoid using methods for simple basic operations.}} +result_div = a.__truediv__(b) # Noncompliant {{Avoid using methods for simple basic operations.}} +result_mod = a.__mod__(b) # Noncompliant {{Avoid using methods for simple basic operations.}} +result_pow = a.__pow__(b) # Noncompliant {{Avoid using methods for simple basic operations.}} + +#Comparisons: +is_equal = x.__eq__(y) # Noncompliant {{Avoid using methods for simple basic operations.}} +is_greater = x.__gt__(y) # Noncompliant {{Avoid using methods for simple basic operations.}} +is_less = x.__lt__(y) # Noncompliant {{Avoid using methods for simple basic operations.}} +is_gte = x.__ge__(y) # Noncompliant {{Avoid using methods for simple basic operations.}} +is_lte = x.__le__(y) # Noncompliant {{Avoid using methods for simple basic operations.}} +is_not_eq = x.__ne__(y) # Noncompliant {{Avoid using methods for simple basic operations.}} + +#Collection size: +my_list = [1, 2, 3, 4, 5] +size = my_list.__len__() # Noncompliant {{Avoid using methods for simple basic operations.}} + +#Found element: +words = ["This", "is", "a", "test"] +found = list.__contains__(words, "test") # Noncompliant {{Avoid using methods for simple basic operations.}} + +#String concatenation: +first = "Hello" +last = "World" +greeting = first.__add__(", " + last) # Noncompliant {{Avoid using methods for simple basic operations.}} +greeting = "".join([first, ", ", last]) # Noncompliant {{Avoid using methods for simple basic operations.}} + +#Operations: +flag_a = True +flag_b = False +result_and = flag_a.__and__(flag_b) # Noncompliant {{Avoid using methods for simple basic operations.}} +result_or = flag_a.__or__(flag_b) # Noncompliant {{Avoid using methods for simple basic operations.}} + +bit_and = n1.__and__(n2) # Noncompliant {{Avoid using methods for simple basic operations.}} +bit_or = n1.__or__(n2) # Noncompliant {{Avoid using methods for simple basic operations.}} +bit_xor = n1.__xor__(n2) # Noncompliant {{Avoid using methods for simple basic operations.}} +bit_lshift = n1.__lshift__(2) # Noncompliant {{Avoid using methods for simple basic operations.}} +bit_rshift = n1.__rshift__(2) # Noncompliant {{Avoid using methods for simple basic operations.}} + +#Sequence repetition: +repeated_str = "abc".__mul__(3) # Noncompliant {{Avoid using methods for simple basic operations.}} +repeated_list = [0].__mul__(5) # Noncompliant {{Avoid using methods for simple basic operations.}} + +#Access / modification / deletion of elements: +data = {"key": 42} +my_list2 = [10, 20, 30] +val = data.__getitem__("key") # Noncompliant {{Avoid using methods for simple basic operations.}} +my_list2.__setitem__(1, 99) # Noncompliant {{Avoid using methods for simple basic operations.}} +my_list2.__delitem__(0) # Noncompliant {{Avoid using methods for simple basic operations.}} + +#Accumulation in a loop: +numbers = range(10) +squares = [] +for n in numbers: + squares.append(n ** 2) # Noncompliant {{Avoid using methods for simple basic operations.}} + +#Trivial user-defined functions wrapping a basic operation: + +def add(a, b): + return a + b # Noncompliant {{Avoid using methods for simple basic operations.}} + +def multiply(a, b): + return a * b # Noncompliant {{Avoid using methods for simple basic operations.}} + +def is_eq(a, b): + return a == b # Noncompliant {{Avoid using methods for simple basic operations.}} + +def is_gt(a, b): + return a > b # Noncompliant {{Avoid using methods for simple basic operations.}} + +def contains(collection, item): + return item in collection # Noncompliant {{Avoid using methods for simple basic operations.}} + +def concat(a, b): + return a + b # Noncompliant {{Avoid using methods for simple basic operations.}} + + +# =========================================================================== + +a = 10 +b = 3 +x = 5 +y = 8 +n1 = 0b1010 +n2 = 0b1100 + +result_add = a + b # Compliant {{Native operator.}} +result_sub = a - b # Compliant {{Native operator.}} +result_mul = a * b # Compliant {{Native operator.}} +result_div = a / b # Compliant {{Native operator.}} +result_mod = a % b # Compliant {{Native operator.}} +result_pow = a ** b # Compliant {{Native operator.}} + +is_equal = x == y # Compliant {{Native operator.}} +is_greater = x > y # Compliant {{Native operator.}} +is_less = x < y # Compliant {{Native operator.}} +is_gte = x >= y # Compliant {{Native operator.}} +is_lte = x <= y # Compliant {{Native operator.}} +is_not_eq = x != y # Compliant {{Native operator.}} + +my_list = [1, 2, 3, 4, 5] +size = len(my_list) # Compliant {{use of len().}} + +words = ["This", "is", "a", "test"] +found = "test" in words # Compliant {{use of in.}} + +first = "Hello" +last = "World" +greeting = first + ", " + last # Compliant {{Native operator.}} +greeting = f"{first}, {last}" # Compliant {{use of f-string.}} + +words = ["word"] * 1000 +sentence = " ".join(words) # Compliant {{use of join().}} + +flag_a = True +flag_b = False +result_and = flag_a and flag_b # Compliant {{Native operator.}} +result_or = flag_a or flag_b # Compliant {{Native operator.}} + +bit_and = n1 & n2 # Compliant {{Native operator.}} +bit_or = n1 | n2 # Compliant {{Native operator.}} +bit_xor = n1 ^ n2 # Compliant {{Native operator.}} +bit_lshift = n1 << 2 # Compliant {{Native operator.}} +bit_rshift = n1 >> 2 # Compliant {{Native operator.}} + +repeated_str = "abc" * 3 # Compliant {{Native operator.}} +repeated_list = [0] * 5 # Compliant {{Native operator.}} + +data = {"key": 42} +my_list2 = [10, 20, 30] +val = data["key"] # Compliant {{Native subscript syntax.}} +my_list2[1] = 99 # Compliant {{Native subscript syntax.}} +del my_list2[0] # Compliant {{Native operator.}} + +numbers = range(10) +squares = [n ** 2 for n in numbers] # Compliant {{Native syntax.}} + +values = [3, 1, 7, 2] +total = sum(values) # Compliant {Use of sum().}} +minimum = min(values) # Compliant {{Use of min().}} +maximum = max(values) # Compliant {{Use of max().}} +flags = [True, False, True] +check = all(flags) # Compliant {{Use of all().}} +check = any(flags) # Compliant {{Use of any().}} + +def is_adult(age): + return age >= 18 # Compliant {{Not a trivial operator wrapper.}} + +def tax_rate_applies(amount): + return amount > 1000 # Compliant {{Not a trivial operator wrapper.}} \ No newline at end of file diff --git a/src/main/java/org/greencodeinitiative/creedengo/python/PythonRuleRepository.java b/src/main/java/org/greencodeinitiative/creedengo/python/PythonRuleRepository.java index a0bb6d86..989bd4b5 100644 --- a/src/main/java/org/greencodeinitiative/creedengo/python/PythonRuleRepository.java +++ b/src/main/java/org/greencodeinitiative/creedengo/python/PythonRuleRepository.java @@ -56,7 +56,9 @@ public record PythonRuleRepository(SonarRuntime sonarRuntime) implements RulesDe GCI104AvoidCreatingTensorUsingNumpyOrNativePython.class, GCI110AvoidWildcardImportsCheck.class, GCI109AvoidExceptionsForControlFlowCheck.class, - GCI112UsingSlotsOnDataClasses.class + GCI112UsingSlotsOnDataClasses.class, + GCI22AvoidUseOfMethodForBasicOperations.class + ); public static final String LANGUAGE = "py"; diff --git a/src/main/java/org/greencodeinitiative/creedengo/python/checks/GCI22AvoidUseOfMethodForBasicOperations.java b/src/main/java/org/greencodeinitiative/creedengo/python/checks/GCI22AvoidUseOfMethodForBasicOperations.java new file mode 100644 index 00000000..16af18b0 --- /dev/null +++ b/src/main/java/org/greencodeinitiative/creedengo/python/checks/GCI22AvoidUseOfMethodForBasicOperations.java @@ -0,0 +1,202 @@ +/* + * 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.*; +import org.sonar.plugins.python.api.tree.Expression; + +import java.util.List; +import java.util.Set; + +import org.sonar.plugins.python.api.tree.BinaryExpression; + +@Rule(key = "GCI22") +public class GCI22AvoidUseOfMethodForBasicOperations extends PythonSubscriptionCheck { + + static final String MESSAGE = + "Avoid using methods for simple basic operations."; + + /** + * Dunder methods that have a direct native-operator equivalent in Python. + */ + private static final Set DUNDER_METHODS = Set.of( + //arithmetic + "__add__", "__sub__", "__mul__", "__truediv__", "__floordiv__", + "__mod__", "__pow__", "__matmul__", + //reflected arithmetic + "__radd__", "__rsub__", "__rmul__", "__rtruediv__", "__rfloordiv__", + "__rmod__", "__rpow__", + //in-place arithmetic + "__iadd__", "__isub__", "__imul__", "__itruediv__", "__ifloordiv__", + "__imod__", "__ipow__", + //comparisons + "__eq__", "__ne__", "__lt__", "__le__", "__gt__", "__ge__", + //logical + "__and__", "__or__", "__xor__", "__lshift__", "__rshift__", + "__iand__", "__ior__", "__ixor__", "__ilshift__", "__irshift__", + "__invert__", "__neg__", "__pos__", + //container / subscript + "__len__", "__contains__", "__getitem__", "__setitem__", "__delitem__", + //string / sequence + "__bool__", "__int__", "__float__", "__str__" + ); + + /** + * Generic trivial function names that are likely pure operator wrappers. + * We only flag functions whose *name* is in this set AND whose body is + * a single basic expression (or assign + return of a basic expression). + * + * Business-meaningful names (is_adult, tax_rate_applies…) are intentionally + * NOT in this set. + */ + private static final Set TRIVIAL_FUNCTION_NAMES = Set.of( + "add", "sub", "subtract", "mul", "multiply", "div", "divide", + "mod", "pow", "neg", "negate", + "eq", "ne", "lt", "le", "gt", "ge", + "is_eq", "is_gt", "is_lt", "is_gte", "is_lte", "is_ne", + "compare", "cmp", + "contains", "concat", "join_two", + "add_op", "add_one", "increment", "decrement", + "identity", "wrapper", "bool_wrapper" + ); + + @Override + public void initialize(Context context) { + // Detect explicit dunder calls and bad join() at every expression level + context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, this::visitCallExpr); + // Detect append() inside a for-loop + context.registerSyntaxNodeConsumer(Tree.Kind.FOR_STMT, this::visitForLoop); + // Detect trivial wrapper functions + context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, this::visitFunctionDef); + } + + private void visitCallExpr(SubscriptionContext ctx) { + CallExpression call = (CallExpression) ctx.syntaxNode(); + + Expression callee = call.callee(); + + // Pattern: obj.__dunder__(args) => callee is a QualifiedExpression + if (callee instanceof QualifiedExpression qualifiedExpr) { + String methodName = qualifiedExpr.name().name(); + + if (DUNDER_METHODS.contains(methodName)) { + ctx.addIssue(call, MESSAGE); + return; + } + + if ("join".equals(methodName) + && qualifiedExpr.qualifier() instanceof StringLiteral + && hasListLiteralArgument(call)) { + ctx.addIssue(call, MESSAGE); + } + } + } + + /** + * Returns true if the call has exactly one argument and that argument is + * an inline list literal (e.g. [a, ", ", b]). + */ + private boolean hasListLiteralArgument(CallExpression call) { + if (call.argumentList() == null) return false; + List args = call.argumentList().arguments(); + if (args.size() != 1) return false; + Argument arg = args.get(0); + return arg instanceof RegularArgument regArg + && regArg.expression() instanceof ListLiteral; + } + + private void visitForLoop(SubscriptionContext ctx) { + ForStatement forStmt = (ForStatement) ctx.syntaxNode(); + for (Statement stmt : forStmt.body().statements()) { + findAppendCalls(ctx, stmt); + } + } + + private void findAppendCalls(SubscriptionContext ctx, Statement stmt) { + // We look for ExpressionStatement wrapping a CallExpression to .append() + if (!(stmt instanceof ExpressionStatement exprStmt)) return; + if (!(exprStmt.expressions().get(0) instanceof CallExpression call)) return; + if (!(call.callee() instanceof QualifiedExpression qe)) return; + if ("append".equals(qe.name().name())) { + ctx.addIssue(call, MESSAGE); + } + } + + private void visitFunctionDef(SubscriptionContext ctx) { + FunctionDef funcDef = (FunctionDef) ctx.syntaxNode(); + String name = funcDef.name().name(); + + if (!TRIVIAL_FUNCTION_NAMES.contains(name)) { + return; // business function or unknown name => not concern + } + + List stmts = funcDef.body().statements(); + + if (stmts.size() == 1) { + checkPatternA(ctx, stmts.get(0)); + } else if (stmts.size() == 2) { + checkPatternB(ctx, stmts.get(0), stmts.get(1)); + } + } + + /** + * Pattern A: return + * Flags the ReturnStatement if the returned expression is basic. + */ + private void checkPatternA(SubscriptionContext ctx, Statement stmt) { + if (!(stmt instanceof ReturnStatement returnStmt)) return; + List exprs = returnStmt.expressions(); + if (exprs.size() == 1 && isBasicExpression(exprs.get(0))) { + ctx.addIssue(returnStmt, MESSAGE); + } + } + + /** + * Pattern B: var = followed by return var + * Flags the ReturnStatement when the assignment feeds straight into the return. + */ + private void checkPatternB(SubscriptionContext ctx, Statement first, Statement second) { + if (!(first instanceof AssignmentStatement assignStmt)) return; + if (!(second instanceof ReturnStatement returnStmt)) return; + + // The return must reference the variable that was just assigned + List returnExprs = returnStmt.expressions(); + if (returnExprs.size() != 1) return; + if (!(returnExprs.get(0) instanceof Name returnedName)) return; + + ExpressionList lhsExprs = assignStmt.lhsExpressions().get(0); + if (lhsExprs.expressions().size() != 1) return; + if (!(lhsExprs.expressions().get(0) instanceof Name assignedName)) return; + + if (!assignedName.name().equals(returnedName.name())) return; + + if (isBasicExpression(assignStmt.assignedValue())) { + ctx.addIssue(returnStmt, MESSAGE); + } + } + + private boolean isBasicExpression(Tree expr) { + return expr instanceof BinaryExpression + || expr instanceof UnaryExpression + || expr instanceof Name; + } + +} diff --git a/src/main/resources/org/greencodeinitiative/creedengo/python/creedengo_way_profile.json b/src/main/resources/org/greencodeinitiative/creedengo/python/creedengo_way_profile.json index 6a107ce8..23a203d0 100644 --- a/src/main/resources/org/greencodeinitiative/creedengo/python/creedengo_way_profile.json +++ b/src/main/resources/org/greencodeinitiative/creedengo/python/creedengo_way_profile.json @@ -6,6 +6,7 @@ "GCI4", "GCI7", "GCI10", + "GCI22", "GCI24", "GCI35", "GCI72", diff --git a/src/test/java/org/greencodeinitiative/creedengo/python/checks/GCI22AvoidUseOfMethodForBasicOperationsTest.java b/src/test/java/org/greencodeinitiative/creedengo/python/checks/GCI22AvoidUseOfMethodForBasicOperationsTest.java new file mode 100644 index 00000000..94cb24c7 --- /dev/null +++ b/src/test/java/org/greencodeinitiative/creedengo/python/checks/GCI22AvoidUseOfMethodForBasicOperationsTest.java @@ -0,0 +1,28 @@ +/* + * 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.jupiter.api.Test; +import org.sonar.python.checks.utils.PythonCheckVerifier; + +public class GCI22AvoidUseOfMethodForBasicOperationsTest { + @Test + public void test() { + PythonCheckVerifier.verify(System.getProperty("testfiles.path") + "/GCI22/avoidUseOfMethodForBasicOperations.py", new GCI22AvoidUseOfMethodForBasicOperations()); + } +}