Skip to content

Commit

Permalink
Issue #12155: BDD configuration comments should be in XML
Browse files Browse the repository at this point in the history
  • Loading branch information
@stoyanK7 @nrmancuso
stoyanK7 authored and nrmancuso committed Jun 30, 2023
1 parent c9876a6 commit 917c687
Show file tree
Hide file tree
Showing 91 changed files with 839 additions and 429 deletions.
9 changes: 8 additions & 1 deletion config/checkstyle-examples-checks.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@
<message key="regexp.filename.mismatch"
value="All example files should only match ''{1}'' exactly"/>
</module>
<module name="RegexpSingleline">
<property name="id" value="exampleFileXmlConfigPresence"/>
<property name="format" value="\/\*xml"/>
<property name="minimum" value="1"/>
<property name="maximum" value="1"/>
<property name="message" value="Example files must start with /*xml and have an XML config"/>
</module>
<module name="RegexpSingleline">
<property name="id" value="noTrailingWhitespace"/>
<property name="format" value="\s+$"/>
Expand Down Expand Up @@ -79,7 +86,7 @@

<!-- Size -->
<module name="FileLength">
<property name="max" value="45"/>
<property name="max" value="60"/>
</module>
<module name="LineLength">
<property name="max" value="85"/>
Expand Down
2 changes: 2 additions & 0 deletions config/checkstyle-examples-suppressions.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
files="abbreviationaswordinname[\\/]SuperClass.java"/>
<suppress id="xdocEndComment"
files="abbreviationaswordinname[\\/]SuperClass.java"/>
<suppress id="exampleFileXmlConfigPresence"
files="abbreviationaswordinname[\\/]SuperClass.java"/>

<!-- Check deals with tab characters. -->
<suppress checks="FileTabCharacter|Indentation"
Expand Down
16 changes: 8 additions & 8 deletions src/main/java/com/puppycrawl/tools/checkstyle/ConfigurationLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ public enum IgnoredModulesOptions {

}

/** The new public ID for version 1_3 of the configuration dtd. */
public static final String DTD_PUBLIC_CS_ID_1_3 =
"-//Checkstyle//DTD Checkstyle Configuration 1.3//EN";

/** The resource for version 1_3 of the configuration dtd. */
public static final String DTD_CONFIGURATION_NAME_1_3 =
"com/puppycrawl/tools/checkstyle/configuration_1_3.dtd";

/** Format of message for sax parse exception. */
private static final String SAX_PARSE_EXCEPTION_FORMAT = "%s - %s:%s:%s";

Expand Down Expand Up @@ -111,14 +119,6 @@ public enum IgnoredModulesOptions {
private static final String DTD_PUBLIC_ID_1_3 =
"-//Puppy Crawl//DTD Check Configuration 1.3//EN";

/** The new public ID for version 1_3 of the configuration dtd. */
private static final String DTD_PUBLIC_CS_ID_1_3 =
"-//Checkstyle//DTD Checkstyle Configuration 1.3//EN";

/** The resource for version 1_3 of the configuration dtd. */
private static final String DTD_CONFIGURATION_NAME_1_3 =
"com/puppycrawl/tools/checkstyle/configuration_1_3.dtd";

/** Prefix for the exception when unable to parse resource. */
private static final String UNABLE_TO_PARSE_EXCEPTION_PREFIX = "unable to parse"
+ " configuration stream";
Expand Down
103 changes: 99 additions & 4 deletions src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,14 @@
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import org.xml.sax.InputSource;

import com.puppycrawl.tools.checkstyle.ConfigurationLoader;
import com.puppycrawl.tools.checkstyle.PropertiesExpander;
import com.puppycrawl.tools.checkstyle.api.CheckstyleException;
import com.puppycrawl.tools.checkstyle.api.Configuration;

public final class InlineConfigParser {

Expand Down Expand Up @@ -91,6 +97,11 @@ public final class InlineConfigParser {
/** The String "(null)". */
private static final String NULL_STRING = "(null)";

private static final String LATEST_DTD = String.format(Locale.ROOT,
"<!DOCTYPE module PUBLIC \"%s\" \"%s\">%n",
ConfigurationLoader.DTD_PUBLIC_CS_ID_1_3,
ConfigurationLoader.DTD_PUBLIC_CS_ID_1_3);

/**
* Checks in which violation message is not specified in input file and have more than
* one violation message key.
Expand Down Expand Up @@ -164,17 +175,72 @@ private static void setModules(TestInputConfiguration.Builder testInputConfigBui
throw new CheckstyleException("Config not specified on top."
+ "Please see other inputs for examples of what is required.");
}
int lineNo = 1;
do {

final List<String> inlineConfig = getInlineConfig(lines);
final boolean isXmlConfig = "/*xml".equals(lines.get(0));

if (isXmlConfig) {
final String stringXmlConfig = LATEST_DTD + String.join("", inlineConfig);
final InputSource inputSource = new InputSource(new StringReader(stringXmlConfig));
final Configuration xmlConfig = ConfigurationLoader.loadConfiguration(
inputSource, new PropertiesExpander(System.getProperties()),
ConfigurationLoader.IgnoredModulesOptions.EXECUTE
);
final String configName = xmlConfig.getName();
if (!"Checker".equals(configName)) {
throw new CheckstyleException(
"First module should be Checker, but was " + configName);
}
handleXmlConfig(testInputConfigBuilder, inputFilePath, xmlConfig.getChildren());
}
else {
handleKeyValueConfig(testInputConfigBuilder, inputFilePath, inlineConfig);
}
}

private static List<String> getInlineConfig(List<String> lines) {
return lines.stream()
.skip(1)
.takeWhile(line -> !line.startsWith("*/"))
.collect(Collectors.toList());
}

private static void handleXmlConfig(TestInputConfiguration.Builder testInputConfigBuilder,
String inputFilePath,
Configuration... modules)
throws CheckstyleException {

for (Configuration module: modules) {
final String moduleName = module.getName();
if ("TreeWalker".equals(moduleName)) {
handleXmlConfig(testInputConfigBuilder, inputFilePath, module.getChildren());
}
else {
final ModuleInputConfiguration.Builder moduleInputConfigBuilder =
new ModuleInputConfiguration.Builder();
setModuleName(moduleInputConfigBuilder, inputFilePath, moduleName);
setProperties(inputFilePath, module, moduleInputConfigBuilder);
testInputConfigBuilder.addChildModule(moduleInputConfigBuilder.build());
}
}
}

private static void handleKeyValueConfig(TestInputConfiguration.Builder testInputConfigBuilder,
String inputFilePath, List<String> lines)
throws CheckstyleException, IOException {
int lineNo = 0;
while (lineNo < lines.size()) {
final ModuleInputConfiguration.Builder moduleInputConfigBuilder =
new ModuleInputConfiguration.Builder();
setModuleName(moduleInputConfigBuilder, inputFilePath, lines.get(lineNo));
setProperties(moduleInputConfigBuilder, inputFilePath, lines, lineNo + 1);
testInputConfigBuilder.addChildModule(moduleInputConfigBuilder.build());
do {
lineNo++;
} while (lines.get(lineNo).isEmpty() || !lines.get(lineNo - 1).isEmpty());
} while (!lines.get(lineNo).startsWith("*/"));
} while (lineNo < lines.size()
&& lines.get(lineNo).isEmpty()
|| !lines.get(lineNo - 1).isEmpty());
}
}

private static String getFullyQualifiedClassName(String filePath, String moduleName)
Expand Down Expand Up @@ -295,6 +361,35 @@ else if (value.startsWith("(default)")) {
}
}

private static void setProperties(String inputFilePath, Configuration module,
ModuleInputConfiguration.Builder moduleInputConfigBuilder)
throws CheckstyleException {
final String[] getPropertyNames = module.getPropertyNames();
for (final String propertyName : getPropertyNames) {
final String propertyValue = module.getProperty(propertyName);

if ("file".equals(propertyName)) {
final String filePath = getResolvedPath(propertyValue, inputFilePath);
moduleInputConfigBuilder.addNonDefaultProperty(propertyName, filePath);
}
else {
if (NULL_STRING.equals(propertyValue)) {
moduleInputConfigBuilder.addNonDefaultProperty(propertyName, null);
}
else {
moduleInputConfigBuilder.addNonDefaultProperty(propertyName, propertyValue);
}
}
}

final Map<String, String> messages = module.getMessages();
for (final Map.Entry<String, String> entry : messages.entrySet()) {
final String key = entry.getKey();
final String value = entry.getValue();
moduleInputConfigBuilder.addModuleMessage(key, value);
}
}

private static void setViolations(TestInputConfiguration.Builder inputConfigBuilder,
List<String> lines, boolean useFilteredViolations)
throws ClassNotFoundException, CheckstyleException {
Expand Down
38 changes: 19 additions & 19 deletions ...a/com/puppycrawl/tools/checkstyle/checks/naming/AbbreviationAsWordInNameExamplesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ protected String getPackageLocation() {
@Test
public void testExample1() throws Exception {
final String[] expected = {
"14:7: " + getCheckMessage(MSG_KEY, "CURRENT_COUNTER", DEFAULT_EXPECTED_CAPITAL_COUNT),
"23:8: " + getCheckMessage(MSG_KEY, "incrementCOUNTER", DEFAULT_EXPECTED_CAPITAL_COUNT),
"27:15: " + getCheckMessage(MSG_KEY, "incrementGLOBAL", DEFAULT_EXPECTED_CAPITAL_COUNT),
"18:7: " + getCheckMessage(MSG_KEY, "CURRENT_COUNTER", DEFAULT_EXPECTED_CAPITAL_COUNT),
"27:8: " + getCheckMessage(MSG_KEY, "incrementCOUNTER", DEFAULT_EXPECTED_CAPITAL_COUNT),
"31:15: " + getCheckMessage(MSG_KEY, "incrementGLOBAL", DEFAULT_EXPECTED_CAPITAL_COUNT),
};

verifyWithInlineConfigParser(getPath("Example1.java"), expected);
Expand All @@ -53,11 +53,11 @@ public void testExample1() throws Exception {
@Test
public void testExample2() throws Exception {
final String[] expected = {
"16:7: " + getCheckMessage(MSG_KEY, "CURRENT_COUNTER", DEFAULT_EXPECTED_CAPITAL_COUNT),
"18:14: " + getCheckMessage(MSG_KEY, "GLOBAL_COUNTER", DEFAULT_EXPECTED_CAPITAL_COUNT),
"22:15: " + getCheckMessage(MSG_KEY, "printCOUNTER", DEFAULT_EXPECTED_CAPITAL_COUNT),
"26:8: " + getCheckMessage(MSG_KEY, "incrementCOUNTER", DEFAULT_EXPECTED_CAPITAL_COUNT),
"30:15: " + getCheckMessage(MSG_KEY, "incrementGLOBAL", DEFAULT_EXPECTED_CAPITAL_COUNT),
"21:7: " + getCheckMessage(MSG_KEY, "CURRENT_COUNTER", DEFAULT_EXPECTED_CAPITAL_COUNT),
"23:14: " + getCheckMessage(MSG_KEY, "GLOBAL_COUNTER", DEFAULT_EXPECTED_CAPITAL_COUNT),
"27:15: " + getCheckMessage(MSG_KEY, "printCOUNTER", DEFAULT_EXPECTED_CAPITAL_COUNT),
"31:8: " + getCheckMessage(MSG_KEY, "incrementCOUNTER", DEFAULT_EXPECTED_CAPITAL_COUNT),
"35:15: " + getCheckMessage(MSG_KEY, "incrementGLOBAL", DEFAULT_EXPECTED_CAPITAL_COUNT),
};

verifyWithInlineConfigParser(getPath("Example2.java"), expected);
Expand All @@ -68,8 +68,8 @@ public void testExample3() throws Exception {
final int expectedCapitalCount = 1;

final String[] expected = {
"16:7: " + getCheckMessage(MSG_KEY, "secondNUM", expectedCapitalCount),
"18:14: " + getCheckMessage(MSG_KEY, "fourthNUm", expectedCapitalCount),
"21:7: " + getCheckMessage(MSG_KEY, "secondNUM", expectedCapitalCount),
"23:14: " + getCheckMessage(MSG_KEY, "fourthNUm", expectedCapitalCount),
};

verifyWithInlineConfigParser(getPath("Example3.java"), expected);
Expand All @@ -80,9 +80,9 @@ public void testExample4() throws Exception {
final int expectedCapitalCount = 2;

final String[] expected = {
"17:7: " + getCheckMessage(MSG_KEY, "secondMYNum", expectedCapitalCount),
"18:7: " + getCheckMessage(MSG_KEY, "thirdNUM", expectedCapitalCount),
"21:10: " + getCheckMessage(MSG_KEY, "firstXML", expectedCapitalCount),
"22:7: " + getCheckMessage(MSG_KEY, "secondMYNum", expectedCapitalCount),
"23:7: " + getCheckMessage(MSG_KEY, "thirdNUM", expectedCapitalCount),
"26:10: " + getCheckMessage(MSG_KEY, "firstXML", expectedCapitalCount),
};

verifyWithInlineConfigParser(getPath("Example4.java"), expected);
Expand All @@ -93,9 +93,9 @@ public void testExample5() throws Exception {
final int expectedCapitalCount = 1;

final String[] expected = {
"16:7: " + getCheckMessage(MSG_KEY, "counterXYZ", expectedCapitalCount),
"18:13: " + getCheckMessage(MSG_KEY, "customerID", expectedCapitalCount),
"19:14: " + getCheckMessage(MSG_KEY, "nextID", expectedCapitalCount),
"21:7: " + getCheckMessage(MSG_KEY, "counterXYZ", expectedCapitalCount),
"23:13: " + getCheckMessage(MSG_KEY, "customerID", expectedCapitalCount),
"24:14: " + getCheckMessage(MSG_KEY, "nextID", expectedCapitalCount),
};

verifyWithInlineConfigParser(getPath("Example5.java"), expected);
Expand All @@ -106,9 +106,9 @@ public void testExample6() throws Exception {
final int expectedCapitalCount = 1;

final String[] expected = {
"16:7: " + getCheckMessage(MSG_KEY, "counterXYZ", expectedCapitalCount),
"18:13: " + getCheckMessage(MSG_KEY, "customerID", expectedCapitalCount),
"21:20: " + getCheckMessage(MSG_KEY, "MAX_ALLOWED", expectedCapitalCount),
"21:7: " + getCheckMessage(MSG_KEY, "counterXYZ", expectedCapitalCount),
"23:13: " + getCheckMessage(MSG_KEY, "customerID", expectedCapitalCount),
"26:20: " + getCheckMessage(MSG_KEY, "MAX_ALLOWED", expectedCapitalCount),
};

verifyWithInlineConfigParser(getPath("Example6.java"), expected);
Expand Down
18 changes: 9 additions & 9 deletions ...ava/com/puppycrawl/tools/checkstyle/checks/naming/AbstractClassNameCheckExamplesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ protected String getPackageLocation() {
@Test
public void testExample1() throws Exception {
final String[] expected = {
"12:3: " + getCheckMessage(MSG_ILLEGAL_ABSTRACT_CLASS_NAME, "Second", DEFAULT_PATTERN),
"13:3: " + getCheckMessage(MSG_NO_ABSTRACT_CLASS_MODIFIER, "AbstractThird",
"16:3: " + getCheckMessage(MSG_ILLEGAL_ABSTRACT_CLASS_NAME, "Second", DEFAULT_PATTERN),
"17:3: " + getCheckMessage(MSG_NO_ABSTRACT_CLASS_MODIFIER, "AbstractThird",
DEFAULT_PATTERN),
"15:3: " + getCheckMessage(MSG_ILLEGAL_ABSTRACT_CLASS_NAME, "GeneratorFifth",
"19:3: " + getCheckMessage(MSG_ILLEGAL_ABSTRACT_CLASS_NAME, "GeneratorFifth",
DEFAULT_PATTERN),
};

Expand All @@ -55,8 +55,8 @@ public void testExample1() throws Exception {
@Test
public void testExample2() throws Exception {
final String[] expected = {
"13:3: " + getCheckMessage(MSG_ILLEGAL_ABSTRACT_CLASS_NAME, "Second", DEFAULT_PATTERN),
"16:3: " + getCheckMessage(MSG_ILLEGAL_ABSTRACT_CLASS_NAME, "GeneratorFifth",
"18:3: " + getCheckMessage(MSG_ILLEGAL_ABSTRACT_CLASS_NAME, "Second", DEFAULT_PATTERN),
"21:3: " + getCheckMessage(MSG_ILLEGAL_ABSTRACT_CLASS_NAME, "GeneratorFifth",
DEFAULT_PATTERN),
};

Expand All @@ -66,7 +66,7 @@ public void testExample2() throws Exception {
@Test
public void testExample3() throws Exception {
final String[] expected = {
"14:3: " + getCheckMessage(MSG_NO_ABSTRACT_CLASS_MODIFIER, "AbstractThird",
"19:3: " + getCheckMessage(MSG_NO_ABSTRACT_CLASS_MODIFIER, "AbstractThird",
DEFAULT_PATTERN),
};

Expand All @@ -78,9 +78,9 @@ public void testExample4() throws Exception {
final String pattern = "^Generator.+$";

final String[] expected = {
"13:3: " + getCheckMessage(MSG_ILLEGAL_ABSTRACT_CLASS_NAME, "AbstractFirst", pattern),
"14:3: " + getCheckMessage(MSG_ILLEGAL_ABSTRACT_CLASS_NAME, "Second", pattern),
"18:3: " + getCheckMessage(MSG_NO_ABSTRACT_CLASS_MODIFIER, "GeneratorSixth",
"18:3: " + getCheckMessage(MSG_ILLEGAL_ABSTRACT_CLASS_NAME, "AbstractFirst", pattern),
"19:3: " + getCheckMessage(MSG_ILLEGAL_ABSTRACT_CLASS_NAME, "Second", pattern),
"23:3: " + getCheckMessage(MSG_NO_ABSTRACT_CLASS_MODIFIER, "GeneratorSixth",
pattern),
};

Expand Down
26 changes: 13 additions & 13 deletions ...les/java/com/puppycrawl/tools/checkstyle/checks/naming/ConstantNameCheckExamplesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ protected String getPackageLocation() {
@Test
public void testExample1() throws Exception {
final String[] expected = {
"13:20: " + getCheckMessage(MSG_INVALID_PATTERN, "third_Constant3", DEFAULT_PATTERN),
"14:28: " + getCheckMessage(MSG_INVALID_PATTERN, "fourth_Const4", DEFAULT_PATTERN),
"15:27: " + getCheckMessage(MSG_INVALID_PATTERN, "log", DEFAULT_PATTERN),
"16:30: " + getCheckMessage(MSG_INVALID_PATTERN, "logger", DEFAULT_PATTERN),
"17:20: " + getCheckMessage(MSG_INVALID_PATTERN, "loggerMYSELF", DEFAULT_PATTERN),
"19:30: " + getCheckMessage(MSG_INVALID_PATTERN, "myselfConstant", DEFAULT_PATTERN),
"17:20: " + getCheckMessage(MSG_INVALID_PATTERN, "third_Constant3", DEFAULT_PATTERN),
"18:28: " + getCheckMessage(MSG_INVALID_PATTERN, "fourth_Const4", DEFAULT_PATTERN),
"19:27: " + getCheckMessage(MSG_INVALID_PATTERN, "log", DEFAULT_PATTERN),
"20:30: " + getCheckMessage(MSG_INVALID_PATTERN, "logger", DEFAULT_PATTERN),
"21:20: " + getCheckMessage(MSG_INVALID_PATTERN, "loggerMYSELF", DEFAULT_PATTERN),
"23:30: " + getCheckMessage(MSG_INVALID_PATTERN, "myselfConstant", DEFAULT_PATTERN),
};

verifyWithInlineConfigParser(getPath("Example1.java"), expected);
Expand All @@ -57,10 +57,10 @@ public void testExample2() throws Exception {
final String pattern = "^log(ger)?$|^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$";

final String[] expected = {
"14:20: " + getCheckMessage(MSG_INVALID_PATTERN, "third_Constant3", pattern),
"15:28: " + getCheckMessage(MSG_INVALID_PATTERN, "fourth_Const4", pattern),
"18:20: " + getCheckMessage(MSG_INVALID_PATTERN, "loggerMYSELF", pattern),
"20:30: " + getCheckMessage(MSG_INVALID_PATTERN, "myselfConstant", pattern),
"19:20: " + getCheckMessage(MSG_INVALID_PATTERN, "third_Constant3", pattern),
"20:28: " + getCheckMessage(MSG_INVALID_PATTERN, "fourth_Const4", pattern),
"23:20: " + getCheckMessage(MSG_INVALID_PATTERN, "loggerMYSELF", pattern),
"25:30: " + getCheckMessage(MSG_INVALID_PATTERN, "myselfConstant", pattern),
};

verifyWithInlineConfigParser(getPath("Example2.java"), expected);
Expand All @@ -69,9 +69,9 @@ public void testExample2() throws Exception {
@Test
public void testExample3() throws Exception {
final String[] expected = {
"15:20: " + getCheckMessage(MSG_INVALID_PATTERN, "third_Constant3", DEFAULT_PATTERN),
"16:28: " + getCheckMessage(MSG_INVALID_PATTERN, "fourth_Const4", DEFAULT_PATTERN),
"19:20: " + getCheckMessage(MSG_INVALID_PATTERN, "loggerMYSELF", DEFAULT_PATTERN),
"20:20: " + getCheckMessage(MSG_INVALID_PATTERN, "third_Constant3", DEFAULT_PATTERN),
"21:28: " + getCheckMessage(MSG_INVALID_PATTERN, "fourth_Const4", DEFAULT_PATTERN),
"24:20: " + getCheckMessage(MSG_INVALID_PATTERN, "loggerMYSELF", DEFAULT_PATTERN),
};

verifyWithInlineConfigParser(getPath("Example3.java"), expected);
Expand Down

0 comments on commit 917c687

Please sign in to comment.