Issue #1566: MultipleStringLiterals violations fixed

checkstyle · Aug 25, 2015 · 085ce12 · 085ce12
1 parent 8aec5bb
commit 085ce12
Show file tree

Hide file tree

Showing 20 changed files with 137 additions and 80 deletions.
diff --git a/config/checkstyle_checks.xml b/config/checkstyle_checks.xml
@@ -319,6 +319,7 @@
     <module name="AvoidInlineConditionals"/>
     <module name="IllegalType"/>
     <module name="TrailingComment"/>
+    <module name="MultipleStringLiterals"/>
 
 <!--
     <module name="ClassDataAbstractionCoupling"/>
@@ -330,7 +331,6 @@
     <module name="JavadocTagContinuationIndentation"/>
     <module name="JavaNCSS"/>
     <module name="MissingCtor"/>
-    <module name="MultipleStringLiterals"/>
     <module name="NPathComplexity"/>
     <module name="OneTopLevelClass"/>
     <module name="OverloadMethodsDeclarationOrder"/>

diff --git a/config/suppressions.xml b/config/suppressions.xml
@@ -67,4 +67,7 @@
     <!-- Till https://github.com/checkstyle/checkstyle/issues/1854 -->
     <suppress checks="TrailingComment" files="(InnerAssignmentCheck\.java|OperatorWrapCheck\.java|XMLLoggerTest\.java|AbbreviationAsWordInNameCheckTest\.java)"/>
 
+    <!-- Fixing these cases will decrease code readability -->
+    <suppress checks="MultipleStringLiterals" files="JavadocStyleCheck\.java|AbstractTypeAwareCheck\.java|XMLLogger\.java"/>
+    <suppress checks="MultipleStringLiterals" files=".*[\\/]src[\\/]test[\\/]"/>
 </suppressions>
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/ConfigurationLoader.java b/src/main/java/com/puppycrawl/tools/checkstyle/ConfigurationLoader.java
@@ -96,6 +96,9 @@ public final class ConfigurationLoader {
     private static final String UNABLE_TO_PARSE_EXCEPTION_PREFIX = "unable to parse"
             + " configuration stream";
 
+    /** Dollar sign literal. */
+    private static final char DOLLAR_SIGN = '$';
+
     /** The SAX document handler */
     private final InternalLoader saxHandler;
 
@@ -261,9 +264,9 @@ public static Configuration loadConfiguration(InputSource configSource,
             return loader.configuration;
         }
         catch (final SAXParseException e) {
-            throw new CheckstyleException(UNABLE_TO_PARSE_EXCEPTION_PREFIX
-                    + " - " + e.getMessage() + ":" + e.getLineNumber()
-                    + ":" + e.getColumnNumber(), e);
+            final String message = String.format("%s - %s:%s:%s", UNABLE_TO_PARSE_EXCEPTION_PREFIX,
+                    e.getMessage(), e.getLineNumber(), e.getColumnNumber());
+            throw new CheckstyleException(message, e);
         }
         catch (final ParserConfigurationException | IOException | SAXException e) {
             throw new CheckstyleException(UNABLE_TO_PARSE_EXCEPTION_PREFIX, e);
@@ -352,7 +355,7 @@ private static void parsePropertyString(String value,
         throws CheckstyleException {
         int prev = 0;
         //search for the next instance of $ from the 'prev' position
-        int pos = value.indexOf('$', prev);
+        int pos = value.indexOf(DOLLAR_SIGN, prev);
         while (pos >= 0) {
 
             //if there was any text before this, add it as a fragment
@@ -362,13 +365,13 @@ private static void parsePropertyString(String value,
             //if we are at the end of the string, we tack on a $
             //then move past it
             if (pos == value.length() - 1) {
-                fragments.add("$");
+                fragments.add(String.valueOf(DOLLAR_SIGN));
                 prev = pos + 1;
             }
             else if (value.charAt(pos + 1) != '{') {
-                if (value.charAt(pos + 1) == '$') {
+                if (value.charAt(pos + 1) == DOLLAR_SIGN) {
                     //backwards compatibility two $ map to one mode
-                    fragments.add("$");
+                    fragments.add(String.valueOf(DOLLAR_SIGN));
                     prev = pos + 2;
                 }
                 else {
@@ -392,7 +395,7 @@ else if (value.charAt(pos + 1) != '{') {
             }
 
             //search for the next instance of $ from the 'prev' position
-            pos = value.indexOf('$', prev);
+            pos = value.indexOf(DOLLAR_SIGN, prev);
         }
         //no more $ signs found
         //if there is any tail to the file, append it

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java b/src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java
@@ -265,9 +265,10 @@ private void registerCheck(Check check)
                     registerCheck(token, check);
                 }
                 else {
-                    throw new CheckstyleException("Token \""
-                        + token + "\" was not found in Acceptable tokens list"
-                                + " in check " + check.getClass().getName());
+                    final String message = String.format("Token \"%s\" was not found in "
+                            + "Acceptable tokens list in check %s",
+                            token, check.getClass().getName());
+                    throw new CheckstyleException(message);
                 }
             }
         }
@@ -293,8 +294,9 @@ private static void validateDefaultTokens(Check check) throws CheckstyleExceptio
             Arrays.sort(defaultTokens);
             for (final int token : check.getRequiredTokens()) {
                 if (Arrays.binarySearch(defaultTokens, token) < 0) {
-                    throw new CheckstyleException("Token \"" + token + "\" from required tokens was"
-                        + " not found in default tokens list in check " + check);
+                    final String message = String.format("Token \"%s\" from required tokens was"
+                            + " not found in default tokens list in check %s", token, check);
+                    throw new CheckstyleException(message);
                 }
             }
         }

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java b/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java
@@ -65,6 +65,9 @@ public class CheckstyleAntTask extends Task {
     /** Poor man's enum for an plain formatter */
     private static final String E_PLAIN = "plain";
 
+    /** Suffix for time string. */
+    private static final String TIME_SUFFIX = " ms.";
+
     /** Class path to locate class files */
     private Path classpath;
 
@@ -252,7 +255,7 @@ public void execute() {
         }
         finally {
             final long endTime = System.currentTimeMillis();
-            log("Total execution took " + (endTime - startTime) + " ms.",
+            log("Total execution took " + (endTime - startTime) + TIME_SUFFIX,
                 Project.MSG_VERBOSE);
         }
     }
@@ -295,7 +298,7 @@ private void realExecute() {
             long startTime = System.currentTimeMillis();
             final List<File> files = scanFileSets();
             long endTime = System.currentTimeMillis();
-            log("To locate the files took " + (endTime - startTime) + " ms.",
+            log("To locate the files took " + (endTime - startTime) + TIME_SUFFIX,
                 Project.MSG_VERBOSE);
 
             log("Running Checkstyle " + version + " on " + files.size()
@@ -305,7 +308,7 @@ private void realExecute() {
             startTime = System.currentTimeMillis();
             final int numErrs = checker.process(files);
             endTime = System.currentTimeMillis();
-            log("To process the files took " + (endTime - startTime) + " ms.",
+            log("To process the files took " + (endTime - startTime) + TIME_SUFFIX,
                 Project.MSG_VERBOSE);
             final int numWarnings = warningCounter.getCount();
             final boolean ok = numErrs <= maxErrors

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/api/AutomaticBean.java b/src/main/java/com/puppycrawl/tools/checkstyle/api/AutomaticBean.java
@@ -159,10 +159,9 @@ private void tryCopyProperty(String moduleName, String key, Object value, boolea
                 final PropertyDescriptor pd =
                         PropertyUtils.getPropertyDescriptor(this, key);
                 if (pd == null) {
-                    throw new CheckstyleException(
-                            "Property '" + key + "' in module "
-                             + moduleName
-                             + " does not exist, please check the documentation");
+                    final String message = String.format("Property '%s' in module %s does not "
+                            + "exist, please check the documentation", key, moduleName);
+                    throw new CheckstyleException(message);
                 }
             }
             // finally we can set the bean property
@@ -174,14 +173,14 @@ private void tryCopyProperty(String moduleName, String key, Object value, boolea
             // as we do PropertyUtils.getPropertyDescriptor before beanUtils.copyProperty
             // so we have to join these exceptions with InvocationTargetException
             // to satisfy UTs coverage
-            throw new CheckstyleException(
-                "Cannot set property '" + key + "' to '" + value
-                + "' in module "  + moduleName, e);
+            final String message = String.format("Cannot set property '%s' to '%s' in module %s",
+                    key, value, moduleName);
+            throw new CheckstyleException(message, e);
         }
         catch (final IllegalArgumentException | ConversionException e) {
-            throw new CheckstyleException(
-                "illegal value '" + value + "' for property '" + key
-                + "' of module " + moduleName, e);
+            final String message = String.format("illegal value '%s' for property '%s' of "
+                    + "module %s", value, key, moduleName);
+            throw new CheckstyleException(message, e);
         }
     }
 

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/api/Comment.java b/src/main/java/com/puppycrawl/tools/checkstyle/api/Comment.java
@@ -98,7 +98,8 @@ public boolean intersects(int startLine, int startCol,
 
     @Override
     public String toString() {
-        return "Comment[" + startLineNo + ":" + startColNo + "-"
-            + endLineNo + ":" + endColNo + "]";
+        final String separator = ":";
+        return "Comment[" + startLineNo + separator + startColNo + "-"
+            + endLineNo + separator + endColNo + "]";
     }
 }
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/AbstractOptionCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/AbstractOptionCheck.java
@@ -37,6 +37,10 @@
  */
 public abstract class AbstractOptionCheck<T extends Enum<T>>
     extends Check {
+
+    /** Semicolon literal. */
+    protected static final String SEMICOLON = ";";
+
     /** Since I cannot get this by going <tt>T.class</tt>. */
     private final Class<T> optionClass;
     /** The policy to enforce */

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/ClassResolver.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/ClassResolver.java
@@ -30,6 +30,12 @@
  * @author Oliver Burn
  */
 public class ClassResolver {
+
+    /** Period literal. */
+    private static final String PERIOD = ".";
+    /** Dollar sign literal. */
+    private static final String DOLLAR_SIGN = "$";
+
     /** Name of the package to check if the class belongs to **/
     private final String pkg;
     /** Set of imports to check against **/
@@ -77,7 +83,7 @@ public Class<?> resolve(String name, String currentClass)
             // when checking for "DataException", it will match on
             // "SecurityDataException". This has been the cause of a very
             // difficult bug to resolve!
-            if (imp.endsWith("." + name)) {
+            if (imp.endsWith(PERIOD + name)) {
                 clazz = resolveQualifiedName(imp);
                 if (clazz != null) {
                     return clazz;
@@ -88,7 +94,7 @@ public Class<?> resolve(String name, String currentClass)
 
         // See if in the package
         if (pkg != null && !pkg.isEmpty()) {
-            clazz = resolveQualifiedName(pkg + "." + name);
+            clazz = resolveQualifiedName(pkg + PERIOD + name);
             if (clazz != null) {
                 return clazz;
             }
@@ -121,10 +127,10 @@ private Class<?> resolveInnerClass(String name, String currentClass)
             throws ClassNotFoundException {
         Class<?> clazz = null;
         if (!currentClass.isEmpty()) {
-            String innerClass = currentClass + "$" + name;
+            String innerClass = currentClass + DOLLAR_SIGN + name;
 
             if (!pkg.isEmpty()) {
-                innerClass = pkg + "." + innerClass;
+                innerClass = pkg + PERIOD + innerClass;
             }
 
             if (isLoadable(innerClass)) {
@@ -198,7 +204,7 @@ private Class<?> resolveQualifiedName(final String name) {
                 final int dot = name.lastIndexOf('.');
                 if (dot != -1) {
                     final String innerName =
-                        name.substring(0, dot) + "$" + name.substring(dot + 1);
+                        name.substring(0, dot) + DOLLAR_SIGN + name.substring(dot + 1);
                     if (isLoadable(innerName)) {
                         classObj = safeLoad(innerName);
                     }

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/blocks/LeftCurlyCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/blocks/LeftCurlyCheck.java
@@ -95,7 +95,10 @@ public class LeftCurlyCheck
      */
     public static final String MSG_KEY_LINE_BREAK_AFTER = "line.break.after";
 
-    /** If true, Check will ignore enums*/
+    /** Open curly brace literal. */
+    private static final String OPEN_CURLY_BRACE = "{";
+
+    /** If true, Check will ignore enums. */
     private boolean ignoreEnums = true;
 
     /**
@@ -289,7 +292,7 @@ private void verifyBrace(final DetailAST brace,
                 || braceLine.charAt(brace.getColumnNo() + 1) != '}') {
             if (getAbstractOption() == LeftCurlyOption.NL) {
                 if (!Utils.whitespaceBefore(brace.getColumnNo(), braceLine)) {
-                    log(brace, MSG_KEY_LINE_NEW, "{", brace.getColumnNo() + 1);
+                    log(brace, MSG_KEY_LINE_NEW, OPEN_CURLY_BRACE, brace.getColumnNo() + 1);
                 }
             }
             else if (getAbstractOption() == LeftCurlyOption.EOL) {
@@ -311,10 +314,10 @@ else if (startToken.getLineNo() != brace.getLineNo()) {
      */
     private void validateEol(DetailAST brace, String braceLine) {
         if (Utils.whitespaceBefore(brace.getColumnNo(), braceLine)) {
-            log(brace, MSG_KEY_LINE_PREVIOUS, "{", brace.getColumnNo() + 1);
+            log(brace, MSG_KEY_LINE_PREVIOUS, OPEN_CURLY_BRACE, brace.getColumnNo() + 1);
         }
         if (!hasLineBreakAfter(brace)) {
-            log(brace, MSG_KEY_LINE_BREAK_AFTER, "{", brace.getColumnNo() + 1);
+            log(brace, MSG_KEY_LINE_BREAK_AFTER, OPEN_CURLY_BRACE, brace.getColumnNo() + 1);
         }
     }
 
@@ -329,14 +332,14 @@ private void validateNewLinePosion(DetailAST brace, DetailAST startToken,
         // not on the same line
         if (startToken.getLineNo() + 1 == brace.getLineNo()) {
             if (Utils.whitespaceBefore(brace.getColumnNo(), braceLine)) {
-                log(brace, MSG_KEY_LINE_PREVIOUS, "{", brace.getColumnNo() + 1);
+                log(brace, MSG_KEY_LINE_PREVIOUS, OPEN_CURLY_BRACE, brace.getColumnNo() + 1);
             }
             else {
-                log(brace, MSG_KEY_LINE_NEW, "{", brace.getColumnNo() + 1);
+                log(brace, MSG_KEY_LINE_NEW, OPEN_CURLY_BRACE, brace.getColumnNo() + 1);
             }
         }
         else if (!Utils.whitespaceBefore(brace.getColumnNo(), braceLine)) {
-            log(brace, MSG_KEY_LINE_NEW, "{", brace.getColumnNo() + 1);
+            log(brace, MSG_KEY_LINE_NEW, OPEN_CURLY_BRACE, brace.getColumnNo() + 1);
         }
     }
 

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/AvoidStarImportCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/AvoidStarImportCheck.java
@@ -72,6 +72,9 @@ public class AvoidStarImportCheck
      */
     public static final String MSG_KEY = "import.avoidStar";
 
+    /** Suffix for the star import. */
+    private static final String STAR_IMPORT_SUFFIX = ".*";
+
     /** The packages/classes to exempt from this check. */
     private final List<String> excludes = Lists.newArrayList();
 
@@ -113,14 +116,13 @@ public int[] getRequiredTokens() {
      */
     public void setExcludes(String... excludesParam) {
         excludes.clear();
-        final String suffix = ".*";
 
         for (final String exclude : excludesParam) {
-            if (exclude.endsWith(suffix)) {
+            if (exclude.endsWith(STAR_IMPORT_SUFFIX)) {
                 excludes.add(exclude);
             }
             else {
-                excludes.add(exclude + suffix);
+                excludes.add(exclude + STAR_IMPORT_SUFFIX);
             }
         }
     }
@@ -163,7 +165,7 @@ else if (!allowStaticMemberImports
     private void logsStarredImportViolation(DetailAST startingDot) {
         final FullIdent name = FullIdent.createFullIdent(startingDot);
         final String importText = name.getText();
-        if (importText.endsWith(".*") && !excludes.contains(importText)) {
+        if (importText.endsWith(STAR_IMPORT_SUFFIX) && !excludes.contains(importText)) {
             log(startingDot.getLineNo(), MSG_KEY, importText);
         }
     }

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/UnusedImportsCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/UnusedImportsCheck.java
@@ -69,6 +69,9 @@ public class UnusedImportsCheck extends Check {
     private static final Pattern ARGUMENT_NAME = Pattern.compile(
            "[(,]\\s*" + CLASS_NAME.pattern());
 
+    /** Suffix for the star import. */
+    private static final String STAR_IMPORT_SUFFIX = ".*";
+
     /** Flag to indicate when time to start collecting references. */
     private boolean collect;
     /** Flag whether to process Javdoc comments. */
@@ -195,7 +198,7 @@ private void processIdent(DetailAST ast) {
      */
     private void processImport(DetailAST ast) {
         final FullIdent name = FullIdent.createFullIdentBelow(ast);
-        if (!name.getText().endsWith(".*")) {
+        if (!name.getText().endsWith(STAR_IMPORT_SUFFIX)) {
             imports.add(name);
         }
     }
@@ -208,7 +211,7 @@ private void processStaticImport(DetailAST ast) {
         final FullIdent name =
             FullIdent.createFullIdent(
                 ast.getFirstChild().getNextSibling());
-        if (!name.getText().endsWith(".*")) {
+        if (!name.getText().endsWith(STAR_IMPORT_SUFFIX)) {
             imports.add(name);
         }
     }

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AtclauseOrderCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AtclauseOrderCheck.java
@@ -69,6 +69,8 @@ public class AtclauseOrderCheck extends AbstractJavadocCheck {
      */
     public static final String MSG_KEY = "at.clause.order";
 
+    /** Comma literal. */
+    private static final String COMMA_SEPARATOR = ",";
     /**
      * Default order of atclauses.
      */
@@ -104,7 +106,7 @@ public class AtclauseOrderCheck extends AbstractJavadocCheck {
      */
     public void setTarget(String target) {
         final List<Integer> customTarget = new ArrayList<>();
-        final String[] sTarget = target.split(",");
+        final String[] sTarget = target.split(COMMA_SEPARATOR);
         for (String aSTarget : sTarget) {
             customTarget.add(Utils.getTokenId(aSTarget.trim()));
         }
@@ -117,7 +119,7 @@ public void setTarget(String target) {
      */
     public void setTagOrder(String order) {
         final List<String> customOrder = new ArrayList<>();
-        final String[] sOrder = order.split(",");
+        final String[] sOrder = order.split(COMMA_SEPARATOR);
         for (String aSOrder : sOrder) {
             customOrder.add(aSOrder.trim());
         }