Issue #6241: resolved teamcity 2018.3 violations

.ci/jsoref-spellchecker/whitelist.words

@@ -1034,6 +1034,7 @@ RBRACK
 rcurly
 rdiachenko
 RDz
+reactivex
 README
 Readonly
 rebasing
@@ -1083,6 +1084,7 @@ ru
 ruleset
 Ruslan
 rw
+rx
 sabaka
 Sameline
 saxonica

config/intellij-idea-inspections.xml

@@ -1137,7 +1137,9 @@
                      enabled_by_default="true"/>
     <inspection_tool class="ClassOnlyUsedInOneModule" enabled="true" level="ERROR"
                      enabled_by_default="true"/>
-    <inspection_tool class="ClassOnlyUsedInOnePackage" enabled="true" level="ERROR"
+    <!-- packages should be where we define.
+         we are a library project and other projects might use them. -->
+    <inspection_tool class="ClassOnlyUsedInOnePackage" enabled="false" level="ERROR"
                      enabled_by_default="true"/>
     <inspection_tool class="ClassReferencesSubclass" enabled="true" level="ERROR"
                      enabled_by_default="true"/>
@@ -4952,6 +4954,22 @@
                      enabled_by_default="false"/>
     <inspection_tool class="UnsecureRandomNumberGeneration" enabled="true" level="ERROR"
                      enabled_by_default="true"/>
+    <inspection_tool class="UnstableApiUsage" enabled="true" level="WARNING"
+                     enabled_by_default="true">
+      <option name="unstableApiAnnotations">
+        <set>
+          <!-- removed guava's Beta. Unstable doesn't mean it isn't safe to use, just might
+               change between versions and isn't stable -->
+          <option value="io.reactivex.annotations.Beta" />
+          <option value="io.reactivex.annotations.Experimental" />
+          <option value="org.apache.http.annotation.Beta" />
+          <option value="org.gradle.api.Incubating" />
+          <option value="org.jetbrains.annotations.ApiStatus.Experimental" />
+          <option value="rx.annotations.Beta" />
+          <option value="rx.annotations.Experimental" />
+        </set>
+      </option>
+    </inspection_tool>
     <inspection_tool class="UnterminatedStatementJS" enabled="true" level="ERROR"
                      enabled_by_default="true">
         <option name="ignoreSemicolonAtEndOfBlock" value="true"/>

pom.xml

@@ -835,7 +835,7 @@
                     <exclude>com.puppycrawl.tools.checkstyle.gui.ParseTreeTableModel*</exclude>
                     <exclude>com.puppycrawl.tools.checkstyle.gui.TreeTableCellRenderer*</exclude>
                     <exclude>com.puppycrawl.tools.checkstyle.gui.TreeTableModelAdapter*</exclude>
-                    <!-- Deprecated classes -->
+                    <!-- This class is in deprecation phase -->
                     <exclude>
                       com.puppycrawl.tools.checkstyle.checks.naming.AbstractTypeParameterNameCheck
                     </exclude>
@@ -2180,7 +2180,7 @@
                 <param>com.puppycrawl.tools.checkstyle.checks.naming.*</param>
               </targetTests>
               <excludedClasses>
-                <!-- deprecated class -->
+                <!-- This class is in deprecation phase -->
                 <param>
                   com.puppycrawl.tools.checkstyle.checks.naming.AbstractTypeParameterNameCheck
                 </param>

src/main/java/com/puppycrawl/tools/checkstyle/AuditEventDefaultFormatter.java

@@ -114,7 +114,7 @@ private static String getCheckShortName(AuditEvent event) {
                     checkFullName.lastIndexOf(SUFFIX));
             }
             else {
-                checkShortName = checkFullName.substring(lastDotIndex + 1, checkFullName.length());
+                checkShortName = checkFullName.substring(lastDotIndex + 1);
             }
         }
         return checkShortName;

src/main/java/com/puppycrawl/tools/checkstyle/ConfigurationLoader.java

@@ -163,8 +163,11 @@ private ConfigurationLoader(final PropertyResolver overrideProps,
     }
 
     /**
-     * Creates mapping between local resources and dtd ids.
+     * Creates mapping between local resources and dtd ids. This method can't be
+     * moved to inner class because it must stay static because it is called
+     * from constructor and inner class isn't static.
      * @return map between local resources and dtd ids.
+     * @noinspection MethodOnlyUsedFromInnerClass
      */
     private static Map<String, String> createIdToResourceNameMap() {
         final Map<String, String> map = new HashMap<>();
@@ -453,7 +456,8 @@ public static Configuration loadConfiguration(InputSource configSource,
 
     /**
      * Replaces {@code ${xxx}} style constructions in the given value
-     * with the string value of the corresponding data types.
+     * with the string value of the corresponding data types. This method must remain
+     * outside inner class for easier testing since inner class requires an instance.
      *
      * <p>Code copied from ant -
      * http://cvs.apache.org/viewcvs/jakarta-ant/src/main/org/apache/tools/ant/ProjectHelper.java
@@ -471,7 +475,7 @@ public static Configuration loadConfiguration(InputSource configSource,
      * @throws CheckstyleException if the string contains an opening
      *                           {@code ${} without a closing
      *                           {@code }}
-     * @noinspection MethodWithMultipleReturnPoints
+     * @noinspection MethodWithMultipleReturnPoints, MethodOnlyUsedFromInnerClass
      */
     private static String replaceProperties(
             String value, PropertyResolver props, String defaultValue)

src/main/java/com/puppycrawl/tools/checkstyle/PackageObjectFactory.java

@@ -25,6 +25,8 @@
 import java.util.HashMap;
 import java.util.LinkedHashSet;
 import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -316,12 +318,13 @@ private Map<String, Set<String>> generateThirdPartyNameToFullModuleName(ClassLoa
      */
     public static String getShortFromFullModuleNames(String fullName) {
         String result = fullName;
-        if (NAME_TO_FULL_MODULE_NAME.containsValue(fullName)) {
-            result = NAME_TO_FULL_MODULE_NAME
-                    .entrySet()
-                    .stream()
-                    .filter(entry -> entry.getValue().equals(fullName))
-                    .findFirst().get().getKey();
+        final Optional<Entry<String, String>> optional = NAME_TO_FULL_MODULE_NAME
+                .entrySet()
+                .stream()
+                .filter(entry -> entry.getValue().equals(fullName))
+                .findFirst();
+        if (optional.isPresent()) {
+            result = optional.get().getKey();
         }
 
         return result;

src/main/java/com/puppycrawl/tools/checkstyle/XpathFileGeneratorAstFilter.java

@@ -22,7 +22,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 import com.puppycrawl.tools.checkstyle.api.AuditEvent;
 import com.puppycrawl.tools.checkstyle.api.AutomaticBean;
@@ -77,7 +76,7 @@ public boolean accept(TreeWalkerAuditEvent event) {
                     new XpathQueryGenerator(event, tabWidth);
             final List<String> xpathQueries = xpathQueryGenerator.generate();
             if (!xpathQueries.isEmpty()) {
-                final String query = xpathQueries.stream().collect(Collectors.joining(DELIMITER));
+                final String query = String.join(DELIMITER, xpathQueries);
                 MESSAGE_QUERY_MAP.put(event.getLocalizedMessage(), query);
             }
         }

src/main/java/com/puppycrawl/tools/checkstyle/api/LocalizedMessage.java

@@ -283,6 +283,11 @@ public LocalizedMessage(
                 sourceClass, customMessage);
     }
 
+    /**
+     * Indicates whether some other object is "equal to" this one.
+     * Suppression on enumeration is needed so code stays consistent.
+     * @noinspection EqualsCalledOnEnumConstant
+     */
     // -@cs[CyclomaticComplexity] equals - a lot of fields to check.
     @Override
     public boolean equals(Object object) {

src/main/java/com/puppycrawl/tools/checkstyle/checks/TranslationCheck.java

@@ -411,7 +411,7 @@ else if (languageMatcher.matches()) {
         }
         // We use substring(...) instead of replace(...), so that the regular expression does
         // not have to be compiled each time it is used inside 'replace' method.
-        final String removePattern = regexp.substring("^.+".length(), regexp.length());
+        final String removePattern = regexp.substring("^.+".length());
         return fileName.replaceAll(removePattern, "");
     }
 

src/main/java/com/puppycrawl/tools/checkstyle/checks/blocks/RightCurlyCheck.java

@@ -495,19 +495,14 @@ private static Details getDetailsForOthers(DetailAST ast) {
                 rcurly = child.getLastChild();
                 nextToken = ast;
             }
-            else if (tokenType == TokenTypes.METHOD_DEF) {
+            else {
                 lcurly = ast.findFirstToken(TokenTypes.SLIST);
                 if (lcurly != null) {
                     // SLIST could be absent if method is abstract
                     rcurly = lcurly.getLastChild();
                 }
                 nextToken = getNextToken(ast);
             }
-            else {
-                lcurly = ast.findFirstToken(TokenTypes.SLIST);
-                rcurly = lcurly.getLastChild();
-                nextToken = getNextToken(ast);
-            }
             return new Details(lcurly, rcurly, nextToken, false);
         }
 

src/main/java/com/puppycrawl/tools/checkstyle/checks/design/VisibilityModifierCheck.java

@@ -828,8 +828,7 @@ private static List<String> getClassShortNames(List<String> canonicalClassNames)
         final List<String> shortNames = new ArrayList<>();
         for (String canonicalClassName : canonicalClassNames) {
             final String shortClassName = canonicalClassName
-                    .substring(canonicalClassName.lastIndexOf('.') + 1,
-                    canonicalClassName.length());
+                    .substring(canonicalClassName.lastIndexOf('.') + 1);
             shortNames.add(shortClassName);
         }
         return shortNames;
@@ -842,8 +841,7 @@ private static List<String> getClassShortNames(List<String> canonicalClassNames)
      */
     private static String getClassShortName(String canonicalClassName) {
         return canonicalClassName
-                .substring(canonicalClassName.lastIndexOf('.') + 1,
-                canonicalClassName.length());
+                .substring(canonicalClassName.lastIndexOf('.') + 1);
     }
 
     /**

src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AbstractJavadocCheck.java

@@ -87,7 +87,6 @@ public abstract class AbstractJavadocCheck extends AbstractCheck {
      * generally tend to be fine with non tight html. It can be set through config file if a check
      * is to log violation upon encountering non-tight HTML in javadoc.
      *
-     * @see ParseStatus#firstNonTightHtmlTag
      * @see ParseStatus#isNonTight()
      * @see <a href="https://checkstyle.org/writingjavadocchecks.html#Tight-HTML_rules">
      *     Tight HTML rules</a>

src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/TagParser.java

@@ -156,10 +156,9 @@ private static boolean isTag(String[] javadocText, Point pos) {
 
         //Character.isJavaIdentifier... may not be a valid HTML
         //identifier but is valid for generics
-        return column < text.length()
-                && (Character.isJavaIdentifierStart(text.charAt(column))
-                    || text.charAt(column) == '/')
-                || column >= text.length();
+        return column >= text.length()
+                || Character.isJavaIdentifierStart(text.charAt(column))
+                    || text.charAt(column) == '/';
     }
 
     /**

src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressWithPlainTextCommentFilter.java

@@ -290,6 +290,11 @@ protected Suppression(
             }
         }
 
+        /**
+         * Indicates whether some other object is "equal to" this one.
+         * Suppression on enumeration is needed so code stays consistent.
+         * @noinspection EqualsCalledOnEnumConstant
+         */
         @Override
         public boolean equals(Object other) {
             if (this == other) {

src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilter.java

@@ -414,6 +414,11 @@ public int compareTo(Tag object) {
             return result;
         }
 
+        /**
+         * Indicates whether some other object is "equal to" this one.
+         * Suppression on enumeration is needed so code stays consistent.
+         * @noinspection EqualsCalledOnEnumConstant
+         */
         @Override
         public boolean equals(Object other) {
             if (this == other) {

src/main/java/com/puppycrawl/tools/checkstyle/gui/TreeTableModelAdapter.java

@@ -108,14 +108,6 @@ private Object nodeForRow(int row) {
         return treePath.getLastPathComponent();
     }
 
-    /**
-     * Invokes fireTableDataChanged after all the pending events have been
-     * processed. SwingUtilities.invokeLater is used to handle this.
-     */
-    private void delayedFireTableDataChanged() {
-        SwingUtilities.invokeLater(this::fireTableDataChanged);
-    }
-
     /**
      * TreeExpansionListener that can update the table when tree changes.
      */
@@ -160,6 +152,14 @@ public void treeStructureChanged(TreeModelEvent event) {
             delayedFireTableDataChanged();
         }
 
+        /**
+         * Invokes fireTableDataChanged after all the pending events have been
+         * processed. SwingUtilities.invokeLater is used to handle this.
+         */
+        private void delayedFireTableDataChanged() {
+            SwingUtilities.invokeLater(TreeTableModelAdapter.this::fireTableDataChanged);
+        }
+
     }
 
 }

src/test/java/com/puppycrawl/tools/checkstyle/MainTest.java

@@ -582,10 +582,9 @@ public void testLoadPropertiesIoException() throws Exception {
                     .equals(localizedMessage
                             .substring(0, localizedMessage.indexOf(' ')));
             final boolean sameSuffix =
-                    causeMessage.substring(causeMessage.lastIndexOf(' '), causeMessage.length())
+                    causeMessage.substring(causeMessage.lastIndexOf(' '))
                     .equals(localizedMessage
-                            .substring(localizedMessage.lastIndexOf(' '),
-                                    localizedMessage.length()));
+                            .substring(localizedMessage.lastIndexOf(' ')));
             assertTrue("Invalid error message", samePrefix || sameSuffix);
             assertTrue("Invalid error message", causeMessage.contains(".'"));
         }

src/test/java/com/puppycrawl/tools/checkstyle/checks/UniquePropertiesCheckTest.java

@@ -83,6 +83,7 @@ public void testDefault() throws Exception {
     /**
      * Tests the {@link UniquePropertiesCheck#getLineNumber(FileText, String)}
      * method return value.
+     * @noinspection JavadocReference Test javadocs should explain all.
      */
     @Test
     public void testNotFoundKey() throws Exception {

src/test/java/com/puppycrawl/tools/checkstyle/internal/AllTestsTest.java

@@ -22,12 +22,15 @@
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.function.Consumer;
+import java.util.stream.Stream;
 
 import org.junit.Assert;
 import org.junit.Test;
@@ -42,38 +45,39 @@ public class AllTestsTest {
     public void testAllInputsHaveTest() throws Exception {
         final Map<String, List<String>> allTests = new HashMap<>();
 
-        Files.walk(Paths.get("src/test/java"))
-            .forEach(filePath -> {
-                grabAllTests(allTests, filePath.toFile());
-            });
+        walk(Paths.get("src/test/java"), filePath -> {
+            grabAllTests(allTests, filePath.toFile());
+        });
 
         Assert.assertTrue("found tests", !allTests.keySet().isEmpty());
 
-        Files.walk(Paths.get("src/test/resources/com/puppycrawl"))
-            .forEach(filePath -> {
-                verifyInputFile(allTests, filePath.toFile());
-            });
-        Files.walk(Paths.get("src/test/resources-noncompilable/com/puppycrawl"))
-            .forEach(filePath -> {
-                verifyInputFile(allTests, filePath.toFile());
-            });
+        walk(Paths.get("src/test/resources/com/puppycrawl"), filePath -> {
+            verifyInputFile(allTests, filePath.toFile());
+        });
+        walk(Paths.get("src/test/resources-noncompilable/com/puppycrawl"), filePath -> {
+            verifyInputFile(allTests, filePath.toFile());
+        });
     }
 
     @Test
     public void testAllTestsHaveProductionCode() throws Exception {
         final Map<String, List<String>> allTests = new HashMap<>();
 
-        Files.walk(Paths.get("src/main/java"))
-            .forEach(filePath -> {
-                grabAllFiles(allTests, filePath.toFile());
-            });
+        walk(Paths.get("src/main/java"), filePath -> {
+            grabAllFiles(allTests, filePath.toFile());
+        });
 
         Assert.assertTrue("found tests", !allTests.keySet().isEmpty());
 
-        Files.walk(Paths.get("src/test/java"))
-            .forEach(filePath -> {
-                verifyHasProductionFile(allTests, filePath.toFile());
-            });
+        walk(Paths.get("src/test/java"), filePath -> {
+            verifyHasProductionFile(allTests, filePath.toFile());
+        });
+    }
+
+    private static void walk(Path path, Consumer<Path> action) throws IOException {
+        try (Stream<Path> walk = Files.walk(path)) {
+            walk.forEach(action);
+        }
     }
 
     private static void grabAllTests(Map<String, List<String>> allTests, File file) {