Merge pull request #9 from donhui/add_rule

add rule for check select *
donhui · Sep 1, 2019 · 6b0a3a9 · 6b0a3a9
2 parents fe3b078 + b9ced2d
commit 6b0a3a9
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 16 deletions.
diff --git a/src/main/java/org/sonarsource/plugins/mybatis/languages/MyBatisQualityProfile.java b/src/main/java/org/sonarsource/plugins/mybatis/languages/MyBatisQualityProfile.java
@@ -34,6 +34,9 @@ public void define(Context context) {
         NewBuiltInActiveRule rule6 = profile.activateRule(REPO_KEY, "MyBatisMapperCheckRule6");
         rule6.overrideSeverity(Severity.CRITICAL);
 
+        NewBuiltInActiveRule rule7 = profile.activateRule(REPO_KEY, "MyBatisMapperCheckRule7");
+        rule7.overrideSeverity(Severity.MINOR);
+
         profile.done();
     }
 

diff --git a/src/main/java/org/sonarsource/plugins/mybatis/rules/MyBatisLintSensor.java b/src/main/java/org/sonarsource/plugins/mybatis/rules/MyBatisLintSensor.java
@@ -47,6 +47,13 @@ public class MyBatisLintSensor implements Sensor {
     private static final Logger LOGGER = Loggers.get(MyBatisLintSensor.class);
 
     private static final String LEFT_SLASH = "/";
+    private static final String SELECT = "select";
+    private static final String UPDATE = "update";
+    private static final String DELETE = "delete";
+    private static final String WHERE = "where";
+    private static final String COUNT_STAR ="count(*)";
+    private static final String STAR ="*";
+
 
     protected final Configuration config;
     protected final FileSystem fileSystem;
@@ -164,7 +171,7 @@ private void parseStatement(Set<MappedStatement> stmts, Map mybatisMapperMap) {
                                 mapperResource.substring(mapperResource.indexOf('[') + 1, mapperResource.indexOf(']'));
 
                             // windows environment
-                            if(!reducedXmlFilePath.startsWith(LEFT_SLASH)){
+                            if (!reducedXmlFilePath.startsWith(LEFT_SLASH)) {
                                 reducedXmlFilePath = LEFT_SLASH + reducedXmlFilePath.replace("\\", LEFT_SLASH);
                             }
                             LOGGER.debug("reducedMapperFilePath: " + reducedXmlFilePath);
@@ -201,7 +208,7 @@ private void cleanFiles(List<File> files) {
         for (File file : files) {
             if (file.exists() && file.isFile()) {
                 try {
-                    Files.delete(Paths.get(new URI("file:///" + file.getAbsolutePath().replace("\\",LEFT_SLASH))));
+                    Files.delete(Paths.get(new URI("file:///" + file.getAbsolutePath().replace("\\", LEFT_SLASH))));
                 } catch (IOException | URISyntaxException e) {
                     LOGGER.warn(e.toString());
                 }
@@ -224,35 +231,43 @@ private void matchRuleAndSaveIssue(String sql, String sourceMapperFilePath, Inte
         String errorMessage = "";
         String ruleId = "";
         if (containsOneEqualsOne(sql)) {
-            if (sql.startsWith("delete")) {
+            if (sql.startsWith(DELETE)) {
                 // delete statement contains 1=1
                 errorMessage = "delete statement should not include 1=1";
                 ruleId = "MyBatisMapperCheckRule3";
-            } else if (sql.startsWith("update")) {
+            } else if (sql.startsWith(UPDATE)) {
                 // update statement contains 1=1
                 errorMessage = "update statement should not include 1=1";
                 ruleId = "MyBatisMapperCheckRule2";
-            } else if (sql.startsWith("select") && !containsFunctionOrLimit(sql)) {
+            } else if (sql.startsWith(SELECT) && !containsFunctionOrLimit(sql)) {
                 // select statement contains 1=1
                 errorMessage = "select statement should not include 1=1";
                 ruleId = "MyBatisMapperCheckRule1";
             }
-        } else if (!sql.contains("where")) {
-            if (sql.startsWith("delete")) {
+        } else if (!sql.contains(WHERE)) {
+            if (sql.startsWith(DELETE)) {
                 // Where condition not found in delete statement
-                errorMessage = "Where condition not found in delete statement";
+                errorMessage = "where condition not found in delete statement";
                 ruleId = "MyBatisMapperCheckRule6";
-            } else if (sql.startsWith("update")) {
+            } else if (sql.startsWith(UPDATE)) {
                 // Where condition not found in update statement
-                errorMessage = "Where condition not found in update statement";
+                errorMessage = "where condition not found in update statement";
                 ruleId = "MyBatisMapperCheckRule5";
-            } else if (sql.startsWith("select") && !containsFunctionOrLimit(sql)) {
+            } else if (sql.startsWith(SELECT) && !containsFunctionOrLimit(sql)) {
                 // Where condition not found in select statement
-                errorMessage = "Where condition not found in select statement";
+                errorMessage = "where condition not found in select statement";
                 ruleId = "MyBatisMapperCheckRule4";
             }
         }
 
+        if (sql.startsWith(SELECT) && sql.contains(STAR)) {
+            sql = sql.replace(" ", "");
+            if (!sql.contains(COUNT_STAR)) {
+                errorMessage = "select statement should not include *";
+                ruleId = "MyBatisMapperCheckRule7";
+            }
+        }
+
         if (!"".equals(ruleId)) {
             ErrorDataFromLinter mybatisError =
                 new ErrorDataFromLinter(ruleId, errorMessage, sourceMapperFilePath, lineNumber);

diff --git a/src/main/resources/mybatis/mybatislint-rules.xml b/src/main/resources/mybatis/mybatislint-rules.xml
@@ -49,7 +49,7 @@
     </rule>
     <rule>
         <key>MyBatisMapperCheckRule4</key>
-        <name>Where condition not found in select statement</name>
+        <name>where condition not found in select statement</name>
         <internalKey>MyBatisMapperCheckRule4</internalKey>
         <description>If all parameters in the update statement of Mapper XML file are null,
             the sql will not have where condition, then it will select all records from the table, which may lead to
@@ -67,7 +67,7 @@
     </rule>
     <rule>
         <key>MyBatisMapperCheckRule5</key>
-        <name>Where condition not found in update statement</name>
+        <name>where condition not found in update statement</name>
         <internalKey>MyBatisMapperCheckRule5</internalKey>
         <description>If all parameters in the update statement of Mapper XML file are null,
             the sql will not have where condition, then it will update all records from the table, which will result
@@ -85,7 +85,7 @@
     </rule>
     <rule>
         <key>MyBatisMapperCheckRule6</key>
-        <name>Where condition not found in delete statement</name>
+        <name>where condition not found in delete statement</name>
         <internalKey>MyBatisMapperCheckRule6</internalKey>
         <description>If all parameters in the delete statement of Mapper XML file are null,
             the sql will not have where condition, then it will delete all records from the table, which will result
@@ -97,9 +97,23 @@
         <status>READY</status>
         <type>BUG</type>
         <tag>mybatis</tag>
-        <tag>mybatis</tag>
         <tag>delete</tag>
         <remediationFunction>LINEAR</remediationFunction>
         <remediationFunctionGapMultiplier>20min</remediationFunctionGapMultiplier>
     </rule>
+    <rule>
+        <key>MyBatisMapperCheckRule7</key>
+        <name>select statement should not include *</name>
+        <internalKey>MyBatisMapperCheckRule7</internalKey>
+        <description>select statement should not include *, it is not a bad practice, suggest using specific fields.
+        </description>
+        <severity>MINOR</severity>
+        <cardinality>SINGLE</cardinality>
+        <status>READY</status>
+        <type>BUG</type>
+        <tag>mybatis</tag>
+        <tag>select</tag>
+        <remediationFunction>LINEAR</remediationFunction>
+        <remediationFunctionGapMultiplier>10min</remediationFunctionGapMultiplier>
+    </rule>
 </mybatislint-rules>