diff --git a/pom.xml b/pom.xml index dd0015f0e..d0a054d0b 100644 --- a/pom.xml +++ b/pom.xml @@ -1077,8 +1077,6 @@ 95 .*.Checker7984 - .*.ConfigurationLoader8679 - .*.ConfigurationLoader\$.*6584 .*.DefaultLogger7576 .*.PackageNamesLoader7872 .*.TreeWalker9491 diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/ConfigurationLoader.java b/src/main/java/com/puppycrawl/tools/checkstyle/ConfigurationLoader.java index 6a226b593..2959416af 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/ConfigurationLoader.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/ConfigurationLoader.java @@ -109,6 +109,8 @@ public final class ConfigurationLoader { private static final String SEVERITY = "severity"; /** name of the message element */ private static final String MESSAGE = "message"; + /** name of the message element */ + private static final String METADATA = "metadata"; /** name of the key attribute */ private static final String KEY = "key"; @@ -174,6 +176,11 @@ public final class ConfigurationLoader { final DefaultConfiguration top = configStack.peek(); top.addMessage(key, value); } + else { + if (!qName.equals(METADATA)) { + throw new IllegalStateException("Unknown name:" + qName + "."); + } + } } @Override @@ -300,13 +307,10 @@ public final class ConfigurationLoader { final URL url = new URL(config); uri = url.toURI(); } - catch (final MalformedURLException ex) { - uri = null; - } - catch (final URISyntaxException ex) { - // URL violating RFC 2396 + catch (final URISyntaxException | MalformedURLException ex) { uri = null; } + if (uri == null) { final File file = new File(config); if (file.exists()) { @@ -323,7 +327,7 @@ public final class ConfigurationLoader { uri = configUrl.toURI(); } catch (final URISyntaxException e) { - throw new CheckstyleException("unable to find " + config); + throw new CheckstyleException("unable to find " + config, e); } } } @@ -379,21 +383,13 @@ public final class ConfigurationLoader { loader.parseInputSource(configSource); return loader.getConfiguration(); } - catch (final ParserConfigurationException e) { - throw new CheckstyleException( - "unable to parse configuration stream", e); - } catch (final SAXParseException e) { throw new CheckstyleException("unable to parse configuration stream" + " - " + e.getMessage() + ":" + e.getLineNumber() + ":" + e.getColumnNumber(), e); } - catch (final SAXException e) { - throw new CheckstyleException("unable to parse configuration stream" - + " - " + e.getMessage(), e); - } - catch (final IOException e) { - throw new CheckstyleException("unable to read from stream", e); + catch (final ParserConfigurationException | IOException | SAXException e) { + throw new CheckstyleException("unable to parse configuration stream", e); } } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/ConfigurationLoaderTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/ConfigurationLoaderTest.java index fad2b8b7c..22adcbff2 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/ConfigurationLoaderTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/ConfigurationLoaderTest.java @@ -22,11 +22,23 @@ package com.puppycrawl.tools.checkstyle; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.powermock.api.mockito.PowerMockito.mockStatic; +import static org.powermock.api.mockito.PowerMockito.when; import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.net.URISyntaxException; import java.util.Properties; import org.junit.Test; +import org.junit.runner.RunWith; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import org.xml.sax.Attributes; import com.puppycrawl.tools.checkstyle.api.CheckstyleException; import com.puppycrawl.tools.checkstyle.api.Configuration; @@ -35,7 +47,10 @@ import com.puppycrawl.tools.checkstyle.api.Configuration; * @author Rick Giles * @author lkuehne */ +@RunWith(PowerMockRunner.class) +@PrepareForTest({ ConfigurationLoader.class, ConfigurationLoaderTest.class }) public class ConfigurationLoaderTest { + private Configuration loadConfiguration(String name) throws CheckstyleException { return loadConfiguration(name, new Properties()); @@ -47,7 +62,7 @@ public class ConfigurationLoaderTest { "src/test/resources/com/puppycrawl/tools/checkstyle/configs/" + name; return ConfigurationLoader.loadConfiguration( - fName, new PropertiesExpander(props)); + fName, new PropertiesExpander(props)); } @@ -331,7 +346,7 @@ public class ConfigurationLoaderTest { "src/test/resources/com/puppycrawl/tools/checkstyle/configs/subdir/including.xml"); final DefaultConfiguration config = (DefaultConfiguration) ConfigurationLoader.loadConfiguration( - file.toURI().toString(), new PropertiesExpander(props)); + file.toURI().toString(), new PropertiesExpander(props)); final Properties atts = new Properties(); atts.put("tabWidth", "4"); @@ -339,4 +354,175 @@ public class ConfigurationLoaderTest { verifyConfigNode(config, "Checker", 2, atts); } + @Test + public void testIncorrectTag() throws Exception { + try { + Class aClassParent = ConfigurationLoader.class; + Constructor ctorParent = null; + Constructor[] parentConstructors = aClassParent.getDeclaredConstructors(); + for (Constructor constr: parentConstructors) { + constr.setAccessible(true); + ctorParent = constr; + } + Object objParent = ctorParent.newInstance(null, true); + + Class aClass = Class.forName("com.puppycrawl.tools.checkstyle." + + "ConfigurationLoader$InternalLoader"); + Constructor constructor = null; + Constructor[] constructors = aClass.getDeclaredConstructors(); + for (Constructor constr: constructors) { + constr.setAccessible(true); + constructor = constr; + } + + Object obj = constructor.newInstance(objParent); + + + Class[] param = new Class[4]; + param[0] = String.class; + param[1] = String.class; + param[2] = String.class; + param[3] = Attributes.class; + Method method = aClass.getDeclaredMethod("startElement", param); + + method.invoke(obj, "", "", "hello", null); + + fail("Exception is expected"); + + } + catch (InvocationTargetException e) { + assertTrue(e.getCause() instanceof IllegalStateException); + assertEquals("Unknown name:" + "hello" + ".", e.getCause().getMessage()); + } + } + + @Test + public void testNonExistingPropertyName() { + try { + loadConfiguration("config_nonexisting_property.xml"); + fail("exception in expected"); + } + catch (CheckstyleException ex) { + assertEquals("unable to parse configuration stream", ex.getMessage()); + assertEquals("Property ${nonexisting} has not been set", + ex.getCause().getMessage()); + } + } + + @Test + public void testConfigWithIgnore() throws CheckstyleException { + + final DefaultConfiguration config = + (DefaultConfiguration) ConfigurationLoader.loadConfiguration( + "src/test/resources/com/puppycrawl/tools/checkstyle/configs/" + + "config_with_ignore.xml", + new PropertiesExpander(new Properties()), true); + + final Configuration[] children = config.getChildren(); + assertTrue(children[0].getChildren().length == 0); + } + + @Test + public void testConfigCheckerWithIgnore() throws CheckstyleException { + + final DefaultConfiguration config = + (DefaultConfiguration) ConfigurationLoader.loadConfiguration( + "src/test/resources/com/puppycrawl/tools/checkstyle/configs/" + + "config_with_checker_ignore.xml", + new PropertiesExpander(new Properties()), true); + + final Configuration[] children = config.getChildren(); + assertTrue(children.length == 0); + } + + @Test + public void testLoadConfiguration_WrongURL() throws CheckstyleException { + try { + final DefaultConfiguration config = + (DefaultConfiguration) ConfigurationLoader.loadConfiguration( + ";config_with_ignore.xml", + new PropertiesExpander(new Properties()), true); + + final Configuration[] children = config.getChildren(); + assertTrue(children[0].getChildren().length == 0); + fail("Exception is expected"); + } + catch (CheckstyleException ex) { + assertEquals("unable to find ;config_with_ignore.xml", ex.getMessage()); + } + } + + @Test + public void testLoadConfiguration_URISyntaxException() throws CheckstyleException { + mockStatic(ConfigurationLoader.class); + + PropertiesExpander expander = new PropertiesExpander(new Properties()); + + when(ConfigurationLoader.class.getResource("config_with_ignore.xml")) + .thenThrow(URISyntaxException.class); + when(ConfigurationLoader.loadConfiguration("config_with_ignore.xml", + expander, + true)) + .thenCallRealMethod(); + + try { + ConfigurationLoader.loadConfiguration( + "config_with_ignore.xml", expander, true); + + fail("Exception is expected"); + } + catch (CheckstyleException ex) { + assertTrue(ex.getCause() instanceof URISyntaxException); + assertEquals("unable to find config_with_ignore.xml", ex.getMessage()); + } + } + + @Test + public void testLoadConfiguration_Deprecated() throws CheckstyleException { + try { + final DefaultConfiguration config = + (DefaultConfiguration) ConfigurationLoader.loadConfiguration( + new FileInputStream( + "src/test/resources/com/puppycrawl/tools/checkstyle/configs/" + + "config_with_ignore.xml"), + new PropertiesExpander(new Properties()), true); + + final Configuration[] children = config.getChildren(); + assertTrue(children[0].getChildren().length == 0); + } + catch (CheckstyleException ex) { + fail("unexpected exception"); + } + catch (FileNotFoundException e) { + fail("unexpected exception"); + } + } + + @Test + public void testReplacePropertiesDefault() throws Exception { + final Properties props = new Properties(); + String defaultValue = "defaultValue"; + + String value = ConfigurationLoader.replaceProperties("${checkstyle.basedir}", + new PropertiesExpander(props), defaultValue); + + assertEquals(defaultValue, value); + } + + @Test + public void testLoadConfigurationFromClassPath() throws CheckstyleException { + try { + final DefaultConfiguration config = + (DefaultConfiguration) ConfigurationLoader.loadConfiguration( + "/com/puppycrawl/tools/checkstyle/configs/" + + "config_with_ignore.xml", + new PropertiesExpander(new Properties()), true); + + final Configuration[] children = config.getChildren(); + assertTrue(children[0].getChildren().length == 0); + } + catch (CheckstyleException ex) { + fail("unexpected exception"); + } + } } diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/configs/checkstyle_checks.xml b/src/test/resources/com/puppycrawl/tools/checkstyle/configs/checkstyle_checks.xml index 96737f513..f12f779e8 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/configs/checkstyle_checks.xml +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/configs/checkstyle_checks.xml @@ -5,6 +5,8 @@ "http://www.puppycrawl.com/dtds/configuration_1_1.dtd"> + + diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/configs/config_nonexisting_property.xml b/src/test/resources/com/puppycrawl/tools/checkstyle/configs/config_nonexisting_property.xml new file mode 100644 index 000000000..cef7c5398 --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/configs/config_nonexisting_property.xml @@ -0,0 +1,9 @@ + + + + + + + \ No newline at end of file diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/configs/config_with_checker_ignore.xml b/src/test/resources/com/puppycrawl/tools/checkstyle/configs/config_with_checker_ignore.xml new file mode 100644 index 000000000..4316919aa --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/configs/config_with_checker_ignore.xml @@ -0,0 +1,9 @@ + + + + + + + \ No newline at end of file diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/configs/config_with_ignore.xml b/src/test/resources/com/puppycrawl/tools/checkstyle/configs/config_with_ignore.xml new file mode 100644 index 000000000..3a99dbf18 --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/configs/config_with_ignore.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + \ No newline at end of file