diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/PackageNamesLoader.java b/src/main/java/com/puppycrawl/tools/checkstyle/PackageNamesLoader.java index 0b347b5b2..eef3f7f04 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/PackageNamesLoader.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/PackageNamesLoader.java @@ -92,11 +92,8 @@ public final class PackageNamesLoader Attributes atts) throws SAXException { if ("package".equals(qName)) { - //push package name + //push package name, name is mandatory attribute with not empty value by DTD final String name = atts.getValue("name"); - if (name == null) { - throw new SAXException("missing package name"); - } packageStack.push(name); } } @@ -139,76 +136,43 @@ public final class PackageNamesLoader * @throws CheckstyleException if an error occurs. */ public static Set getPackageNames(ClassLoader classLoader) - throws CheckstyleException { + throws CheckstyleException { - Enumeration packageFiles = null; + Set result; try { - packageFiles = classLoader.getResources(CHECKSTYLE_PACKAGES); + //create the loader outside the loop to prevent PackageObjectFactory + //being created anew for each file + final PackageNamesLoader namesLoader = new PackageNamesLoader(); + + final Enumeration 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 e) { + throw new CheckstyleException("unable to open " + packageFile, e); + } + finally { + Closeables.closeQuietly(stream); + } + } + + result = namesLoader.getPackageNames(); + } catch (IOException e) { - throw new CheckstyleException( - "unable to get package file resources", e); + throw new CheckstyleException("unable to get package file resources", e); + } + catch (ParserConfigurationException | SAXException e) { + throw new CheckstyleException("unable to open one of package files", e); } - //create the loader outside the loop to prevent PackageObjectFactory - //being created anew for each file - final PackageNamesLoader namesLoader = newPackageNamesLoader(); - - while (packageFiles.hasMoreElements()) { - final URL packageFile = packageFiles.nextElement(); - InputStream stream = null; - - try { - stream = new BufferedInputStream(packageFile.openStream()); - final InputSource source = new InputSource(stream); - loadPackageNamesSource(source, "default package names", - namesLoader); - } - catch (IOException e) { - throw new CheckstyleException( - "unable to open " + packageFile, e); - } - finally { - Closeables.closeQuietly(stream); - } - } - return namesLoader.getPackageNames(); - } - - /** - * Creates a PackageNamesLoader instance. - * @return the PackageNamesLoader - * @throws CheckstyleException if the creation failed - */ - private static PackageNamesLoader newPackageNamesLoader() - throws CheckstyleException { - try { - return new PackageNamesLoader(); - } - catch (final ParserConfigurationException | SAXException e) { - throw new CheckstyleException( - "unable to create PackageNamesLoader - " - + e.getMessage(), e); - } - } - - /** - * Returns the list of package names in a specified source. - * @param source the source for the list. - * @param sourceName the name of the source. - * @param nameLoader the PackageNamesLoader instance - * @throws CheckstyleException if an error occurs. - */ - private static void loadPackageNamesSource( - InputSource source, String sourceName, - PackageNamesLoader nameLoader) - throws CheckstyleException { - try { - nameLoader.parseInputSource(source); - } - catch (final SAXException | IOException e) { - throw new CheckstyleException("Unable to parse " - + sourceName + " - " + e.getMessage(), e); - } + return result; } } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/PackageNamesLoaderTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/PackageNamesLoaderTest.java index bcfa21cb1..93fe630a8 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/PackageNamesLoaderTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/PackageNamesLoaderTest.java @@ -20,11 +20,29 @@ package com.puppycrawl.tools.checkstyle; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.powermock.api.mockito.PowerMockito.mock; +import static org.powermock.api.mockito.PowerMockito.when; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.net.URL; +import java.net.URLConnection; +import java.net.URLStreamHandler; import java.util.Arrays; +import java.util.Enumeration; +import java.util.LinkedHashSet; import java.util.Set; +import org.junit.Assert; import org.junit.Test; +import org.mockito.Mockito; +import org.xml.sax.Attributes; +import org.xml.sax.SAXException; import com.google.common.collect.Sets; import com.puppycrawl.tools.checkstyle.api.CheckstyleException; @@ -72,4 +90,105 @@ public class PackageNamesLoaderTest { Sets.newHashSet(Arrays.asList(checkstylePackages)); assertEquals("names set.", checkstylePackagesSet, pkgNames); } + + @Test + public void testPackagesWithDots() throws Exception { + + Constructor constructor = + (Constructor) PackageNamesLoader.class + .getDeclaredConstructors()[0]; + constructor.setAccessible(true); + PackageNamesLoader loader = constructor.newInstance(); + + Attributes attributes = mock(Attributes.class); + when(attributes.getValue("name")).thenReturn("coding."); + loader.startElement("", "", "package", attributes); + loader.endElement("", "", "package"); + + Field field = PackageNamesLoader.class.getDeclaredField("packageNames"); + field.setAccessible(true); + LinkedHashSet list = (LinkedHashSet) field.get(loader); + Assert.assertEquals("coding.", list.iterator().next()); + } + + @Test + public void testPackagesWithSaxException() throws Exception { + + final URLConnection mockConnection = Mockito.mock(URLConnection.class); + when(mockConnection.getInputStream()).thenReturn( + new ByteArrayInputStream(new byte[]{})); + + URL url = getMockUrl(mockConnection); + + Enumeration enumer = (Enumeration) mock(Enumeration.class); + when(enumer.hasMoreElements()).thenReturn(true); + when(enumer.nextElement()).thenReturn(url); + + ClassLoader classLoader = mock(ClassLoader.class); + when(classLoader.getResources("checkstyle_packages.xml")).thenReturn(enumer); + + try { + final Set packageNames = PackageNamesLoader + .getPackageNames(classLoader); + fail(); + } + catch (CheckstyleException ex) { + assertTrue(ex.getCause() instanceof SAXException); + } + } + + @Test + public void testPackagesWithIoException() throws Exception { + + final URLConnection mockConnection = Mockito.mock(URLConnection.class); + when(mockConnection.getInputStream()).thenReturn(null); + + URL url = getMockUrl(mockConnection); + + Enumeration enumer = (Enumeration) mock(Enumeration.class); + when(enumer.hasMoreElements()).thenReturn(true); + when(enumer.nextElement()).thenReturn(url); + + ClassLoader classLoader = mock(ClassLoader.class); + when(classLoader.getResources("checkstyle_packages.xml")).thenReturn(enumer); + + try { + final Set packageNames = PackageNamesLoader + .getPackageNames(classLoader); + fail(); + } + catch (CheckstyleException ex) { + assertTrue(ex.getCause() instanceof IOException); + assertNotEquals("unable to get package file resources", ex.getMessage()); + } + } + + @Test + public void testPackagesWithIoException_getResources() throws Exception { + + ClassLoader classLoader = mock(ClassLoader.class); + when(classLoader.getResources("checkstyle_packages.xml")).thenThrow(IOException.class); + + try { + PackageNamesLoader.getPackageNames(classLoader); + fail(); + } + catch (CheckstyleException ex) { + assertTrue(ex.getCause() instanceof IOException); + assertEquals("unable to get package file resources", ex.getMessage()); + } + } + + public static URL getMockUrl(final URLConnection connection) throws IOException { + final URLStreamHandler handler = new URLStreamHandler() { + @Override + protected URLConnection openConnection(final URL arg0) throws IOException { + return connection; + } + }; + final URL url = new URL("http://foo.bar", "foo.bar", 80, "", handler); + return url; + } + + }