Skip to content

Commit

Permalink
Issue checkstyle#6340: TranslationCheck print file on IllegalArgument…
Browse files Browse the repository at this point in the history
…Exception
  • Loading branch information
rnveach committed Jan 6, 2019
1 parent a262bad commit 1bfb6a3
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 16 deletions.
1 change: 1 addition & 0 deletions .ci/jsoref-spellchecker/whitelist.words
Expand Up @@ -1289,6 +1289,7 @@ UTests
utf
UUID
UWF
uxxxx
Validator
VALUEEEE
vararg
Expand Down
10 changes: 7 additions & 3 deletions config/pmd.xml
Expand Up @@ -200,11 +200,15 @@
</rule>
<rule ref="category/java/design.xml/AvoidCatchingGenericException">
<properties>
<!-- There is no other way to deliver the filename that was under processing.
See https://github.com/checkstyle/checkstyle/issues/2285 -->
<!-- Checker: There is no other way to deliver the filename that was under processing.
See https://github.com/checkstyle/checkstyle/issues/2285
TranslationCheck: It is better to catch all exceptions since it can throw
a runtime exception. -->
<property name="violationSuppressXPath"
value="//ClassOrInterfaceDeclaration[@Image='Checker']
//MethodDeclaration[@Name='processFiles']"/>
//MethodDeclaration[@Name='processFiles']
|//ClassOrInterfaceDeclaration[@Image='TranslationCheck']
//MethodDeclaration[@Name='getTranslationKeys']"/>
</properties>
</rule>
<rule ref="category/java/design.xml/AvoidDeeplyNestedIfStmts">
Expand Down
Expand Up @@ -20,7 +20,6 @@
package com.puppycrawl.tools.checkstyle.checks;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
Expand Down Expand Up @@ -481,21 +480,27 @@ private Set<String> getTranslationKeys(File file) {
translations.load(inStream);
keys = translations.stringPropertyNames();
}
catch (final IOException ex) {
logIoException(ex, file);
// -@cs[IllegalCatch] It is better to catch all exceptions since it can throw
// a runtime exception.
catch (final Exception ex) {
logException(ex, file);
}
return keys;
}

/**
* Helper method to log an io exception.
* Helper method to log an exception.
* @param exception the exception that occurred
* @param file the file that could not be processed
*/
private void logIoException(IOException exception, File file) {
String[] args = null;
String key = "general.fileNotFound";
if (!(exception instanceof NoSuchFileException)) {
private void logException(Exception exception, File file) {
final String[] args;
final String key;
if (exception instanceof NoSuchFileException) {
args = null;
key = "general.fileNotFound";
}
else {
args = new String[] {exception.getMessage()};
key = "general.exception";
}
Expand All @@ -510,7 +515,7 @@ private void logIoException(IOException exception, File file) {
final SortedSet<LocalizedMessage> messages = new TreeSet<>();
messages.add(message);
getMessageDispatcher().fireErrors(file.getPath(), messages);
log.debug("IOException occurred.", exception);
log.debug("Exception occurred.", exception);
}

/** Class which represents a resource bundle. */
Expand Down
Expand Up @@ -273,18 +273,38 @@ public void testLogIoException() throws Exception {
check.configure(checkConfig);
check.setMessageDispatcher(dispatcher);

final Method logIoException = check.getClass().getDeclaredMethod("logIoException",
IOException.class,
final Method logException = check.getClass().getDeclaredMethod("logException",
Exception.class,
File.class);
logIoException.setAccessible(true);
logException.setAccessible(true);
final File file = new File("");
logIoException.invoke(check, new IOException("test exception"), file);
logException.invoke(check, new IOException("test exception"), file);

Mockito.verify(dispatcher, times(1)).fireErrors(any(String.class), captor.capture());
final String actual = captor.getValue().first().getMessage();
assertThat("Invalid message: " + actual, actual, endsWith("test exception"));
}

@Test
public void testLogIllegalArgumentException() throws Exception {
final DefaultConfiguration checkConfig = createModuleConfig(TranslationCheck.class);
checkConfig.addAttribute("baseName", "^bad.*$");
final String[] expected = {
"0: " + new LocalizedMessage(1, Definitions.CHECKSTYLE_BUNDLE, "general.exception",
new String[] {"Malformed \\uxxxx encoding." }, null, getClass(), null).getMessage(),
"1: " + getCheckMessage(MSG_KEY, "test"),
};
final File[] propertyFiles = {
new File(getPath("bad.properties")),
new File(getPath("bad_es.properties")),
};
verify(
createChecker(checkConfig),
propertyFiles,
getPath("bad.properties"),
expected);
}

@Test
public void testDefaultTranslationFileIsMissing() throws Exception {
final DefaultConfiguration checkConfig = createModuleConfig(TranslationCheck.class);
Expand Down
@@ -0,0 +1 @@
test=\u@\h:\W \!$
@@ -0,0 +1 @@
test=test

0 comments on commit 1bfb6a3

Please sign in to comment.