Skip to content

Commit

Permalink
Issue checkstyle#4814: added try/catch to setting up TreeWalker children
Browse files Browse the repository at this point in the history
  • Loading branch information
rnveach committed Feb 20, 2019
1 parent 88907c5 commit 2bbe740
Show file tree
Hide file tree
Showing 24 changed files with 160 additions and 155 deletions.
18 changes: 13 additions & 5 deletions src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java
Expand Up @@ -148,11 +148,19 @@ public void finishLocalSetup() {
public void setupChild(Configuration childConf)
throws CheckstyleException {
final String name = childConf.getName();
final Object module = moduleFactory.createModule(name);
if (module instanceof AutomaticBean) {
final AutomaticBean bean = (AutomaticBean) module;
bean.contextualize(childContext);
bean.configure(childConf);
final Object module;

try {
module = moduleFactory.createModule(name);
if (module instanceof AutomaticBean) {
final AutomaticBean bean = (AutomaticBean) module;
bean.contextualize(childContext);
bean.configure(childConf);
}
}
catch (final CheckstyleException ex) {
throw new CheckstyleException("cannot initialize module " + name
+ " - " + ex.getMessage(), ex);
}
if (module instanceof AbstractCheck) {
final AbstractCheck check = (AbstractCheck) module;
Expand Down
Expand Up @@ -188,7 +188,7 @@ public final void configure(Configuration config)
for (final String key : attributes) {
final String value = config.getAttribute(key);

tryCopyProperty(config.getName(), key, value, true);
tryCopyProperty(key, value, true);
}

finishLocalSetup();
Expand All @@ -201,13 +201,12 @@ public final void configure(Configuration config)

/**
* Recheck property and try to copy it.
* @param moduleName name of the module/class
* @param key key of value
* @param value value
* @param recheck whether to check for property existence before copy
* @throws CheckstyleException then property defined incorrectly
*/
private void tryCopyProperty(String moduleName, String key, Object value, boolean recheck)
private void tryCopyProperty(String key, Object value, boolean recheck)
throws CheckstyleException {
final BeanUtilsBean beanUtils = createBeanUtilsBean();

Expand All @@ -219,8 +218,8 @@ private void tryCopyProperty(String moduleName, String key, Object value, boolea
final PropertyDescriptor descriptor =
PropertyUtils.getPropertyDescriptor(this, key);
if (descriptor == null) {
final String message = String.format(Locale.ROOT, "Property '%s' in module %s "
+ "does not exist, please check the documentation", key, moduleName);
final String message = String.format(Locale.ROOT, "Property '%s' "
+ "does not exist, please check the documentation", key);
throw new CheckstyleException(message);
}
}
Expand All @@ -234,12 +233,12 @@ private void tryCopyProperty(String moduleName, String key, Object value, boolea
// so we have to join these exceptions with InvocationTargetException
// to satisfy UTs coverage
final String message = String.format(Locale.ROOT,
"Cannot set property '%s' to '%s' in module %s", key, value, moduleName);
"Cannot set property '%s' to '%s'", key, value);
throw new CheckstyleException(message, ex);
}
catch (final IllegalArgumentException | ConversionException ex) {
final String message = String.format(Locale.ROOT, "illegal value '%s' for property "
+ "'%s' of module %s", value, key, moduleName);
+ "'%s'", value, key);
throw new CheckstyleException(message, ex);
}
}
Expand All @@ -256,7 +255,7 @@ public final void contextualize(Context context)
for (final String key : attributes) {
final Object value = context.get(key);

tryCopyProperty(getClass().getName(), key, value, false);
tryCopyProperty(key, value, false);
}
}

Expand Down
Expand Up @@ -459,7 +459,8 @@ public void testSetupChildInvalidProperty() throws Exception {
catch (CheckstyleException ex) {
assertEquals("Error message is not expected",
"cannot initialize module com.puppycrawl.tools.checkstyle.TreeWalker"
+ " - Property '$$No such property' in module " + checkConfig.getName()
+ " - cannot initialize module " + checkConfig.getName()
+ " - Property '$$No such property'"
+ " does not exist, please check the documentation", ex.getMessage());
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/test/java/com/puppycrawl/tools/checkstyle/MainTest.java
Expand Up @@ -549,8 +549,9 @@ public void testExistingIncorrectChildrenInConfigFile2()
final String output = errorCounterOneMessage.getMessage() + EOL;
assertEquals("Unexpected output log", output, systemOut.getLog());
final String errorOutput = "com.puppycrawl.tools.checkstyle.api."
+ "CheckstyleException: cannot initialize module TreeWalker"
+ " - JavadocVariable is not allowed as a child in JavadocMethod";
+ "CheckstyleException: cannot initialize module TreeWalker - "
+ "cannot initialize module JavadocMethod - "
+ "JavadocVariable is not allowed as a child in JavadocMethod";
assertTrue("Unexpected system error log", systemErr.getLog().startsWith(errorOutput));
});
Main.main("-c", getPath("InputMainConfig-incorrectChildren2.xml"),
Expand Down
Expand Up @@ -48,10 +48,10 @@ public void testConfigureNoSuchAttribute() {
fail("Exception is expected");
}
catch (CheckstyleException ex) {
final String expected = "Property 'NonExistent' in module ";
assertNull("Exceptions cause should be null", ex.getCause());
assertTrue("Invalid exception message, should start with: " + expected,
ex.getMessage().startsWith(expected));
assertEquals("Invalid exception message",
"Property 'NonExistent' does not exist, please check the documentation",
ex.getMessage());
}
}

Expand All @@ -65,10 +65,10 @@ public void testConfigureNoSuchAttribute2() {
fail("Exception is expected");
}
catch (CheckstyleException ex) {
final String expected = "Property 'privateField' in module ";
assertNull("Exceptions cause should be null", ex.getCause());
assertTrue("Invalid exception message, should start with: " + expected,
ex.getMessage().startsWith(expected));
assertEquals("Invalid exception message",
"Property 'privateField' does not exist, please check the documentation",
ex.getMessage());
}
}

Expand Down
Expand Up @@ -150,11 +150,11 @@ public void testSetLineSeparatorFailure()
fail("exception expected");
}
catch (CheckstyleException ex) {
assertTrue("Error message is unexpected",
ex.getMessage().startsWith(
assertEquals("Error message is unexpected",
"cannot initialize module com.puppycrawl.tools.checkstyle."
+ "checks.NewlineAtEndOfFileCheck - "
+ "Cannot set property 'lineSeparator' to 'ct' in module"));
+ "Cannot set property 'lineSeparator' to 'ct'",
ex.getMessage());
}
}

Expand Down
Expand Up @@ -22,7 +22,6 @@
import static com.puppycrawl.tools.checkstyle.checks.blocks.EmptyBlockCheck.MSG_KEY_BLOCK_EMPTY;
import static com.puppycrawl.tools.checkstyle.checks.blocks.EmptyBlockCheck.MSG_KEY_BLOCK_NO_STATEMENT;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import org.junit.Test;
Expand Down Expand Up @@ -149,12 +148,12 @@ public void testInvalidOption() throws Exception {
fail("exception expected");
}
catch (CheckstyleException ex) {
final String messageStart =
"cannot initialize module com.puppycrawl.tools.checkstyle.TreeWalker - "
+ "Cannot set property 'option' to 'invalid_option' in module";

assertTrue("Invalid exception message, should start with: " + messageStart,
ex.getMessage().startsWith(messageStart));
assertEquals("Invalid exception message",
"cannot initialize module com.puppycrawl.tools.checkstyle.TreeWalker - "
+ "cannot initialize module com.puppycrawl.tools.checkstyle.checks."
+ "blocks.EmptyBlockCheck - "
+ "Cannot set property 'option' to 'invalid_option'",
ex.getMessage());
}
}

Expand Down
Expand Up @@ -24,7 +24,6 @@
import static com.puppycrawl.tools.checkstyle.checks.blocks.LeftCurlyCheck.MSG_KEY_LINE_PREVIOUS;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import org.junit.Test;
Expand Down Expand Up @@ -434,12 +433,12 @@ public void testInvalidOption() throws Exception {
fail("exception expected");
}
catch (CheckstyleException ex) {
final String messageStart =
assertEquals("Invalid exception message",
"cannot initialize module com.puppycrawl.tools.checkstyle.TreeWalker - "
+ "Cannot set property 'option' to 'invalid_option' in module";

assertTrue("Invalid exception message, should start with: " + messageStart,
ex.getMessage().startsWith(messageStart));
+ "cannot initialize module com.puppycrawl.tools.checkstyle.checks."
+ "blocks.LeftCurlyCheck - "
+ "Cannot set property 'option' to 'invalid_option'",
ex.getMessage());
}
}

Expand Down
Expand Up @@ -23,7 +23,6 @@
import static com.puppycrawl.tools.checkstyle.checks.blocks.RightCurlyCheck.MSG_KEY_LINE_BREAK_BEFORE;
import static com.puppycrawl.tools.checkstyle.checks.blocks.RightCurlyCheck.MSG_KEY_LINE_SAME;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import org.junit.Test;
Expand Down Expand Up @@ -317,11 +316,12 @@ public void testInvalidOption() throws Exception {
fail("exception expected");
}
catch (CheckstyleException ex) {
final String messageStart =
assertEquals("Invalid exception message",
"cannot initialize module com.puppycrawl.tools.checkstyle.TreeWalker - "
+ "Cannot set property 'option' to 'invalid_option' in module";
assertTrue("Invalid exception message, should start with: " + messageStart,
ex.getMessage().startsWith(messageStart));
+ "cannot initialize module com.puppycrawl.tools.checkstyle.checks."
+ "blocks.RightCurlyCheck - "
+ "Cannot set property 'option' to 'invalid_option'",
ex.getMessage());
}
}

Expand Down
Expand Up @@ -122,8 +122,7 @@ public void testInvalidCharset() throws Exception {
catch (CheckstyleException ex) {
assertEquals("Invalid exception message", "cannot initialize module"
+ " com.puppycrawl.tools.checkstyle.checks.header.HeaderCheck"
+ " - Cannot set property 'charset' to 'XSO-8859-1' in module"
+ " com.puppycrawl.tools.checkstyle.checks.header.HeaderCheck",
+ " - Cannot set property 'charset' to 'XSO-8859-1'",
ex.getMessage());
assertEquals("Invalid exception message", "unsupported charset: 'XSO-8859-1'",
ex.getCause().getCause().getCause().getMessage());
Expand All @@ -141,8 +140,7 @@ public void testEmptyFilename() throws Exception {
catch (CheckstyleException ex) {
assertEquals("Invalid exception message", "cannot initialize module"
+ " com.puppycrawl.tools.checkstyle.checks.header.HeaderCheck"
+ " - Cannot set property 'headerFile' to '' in module"
+ " com.puppycrawl.tools.checkstyle.checks.header.HeaderCheck",
+ " - Cannot set property 'headerFile' to ''",
ex.getMessage());
assertEquals("Invalid exception message",
"property 'headerFile' is missing or invalid in module"
Expand All @@ -162,8 +160,7 @@ public void testNullFilename() throws Exception {
catch (CheckstyleException ex) {
assertEquals("Invalid exception message", "cannot initialize module"
+ " com.puppycrawl.tools.checkstyle.checks.header.HeaderCheck"
+ " - Cannot set property 'headerFile' to 'null' in module"
+ " com.puppycrawl.tools.checkstyle.checks.header.HeaderCheck",
+ " - Cannot set property 'headerFile' to 'null'",
ex.getMessage());
}
}
Expand Down
Expand Up @@ -129,8 +129,7 @@ public void testEmptyFilename() throws Exception {
catch (CheckstyleException ex) {
assertEquals("Invalid exception message", "cannot initialize module"
+ " com.puppycrawl.tools.checkstyle.checks.header.RegexpHeaderCheck"
+ " - Cannot set property 'headerFile' to '' in"
+ " module com.puppycrawl.tools.checkstyle.checks.header.RegexpHeaderCheck",
+ " - Cannot set property 'headerFile' to ''",
ex.getMessage());
}
}
Expand Down Expand Up @@ -180,8 +179,7 @@ public void testFailureForMultilineRegexp() throws Exception {
catch (CheckstyleException ex) {
assertEquals("Invalid exception message", "cannot initialize module"
+ " com.puppycrawl.tools.checkstyle.checks.header.RegexpHeaderCheck"
+ " - Cannot set property 'header' to '^(.*\\n.*)' in module"
+ " com.puppycrawl.tools.checkstyle.checks.header.RegexpHeaderCheck",
+ " - Cannot set property 'header' to '^(.*\\n.*)'",
ex.getMessage());
}
}
Expand Down
Expand Up @@ -26,7 +26,6 @@
import static com.puppycrawl.tools.checkstyle.checks.imports.CustomImportOrderCheck.MSG_ORDER;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.io.File;
Expand Down Expand Up @@ -526,15 +525,16 @@ public void testSamePackageDepthNegative() throws Exception {
fail("exception expected");
}
catch (CheckstyleException ex) {
final String messageStart =
assertEquals("Invalid exception message",
"cannot initialize module com.puppycrawl.tools.checkstyle.TreeWalker - "
+ "Cannot set property 'customImportOrderRules' to "
+ "'SAME_PACKAGE(-1)' in module";
assertTrue("Invalid exception message, should start with: " + messageStart,
ex.getMessage().startsWith(messageStart));
+ "cannot initialize module com.puppycrawl.tools.checkstyle.checks"
+ ".imports.CustomImportOrderCheck - "
+ "Cannot set property 'customImportOrderRules' to "
+ "'SAME_PACKAGE(-1)'",
ex.getMessage());
assertEquals("Invalid exception message",
"SAME_PACKAGE rule parameter should be positive integer: SAME_PACKAGE(-1)",
ex.getCause().getCause().getCause().getMessage());
ex.getCause().getCause().getCause().getCause().getMessage());
}
}

Expand All @@ -554,15 +554,16 @@ public void testSamePackageDepthZero() throws Exception {
fail("exception expected");
}
catch (CheckstyleException ex) {
final String messageStart =
assertEquals("Invalid exception message",
"cannot initialize module com.puppycrawl.tools.checkstyle.TreeWalker - "
+ "Cannot set property 'customImportOrderRules' to "
+ "'SAME_PACKAGE(0)' in module";
assertTrue("Invalid exception message, should start with: " + messageStart,
ex.getMessage().startsWith(messageStart));
+ "cannot initialize module com.puppycrawl.tools.checkstyle.checks"
+ ".imports.CustomImportOrderCheck - "
+ "Cannot set property 'customImportOrderRules' to "
+ "'SAME_PACKAGE(0)'",
ex.getMessage());
assertEquals("Invalid exception message",
"SAME_PACKAGE rule parameter should be positive integer: SAME_PACKAGE(0)",
ex.getCause().getCause().getCause().getMessage());
ex.getCause().getCause().getCause().getCause().getMessage());
}
}

Expand All @@ -581,14 +582,15 @@ public void testUnsupportedRule() throws Exception {
fail("exception expected");
}
catch (CheckstyleException ex) {
final String messageStart =
assertEquals("Invalid exception message",
"cannot initialize module com.puppycrawl.tools.checkstyle.TreeWalker - "
+ "Cannot set property 'customImportOrderRules' to "
+ "'SAME_PACKAGE(3)###UNSUPPORTED_RULE' in module";
assertTrue("Invalid exception message, should start with: " + messageStart,
ex.getMessage().startsWith(messageStart));
+ "cannot initialize module com.puppycrawl.tools.checkstyle.checks"
+ ".imports.CustomImportOrderCheck - "
+ "Cannot set property 'customImportOrderRules' to "
+ "'SAME_PACKAGE(3)###UNSUPPORTED_RULE'",
ex.getMessage());
assertEquals("Invalid exception message", "Unexpected rule: UNSUPPORTED_RULE", ex
.getCause().getCause().getCause().getMessage());
.getCause().getCause().getCause().getCause().getMessage());
}
}

Expand All @@ -606,15 +608,16 @@ public void testSamePackageDepthNotInt() throws Exception {
fail("exception expected");
}
catch (CheckstyleException ex) {
final String messageStart =
assertEquals("Invalid exception message",
"cannot initialize module com.puppycrawl.tools.checkstyle.TreeWalker - "
+ "Cannot set property 'customImportOrderRules' to "
+ "'SAME_PACKAGE(INT_IS_REQUIRED_HERE)' in module";
assertTrue("Invalid exception message, should start with: " + messageStart,
ex.getMessage().startsWith(messageStart));
+ "cannot initialize module com.puppycrawl.tools.checkstyle.checks"
+ ".imports.CustomImportOrderCheck - "
+ "Cannot set property 'customImportOrderRules' to "
+ "'SAME_PACKAGE(INT_IS_REQUIRED_HERE)'",
ex.getMessage());
assertEquals("Invalid exception message",
"For input string: \"INT_IS_REQUIRED_HERE\"",
ex.getCause().getCause().getCause().getMessage());
ex.getCause().getCause().getCause().getCause().getMessage());
}
}

Expand Down
Expand Up @@ -451,7 +451,7 @@ public void testFileNameNoExtension() throws Exception {
* @return String message of original exception
*/
private static String getCheckstyleExceptionMessage(CheckstyleException exception) {
return exception.getCause().getCause().getCause().getMessage();
return exception.getCause().getCause().getCause().getCause().getMessage();
}

}

0 comments on commit 2bbe740

Please sign in to comment.