From a63dca93fc38511a7c35d0aa703d9b1ed44b592a Mon Sep 17 00:00:00 2001 From: Benjamin DELAHAIS Date: Thu, 30 May 2024 13:35:03 +0200 Subject: [PATCH 1/5] [Issue #26][EC1337] Avoid unlimited cache --- CHANGELOG.md | 2 + .../python/PythonRuleRepository.java | 1 + .../python/checks/AvoidUnlimitedCache.java | 92 +++++++++++++++++++ .../python/PythonRuleRepositoryTest.java | 2 +- .../checks/AvoidUnlimitedCacheTest.java | 29 ++++++ .../checks/avoidUnlimitedCacheCompliant.py | 9 ++ .../checks/avoidUnlimitedCacheNonCompliant.py | 14 +++ 7 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 src/main/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCache.java create mode 100644 src/test/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCacheTest.java create mode 100644 src/test/resources/checks/avoidUnlimitedCacheCompliant.py create mode 100644 src/test/resources/checks/avoidUnlimitedCacheNonCompliant.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 44f7fc9..06ef6d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +[#26](https://github.com/green-code-initiative/ecoCode-python/issues/26) [EC1337] Avoid unlimited cache + ### Changed ### Deleted diff --git a/src/main/java/fr/greencodeinitiative/python/PythonRuleRepository.java b/src/main/java/fr/greencodeinitiative/python/PythonRuleRepository.java index c9ba42d..fb3b20f 100644 --- a/src/main/java/fr/greencodeinitiative/python/PythonRuleRepository.java +++ b/src/main/java/fr/greencodeinitiative/python/PythonRuleRepository.java @@ -60,6 +60,7 @@ public List checkClasses() { AvoidGlobalVariableInFunctionCheck.class, AvoidSQLRequestInLoop.class, AvoidTryCatchWithFileOpenedCheck.class, + AvoidUnlimitedCache.class, AvoidUnoptimizedVectorImagesCheck.class, NoFunctionCallWhenDeclaringForLoop.class, AvoidFullSQLRequest.class, diff --git a/src/main/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCache.java b/src/main/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCache.java new file mode 100644 index 0000000..b625664 --- /dev/null +++ b/src/main/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCache.java @@ -0,0 +1,92 @@ +/* + * ecoCode - Python language - Provides rules to reduce the environmental footprint of your Python programs + * Copyright © 2023 Green Code Initiative (https://www.ecocode.io) + * + * 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 fr.greencodeinitiative.python.checks; + +import org.checkerframework.checker.units.qual.C; +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.CallExpression; +import org.sonar.plugins.python.api.tree.Decorator; +import org.sonar.plugins.python.api.tree.FunctionDef; +import org.sonar.plugins.python.api.tree.Name; +import org.sonar.plugins.python.api.tree.RegularArgument; +import org.sonar.plugins.python.api.tree.Tree; +import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey; + +@Rule(key = "EC1337") +public class AvoidUnlimitedCache extends PythonSubscriptionCheck { + + public static final String DESCRIPTION = "Do not set cache size to unlimited"; + + public static final String LRU_CACHE = "lru_cache"; + public static final String MAX_SIZE_ARGUMENT = "maxsize"; + + + public static final String CACHE = "cache"; + + @Override + public void initialize(Context context) { + // Check function decorators + context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, this::check_function); + } + + private void check_function(SubscriptionContext ctx) { + FunctionDef function = (FunctionDef) ctx.syntaxNode(); + + function.decorators().forEach(decorator -> { + // If decorator is @cache + if (is_cache_decorator(decorator)) { + ctx.addIssue(decorator, AvoidUnlimitedCache.DESCRIPTION); + // If decorator is @lru_cache + } else if (is_lru_cache_decorator(decorator)) { + if (decorator.arguments() != null) { + decorator.arguments().arguments().forEach(arg -> { + RegularArgument regArg = (RegularArgument) arg; + if (MAX_SIZE_ARGUMENT.equals(regArg.keywordArgument().name()) && regArg.expression().is(Tree.Kind.NONE)) { + ctx.addIssue(decorator, AvoidUnlimitedCache.DESCRIPTION); + } + }); + } + } + }); + } + + private boolean is_cache_decorator(Decorator decorator) { + return is_decorator(decorator, CACHE); + } + + private boolean is_lru_cache_decorator(Decorator decorator) { + return is_decorator(decorator, LRU_CACHE); + } + + private boolean is_decorator(Decorator decorator, String expression) { + Name name = null; + // Manage decarator detected as simple expression + if (decorator.expression().is(Tree.Kind.NAME)) { + name = (Name) decorator.expression(); + // manage decorator detected as callable expression + } else if(decorator.expression().is(Tree.Kind.CALL_EXPR)) { + CallExpression callExpression = (CallExpression) decorator.expression(); + if (callExpression.callee().is(Tree.Kind.NAME)) { + name = (Name) callExpression.callee(); + } + } + return name != null && expression.equals(name.name()); + } +} diff --git a/src/test/java/fr/greencodeinitiative/python/PythonRuleRepositoryTest.java b/src/test/java/fr/greencodeinitiative/python/PythonRuleRepositoryTest.java index ca37f45..77db56f 100644 --- a/src/test/java/fr/greencodeinitiative/python/PythonRuleRepositoryTest.java +++ b/src/test/java/fr/greencodeinitiative/python/PythonRuleRepositoryTest.java @@ -78,7 +78,7 @@ void testMetadata() { @Test void testRegistredRules() { - assertThat(repository.rules()).hasSize(11); + assertThat(repository.rules()).hasSize(12); } @Test diff --git a/src/test/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCacheTest.java b/src/test/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCacheTest.java new file mode 100644 index 0000000..e99f092 --- /dev/null +++ b/src/test/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCacheTest.java @@ -0,0 +1,29 @@ +/* + * ecoCode - Python language - Provides rules to reduce the environmental footprint of your Python programs + * Copyright © 2023 Green Code Initiative (https://www.ecocode.io) + * + * 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 fr.greencodeinitiative.python.checks; + +import org.junit.Test; +import org.sonar.python.checks.utils.PythonCheckVerifier; + +public class AvoidUnlimitedCacheTest { + @Test + public void test() { + PythonCheckVerifier.verify("src/test/resources/checks/avoidUnlimitedCacheNonCompliant.py", new AvoidUnlimitedCache()); + PythonCheckVerifier.verifyNoIssue("src/test/resources/checks/avoidUnlimitedCacheCompliant.py", new AvoidUnlimitedCache()); + } +} diff --git a/src/test/resources/checks/avoidUnlimitedCacheCompliant.py b/src/test/resources/checks/avoidUnlimitedCacheCompliant.py new file mode 100644 index 0000000..2eef69f --- /dev/null +++ b/src/test/resources/checks/avoidUnlimitedCacheCompliant.py @@ -0,0 +1,9 @@ +import functools + +@lru_cache +def cached_function(): + print('a') + +@lru_cache(maxsize=30) +def cached_function(): + print('b') \ No newline at end of file diff --git a/src/test/resources/checks/avoidUnlimitedCacheNonCompliant.py b/src/test/resources/checks/avoidUnlimitedCacheNonCompliant.py new file mode 100644 index 0000000..e58648a --- /dev/null +++ b/src/test/resources/checks/avoidUnlimitedCacheNonCompliant.py @@ -0,0 +1,14 @@ +import functools + +class A: + @cache # Noncompliant {{Do not set cache size to unlimited}} + def cached_method_a(): + print('a') + + @lru_cache(maxsize=None) # Noncompliant {{Do not set cache size to unlimited}} + def cached_method_b(): + print('b') + +@cache # Noncompliant {{Do not set cache size to unlimited}} +def cached_function(): + print('a') From 36da4230f401632c48617f3c097f2e98b518e5ae Mon Sep 17 00:00:00 2001 From: Benjamin DELAHAIS Date: Fri, 31 May 2024 11:54:24 +0200 Subject: [PATCH 2/5] [Issue #26][EC1337] Avoid unlimited cache --- .../python/checks/AvoidUnlimitedCache.java | 38 +++++++++---------- .../checks/avoidUnlimitedCacheCompliant.py | 4 ++ .../checks/avoidUnlimitedCacheNonCompliant.py | 4 ++ 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/main/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCache.java b/src/main/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCache.java index b625664..06e1d48 100644 --- a/src/main/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCache.java +++ b/src/main/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCache.java @@ -17,7 +17,6 @@ */ package fr.greencodeinitiative.python.checks; -import org.checkerframework.checker.units.qual.C; import org.sonar.check.Rule; import org.sonar.plugins.python.api.PythonSubscriptionCheck; import org.sonar.plugins.python.api.SubscriptionContext; @@ -27,7 +26,6 @@ import org.sonar.plugins.python.api.tree.Name; import org.sonar.plugins.python.api.tree.RegularArgument; import org.sonar.plugins.python.api.tree.Tree; -import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey; @Rule(key = "EC1337") public class AvoidUnlimitedCache extends PythonSubscriptionCheck { @@ -37,45 +35,45 @@ public class AvoidUnlimitedCache extends PythonSubscriptionCheck { public static final String LRU_CACHE = "lru_cache"; public static final String MAX_SIZE_ARGUMENT = "maxsize"; - public static final String CACHE = "cache"; @Override public void initialize(Context context) { // Check function decorators - context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, this::check_function); + context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, this::checkFunction); } - private void check_function(SubscriptionContext ctx) { + private void checkFunction(SubscriptionContext ctx) { FunctionDef function = (FunctionDef) ctx.syntaxNode(); function.decorators().forEach(decorator -> { // If decorator is @cache - if (is_cache_decorator(decorator)) { + if (isCacheDecorator(decorator)) { ctx.addIssue(decorator, AvoidUnlimitedCache.DESCRIPTION); // If decorator is @lru_cache - } else if (is_lru_cache_decorator(decorator)) { - if (decorator.arguments() != null) { - decorator.arguments().arguments().forEach(arg -> { - RegularArgument regArg = (RegularArgument) arg; - if (MAX_SIZE_ARGUMENT.equals(regArg.keywordArgument().name()) && regArg.expression().is(Tree.Kind.NONE)) { - ctx.addIssue(decorator, AvoidUnlimitedCache.DESCRIPTION); - } - }); - } + } else if (isLruCacheDecorator(decorator) + && decorator.arguments() != null + && decorator.arguments().arguments() != null + ) { + decorator.arguments().arguments().forEach(arg -> { + RegularArgument regArg = (RegularArgument) arg; + if (MAX_SIZE_ARGUMENT.equals(regArg.keywordArgument().name()) && regArg.expression().is(Tree.Kind.NONE)) { + ctx.addIssue(decorator, AvoidUnlimitedCache.DESCRIPTION); + } + }); } }); } - private boolean is_cache_decorator(Decorator decorator) { - return is_decorator(decorator, CACHE); + private boolean isCacheDecorator(Decorator decorator) { + return isDecorator(decorator, CACHE); } - private boolean is_lru_cache_decorator(Decorator decorator) { - return is_decorator(decorator, LRU_CACHE); + private boolean isLruCacheDecorator(Decorator decorator) { + return isDecorator(decorator, LRU_CACHE); } - private boolean is_decorator(Decorator decorator, String expression) { + private boolean isDecorator(Decorator decorator, String expression) { Name name = null; // Manage decarator detected as simple expression if (decorator.expression().is(Tree.Kind.NAME)) { diff --git a/src/test/resources/checks/avoidUnlimitedCacheCompliant.py b/src/test/resources/checks/avoidUnlimitedCacheCompliant.py index 2eef69f..b4a18e8 100644 --- a/src/test/resources/checks/avoidUnlimitedCacheCompliant.py +++ b/src/test/resources/checks/avoidUnlimitedCacheCompliant.py @@ -4,6 +4,10 @@ def cached_function(): print('a') +@lru_cache() +def cached_function(): + print('a') + @lru_cache(maxsize=30) def cached_function(): print('b') \ No newline at end of file diff --git a/src/test/resources/checks/avoidUnlimitedCacheNonCompliant.py b/src/test/resources/checks/avoidUnlimitedCacheNonCompliant.py index e58648a..6f26834 100644 --- a/src/test/resources/checks/avoidUnlimitedCacheNonCompliant.py +++ b/src/test/resources/checks/avoidUnlimitedCacheNonCompliant.py @@ -12,3 +12,7 @@ def cached_method_b(): @cache # Noncompliant {{Do not set cache size to unlimited}} def cached_function(): print('a') + +@lru_cache(maxsize=None) # Noncompliant {{Do not set cache size to unlimited}} +def cached_method(): + print('b') \ No newline at end of file From 2699303c5b116891df630e319776689e195818d0 Mon Sep 17 00:00:00 2001 From: Benjamin DELAHAIS Date: Mon, 3 Jun 2024 17:04:56 +0200 Subject: [PATCH 3/5] [Issue #26][EC89] Change EC1337 to EC89 --- CHANGELOG.md | 2 +- .../greencodeinitiative/python/checks/AvoidUnlimitedCache.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06ef6d8..c0c4eb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -[#26](https://github.com/green-code-initiative/ecoCode-python/issues/26) [EC1337] Avoid unlimited cache +[#26](https://github.com/green-code-initiative/ecoCode-python/issues/26) [EC89] Avoid unlimited cache ### Changed diff --git a/src/main/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCache.java b/src/main/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCache.java index 06e1d48..1f62d87 100644 --- a/src/main/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCache.java +++ b/src/main/java/fr/greencodeinitiative/python/checks/AvoidUnlimitedCache.java @@ -27,7 +27,7 @@ import org.sonar.plugins.python.api.tree.RegularArgument; import org.sonar.plugins.python.api.tree.Tree; -@Rule(key = "EC1337") +@Rule(key = "EC89") public class AvoidUnlimitedCache extends PythonSubscriptionCheck { public static final String DESCRIPTION = "Do not set cache size to unlimited"; From d259da4673facff3f080bb079bf648d5df86a7f8 Mon Sep 17 00:00:00 2001 From: B3ND3L Date: Mon, 3 Jun 2024 19:23:10 +0200 Subject: [PATCH 4/5] [Issue #26][EC89] Typo in changelog.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0c4eb3..998fb37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -[#26](https://github.com/green-code-initiative/ecoCode-python/issues/26) [EC89] Avoid unlimited cache +- [#26](https://github.com/green-code-initiative/ecoCode-python/issues/26) [EC89] Avoid unlimited cache ### Changed From 316958a90097161d60d2f773a262b32575a6bfdd Mon Sep 17 00:00:00 2001 From: B3ND3L Date: Wed, 5 Jun 2024 09:00:08 +0200 Subject: [PATCH 5/5] [Issue #26][EC89] Changer version ecocode-rules to 1.6.0 --- pom.xml | 2 +- .../resources/checks/avoidUnlimitedCacheCompliant.py | 11 +++++++---- .../checks/avoidUnlimitedCacheNonCompliant.py | 12 ++++++++---- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/pom.xml b/pom.xml index 554966f..9ceee8d 100644 --- a/pom.xml +++ b/pom.xml @@ -59,7 +59,7 @@ 5.3.1 - 1.4.7 + 1.6.0 2.5.0.1358 diff --git a/src/test/resources/checks/avoidUnlimitedCacheCompliant.py b/src/test/resources/checks/avoidUnlimitedCacheCompliant.py index b4a18e8..aa37e6b 100644 --- a/src/test/resources/checks/avoidUnlimitedCacheCompliant.py +++ b/src/test/resources/checks/avoidUnlimitedCacheCompliant.py @@ -1,13 +1,16 @@ -import functools +from functools import lru_cache + @lru_cache def cached_function(): print('a') + @lru_cache() -def cached_function(): +def cached_function_a(): print('a') + @lru_cache(maxsize=30) -def cached_function(): - print('b') \ No newline at end of file +def cached_function_b(): + print('b') diff --git a/src/test/resources/checks/avoidUnlimitedCacheNonCompliant.py b/src/test/resources/checks/avoidUnlimitedCacheNonCompliant.py index 6f26834..a05c706 100644 --- a/src/test/resources/checks/avoidUnlimitedCacheNonCompliant.py +++ b/src/test/resources/checks/avoidUnlimitedCacheNonCompliant.py @@ -1,18 +1,22 @@ -import functools +from functools import cache +from functools import lru_cache + class A: @cache # Noncompliant {{Do not set cache size to unlimited}} - def cached_method_a(): + def cached_method_a(self): print('a') @lru_cache(maxsize=None) # Noncompliant {{Do not set cache size to unlimited}} - def cached_method_b(): + def cached_method_b(self): print('b') + @cache # Noncompliant {{Do not set cache size to unlimited}} def cached_function(): print('a') + @lru_cache(maxsize=None) # Noncompliant {{Do not set cache size to unlimited}} def cached_method(): - print('b') \ No newline at end of file + print('b')