From 89c779cabcdaac39edb60a5ed839cea31cfab643 Mon Sep 17 00:00:00 2001 From: vasilyeva Date: Sat, 15 Jul 2017 00:24:07 +0300 Subject: [PATCH] Issue #4394: increase coverage of pitest-checkstyle-api profile to 96% --- config/suppressions.xml | 2 +- pom.xml | 12 ++- .../tools/checkstyle/api/AbstractLoader.java | 39 ++++++-- .../api/AbstractViolationReporter.java | 2 +- .../tools/checkstyle/api/DetailAST.java | 12 +-- .../tools/checkstyle/api/FileText.java | 2 +- .../checkstyle/api/AbstractCheckTest.java | 44 +++++++++ .../checkstyle/api/AbstractLoaderTest.java | 65 +++++++++++++ .../tools/checkstyle/api/DetailASTTest.java | 92 +++++++++++++++++++ .../tools/checkstyle/api/FileTextTest.java | 42 ++++++++- .../tools/checkstyle/api/FullIdentTest.java | 20 +++- 11 files changed, 308 insertions(+), 24 deletions(-) create mode 100644 src/test/java/com/puppycrawl/tools/checkstyle/api/AbstractLoaderTest.java diff --git a/config/suppressions.xml b/config/suppressions.xml index 1f17bb5fc8c..9119129ea0b 100644 --- a/config/suppressions.xml +++ b/config/suppressions.xml @@ -21,7 +21,7 @@ files="AbstractClassNameCheck.java"/> + files="AbstractCheckTest.java|AbstractClassNameCheckTest.java|AbstractTypeAwareCheckTest.java|AbstractJavadocCheckTest.java|AbstractViolationReporterTest.java|AbstractFileSetCheckTest.java|AbstractLoaderTest.java"/> diff --git a/pom.xml b/pom.xml index 32df6e02d9a..293a40101a4 100644 --- a/pom.xml +++ b/pom.xml @@ -2195,8 +2195,18 @@ com.puppycrawl.tools.checkstyle.checks.imports.ImportControlCheckTest com.puppycrawl.tools.checkstyle.checks.javadoc.WriteTagCheckTest com.puppycrawl.tools.checkstyle.checks.naming.ParameterNameCheckTest + com.puppycrawl.tools.checkstyle.ConfigurationLoaderTest + com.puppycrawl.tools.checkstyle.checks.TranslationCheckTest - 88 + + + addFeaturesForVerySecureJavaInstallations + + + + com.puppycrawl.tools.checkstyle.api.AbstractLoader$FeaturesForVerySecureJavaInstallations + + 96 ${pitest.plugin.timeout.factor} ${pitest.plugin.timeout.constant} ${pitest.plugin.threads} diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/api/AbstractLoader.java b/src/main/java/com/puppycrawl/tools/checkstyle/api/AbstractLoader.java index a414492f3b5..66a7475fd7d 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/api/AbstractLoader.java +++ b/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 publicIdToResourceNameMap; /** Parser to read XML files. **/ @@ -83,8 +77,7 @@ protected AbstractLoader(Map 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); + } + } } diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/api/AbstractViolationReporter.java b/src/main/java/com/puppycrawl/tools/checkstyle/api/AbstractViolationReporter.java index 0937d3091ed..48e46a8b957 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/api/AbstractViolationReporter.java +++ b/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 { diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/api/DetailAST.java b/src/main/java/com/puppycrawl/tools/checkstyle/api/DetailAST.java index 568db2d42b6..6fb4b99b3c6 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/api/DetailAST.java +++ b/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; diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/api/FileText.java b/src/main/java/com/puppycrawl/tools/checkstyle/api/FileText.java index beb5b3d4235..7ccf526e8fa 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/api/FileText.java +++ b/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); diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/api/AbstractCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/api/AbstractCheckTest.java index 37df889e1fc..59f0ea2f8aa 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/api/AbstractCheckTest.java +++ b/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 messages = + (Collection) 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 getCustomMessages() { + final Map messages = new HashMap<>(); + messages.put("key", "value"); + return messages; + } + } } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/api/AbstractLoaderTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/api/AbstractLoaderTest.java new file mode 100644 index 00000000000..da97c42f9d7 --- /dev/null +++ b/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 publicIdToResourceNameMap) + throws SAXException, ParserConfigurationException { + super(publicIdToResourceNameMap); + } + } + +} diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/api/DetailASTTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/api/DetailASTTest.java index f2e2a5d9947..3c4a1d5b7f0 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/api/DetailASTTest.java +++ b/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> 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 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> clearChildCountCacheMethods = Arrays.asList( + ast -> child.setNextSibling(ast), + ast -> child.addPreviousSibling(ast), + ast -> child.addNextSibling(ast) + ); + + for (Consumer 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") diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/api/FileTextTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/api/FileTextTest.java index b343a2dc2cd..1fd577b1400 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/api/FileTextTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/api/FileTextTest.java @@ -21,12 +21,26 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.times; +import static org.powermock.api.mockito.PowerMockito.doNothing; +import static org.powermock.api.mockito.PowerMockito.mockStatic; +import static org.powermock.api.mockito.PowerMockito.verifyStatic; import java.io.File; import java.io.IOException; +import java.io.Reader; +import java.util.Locale; import org.junit.Test; +import org.junit.runner.RunWith; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import com.google.common.io.Closeables; + +@RunWith(PowerMockRunner.class) +@PrepareForTest(Closeables.class) public class FileTextTest { @Test public void testUnsupportedCharset() throws IOException { @@ -44,10 +58,18 @@ public void testUnsupportedCharset() throws IOException { @Test public void testSupportedCharset() throws IOException { + //check if reader finally closed + mockStatic(Closeables.class); + doNothing().when(Closeables.class); + Closeables.closeQuietly(any(Reader.class)); + final String charsetName = "ISO-8859-1"; final FileText fileText = new FileText(new File("src/test/resources/com/puppycrawl/tools/" + "checkstyle/api/import-control_complete.xml"), charsetName); assertEquals(charsetName, fileText.getCharset().name()); + + verifyStatic(times(1)); + Closeables.closeQuietly(any(Reader.class)); } @Test @@ -66,6 +88,24 @@ public void testLineColumnAfterCopyConstructor() throws IOException { final FileText fileText = new FileText(new File("src/test/resources/com/puppycrawl/tools/" + "checkstyle/api/import-control_complete.xml"), charsetName); final FileText copy = new FileText(fileText); - assertEquals(3, copy.lineColumn(100).getLine()); + final LineColumn lineColumn = copy.lineColumn(100); + assertEquals(3, lineColumn.getLine()); + if (System.getProperty("os.name").toLowerCase(Locale.ENGLISH).startsWith("windows")) { + assertEquals(44, lineColumn.getColumn()); + } + else { + assertEquals(46, lineColumn.getColumn()); + } + } + + @Test + public void testLineColumnAtTheStartOfFile() throws IOException { + final String charsetName = "ISO-8859-1"; + final FileText fileText = new FileText(new File("src/test/resources/com/puppycrawl/tools/" + + "checkstyle/api/import-control_complete.xml"), charsetName); + final FileText copy = new FileText(fileText); + final LineColumn lineColumn = copy.lineColumn(0); + assertEquals(1, lineColumn.getLine()); + assertEquals(0, lineColumn.getColumn()); } } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/api/FullIdentTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/api/FullIdentTest.java index d3b5c76c747..90b780e81b1 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/api/FullIdentTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/api/FullIdentTest.java @@ -37,7 +37,18 @@ public void testToString() { } @Test - public void testNonValidCoordinates() { + public void testNonValidCoordinatesWithNegative() { + final FullIdent fullIdent = prepareFullIdentWithCoordinates(14, 15); + Assert.assertEquals("MyTest.MyTestik[15x14]", fullIdent.toString()); + } + + @Test + public void testNonValidCoordinatesWithZero() { + final FullIdent fullIdent = prepareFullIdentWithCoordinates(0, 0); + Assert.assertEquals("MyTest.MyTestik[15x14]", fullIdent.toString()); + } + + private static FullIdent prepareFullIdentWithCoordinates(int columnNo, int lineNo) { final DetailAST ast = new DetailAST(); ast.setType(TokenTypes.DOT); ast.setColumnNo(1); @@ -46,8 +57,8 @@ public void testNonValidCoordinates() { final DetailAST ast2 = new DetailAST(); ast2.setType(TokenTypes.LE); - ast2.setColumnNo(-14); - ast2.setLineNo(-15); + ast2.setColumnNo(columnNo); + ast2.setLineNo(lineNo); ast2.setText("MyTestik"); final DetailAST ast1 = new DetailAST(); @@ -59,7 +70,6 @@ public void testNonValidCoordinates() { ast.addChild(ast1); ast.addChild(ast2); - final FullIdent indent = FullIdent.createFullIdent(ast); - Assert.assertEquals("MyTest.MyTestik[15x14]", indent.toString()); + return FullIdent.createFullIdent(ast); } }