Merge pull request #1606 from guwirth/quality-flaws-2

quality flaws 2
SonarOpenCommunity · Nov 30, 2018 · 5d8c77b · 5d8c77b
2 parents ee91258 + bb3a4c3
commit 5d8c77b
Show file tree

Hide file tree

Showing 236 changed files with 3,297 additions and 3,565 deletions.
diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/BooleanEqualityComparisonCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/BooleanEqualityComparisonCheck.java
@@ -39,6 +39,11 @@
 @SqaleConstantRemediation("5min")
 public class BooleanEqualityComparisonCheck extends SquidCheck<Grammar> {
 
+  private static boolean hasBooleanLiteralOperand(AstNode node) {
+    return node.hasDirectChildren(CxxGrammarImpl.LITERAL, CxxGrammarImpl.BOOL)
+      && node.hasDescendant(CxxKeyword.TRUE, CxxKeyword.FALSE);
+  }
+
   @Override
   public void init() {
     subscribeTo(CxxGrammarImpl.equalityExpression);
@@ -54,9 +59,4 @@ public void visitNode(AstNode node) {
     }
   }
 
-  private static boolean hasBooleanLiteralOperand(AstNode node) {
-    return node.hasDirectChildren(CxxGrammarImpl.LITERAL, CxxGrammarImpl.BOOL)
-      && node.hasDescendant(CxxKeyword.TRUE, CxxKeyword.FALSE);
-  }
-
 }
diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/CollapsibleIfCandidateCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/CollapsibleIfCandidateCheck.java
@@ -41,22 +41,6 @@
 @SqaleConstantRemediation("5min")
 public class CollapsibleIfCandidateCheck extends SquidCheck<Grammar> {
 
-  @Override
-  public void init() {
-    subscribeTo(CxxGrammarImpl.selectionStatement);
-  }
-
-  @Override
-  public void visitNode(AstNode node) {
-    if (!hasElseClause(node) && !hasDeclaration(node)) {
-      AstNode enclosingIfStatement = getEnclosingIfStatement(node);
-      if (enclosingIfStatement != null && !hasElseClause(enclosingIfStatement)
-        && hasSingleTrueStatement(enclosingIfStatement) && !hasDeclaration(enclosingIfStatement)) {
-        getContext().createLineViolation(this, "Merge this if statement with the enclosing one.", node);
-      }
-    }
-  }
-
   private static boolean hasElseClause(AstNode node) {
     return node.hasDirectChildren(CxxKeyword.ELSE);
   }
@@ -95,4 +79,20 @@ private static boolean hasSingleTrueStatement(AstNode node) {
         .getChildren().size() == 1 : true;
   }
 
+  @Override
+  public void init() {
+    subscribeTo(CxxGrammarImpl.selectionStatement);
+  }
+
+  @Override
+  public void visitNode(AstNode node) {
+    if (!hasElseClause(node) && !hasDeclaration(node)) {
+      AstNode enclosingIfStatement = getEnclosingIfStatement(node);
+      if (enclosingIfStatement != null && !hasElseClause(enclosingIfStatement)
+        && hasSingleTrueStatement(enclosingIfStatement) && !hasDeclaration(enclosingIfStatement)) {
+        getContext().createLineViolation(this, "Merge this if statement with the enclosing one.", node);
+      }
+    }
+  }
+
 }
diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/CommentedCodeCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/CommentedCodeCheck.java
@@ -54,25 +54,9 @@ public class CommentedCodeCheck extends SquidCheck<Grammar> implements AstAndTok
 
   private static final double THRESHOLD = 0.94;
 
-  private final CodeRecognizer codeRecognizer = new CodeRecognizer(THRESHOLD, new CxxRecognizer());
-
   private static final Pattern EOL_PATTERN = Pattern.compile("\\R");
 
-  private static class CxxRecognizer implements LanguageFootprint {
-
-    @Override
-    public Set<Detector> getDetectors() {
-      Set<Detector> detectors = new HashSet<>();
-
-      detectors.add(new EndWithDetector(0.95, '}', ';', '{'));
-      detectors.add(new KeywordsDetector(0.7, "||", "&&"));
-      detectors.add(new KeywordsDetector(0.3, CppKeyword.keywordValues()));
-      detectors.add(new ContainsDetector(0.95, "for(", "if(", "while(", "catch(", "switch(", "try{", "else{"));
-
-      return detectors;
-    }
-
-  }
+  private final CodeRecognizer codeRecognizer = new CodeRecognizer(THRESHOLD, new CxxRecognizer());
 
   @Override
   public void visitToken(Token token) {
@@ -98,4 +82,20 @@ public void visitToken(Token token) {
     }
   }
 
+  private static class CxxRecognizer implements LanguageFootprint {
+
+    @Override
+    public Set<Detector> getDetectors() {
+      Set<Detector> detectors = new HashSet<>();
+
+      detectors.add(new EndWithDetector(0.95, '}', ';', '{'));
+      detectors.add(new KeywordsDetector(0.7, "||", "&&"));
+      detectors.add(new KeywordsDetector(0.3, CppKeyword.keywordValues()));
+      detectors.add(new ContainsDetector(0.95, "for(", "if(", "while(", "catch(", "switch(", "try{", "else{"));
+
+      return detectors;
+    }
+
+  }
+
 }
diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/HardcodedAccountCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/HardcodedAccountCheck.java
@@ -25,7 +25,6 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;
-
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
 import org.sonar.check.RuleProperty;

diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/HardcodedIpCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/HardcodedIpCheck.java
@@ -25,7 +25,6 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;
-
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
 import org.sonar.check.RuleProperty;

diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/MagicNumberCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/MagicNumberCheck.java
@@ -50,39 +50,6 @@ public class MagicNumberCheck extends SquidCheck<Grammar> {
   private static final String DEFAULT_EXCEPTIONS
     = "0,1,0x0,0x00,.0,.1,0.0,1.0,0u,1u,0ul,1ul,1.0f,0.0f,0LL,1LL,0ULL,1ULL";
 
-  @RuleProperty(
-    key = "exceptions",
-    description = "Comma separated list of allowed values (excluding '-' and '+' signs)",
-    defaultValue = DEFAULT_EXCEPTIONS)
-  public String exceptions = DEFAULT_EXCEPTIONS;
-
-  private final Set<String> exceptionsSet = new HashSet<>();
-
-  @Override
-  public void init() {
-    subscribeTo(CxxTokenType.NUMBER);
-    for (String magicNumber : exceptions.split(",")) {
-      magicNumber = magicNumber.trim();
-      if (!magicNumber.isEmpty()) {
-        exceptionsSet.add(magicNumber);
-      }
-    }
-  }
-
-  @Override
-  public void visitNode(AstNode node) {
-    if (!isConstexpr(node)
-      && !isConst(node)
-      && !isExcluded(node)
-      && !isInEnum(node)
-      && !isArrayInitializer(node)
-      && !isGenerated(node)
-      && !isNullPtr(node)) {
-      getContext().createLineViolation(this, "Extract this magic number '" + node.getTokenOriginalValue()
-        + "' into a constant, variable declaration or an enum.", node);
-    }
-  }
-
   private static boolean isConstexpr(AstNode node) {
     AstNode decl = null;
 
@@ -116,10 +83,6 @@ private static boolean isConst(AstNode node) {
     return false;
   }
 
-  private boolean isExcluded(AstNode node) {
-    return exceptionsSet.contains(node.getTokenOriginalValue());
-  }
-
   private static boolean isInEnum(AstNode node) {
     return node.hasAncestor(CxxGrammarImpl.enumeratorList);
   }
@@ -135,5 +98,40 @@ private static boolean isGenerated(AstNode node) {
   private static boolean isNullPtr(AstNode node) {
     return "nullptr".equals(node.getTokenValue());
   }
+  @RuleProperty(
+    key = "exceptions",
+    description = "Comma separated list of allowed values (excluding '-' and '+' signs)",
+    defaultValue = DEFAULT_EXCEPTIONS)
+  public String exceptions = DEFAULT_EXCEPTIONS;
+  private final Set<String> exceptionsSet = new HashSet<>();
+
+  @Override
+  public void init() {
+    subscribeTo(CxxTokenType.NUMBER);
+    for (String magicNumber : exceptions.split(",")) {
+      magicNumber = magicNumber.trim();
+      if (!magicNumber.isEmpty()) {
+        exceptionsSet.add(magicNumber);
+      }
+    }
+  }
+
+  @Override
+  public void visitNode(AstNode node) {
+    if (!isConstexpr(node)
+      && !isConst(node)
+      && !isExcluded(node)
+      && !isInEnum(node)
+      && !isArrayInitializer(node)
+      && !isGenerated(node)
+      && !isNullPtr(node)) {
+      getContext().createLineViolation(this, "Extract this magic number '" + node.getTokenOriginalValue()
+        + "' into a constant, variable declaration or an enum.", node);
+    }
+  }
+
+  private boolean isExcluded(AstNode node) {
+    return exceptionsSet.contains(node.getTokenOriginalValue());
+  }
 
 }
diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/MissingCurlyBracesCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/MissingCurlyBracesCheck.java
@@ -19,17 +19,17 @@
  */
 package org.sonar.cxx.checks;
 
+import com.sonar.sslr.api.AstNode;
+import com.sonar.sslr.api.Grammar;
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
 import org.sonar.cxx.api.CxxKeyword;
-import org.sonar.cxx.parser.CxxGrammarImpl;
-import org.sonar.squidbridge.checks.SquidCheck;
-import com.sonar.sslr.api.AstNode;
-import com.sonar.sslr.api.Grammar;
 import static org.sonar.cxx.checks.utils.CheckUtils.isIfStatement;
+import org.sonar.cxx.parser.CxxGrammarImpl;
+import org.sonar.cxx.tag.Tag;
 import org.sonar.squidbridge.annotations.ActivatedByDefault;
 import org.sonar.squidbridge.annotations.SqaleConstantRemediation;
-import org.sonar.cxx.tag.Tag;
+import org.sonar.squidbridge.checks.SquidCheck;
 
 @Rule(
   key = "MissingCurlyBraces",

diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/NestedStatementsCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/NestedStatementsCheck.java
@@ -58,6 +58,13 @@ public class NestedStatementsCheck extends SquidCheck<Grammar> {
 
   private static final int DEFAULT_MAX = 3;
 
+  /**
+   * @return True if the given node is the 'if' in an 'else if' construct.
+   */
+  private static boolean isElseIf(AstNode node) {
+    return isIfStatement(node) && node.getParent().getPreviousAstNode().getType().equals(CxxKeyword.ELSE);
+  }
+
   /**
    * max
    */
@@ -113,10 +120,4 @@ private void visitChildren(List<AstNode> watchedDescendants) {
     }
   }
 
-  /**
-   * @return True if the given node is the 'if' in an 'else if' construct.
-   */
-  private static boolean isElseIf(AstNode node) {
-    return isIfStatement(node) && node.getParent().getPreviousAstNode().getType().equals(CxxKeyword.ELSE);
-  }
 }
diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/ReservedNamesCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/ReservedNamesCheck.java
@@ -29,7 +29,6 @@
 import java.util.Locale;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
 import org.sonar.cxx.api.CxxKeyword;
@@ -53,9 +52,8 @@
 public class ReservedNamesCheck extends SquidCheck<Grammar> implements CxxCharsetAwareVisitor {
 
   private static final String[] keywords = CxxKeyword.keywordValues();
-  private Charset charset = StandardCharsets.UTF_8;
   private static final Pattern DEFINE_DECLARATION_PATTERN = Pattern.compile("^\\s*#define\\s+([^\\s(]+).*$");
-
+  private Charset charset = StandardCharsets.UTF_8;
 
   @Override
   public void visitFile(AstNode astNode) {

diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/SwitchLastCaseIsDefaultCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/SwitchLastCaseIsDefaultCheck.java
@@ -50,6 +50,20 @@ public class SwitchLastCaseIsDefaultCheck extends SquidCheck<Grammar> {
   private static final String DEFAULT_CASE_IS_NOT_LAST_MESSAGE = "Move this default to the end of the switch.";
   private static final String DEFAULT_CASE_TOKENVALUE = "default";
 
+  private static void getSwitchCases(List<AstNode> result, AstNode node) {
+    if (isSwitchStatement(node)) {
+      return;
+    }
+    if (node.is(CxxGrammarImpl.labeledStatement)) {
+      result.add(node);
+    }
+    if (node.hasChildren()) {
+      for (AstNode child : node.getChildren()) {
+        getSwitchCases(result, child);
+      }
+    }
+  }
+
   @Override
   public void init() {
     subscribeTo(CHECKED_TYPES);
@@ -87,18 +101,4 @@ private List<AstNode> getSwitchCases(AstNode node) {
     return cases;
   }
 
-  private static void getSwitchCases(List<AstNode> result, AstNode node) {
-    if (isSwitchStatement(node)) {
-      return;
-    }
-    if (node.is(CxxGrammarImpl.labeledStatement)) {
-      result.add(node);
-    }
-    if (node.hasChildren()) {
-      for (AstNode child : node.getChildren()) {
-        getSwitchCases(result, child);
-      }
-    }
-  }
-
 }
diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/UnnamedNamespaceInHeaderCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/UnnamedNamespaceInHeaderCheck.java
@@ -39,7 +39,16 @@
 //similar Vera++ rule T017
 public class UnnamedNamespaceInHeaderCheck extends SquidCheck<Grammar> {
 
-  private static final String[] DEFAULT_NAME_SUFFIX = new String[] { ".h", ".hh", ".hpp", ".H" };
+  private static final String[] DEFAULT_NAME_SUFFIX = new String[]{".h", ".hh", ".hpp", ".H"};
+
+  private static boolean isHeader(String name) {
+    for (String suff : DEFAULT_NAME_SUFFIX) {
+      if (name.endsWith(suff)) {
+        return true;
+      }
+    }
+    return false;
+  }
   private Boolean isHeader = false;
 
   @Override
@@ -61,12 +70,4 @@ public void visitNode(AstNode node) {
     }
   }
 
-  private static boolean isHeader(String name) {
-    for (String suff : DEFAULT_NAME_SUFFIX) {
-      if (name.endsWith(suff)) {
-        return true;
-      }
-    }
-    return false;
-  }
 }
diff --git a/cxx-checks/src/main/java/org/sonar/cxx/checks/UsingNamespaceInHeaderCheck.java b/cxx-checks/src/main/java/org/sonar/cxx/checks/UsingNamespaceInHeaderCheck.java
@@ -39,7 +39,16 @@
 //similar Vera++ rule T018
 public class UsingNamespaceInHeaderCheck extends SquidCheck<Grammar> {
 
-  private static final String[] DEFAULT_NAME_SUFFIX = new String[] { ".h", ".hh", ".hpp", ".H" };
+  private static final String[] DEFAULT_NAME_SUFFIX = new String[]{".h", ".hh", ".hpp", ".H"};
+
+  private static boolean isHeader(String name) {
+    for (String suff : DEFAULT_NAME_SUFFIX) {
+      if (name.endsWith(suff)) {
+        return true;
+      }
+    }
+    return false;
+  }
   private Boolean isHeader = false;
 
   @Override
@@ -60,12 +69,4 @@ public void visitNode(AstNode node) {
     }
   }
 
-  private static boolean isHeader(String name) {
-    for (String suff : DEFAULT_NAME_SUFFIX) {
-      if (name.endsWith(suff)) {
-        return true;
-      }
-    }
-    return false;
-  }
 }