From f020066f8bdfb378df36904af3df8b5bc48858fd Mon Sep 17 00:00:00 2001 From: Roman Ivanov Date: Fri, 30 Oct 2015 08:20:57 -0700 Subject: [PATCH] Issue #2109: CLI should print a file name where exception is happen --- config/pmd.xml | 12 +++ config/suppressions.xml | 7 +- .../puppycrawl/tools/checkstyle/Checker.java | 43 +++++---- .../com/puppycrawl/tools/checkstyle/Main.java | 14 +-- .../puppycrawl/tools/checkstyle/MainTest.java | 94 ++++++++++--------- .../javadoc/AbstractTypeAwareCheckTest.java | 8 +- .../SuppressWithNearbyCommentFilterTest.java | 25 ++++- .../filters/SuppressionCommentFilterTest.java | 25 ++++- .../tools/checkstyle/InputIncorrectClass.java | 1 + 9 files changed, 144 insertions(+), 85 deletions(-) create mode 100644 src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/InputIncorrectClass.java diff --git a/config/pmd.xml b/config/pmd.xml index fc6dabd6c..da6fc7d81 100644 --- a/config/pmd.xml +++ b/config/pmd.xml @@ -186,6 +186,12 @@ + + + + + + @@ -252,6 +258,12 @@ + + + + + + diff --git a/config/suppressions.xml b/config/suppressions.xml index 4b82755e8..ae1772572 100644 --- a/config/suppressions.xml +++ b/config/suppressions.xml @@ -9,6 +9,11 @@ files="TokenTypes.java" lines="1"/> + + + - + diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/Checker.java b/src/main/java/com/puppycrawl/tools/checkstyle/Checker.java index ea54aacbe..9fb0011d3 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/Checker.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/Checker.java @@ -252,28 +252,35 @@ public class Checker extends AutomaticBean implements MessageDispatcher { // Process each file for (final File file : files) { - if (!CommonUtils.matchesFileExtension(file, fileExtensions)) { - continue; - } - final String fileName = file.getAbsolutePath(); - fireFileStarted(fileName); - final SortedSet fileMessages = Sets.newTreeSet(); try { - final FileText theText = new FileText(file.getAbsoluteFile(), - charset); - for (final FileSetCheck fsc : fileSetChecks) { - fileMessages.addAll(fsc.process(file, theText)); + if (!CommonUtils.matchesFileExtension(file, fileExtensions)) { + continue; } + final String fileName = file.getAbsolutePath(); + fireFileStarted(fileName); + final SortedSet fileMessages = Sets.newTreeSet(); + try { + final FileText theText = new FileText(file.getAbsoluteFile(), + charset); + for (final FileSetCheck fsc : fileSetChecks) { + fileMessages.addAll(fsc.process(file, theText)); + } + } + catch (final IOException ioe) { + LOG.debug("IOException occurred.", ioe); + fileMessages.add(new LocalizedMessage(0, + Definitions.CHECKSTYLE_BUNDLE, "general.exception", + new String[] {ioe.getMessage()}, null, getClass(), + null)); + } + fireErrors(fileName, fileMessages); + fireFileFinished(fileName); } - catch (final IOException ioe) { - LOG.debug("IOException occurred.", ioe); - fileMessages.add(new LocalizedMessage(0, - Definitions.CHECKSTYLE_BUNDLE, "general.exception", - new String[] {ioe.getMessage()}, null, getClass(), - null)); + catch (Exception ex) { + // We need to catch all exception to put a reason failure(file name) in exception + throw new CheckstyleException("Exception was thrown while processing " + + file.getPath(), ex); } - fireErrors(fileName, fileMessages); - fireFileFinished(fileName); } // Finish up diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/Main.java b/src/main/java/com/puppycrawl/tools/checkstyle/Main.java index fa77ad15b..05979ad7c 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/Main.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/Main.java @@ -82,6 +82,7 @@ public final class Main { * is the number of errors found in all the files. * @param args the command line arguments. * @throws FileNotFoundException if there is a problem with files access + * @noinspection CallToPrintStackTrace **/ public static void main(String... args) throws FileNotFoundException { int errorCounter = 0; @@ -131,7 +132,7 @@ public final class Main { catch (CheckstyleException e) { exitStatus = EXIT_WITH_CHECKSTYLE_EXCEPTION_CODE; errorCounter = 1; - printMessageAndCause(e); + e.printStackTrace(); } finally { // return exit code base on validation of Checker @@ -144,17 +145,6 @@ public final class Main { } } - /** - * Prints message of exception to the first line and cause of exception to the second line. - * @param exception to be written to console - */ - private static void printMessageAndCause(CheckstyleException exception) { - System.out.println(exception.getMessage()); - if (exception.getCause() != null) { - System.out.println("Cause: " + exception.getCause()); - } - } - /** * Parses and executes Checkstyle based on passed arguments. * @param args diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/MainTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/MainTest.java index 97af3de44..34a1df8dc 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/MainTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/MainTest.java @@ -59,6 +59,10 @@ public class MainTest { return "src/test/resources/com/puppycrawl/tools/checkstyle/" + filename; } + private static String getNonCompilablePath(String filename) { + return "src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/" + filename; + } + private static String getFilePath(String filename) throws IOException { return new File(getPath(filename)).getCanonicalPath(); } @@ -143,11 +147,11 @@ public class MainTest { exit.checkAssertionAfterwards(new Assertion() { @Override public void checkAssertion() { - assertEquals(String.format(Locale.ROOT, - "Unable to find: src/main/resources/non_existing_config.xml%n" - + "Checkstyle ends with 1 errors.%n"), + assertEquals(String.format(Locale.ROOT, "Checkstyle ends with 1 errors.%n"), systemOut.getLog()); - assertEquals("", systemErr.getLog()); + final String cause = "com.puppycrawl.tools.checkstyle.api.CheckstyleException:" + + " Unable to find: src/main/resources/non_existing_config.xml"; + assertTrue(systemErr.getLog().startsWith(cause)); } }); Main.main("-c", "src/main/resources/non_existing_config.xml", @@ -172,38 +176,18 @@ public class MainTest { @Test public void testNonExistingClass() throws Exception { exit.expectSystemExitWithStatus(-2); - final String cause = "Unable to instantiate 'NonExistingClass' class," - + " it is also not possible to instantiate it as" - + " com.puppycrawl.tools.checkstyle.checks.annotation.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.checks.blocks.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.checks.coding.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.checks.design.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.checks.header.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.checks.imports.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.checks.indentation.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.checks.javadoc.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.checks.metrics.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.checks.modifier.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.checks.naming.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.checks.regexp.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.checks.sizes.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.checks.whitespace.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.checks.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.filters.NonExistingClass," - + " com.puppycrawl.tools.checkstyle.NonExistingClass." - + " Please recheck that class name is specified as canonical name or read" - + " how to configure short name usage" - + " http://checkstyle.sourceforge.net/config.html#Packages." - + " Please also recheck that provided ClassLoader to Checker is configured correctly."; - final String expectedExceptionMessage = - String.format(Locale.ROOT, "cannot initialize module TreeWalker - %1$s%n" - + "Cause: com.puppycrawl.tools.checkstyle.api.CheckstyleException: %1$s%n" - + "Checkstyle ends with 1 errors.%n", cause); exit.checkAssertionAfterwards(new Assertion() { @Override public void checkAssertion() { + final String expectedExceptionMessage = + String.format(Locale.ROOT, "Checkstyle ends with 1 errors.%n"); assertEquals(expectedExceptionMessage, systemOut.getLog()); - assertEquals("", systemErr.getLog()); + + final String cause = "com.puppycrawl.tools.checkstyle.api.CheckstyleException:" + + " cannot initialize module TreeWalker - " + + "Unable to instantiate 'NonExistingClass' class, " + + "it is also not possible to instantiate it as "; + assertTrue(systemErr.getLog().startsWith(cause)); } }); @@ -434,16 +418,14 @@ public class MainTest { exit.checkAssertionAfterwards(new Assertion() { @Override public void checkAssertion() { - assertTrue(systemOut.getLog().startsWith(String.format(Locale.ROOT, - "unable to parse configuration stream" - + " - Content is not allowed in prolog.:7:1%n" - + "Cause: org.xml.sax.SAXParseException; systemId: file:"))); - assertTrue(systemOut.getLog().endsWith(String.format(Locale.ROOT, - "com/puppycrawl/tools/checkstyle/config-Incorrect.xml;" - + " lineNumber: 7; columnNumber: 1; " - + "Content is not allowed in prolog.%n" - + "Checkstyle ends with 1 errors.%n"))); - assertEquals("", systemErr.getLog()); + final String output = String.format(Locale.ROOT, + "Checkstyle ends with 1 errors.%n"); + assertEquals(output, systemOut.getLog()); + final String errorOuput = String.format(Locale.ROOT, + "com.puppycrawl.tools.checkstyle.api." + + "CheckstyleException: unable to parse configuration stream" + + " - Content is not allowed in prolog.:7:1%n"); + assertTrue(systemErr.getLog().startsWith(errorOuput)); } }); Main.main("-c", getPath("config-Incorrect.xml"), @@ -545,7 +527,7 @@ public class MainTest { }); Main.main("-c", getPath("config-filelength.xml"), - getPath("checks/metrics")); + getPath("checks/metrics")); } @Test @@ -577,4 +559,30 @@ public class MainTest { final List result = (List) method.invoke(null, fileMock); assertEquals(0, result.size()); } + + @Test + public void testFileReferenceDuringException() throws Exception { + exit.expectSystemExitWithStatus(-2); + exit.checkAssertionAfterwards(new Assertion() { + @Override + public void checkAssertion() { + final String expectedExceptionMessage = + String.format(Locale.ROOT, "Starting audit...%n" + + "Checkstyle ends with 1 errors.%n"); + assertEquals(expectedExceptionMessage, systemOut.getLog()); + + final String exceptionFirstLine = String.format(Locale.ROOT, + "com.puppycrawl.tools.checkstyle.api." + + "CheckstyleException: Exception was thrown while processing " + + new File(getNonCompilablePath("InputIncorrectClass.java")).getPath() + + "%n"); + assertTrue(systemErr.getLog().startsWith(exceptionFirstLine)); + } + }); + + // We put xml as source to cause parse excepion + Main.main("-c", getPath("config-classname.xml"), + getNonCompilablePath("InputIncorrectClass.java")); + } + } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AbstractTypeAwareCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AbstractTypeAwareCheckTest.java index 8ddb07cae..941060a58 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AbstractTypeAwareCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AbstractTypeAwareCheckTest.java @@ -37,6 +37,7 @@ import org.junit.Test; import com.puppycrawl.tools.checkstyle.BaseCheckTestSupport; import com.puppycrawl.tools.checkstyle.DefaultConfiguration; +import com.puppycrawl.tools.checkstyle.api.CheckstyleException; import com.puppycrawl.tools.checkstyle.checks.AbstractTypeAwareCheck; @SuppressWarnings("deprecation") @@ -164,10 +165,11 @@ public class AbstractTypeAwareCheckTest extends BaseCheckTestSupport { try { verify(config, getPath("InputLoadErrors.java"), expected); } - catch (IllegalStateException ex) { + catch (CheckstyleException ex) { + final IllegalStateException cause = (IllegalStateException) ex.getCause(); assertEquals("Unable to get" - + " class information for @throws tag 'InvalidExceptionName'.", - ex.getMessage()); + + " class information for @throws tag 'InvalidExceptionName'.", + cause.getMessage()); } } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressWithNearbyCommentFilterTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressWithNearbyCommentFilterTest.java index 3cbbb50e4..532a253ca 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressWithNearbyCommentFilterTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressWithNearbyCommentFilterTest.java @@ -240,22 +240,39 @@ public class SuppressWithNearbyCommentFilterTest return coll.toArray(new String[coll.size()]); } - @Test(expected = ConversionException.class) + @Test public void testInvalidInfluenceFormat() throws Exception { final DefaultConfiguration filterConfig = createFilterConfig(SuppressWithNearbyCommentFilter.class); filterConfig.addAttribute("influenceFormat", "a"); final String[] suppressed = ArrayUtils.EMPTY_STRING_ARRAY; - verifySuppressed(filterConfig, suppressed); + + try { + verifySuppressed(filterConfig, suppressed); + } + catch (CheckstyleException ex) { + final ConversionException cause = (ConversionException) ex.getCause(); + assertEquals("unable to parse influence" + + " from 'SUPPRESS CHECKSTYLE MemberNameCheck' using a", + cause.getMessage()); + } } - @Test(expected = ConversionException.class) + @Test public void testInvalidCheckFormat() throws Exception { final DefaultConfiguration filterConfig = createFilterConfig(SuppressWithNearbyCommentFilter.class); filterConfig.addAttribute("checkFormat", "a[l"); final String[] suppressed = ArrayUtils.EMPTY_STRING_ARRAY; - verifySuppressed(filterConfig, suppressed); + + try { + verifySuppressed(filterConfig, suppressed); + } + catch (CheckstyleException ex) { + final ConversionException cause = (ConversionException) ex.getCause(); + assertEquals("unable to parse expanded comment a[l", + cause.getMessage()); + } } @Test diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilterTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilterTest.java index 8cd16887e..462aa93ff 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilterTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilterTest.java @@ -254,22 +254,39 @@ public class SuppressionCommentFilterTest assertEquals("Tag[line=0; col=1; on=false; text='text']", tag.toString()); } - @Test(expected = ConversionException.class) + @Test public void testInvalidCheckFormat() throws Exception { final DefaultConfiguration filterConfig = createFilterConfig(SuppressionCommentFilter.class); filterConfig.addAttribute("checkFormat", "e[l"); final String[] suppressed = ArrayUtils.EMPTY_STRING_ARRAY; - verifySuppressed(filterConfig, suppressed); + + try { + verifySuppressed(filterConfig, suppressed); + } + catch (CheckstyleException ex) { + final ConversionException cause = (ConversionException) ex.getCause(); + assertEquals("unable to parse expanded comment e[l", + cause.getMessage()); + } } - @Test(expected = ConversionException.class) + @Test public void testInvalidMessageFormat() throws Exception { final DefaultConfiguration filterConfig = createFilterConfig(SuppressionCommentFilter.class); filterConfig.addAttribute("messageFormat", "e[l"); final String[] suppressed = ArrayUtils.EMPTY_STRING_ARRAY; - verifySuppressed(filterConfig, suppressed); + + try { + verifySuppressed(filterConfig, suppressed); + } + catch (CheckstyleException ex) { + final ConversionException cause = (ConversionException) ex.getCause(); + assertEquals("unable to parse expanded comment e[l", + cause.getMessage()); + } + } @Test diff --git a/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/InputIncorrectClass.java b/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/InputIncorrectClass.java new file mode 100644 index 000000000..99a9d4277 --- /dev/null +++ b/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/InputIncorrectClass.java @@ -0,0 +1 @@ +!@#$^$^&%5 \ No newline at end of file