Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue #4814: added try/catch to setting up TreeWalker children #6442

Merged
merged 1 commit into from Feb 23, 2019
Merged

Issue #4814: added try/catch to setting up TreeWalker children #6442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 13 additions & 5 deletions src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java
View file
Open in desktop
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
15 changes: 7 additions & 8 deletions src/main/java/com/puppycrawl/tools/checkstyle/api/AutomaticBean.java
View file
Open in desktop
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
3 changes: 2 additions & 1 deletion src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java
View file
Open in desktop
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
View file
Open in desktop
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
12 changes: 6 additions & 6 deletions src/test/java/com/puppycrawl/tools/checkstyle/api/AutomaticBeanTest.java
View file
Open in desktop
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
6 changes: 3 additions & 3 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/NewlineAtEndOfFileCheckTest.java
View file
Open in desktop
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
13 changes: 6 additions & 7 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/blocks/EmptyBlockCheckTest.java
View file
Open in desktop
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
11 changes: 5 additions & 6 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/blocks/LeftCurlyCheckTest.java
View file
Open in desktop
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
10 changes: 5 additions & 5 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/blocks/RightCurlyCheckTest.java
View file
Open in desktop
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
9 changes: 3 additions & 6 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/header/HeaderCheckTest.java
View file
Open in desktop
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
6 changes: 2 additions & 4 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/header/RegexpHeaderCheckTest.java
View file
Open in desktop
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
53 changes: 28 additions & 25 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheckTest.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportControlCheckTest.java
View file
Open in desktop
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();
}

}