Issue checkstyle#4394: increase coverage of pitest-checkstyle-api pro…

…file to 96%

config/suppressions.xml

@@ -21,7 +21,7 @@
               files="AbstractClassNameCheck.java"/>
     <!-- test should be named as their main class -->
     <suppress checks="AbstractClassNameCheck"
-              files="AbstractCheckTest.java|AbstractClassNameCheckTest.java|AbstractTypeAwareCheckTest.java|AbstractJavadocCheckTest.java|AbstractViolationReporterTest.java|AbstractFileSetCheckTest.java"/>
+              files="AbstractCheckTest.java|AbstractClassNameCheckTest.java|AbstractTypeAwareCheckTest.java|AbstractJavadocCheckTest.java|AbstractViolationReporterTest.java|AbstractFileSetCheckTest.java|AbstractLoaderTest.java"/>
 
     <!-- Tone down the checking for test code -->
     <suppress checks="CyclomaticComplexity" files="[\\/]XdocsPagesTest\.java"/>

pom.xml

@@ -2195,8 +2195,18 @@
                 <param>com.puppycrawl.tools.checkstyle.checks.imports.ImportControlCheckTest</param>
                 <param>com.puppycrawl.tools.checkstyle.checks.javadoc.WriteTagCheckTest</param>
                 <param>com.puppycrawl.tools.checkstyle.checks.naming.ParameterNameCheckTest</param>
+                <param>com.puppycrawl.tools.checkstyle.ConfigurationLoaderTest</param>
+                <param>com.puppycrawl.tools.checkstyle.checks.TranslationCheckTest</param>
               </targetTests>
-              <mutationThreshold>88</mutationThreshold>
+              <excludedMethods>
+                <!--cause of https://github.com/checkstyle/checkstyle/issues/3605-->
+                <param>addFeaturesForVerySecureJavaInstallations</param>
+              </excludedMethods>
+              <avoidCallsTo>
+                <!--cause of https://github.com/checkstyle/checkstyle/issues/3605-->
+                <avoidCallsTo>com.puppycrawl.tools.checkstyle.api.AbstractLoader$FeaturesForVerySecureJavaInstallations</avoidCallsTo>
+              </avoidCallsTo>
+              <mutationThreshold>96</mutationThreshold>
               <timeoutFactor>${pitest.plugin.timeout.factor}</timeoutFactor>
               <timeoutConstant>${pitest.plugin.timeout.constant}</timeoutConstant>
               <threads>${pitest.plugin.threads}</threads>

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

@@ -49,12 +49,6 @@
  */
 public abstract class AbstractLoader
     extends DefaultHandler {
-    /** Feature that enables loading external DTD when loading XML files. */
-    private static final String LOAD_EXTERNAL_DTD =
-        "http://apache.org/xml/features/nonvalidating/load-external-dtd";
-    /** Feature that enables including external general entities in XML files. */
-    private static final String EXTERNAL_GENERAL_ENTITIES =
-        "http://xml.org/sax/features/external-general-entities";
     /** Maps public id to resolve to resource name for the DTD. */
     private final Map<String, String> publicIdToResourceNameMap;
     /** Parser to read XML files. **/
@@ -83,8 +77,7 @@ protected AbstractLoader(Map<String, String> publicIdToResourceNameMap)
             throws SAXException, ParserConfigurationException {
         this.publicIdToResourceNameMap = new HashMap<>(publicIdToResourceNameMap);
         final SAXParserFactory factory = SAXParserFactory.newInstance();
-        factory.setFeature(LOAD_EXTERNAL_DTD, true);
-        factory.setFeature(EXTERNAL_GENERAL_ENTITIES, true);
+        FeaturesForVerySecureJavaInstallations.addFeaturesForVerySecureJavaInstallations(factory);
         factory.setValidating(true);
         factory.setNamespaceAware(true);
         parser = factory.newSAXParser().getXMLReader();
@@ -133,4 +126,34 @@ public void error(SAXParseException exception) throws SAXException {
     public void fatalError(SAXParseException exception) throws SAXException {
         throw exception;
     }
+
+    /**
+     * Used for setting specific for secure java installations features to SAXParserFactory.
+     * Pulled out as a separate class in order to suppress Pitest mutations.
+     */
+    public static final class FeaturesForVerySecureJavaInstallations {
+        /** Feature that enables loading external DTD when loading XML files. */
+        private static final String LOAD_EXTERNAL_DTD =
+                "http://apache.org/xml/features/nonvalidating/load-external-dtd";
+        /** Feature that enables including external general entities in XML files. */
+        private static final String EXTERNAL_GENERAL_ENTITIES =
+                "http://xml.org/sax/features/external-general-entities";
+
+        /** Stop instances being created. **/
+        private FeaturesForVerySecureJavaInstallations() {
+        }
+
+        /**
+         * Configures SAXParserFactory with features requered
+         * for exectution on very secured environments.
+         * @param factory factory to be configured with spectial features
+         * @throws SAXException if an error occurs
+         * @throws ParserConfigurationException if an error occurs
+         */
+        public static void addFeaturesForVerySecureJavaInstallations(SAXParserFactory factory)
+                throws SAXException, ParserConfigurationException {
+            factory.setFeature(LOAD_EXTERNAL_DTD, true);
+            factory.setFeature(EXTERNAL_GENERAL_ENTITIES, true);
+        }
+    }
 }

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

@@ -122,7 +122,7 @@ private static String getMessageBundle(final String className) {
         final String messageBundle;
         final int endIndex = className.lastIndexOf('.');
         final String messages = "messages";
-        if (endIndex < 0) {
+        if (endIndex == -1) {
             messageBundle = messages;
         }
         else {

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

@@ -110,7 +110,7 @@ public void addPreviousSibling(DetailAST ast) {
         clearBranchTokenTypes();
         clearChildCountCache(parent);
         if (ast != null) {
-            ast.setParent(parent);
+            //parent is set in setNextSibling or parent.setFirstChild
             final DetailAST previousSiblingNode = previousSibling;
 
             if (previousSiblingNode != null) {
@@ -135,7 +135,7 @@ public void addNextSibling(DetailAST ast) {
         clearBranchTokenTypes();
         clearChildCountCache(parent);
         if (ast != null) {
-            ast.setParent(parent);
+            //parent is set in setNextSibling
             final DetailAST nextSibling = getNextSibling();
 
             if (nextSibling != null) {
@@ -227,11 +227,11 @@ public int getLineNo() {
             // with initialize(String text)
             resultNo = findLineNo(getFirstChild());
 
-            if (resultNo < 0) {
+            if (resultNo == -1) {
                 resultNo = findLineNo(getNextSibling());
             }
         }
-        if (resultNo < 0) {
+        if (resultNo == -1) {
             resultNo = lineNo;
         }
         return resultNo;
@@ -258,11 +258,11 @@ public int getColumnNo() {
             // with initialize(String text)
             resultNo = findColumnNo(getFirstChild());
 
-            if (resultNo < 0) {
+            if (resultNo == -1) {
                 resultNo = findColumnNo(getNextSibling());
             }
         }
-        if (resultNo < 0) {
+        if (resultNo == -1) {
             resultNo = columnNo;
         }
         return resultNo;

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

@@ -199,7 +199,7 @@ private static String readFile(final File inputFile, final CharsetDecoder decode
             final char[] chars = new char[READ_BUFFER_SIZE];
             while (true) {
                 final int len = reader.read(chars);
-                if (len < 0) {
+                if (len == -1) {
                     break;
                 }
                 buf.append(chars, 0, len);

src/test/java/com/puppycrawl/tools/checkstyle/api/AbstractCheckTest.java

@@ -21,9 +21,14 @@
 
 import java.io.File;
 import java.nio.charset.Charset;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
 
 import org.junit.Assert;
 import org.junit.Test;
+import org.mockito.internal.util.reflection.Whitebox;
 
 import com.puppycrawl.tools.checkstyle.utils.CommonUtils;
 
@@ -196,4 +201,43 @@ public int[] getRequiredTokens() {
         Assert.assertArrayEquals(defaultTokens, check.getAcceptableTokens());
         Assert.assertArrayEquals(requiredTokens, check.getRequiredTokens());
     }
+
+    @Test
+    @SuppressWarnings("unchecked")
+    public void testClearMessages() {
+        final AbstractCheck check = new DummyAbstractCheck();
+
+        check.log(0, "key", "args");
+        final Collection<LocalizedMessage> messages =
+                (Collection<LocalizedMessage>) Whitebox.getInternalState(check, "messages");
+        Assert.assertEquals(1, messages.size());
+        check.clearMessages();
+        Assert.assertEquals(0, messages.size());
+    }
+
+    private static final class DummyAbstractCheck extends AbstractCheck {
+        private static final int[] DUMMY_ARRAY = {6};
+
+        @Override
+        public int[] getDefaultTokens() {
+            return Arrays.copyOf(DUMMY_ARRAY, 1);
+        }
+
+        @Override
+        public int[] getAcceptableTokens() {
+            return Arrays.copyOf(DUMMY_ARRAY, 1);
+        }
+
+        @Override
+        public int[] getRequiredTokens() {
+            return Arrays.copyOf(DUMMY_ARRAY, 1);
+        }
+
+        @Override
+        protected Map<String, String> getCustomMessages() {
+            final Map<String, String> messages = new HashMap<>();
+            messages.put("key", "value");
+            return messages;
+        }
+    }
 }

src/test/java/com/puppycrawl/tools/checkstyle/api/AbstractLoaderTest.java

@@ -0,0 +1,65 @@
+////////////////////////////////////////////////////////////////////////////////
+// checkstyle: Checks Java source code for adherence to a set of rules.
+// Copyright (C) 2001-2017 the original author or authors.
+//
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Lesser General Public
+// License as published by the Free Software Foundation; either
+// version 2.1 of the License, or (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public
+// License along with this library; if not, write to the Free Software
+// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+////////////////////////////////////////////////////////////////////////////////
+
+package com.puppycrawl.tools.checkstyle.api;
+
+import static com.puppycrawl.tools.checkstyle.internal.TestUtils.assertUtilsClassHasPrivateConstructor;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.xml.parsers.ParserConfigurationException;
+
+import org.junit.Test;
+import org.powermock.reflect.Whitebox;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLReader;
+
+import com.sun.org.apache.xerces.internal.impl.Constants;
+
+public class AbstractLoaderTest {
+
+    private static final String NAMESPACES_FEATURE =
+            Constants.SAX_FEATURE_PREFIX + Constants.NAMESPACES_FEATURE;
+
+    @Test
+    public void testParserConfiguratedSuccefully() throws Exception {
+        final DummyLoader dummyLoader = new DummyLoader(new HashMap<>(1));
+        final XMLReader parser = Whitebox.getInternalState(dummyLoader, "parser");
+        assertTrue(parser.getFeature(NAMESPACES_FEATURE));
+        assertEquals(dummyLoader, parser.getEntityResolver());
+    }
+
+    @Test
+    public void testIsProperUtilsClass() throws ReflectiveOperationException {
+        assertUtilsClassHasPrivateConstructor(
+                AbstractLoader.FeaturesForVerySecureJavaInstallations.class);
+    }
+
+    private static final class DummyLoader extends AbstractLoader {
+
+        DummyLoader(Map<String, String> publicIdToResourceNameMap)
+                throws SAXException, ParserConfigurationException {
+            super(publicIdToResourceNameMap);
+        }
+    }
+
+}

src/test/java/com/puppycrawl/tools/checkstyle/api/DetailASTTest.java

@@ -20,14 +20,21 @@
 package com.puppycrawl.tools.checkstyle.api;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import java.io.File;
 import java.lang.reflect.Method;
 import java.text.MessageFormat;
+import java.util.Arrays;
+import java.util.BitSet;
+import java.util.List;
 import java.util.Locale;
+import java.util.function.Consumer;
 
 import org.junit.Test;
+import org.powermock.reflect.Whitebox;
 
 import com.puppycrawl.tools.checkstyle.TreeWalker;
 
@@ -116,11 +123,96 @@ public void testInsertSiblingBetween() throws Exception {
         assertEquals(firstLevelC, firstLevelA.getNextSibling());
     }
 
+    @Test
+    public void testClearBranchTokenTypes() throws Exception {
+        final DetailAST parent = new DetailAST();
+        final DetailAST child = new DetailAST();
+        parent.setFirstChild(child);
+
+        final List<Consumer<DetailAST>> clearBranchTokenTypesMethods = Arrays.asList(
+            ast -> child.setFirstChild(ast),
+            ast -> child.setNextSibling(ast),
+            ast -> child.addPreviousSibling(ast),
+            ast -> child.addNextSibling(ast),
+            ast -> child.addChild(ast),
+            ast -> {
+                try {
+                    Whitebox.invokeMethod(child, "setParent", ast);
+                }
+                // -@cs[IllegalCatch] Cannot avoid catching it.
+                catch (Exception exception) {
+                    throw new IllegalStateException(exception);
+                }
+            }
+        );
+
+        for (Consumer<DetailAST> method : clearBranchTokenTypesMethods) {
+            final BitSet branchTokenTypes = Whitebox.invokeMethod(parent, "getBranchTokenTypes");
+            method.accept(null);
+            final BitSet branchTokenTypes2 = Whitebox.invokeMethod(parent, "getBranchTokenTypes");
+            assertEquals(branchTokenTypes, branchTokenTypes2);
+            assertNotSame(branchTokenTypes, branchTokenTypes2);
+        }
+    }
+
+    @Test
+    public void testClearChildCountCache() throws Exception {
+        final DetailAST parent = new DetailAST();
+        final DetailAST child = new DetailAST();
+        parent.setFirstChild(child);
+
+        final List<Consumer<DetailAST>> clearChildCountCacheMethods = Arrays.asList(
+            ast -> child.setNextSibling(ast),
+            ast -> child.addPreviousSibling(ast),
+            ast -> child.addNextSibling(ast)
+        );
+
+        for (Consumer<DetailAST> method : clearChildCountCacheMethods) {
+            final int startCount = parent.getChildCount();
+            method.accept(null);
+            final int intermediateCount = Whitebox.getInternalState(parent, "childCount");
+            final int finishCount = parent.getChildCount();
+            assertEquals(startCount, finishCount);
+            assertEquals(Integer.MIN_VALUE, intermediateCount);
+        }
+
+        final int startCount = child.getChildCount();
+        child.addChild(null);
+        final int intermediateCount = Whitebox.getInternalState(child, "childCount");
+        final int finishCount = child.getChildCount();
+        assertEquals(startCount, finishCount);
+        assertEquals(Integer.MIN_VALUE, intermediateCount);
+    }
+
+    @Test
+    public void testAddNextSibling() {
+        final DetailAST parent = new DetailAST();
+        final DetailAST child = new DetailAST();
+        final DetailAST sibling = new DetailAST();
+        final DetailAST newSibling = new DetailAST();
+        parent.setFirstChild(child);
+        child.setNextSibling(sibling);
+
+        child.addNextSibling(newSibling);
+        assertTrue(newSibling.getParent().equals(parent));
+        assertTrue(newSibling.getNextSibling().equals(sibling));
+        assertTrue(child.getNextSibling().equals(newSibling));
+    }
+
     @Test
     public void testTreeStructure() throws Exception {
         checkDir(new File("src/test/resources/com/puppycrawl/tools/checkstyle"));
     }
 
+    @Test
+    public void testToString() {
+        final DetailAST ast = new DetailAST();
+        ast.setText("text");
+        ast.setColumnNo(0);
+        ast.setLineNo(0);
+        assertEquals("text[0x0]", ast.toString());
+    }
+
     private static void checkDir(File dir) throws Exception {
         final File[] files = dir.listFiles(file -> {
             return (file.getName().endsWith(".java")