Skip to content

Commit

Permalink
Issue #2109: CLI should print a file name where exception is happen
Browse files Browse the repository at this point in the history
  • Loading branch information
romani committed Nov 1, 2015
1 parent 346387c commit f020066
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 85 deletions.
12 changes: 12 additions & 0 deletions config/pmd.xml
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,12 @@
<property name="violationSuppressXPath" value="//ClassOrInterfaceDeclaration[@Image='Main']"/>
</properties>
</rule>
<rule ref="rulesets/java/logging-java.xml/AvoidPrintStackTrace">
<properties>
<!-- it is ok to use print stack trace in CLI class -->
<property name="violationSuppressXPath" value="//ClassOrInterfaceDeclaration[@Image='Main']"/>
</properties>
</rule>

<rule ref="rulesets/java/migrating.xml"/>

Expand Down Expand Up @@ -252,6 +258,12 @@
<property name="violationSuppressXPath" value="//ClassOrInterfaceDeclaration[@Image='PackageObjectFactory']"/>
</properties>
</rule>
<rule ref="rulesets/java/strictexception.xml/AvoidCatchingGenericException">
<properties>
<!-- There is no other way to deliver filename that was under processing -->
<property name="violationSuppressXPath" value="//MethodDeclaration[@Name='process' and ../../..[@Image='Checker']]"/>
</properties>
</rule>

<rule ref="rulesets/java/strings.xml"/>
<rule ref="rulesets/java/sunsecure.xml/MethodReturnsInternalArray">
Expand Down
7 changes: 6 additions & 1 deletion config/suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
files="TokenTypes.java"
lines="1"/>

<!-- There is no other way to deliver filename that was under processing -->
<suppress checks="IllegalCatch"
files="Checker.java"
lines="279"/>

<!-- we can not change it as, Check name is part of API (used in configurations) -->
<suppress checks="AbbreviationAsWordInName"
files="JavaNCSSCheck.java"
Expand Down Expand Up @@ -95,7 +100,7 @@

<!-- Suppressions from PMD configuration-->
<!-- validateCli is not reasonable to split as encapsulation of logic will be damaged -->
<suppress checks="CyclomaticComplexity" files="Main\.java" lines="179"/>
<suppress checks="CyclomaticComplexity" files="Main\.java" lines="169"/>
<!-- JavadocMethodCheck, JavadocStyleCheck, JavadocUtils.getJavadocTags() - deprecated -->
<suppress checks="CyclomaticComplexity" files="JavadocMethodCheck\.java"/>
<suppress checks="CyclomaticComplexity" files="JavadocStyleCheck\.java"/>
Expand Down
43 changes: 25 additions & 18 deletions src/main/java/com/puppycrawl/tools/checkstyle/Checker.java
Original file line number Diff line number Diff line change
Expand Up @@ -252,28 +252,35 @@ public int process(List<File> files) throws CheckstyleException {

// Process each file
for (final File file : files) {
if (!CommonUtils.matchesFileExtension(file, fileExtensions)) {
continue;
}
final String fileName = file.getAbsolutePath();
fireFileStarted(fileName);
final SortedSet<LocalizedMessage> 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<LocalizedMessage> 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
Expand Down
14 changes: 2 additions & 12 deletions src/main/java/com/puppycrawl/tools/checkstyle/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ private 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;
Expand Down Expand Up @@ -131,7 +132,7 @@ public static void main(String... args) throws FileNotFoundException {
catch (CheckstyleException e) {
exitStatus = EXIT_WITH_CHECKSTYLE_EXCEPTION_CODE;
errorCounter = 1;
printMessageAndCause(e);
e.printStackTrace();
}
finally {
// return exit code base on validation of Checker
Expand All @@ -144,17 +145,6 @@ public static void main(String... args) throws FileNotFoundException {
}
}

/**
* 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
Expand Down
94 changes: 51 additions & 43 deletions src/test/java/com/puppycrawl/tools/checkstyle/MainTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ private static String getPath(String filename) {
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();
}
Expand Down Expand Up @@ -143,11 +147,11 @@ public void testNonExistingConfigFile()
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",
Expand All @@ -172,38 +176,18 @@ public void checkAssertion() {
@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));
}
});

Expand Down Expand Up @@ -434,16 +418,14 @@ public void testExistingIncorrectConfigFile()
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"),
Expand Down Expand Up @@ -545,7 +527,7 @@ public void testExistingDirectoryWithViolations() throws Exception {
});

Main.main("-c", getPath("config-filelength.xml"),
getPath("checks/metrics"));
getPath("checks/metrics"));
}

@Test
Expand Down Expand Up @@ -577,4 +559,30 @@ public void testListFilesDirectoryWithNull() throws Exception {
final List<File> result = (List<File>) 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"));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

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")
Expand Down Expand Up @@ -164,10 +165,11 @@ public void testWithoutLogErrors() throws Exception {
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());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,22 +240,39 @@ private static String[] removeSuppressed(String[] from, String... remove) {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,22 +254,39 @@ public void testToStringOfTagClass() {
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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
!@#$^$^&%5

0 comments on commit f020066

Please sign in to comment.