Skip to content
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

Add java rule EC24 : Optimize Database SQL Queries (Clause LIMIT) #20

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- [#239](https://github.com/green-code-initiative/ecoCode/issues/239) Add new Java rule EC24 : Optimize Database SQL Queries (Clause LIMIT)

### Changed

- [#15](https://github.com/green-code-initiative/ecoCode-java/issues/15) correction NullPointer in EC2 rule
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
<google.re2j>1.7</google.re2j>

<!-- temporary version waiting for real automatic release in ecocode repository -->
<ecocode-rules-specifications.version>1.5.0</ecocode-rules-specifications.version>
<ecocode-rules-specifications.version>1.5.1-SNAPSHOT</ecocode-rules-specifications.version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can wait for the next version of the artifact instead of using a snapshot here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

waiting for approvement of this implementation before releasing ecocode-rules-specifications


</properties>

Expand Down
19 changes: 3 additions & 16 deletions src/main/java/fr/greencodeinitiative/java/JavaCheckRegistrar.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,7 @@
import java.util.Collections;
import java.util.List;

import fr.greencodeinitiative.java.checks.ArrayCopyCheck;
import fr.greencodeinitiative.java.checks.AvoidFullSQLRequest;
import fr.greencodeinitiative.java.checks.AvoidGettingSizeCollectionInLoop;
import fr.greencodeinitiative.java.checks.AvoidMultipleIfElseStatement;
import fr.greencodeinitiative.java.checks.AvoidRegexPatternNotStatic;
import fr.greencodeinitiative.java.checks.AvoidSQLRequestInLoop;
import fr.greencodeinitiative.java.checks.AvoidSetConstantInBatchUpdate;
import fr.greencodeinitiative.java.checks.AvoidSpringRepositoryCallInLoopOrStreamCheck;
import fr.greencodeinitiative.java.checks.AvoidStatementForDMLQueries;
import fr.greencodeinitiative.java.checks.AvoidUsageOfStaticCollections;
import fr.greencodeinitiative.java.checks.FreeResourcesOfAutoCloseableInterface;
import fr.greencodeinitiative.java.checks.IncrementCheck;
import fr.greencodeinitiative.java.checks.InitializeBufferWithAppropriateSize;
import fr.greencodeinitiative.java.checks.NoFunctionCallWhenDeclaringForLoop;
import fr.greencodeinitiative.java.checks.OptimizeReadFileExceptions;
import fr.greencodeinitiative.java.checks.*;
import org.sonar.plugins.java.api.CheckRegistrar;
import org.sonar.plugins.java.api.JavaCheck;
import org.sonarsource.api.sonarlint.SonarLintSide;
Expand Down Expand Up @@ -62,7 +48,8 @@ public class JavaCheckRegistrar implements CheckRegistrar {
InitializeBufferWithAppropriateSize.class,
AvoidSetConstantInBatchUpdate.class,
FreeResourcesOfAutoCloseableInterface.class,
AvoidMultipleIfElseStatement.class
AvoidMultipleIfElseStatement.class,
OptimizeSQLQueriesWithLimit.class
);

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package fr.greencodeinitiative.java.checks;

import org.sonar.check.Rule;
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
import org.sonar.plugins.java.api.tree.LiteralTree;
import org.sonar.plugins.java.api.tree.Tree;

import java.util.List;
import java.util.function.Predicate;

import static java.util.Collections.singletonList;
import static java.util.regex.Pattern.CASE_INSENSITIVE;
import static java.util.regex.Pattern.compile;

@Rule(key = "EC24")
public class OptimizeSQLQueriesWithLimit extends IssuableSubscriptionVisitor {

public static final String MESSAGE_RULE = "Optimize Database SQL Queries (Clause LIMIT)";
private static final Predicate<String> LIMIT_REGEXP =
compile("limit", CASE_INSENSITIVE).asPredicate();
private static final Predicate<String> SELECT_REGEXP =
compile("select", CASE_INSENSITIVE).asPredicate();
private static final Predicate<String> FROM_REGEXP =
compile("from", CASE_INSENSITIVE).asPredicate();

@Override
public List<Tree.Kind> nodesToVisit() {
return singletonList(Tree.Kind.STRING_LITERAL);
}

@Override
public void visitNode(Tree tree) {
String value = ((LiteralTree) tree).value();
if (SELECT_REGEXP.test(value) && FROM_REGEXP.test(value) && !LIMIT_REGEXP.test(value)) {
reportIssue(tree, MESSAGE_RULE);
}
}

}
18 changes: 18 additions & 0 deletions src/test/files/OptimizeSQLQueriesWithLimit.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
class OptimizeSQLQueriesWithLimit {

public void literalSQLrequest() {
dummyCall("SELECT user FROM myTable"); // Noncompliant {{Optimize Database SQL Queries (Clause LIMIT)}}
dummyCall("SELECT user FROM myTable LIMIT 50"); // Compliant
}

@Query("select t from Todo t where t.status != 'COMPLETED'") // Noncompliant {{Optimize Database SQL Queries (Clause LIMIT)}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the JavaScript plugin, I added the WHERE keyword in the list of limiting keywords so as not to be too strict either. We could consider that this query already performs a stricter selection than all the data in the table. I don't know what your opinion is on this? maybe it lacks measure too :p

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@utarwyn update done

@Query("select t from Todo t where t.status != 'COMPLETED' LIMIT 25") // Compliant

private void callQuery() {
String sql1 = "SELECT user FROM myTable"; // Noncompliant {{Optimize Database SQL Queries (Clause LIMIT)}}
String sql2 = "SELECT user FROM myTable LIMIT 50"; // Compliant
}

private void dummyCall(String request) {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void checkNumberRules() {
final JavaCheckRegistrar registrar = new JavaCheckRegistrar();
registrar.register(context);

assertThat(context.checkClasses()).hasSize(15);
assertThat(context.checkClasses()).hasSize(16);
assertThat(context.testCheckClasses()).isEmpty();

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ void init() {
RulesDefinition.Context context = new RulesDefinition.Context();
rulesDefinition.define(context);
repository = context.repository(rulesDefinition.repositoryKey());
rulesSize = 15;
rulesSize = 16;
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package fr.greencodeinitiative.java.checks;

import org.junit.jupiter.api.Test;
import org.sonar.java.checks.verifier.CheckVerifier;

public class OptimizeSQLQueriesWithLimitTest {

@Test
void test() {
CheckVerifier.newVerifier()
.onFile("src/test/files/OptimizeSQLQueriesWithLimit.java")
.withCheck(new OptimizeSQLQueriesWithLimit())
.verifyIssues();
}

}
Loading