-
Notifications
You must be signed in to change notification settings - Fork 47
GCI91 - Create rule 'Use filter before sort' in java context - V.E.R.T. #111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
c872aa3
0fa8332
1cb50f5
60dccd6
f93b677
e669c5b
2f4bf3f
52d6d3b
bc4386d
04e3c8c
fdca436
a70224e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package org.greencodeinitiative.creedengo.java.checks; | ||
|
|
||
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| class UseFilterBeforeSort { | ||
| UseFilterBeforeSort() { | ||
| } | ||
|
|
||
| public void manipulateStream(final List<String> list) { | ||
| list.stream() // Noncompliant {{Use 'filter' before 'sorted' for better efficiency.}} | ||
| .sorted() | ||
| .filter(s -> s.startsWith("A")) | ||
| .collect(Collectors.toList()); | ||
dedece35 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| list.stream() // Compliant {{Use 'filter' before 'sorted' for better efficiency.}} | ||
| .filter(s -> s.startsWith("A")) | ||
| .sorted() | ||
| .collect(Collectors.toList()); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,77 @@ | ||||||
| package org.greencodeinitiative.creedengo.java.checks; | ||||||
|
|
||||||
| import org.sonar.check.Rule; | ||||||
| import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; | ||||||
| import org.sonar.plugins.java.api.tree.*; | ||||||
|
|
||||||
| import java.util.ArrayList; | ||||||
| import java.util.Collections; | ||||||
| import java.util.List; | ||||||
|
|
||||||
| @Rule(key = "GCI91") | ||||||
| public class UseFilterBeforeSort extends IssuableSubscriptionVisitor { | ||||||
| @Override | ||||||
| public List<Tree.Kind> nodesToVisit() { | ||||||
| return Collections.singletonList(Tree.Kind.METHOD_INVOCATION); | ||||||
| } | ||||||
|
|
||||||
| @Override | ||||||
| public void visitNode(Tree tree) { | ||||||
| if (tree instanceof MethodInvocationTree) { | ||||||
| MethodInvocationTree mit = (MethodInvocationTree) tree; | ||||||
|
|
||||||
| if (!isTerminalOperation(mit)) { | ||||||
| return; // On ignore les appels intermédiaires (filter, sorted, etc.) | ||||||
|
||||||
| } | ||||||
|
|
||||||
| List<String> methodChain = extractChainedMethodNames(mit); | ||||||
|
|
||||||
| int sortedIndex = methodChain.indexOf("sorted"); | ||||||
| int filterIndex = methodChain.indexOf("filter"); | ||||||
|
|
||||||
| if (sortedIndex != -1 && filterIndex != -1 && sortedIndex < filterIndex) { | ||||||
dedece35 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are you sure about your algorithms ... I think in streams we can have several select method ... thus you can control only the first one ? or more complex, control all the stream and we want only control th second part ... for example : what is the behaviour for examples with the same complexity (several parts) ? |
||||||
| reportIssue(tree, "Use 'filter' before 'sorted' for better efficiency."); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| private List<String> extractChainedMethodNames(MethodInvocationTree terminalInvocation) { | ||||||
| List<String> methodNames = new ArrayList<>(); | ||||||
| ExpressionTree current = terminalInvocation; | ||||||
|
|
||||||
| while (current instanceof MethodInvocationTree) { | ||||||
| MethodInvocationTree methodCall = (MethodInvocationTree) current; | ||||||
| ExpressionTree methodSelect = methodCall.methodSelect(); | ||||||
|
|
||||||
| if (methodSelect instanceof MemberSelectExpressionTree) { | ||||||
| MemberSelectExpressionTree memberSelect = (MemberSelectExpressionTree) methodSelect; | ||||||
| methodNames.add(memberSelect.identifier().name()); | ||||||
| current = memberSelect.expression(); // remonter à l'appel précédent | ||||||
| } else { | ||||||
| break; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| Collections.reverse(methodNames); // pour avoir l’ordre stream → ... → collect | ||||||
| return methodNames; | ||||||
| } | ||||||
|
|
||||||
| private boolean isTerminalOperation(MethodInvocationTree tree) { | ||||||
| if (!(tree.methodSelect() instanceof MemberSelectExpressionTree)) { | ||||||
| return false; | ||||||
| } | ||||||
| String methodName = ((MemberSelectExpressionTree) tree.methodSelect()).identifier().name(); | ||||||
| // Ajouter ici tous les terminaux connus | ||||||
|
||||||
| // Ajouter ici tous les terminaux connus | |
| // Add all known terminal operations here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please correct this feedback
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |
| "GCI78", | ||
| "GCI79", | ||
| "GCI82", | ||
| "GCI91", | ||
| "GCI94" | ||
| ] | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| /* | ||
| * creedengo - Java language - Provides rules to reduce the environmental footprint of your Java 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 <http://www.gnu.org/licenses/>. | ||
| */ | ||
| package org.greencodeinitiative.creedengo.java.checks; | ||
|
|
||
| class UseFilterBeforeSort { | ||
| UseFilterBeforeSort() { | ||
| } | ||
|
|
||
| public void manipulateStream(List<String> list) { | ||
| list.stream() // Noncompliant {{Use 'filter' before 'sorted' for better efficiency.}} | ||
| .sorted() | ||
| .filter(s -> s.startsWith("A")) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| list.stream() // Noncompliant {{Use 'filter' before 'sorted' for better efficiency.}} | ||
| .sorted() | ||
| .otherMethod() | ||
| .filter(s -> s.startsWith("A")) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| list.stream() // Compliant {{Use 'filter' before 'sorted' for better efficiency.}} | ||
| .filter(s -> s.startsWith("A")) | ||
| .sorted() | ||
| .collect(Collectors.toList()); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package org.greencodeinitiative.creedengo.java.checks; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import org.sonar.java.checks.verifier.CheckVerifier; | ||
|
|
||
| public class UseFilterBeforeSortTest { | ||
| @Test | ||
| void testHasIssues() { | ||
dedece35 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| CheckVerifier.newVerifier() | ||
| .onFile("src/test/files/UseFilterBeforeSort.java") | ||
| .withCheck(new UseFilterBeforeSort()) | ||
| .verifyIssues(); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when I execute the integration test, I have the following error and it's relevant for me. You should modify your startLines, endLines and severity value.
[ERROR] Failures:
[ERROR] GCIRulesIT.testGCI91:559->GCIRulesBase.checkIssuesForFile:84 [Extracted: rule, message, textRange.startLine, textRange.endLine, severity, type, effort]
Expecting ArrayList:
[("creedengo-java:GCI91", "Use 'filter' before 'sorted' for better efficiency.", 11, 14, MAJOR, CODE_SMELL, "5min"),
("creedengo-java:GCI91", "Use 'filter' before 'sorted' for better efficiency.", 16, 20, MAJOR, CODE_SMELL, "5min")]
to contain:
[("creedengo-java:GCI91", "Use 'filter' before 'sorted' for better efficiency.", 25, 25, MINOR, CODE_SMELL, "5min")]
but could not find the following element(s):
[("creedengo-java:GCI91", "Use 'filter' before 'sorted' for better efficiency.", 25, 25, MINOR, CODE_SMELL, "5min")]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the line 25 is the good one and not the bad one.