Issue checkstyle#4713: fix all violations from the third part of idea…

… violations
Nimfadora · Jul 19, 2017 · 7bd0e9a · 7bd0e9a
1 parent c1c5a42
commit 7bd0e9a
Show file tree

Hide file tree

Showing 25 changed files with 171 additions and 125 deletions.
diff --git a/config/intellij-idea-inspections.xml b/config/intellij-idea-inspections.xml
@@ -88,7 +88,7 @@
     <inspection_tool class="AssignmentToDateFieldFromParameter" enabled="true" level="ERROR" enabled_by_default="true">
         <option name="ignorePrivateMethods" value="true" />
     </inspection_tool>
-    <inspection_tool class="AssignmentToForLoopParameter" enabled="false" level="ERROR" enabled_by_default="false">
+    <inspection_tool class="AssignmentToForLoopParameter" enabled="true" level="ERROR" enabled_by_default="true">
         <option name="m_checkForeachParameters" value="true" />
     </inspection_tool>
     <inspection_tool class="AssignmentToForLoopParameterJS" enabled="true" level="ERROR" enabled_by_default="true" />
@@ -277,7 +277,7 @@
     <inspection_tool class="CheckTagEmptyBody" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="CheckValidXmlInScriptTagBody" enabled="true" level="WARNING" enabled_by_default="true" />
     <inspection_tool class="CheckXmlFileWithXercesValidator" enabled="true" level="WARNING" enabled_by_default="true" />
-    <inspection_tool class="CheckedExceptionClass" enabled="false" level="ERROR" enabled_by_default="false" />
+    <inspection_tool class="CheckedExceptionClass" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="ClashingGetters" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="ClashingTraitMethods" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="ClassComplexity" enabled="false" level="ERROR" enabled_by_default="false">
@@ -680,7 +680,7 @@
     <inspection_tool class="ExpectedExceptionNeverThrownTestNG" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="ExplicitGet" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="ExtendsAnnotation" enabled="true" level="ERROR" enabled_by_default="true" />
-    <inspection_tool class="ExtendsConcreteCollection" enabled="false" level="ERROR" enabled_by_default="false" />
+    <inspection_tool class="ExtendsConcreteCollection" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="ExtendsObject" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="ExtendsThread" enabled="true" level="WARNING" enabled_by_default="true" />
     <inspection_tool class="ExtendsThrowable" enabled="false" level="ERROR" enabled_by_default="false" />
@@ -746,8 +746,8 @@
     </inspection_tool>
     <inspection_tool class="ForLoopThatDoesntUseLoopVariable" enabled="true" level="WARNING" enabled_by_default="true" />
     <inspection_tool class="ForLoopThatDoesntUseLoopVariableJS" enabled="true" level="ERROR" enabled_by_default="true" />
-    <inspection_tool class="ForLoopWithMissingComponent" enabled="false" level="ERROR" enabled_by_default="false">
-        <option name="ignoreCollectionLoops" value="false" />
+    <inspection_tool class="ForLoopWithMissingComponent" enabled="true" level="ERROR" enabled_by_default="true">
+        <option name="ignoreCollectionLoops" value="true" />
     </inspection_tool>
     <!-- we are ok to use for-each as we use modern java -->
     <inspection_tool class="ForeachStatement" enabled="false" level="ERROR" enabled_by_default="false" />
@@ -1497,7 +1497,9 @@
     <inspection_tool class="NestedSwitchStatement" enabled="false" level="ERROR" enabled_by_default="false" />
     <inspection_tool class="NestedSwitchStatementJS" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="NestedSynchronizedStatement" enabled="true" level="WARNING" enabled_by_default="true" />
-    <inspection_tool class="NestedTryStatement" enabled="false" level="ERROR" enabled_by_default="false" />
+    <inspection_tool class="NestedTryStatement" enabled="true" level="ERROR" enabled_by_default="true" >
+        <scope name="Tests" level="ERROR" enabled="false" />
+    </inspection_tool>
     <inspection_tool class="NestingDepth" enabled="true" level="ERROR" enabled_by_default="true">
         <option name="m_limit" value="5" />
     </inspection_tool>
@@ -1706,8 +1708,8 @@
     <inspection_tool class="ProblematicVarargsMethodOverride" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="ProblematicWhitespace" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="PropertyValueSetToItself" enabled="true" level="ERROR" enabled_by_default="true" />
-    <inspection_tool class="ProtectedField" enabled="false" level="ERROR" enabled_by_default="false" />
-    <inspection_tool class="ProtectedInnerClass" enabled="false" level="ERROR" enabled_by_default="false">
+    <inspection_tool class="ProtectedField" enabled="true" level="ERROR" enabled_by_default="true" />
+    <inspection_tool class="ProtectedInnerClass" enabled="true" level="ERROR" enabled_by_default="true">
         <option name="ignoreEnums" value="false" />
         <option name="ignoreInterfaces" value="false" />
     </inspection_tool>
@@ -1986,7 +1988,10 @@
     </inspection_tool>
     <inspection_tool class="StaticPseudoFunctionalStyleMethod" enabled="true" level="ERROR" enabled_by_default="true" />
     <inspection_tool class="StaticSuite" enabled="true" level="ERROR" enabled_by_default="true" />
-    <inspection_tool class="StaticVariableInitialization" enabled="false" level="ERROR" enabled_by_default="false">
+    <inspection_tool class="StaticVariableInitialization" enabled="true" level="ERROR" enabled_by_default="true">
+        <scope name="Tests" level="ERROR" enabled="false" >
+            <option name="m_ignorePrimitives" value="false" />
+        </scope>
         <option name="m_ignorePrimitives" value="false" />
     </inspection_tool>
     <inspection_tool class="StaticVariableNamingConvention" enabled="true" level="WARNING" enabled_by_default="true">
@@ -1997,7 +2002,7 @@
     </inspection_tool>
     <!-- it lacks option to skip inner field of the same size that we use give convenient ready to use instances -->
     <inspection_tool class="StaticVariableOfConcreteClass" enabled="false" level="ERROR" enabled_by_default="false" />
-    <inspection_tool class="StaticVariableUninitializedUse" enabled="false" level="WARNING" enabled_by_default="true">
+    <inspection_tool class="StaticVariableUninitializedUse" enabled="true" level="WARNING" enabled_by_default="true">
         <scope name="Tests" level="WARNING" enabled="false">
             <option name="m_ignorePrimitives" value="false" />
         </scope>
@@ -2071,6 +2076,10 @@
                 <option value="UseOfSystemOutOrSystemErr" />
                 <option value="SuspiciousArrayCast" />
                 <option value="AbstractClassWithOnlyOneDirectInheritor" />
+                <option value="ProtectedInnerClass" />
+                <option value="CheckedExceptionClass" />
+                <option value="StaticVariableMayNotBeInitialized" />
+                <option value="StaticVariableUsedBeforeInitialization" />
             </list>
         </option>
     </inspection_tool>

diff --git a/src/it/java/com/google/checkstyle/test/base/BaseCheckTestSupport.java b/src/it/java/com/google/checkstyle/test/base/BaseCheckTestSupport.java
@@ -59,7 +59,15 @@ public class BaseCheckTestSupport {
 
     private static Configuration configuration;
 
-    protected final ByteArrayOutputStream stream = new ByteArrayOutputStream();
+    private final ByteArrayOutputStream stream = new ByteArrayOutputStream();
+
+    /**
+     * Returns test logger.
+     * @return logger test logger
+     */
+    public BriefUtLogger getBriefUtLogger() {
+        return new BriefUtLogger(stream);
+    }
 
     /**
      * Returns {@link Configuration} based on Google's checks xml-configuration (google_checks.xml).
@@ -103,7 +111,7 @@ protected Checker createChecker(Configuration checkConfig)
         checker.setLocaleLanguage(locale.getLanguage());
         checker.setModuleClassLoader(Thread.currentThread().getContextClassLoader());
         checker.configure(dc);
-        checker.addListener(new BriefUtLogger(stream));
+        checker.addListener(getBriefUtLogger());
         return checker;
     }
 

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/PackageNamesLoader.java b/src/main/java/com/puppycrawl/tools/checkstyle/PackageNamesLoader.java
@@ -141,24 +141,10 @@ public static Set<String> getPackageNames(ClassLoader classLoader)
             final Enumeration<URL> packageFiles = classLoader.getResources(CHECKSTYLE_PACKAGES);
 
             while (packageFiles.hasMoreElements()) {
-                final URL packageFile = packageFiles.nextElement();
-                InputStream stream = null;
-
-                try {
-                    stream = new BufferedInputStream(packageFile.openStream());
-                    final InputSource source = new InputSource(stream);
-                    namesLoader.parseInputSource(source);
-                }
-                catch (IOException ex) {
-                    throw new CheckstyleException("unable to open " + packageFile, ex);
-                }
-                finally {
-                    Closeables.closeQuietly(stream);
-                }
+                processFile(packageFiles.nextElement(), namesLoader);
             }
 
             result = namesLoader.packageNames;
-
         }
         catch (IOException ex) {
             throw new CheckstyleException("unable to get package file resources", ex);
@@ -169,4 +155,27 @@ public static Set<String> getPackageNames(ClassLoader classLoader)
 
         return result;
     }
+
+    /**
+     * Reads the file provided and parses it with package names loader.
+     * @param packageFile file from package
+     * @param namesLoader package names loader
+     * @throws SAXException if an error while parsing occurs
+     * @throws CheckstyleException if unable to open file
+     */
+    private static void processFile(URL packageFile, PackageNamesLoader namesLoader)
+            throws SAXException, CheckstyleException {
+        InputStream stream = null;
+        try {
+            stream = new BufferedInputStream(packageFile.openStream());
+            final InputSource source = new InputSource(stream);
+            namesLoader.parseInputSource(source);
+        }
+        catch (IOException ex) {
+            throw new CheckstyleException("unable to open " + packageFile, ex);
+        }
+        finally {
+            Closeables.closeQuietly(stream);
+        }
+    }
 }
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java b/src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java
@@ -219,18 +219,11 @@ public void remove(String checkedFileName) {
      * @return the hashcode for serializable object.
      */
     private static String getHashCodeBasedOnObjectContent(Serializable object) {
-        try {
-            // im-memory serialization of Configuration
+        final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+        try (ObjectOutputStream oos = new ObjectOutputStream(outputStream)) {
 
-            final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
-            ObjectOutputStream oos = null;
-            try {
-                oos = new ObjectOutputStream(outputStream);
-                oos.writeObject(object);
-            }
-            finally {
-                flushAndCloseOutStream(oos);
-            }
+            oos.writeObject(object);
+            Flushables.flush(oos, false);
 
             // Instead of hexEncoding outputStream.toByteArray() directly we
             // use a message digest here to keep the length of the

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/api/CheckstyleException.java b/src/main/java/com/puppycrawl/tools/checkstyle/api/CheckstyleException.java
@@ -23,6 +23,7 @@
  * Represents an error condition within Checkstyle.
  *
  * @author Oliver Burn
+ * @noinspection CheckedExceptionClass
  */
 public class CheckstyleException extends Exception {
     /** For Serialisation that will never happen. */

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/AbstractTypeAwareCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/AbstractTypeAwareCheck.java
@@ -397,6 +397,7 @@ protected final AbstractClassInfo findClassAlias(final String name) {
 
     /**
      * Contains class's {@code Token}.
+     * @noinspection ProtectedInnerClass
      */
     protected abstract static class AbstractClassInfo {
         /** {@code FullIdent} associated with this class. */
@@ -509,6 +510,7 @@ public String toString() {
 
     /**
      * Represents text element with location in the text.
+     * @noinspection ProtectedInnerClass
      */
     protected static class Token {
         /** Token's column number. */

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/FileContentsHolder.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/FileContentsHolder.java
@@ -30,6 +30,7 @@
  * a filter can access the current file contents through this module.
  * @author Mike McMahon
  * @author Rick Giles
+ * @noinspection StaticVariableMayNotBeInitialized, StaticVariableUsedBeforeInitialization
  */
 public class FileContentsHolder extends AbstractCheck {
     /** The current file contents. */

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/NewlineAtEndOfFileCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/NewlineAtEndOfFileCheck.java
@@ -76,19 +76,8 @@ public class NewlineAtEndOfFileCheck
 
     @Override
     protected void processFiltered(File file, FileText fileText) {
-        // Cannot use lines as the line separators have been removed!
         try {
-            final RandomAccessFile randomAccessFile = new RandomAccessFile(file, "r");
-            boolean threw = true;
-            try {
-                if (!endsWithNewline(randomAccessFile)) {
-                    log(0, MSG_KEY_NO_NEWLINE_EOF, file.getPath());
-                }
-                threw = false;
-            }
-            finally {
-                Closeables.close(randomAccessFile, threw);
-            }
+            readAndCheckFile(file);
         }
         catch (final IOException ignored) {
             log(0, MSG_KEY_UNABLE_OPEN, file.getPath());
@@ -113,6 +102,27 @@ public void setLineSeparator(String lineSeparatorParam) {
         }
     }
 
+    /**
+     * Reads the file provided and checks line separators.
+     * @param file the file to be processed
+     * @throws IOException When an IO error occurred while reading from the
+     *         file provided
+     */
+    private void readAndCheckFile(File file) throws IOException {
+        // Cannot use lines as the line separators have been removed!
+        final RandomAccessFile randomAccessFile = new RandomAccessFile(file, "r");
+        boolean threw = true;
+        try {
+            if (!endsWithNewline(randomAccessFile)) {
+                log(0, MSG_KEY_NO_NEWLINE_EOF, file.getPath());
+            }
+            threw = false;
+        }
+        finally {
+            Closeables.close(randomAccessFile, threw);
+        }
+    }
+
     /**
      * Checks whether the content provided by the Reader ends with the platform
      * specific line separator.

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/UniquePropertiesCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/UniquePropertiesCheck.java
@@ -65,16 +65,8 @@ public UniquePropertiesCheck() {
     @Override
     protected void processFiltered(File file, FileText fileText) {
         final UniqueProperties properties = new UniqueProperties();
-
-        try {
-            final FileInputStream fileInputStream = new FileInputStream(file);
-            try {
-                // As file is already read, there should not be any exceptions.
-                properties.load(fileInputStream);
-            }
-            finally {
-                fileInputStream.close();
-            }
+        try (FileInputStream fileInputStream = new FileInputStream(file)) {
+            properties.load(fileInputStream);
         }
         catch (IOException ex) {
             log(0, MSG_IO_EXCEPTION_KEY, file.getPath(),
@@ -136,6 +128,7 @@ private static Pattern getKeyPattern(String keyName) {
      * Properties subclass to store duplicated property keys in a separate map.
      *
      * @author Pavel Baranchikov
+     * @noinspection ClassExtendsConcreteCollection
      */
     private static class UniqueProperties extends Properties {
         private static final long serialVersionUID = 1L;

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/annotation/MissingDeprecatedCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/annotation/MissingDeprecatedCheck.java
@@ -241,8 +241,8 @@ private boolean checkTagAtTheRestOfComment(String[] lines, boolean foundBefore,
             int currentLine, int index) {
 
         boolean found = false;
-        for (int reindex = index + 1;
-            reindex < lines.length;) {
+        int reindex = index + 1;
+        while (reindex < lines.length) {
             final Matcher multilineCont = MATCH_DEPRECATED_MULTILINE_CONT.matcher(lines[reindex]);
 
             if (multilineCont.find()) {

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocStyleCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocStyleCheck.java
@@ -288,20 +288,21 @@ private static String getCommentText(String... comments) {
      */
     private static int findTextStart(String line) {
         int textStart = -1;
-        for (int i = 0; i < line.length();) {
-            if (!Character.isWhitespace(line.charAt(i))) {
-                if (line.regionMatches(i, "/**", 0, "/**".length())) {
-                    i += 2;
+        int index = 0;
+        while (index < line.length()) {
+            if (!Character.isWhitespace(line.charAt(index))) {
+                if (line.regionMatches(index, "/**", 0, "/**".length())) {
+                    index += 2;
                 }
-                else if (line.regionMatches(i, "*/", 0, 2)) {
-                    i++;
+                else if (line.regionMatches(index, "*/", 0, 2)) {
+                    index++;
                 }
-                else if (line.charAt(i) != '*') {
-                    textStart = i;
+                else if (line.charAt(index) != '*') {
+                    textStart = index;
                     break;
                 }
             }
-            i++;
+            index++;
         }
         return textStart;
     }

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressWithNearbyCommentFilter.java b/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressWithNearbyCommentFilter.java
@@ -320,17 +320,12 @@ public Tag(String text, int line, SuppressWithNearbyCommentFilter filter) {
                 }
                 format = CommonUtils.fillTemplateWithStringsByRegexp(
                         filter.influenceFormat, text, filter.commentFormat);
-                final int influence;
-                try {
-                    if (CommonUtils.startsWithChar(format, '+')) {
-                        format = format.substring(1);
-                    }
-                    influence = Integer.parseInt(format);
-                }
-                catch (final NumberFormatException ex) {
-                    throw new IllegalArgumentException("unable to parse influence from '" + text
-                            + "' using " + filter.influenceFormat, ex);
+
+                if (CommonUtils.startsWithChar(format, '+')) {
+                    format = format.substring(1);
                 }
+                final int influence = parseInfluence(format, filter.influenceFormat, text);
+
                 if (influence >= 1) {
                     firstLine = line;
                     lastLine = line + influence;
@@ -346,6 +341,24 @@ public Tag(String text, int line, SuppressWithNearbyCommentFilter filter) {
             }
         }
 
+        /**
+         * Gets influence from suppress filter influence format param.
+         *
+         * @param format          influence format to parse
+         * @param influenceFormat raw influence format
+         * @param text            text of the suppression
+         * @return parsed influence
+         */
+        private static int parseInfluence(String format, String influenceFormat, String text) {
+            try {
+                return Integer.parseInt(format);
+            }
+            catch (final NumberFormatException ex) {
+                throw new IllegalArgumentException("unable to parse influence from '" + text
+                        + "' using " + influenceFormat, ex);
+            }
+        }
+
         @Override
         public boolean equals(Object other) {
             if (this == other) {