Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Issue checkstyle#3102: Fix DesignForExtension violations on Checkstyl…
…e's source code
  • Loading branch information
MEZk authored and agcuda committed Oct 30, 2016
1 parent 05c6ffb commit 4019a5b
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 7 deletions.
6 changes: 1 addition & 5 deletions config/checkstyle_checks.xml
Expand Up @@ -157,11 +157,7 @@

<!-- Class Design -->
<module name="DesignForExtension">
<!--
We should postpone DesignForExtension Check enforcement till next major release
as it will seriously brake backward compatibility with existing usage of our library
-->
<property name="severity" value="ignore"/>
<property name="ignoredAnnotations" value="Override, Test, Before, After, BeforeClass, AfterClass"/>
</module>
<module name="FinalClass"/>
<module name="HideUtilityClassConstructor"/>
Expand Down
Expand Up @@ -61,6 +61,13 @@ public class BaseCheckTestSupport {

protected final ByteArrayOutputStream stream = new ByteArrayOutputStream();

/**
* Returns {@link Configuration} based on Google's checks xml-configuration (google_checks.xml).
* This implementation uses {@link ConfigurationLoader} in order to load configuration
* from xml-file.
* @return {@link Configuration} based on Google's checks xml-configuration (google_checks.xml).
* @throws CheckstyleException if exception occurs during configuration loading.
*/
protected static Configuration getConfiguration() throws CheckstyleException {
if (configuration == null) {
configuration = ConfigurationLoader.loadConfiguration(XML_NAME, new PropertiesExpander(
Expand All @@ -70,10 +77,21 @@ protected static Configuration getConfiguration() throws CheckstyleException {
return configuration;
}

/**
* Creates {@link DefaultConfiguration} instance for the given check class.
* @param clazz check class.
* @return {@link DefaultConfiguration} instance.
*/
protected static DefaultConfiguration createCheckConfig(Class<?> clazz) {
return new DefaultConfiguration(clazz.getName());
}

/**
* Creates {@link Checker} instance based on specified {@link Configuration}.
* @param checkConfig {@link Configuration} instance.
* @return {@link Checker} instance.
* @throws CheckstyleException if an exception occurs during checker configuration.
*/
protected Checker createChecker(Configuration checkConfig)
throws Exception {
final DefaultConfiguration dc = createCheckerConfig(checkConfig);
Expand All @@ -89,6 +107,12 @@ protected Checker createChecker(Configuration checkConfig)
return checker;
}

/**
* Creates {@link DefaultConfiguration} or the {@link Checker}.
* based on the given {@link Configuration}.
* @param config {@link Configuration} instance.
* @return {@link DefaultConfiguration} for the {@link Checker}.
*/
protected DefaultConfiguration createCheckerConfig(Configuration config) {
final DefaultConfiguration dc =
new DefaultConfiguration("configuration");
Expand All @@ -100,18 +124,48 @@ protected DefaultConfiguration createCheckerConfig(Configuration config) {
return dc;
}

/**
* Returns canonical path for the file with the given file name.
* The path is formed based on the specific root location.
* This implementation uses 'src/it/resources/com/google/checkstyle/test/' as a root location.
* @param fileName file name.
* @return canonical path for the the file with the given file name.
* @throws IOException if I/O exception occurs while forming the path.
*/
protected String getPath(String fileName) throws IOException {
return new File("src/it/resources/com/google/checkstyle/test/" + fileName)
.getCanonicalPath();
}

/**
* Performs verification of the file with given file name. Uses specified configuration.
* Expected messages are represented by the array of strings, warning line numbers are
* represented by the array of integers.
* This implementation uses overloaded
* {@link BaseCheckTestSupport#verify(Checker, File[], String, String[], Integer...)} method
* inside.
* @param config configuration.
* @param fileName file name to verify.
* @param expected an array of expected messages.
* @param warnsExpected an array of expected warning numbers.
* @throws Exception if exception occurs during verification process.
*/
protected void verify(Configuration config, String fileName, String[] expected,
Integer... warnsExpected) throws Exception {
verify(createChecker(config),
new File[] {new File(fileName)},
fileName, expected, warnsExpected);
}

/**
* Performs verification of files. Uses provided {@link Checker} instance.
* @param checker {@link Checker} instance.
* @param processedFiles files to process.
* @param messageFileName message file name.
* @param expected an array of expected messages.
* @param warnsExpected an array of expected warning line numbers.
* @throws Exception if exception occurs during verification process.
*/
protected void verify(Checker checker,
File[] processedFiles,
String messageFileName,
Expand Down Expand Up @@ -192,6 +246,13 @@ protected String getCheckMessage(Map<String, String> messages, String messageKey
return null;
}

/**
* Returns {@link Configuration} instance for the given check name.
* This implementation uses {@link BaseCheckTestSupport#getConfiguration()} method inside.
* @param checkName check name.
* @return {@link Configuration} instance for the given check name.
* @throws CheckstyleException if exception occurs during configuration loading.
*/
protected static Configuration getCheckConfig(String checkName) throws CheckstyleException {
Configuration result = null;
for (Configuration currentConfig : getConfiguration().getChildren()) {
Expand Down Expand Up @@ -219,6 +280,13 @@ private static String removeDeviceFromPathOnWindows(String path) {
return path;
}

/**
* Returns an array of integers which represents the warning line numbers in the file
* with the given file name.
* @param fileName file name.
* @return an array of integers which represents the warning line numbers.
* @throws IOException if I/O exception occurs while reading the file.
*/
protected Integer[] getLinesWithWarn(String fileName) throws IOException {
final List<Integer> result = new ArrayList<>();
try (BufferedReader br = new BufferedReader(new InputStreamReader(
Expand Down
Expand Up @@ -36,7 +36,7 @@ public class PackageNameTest extends BaseCheckTestSupport {
private static Configuration checkConfig;
private static String format;

protected String getPath(String packageName, String fileName) throws IOException {
private String getPath(String packageName, String fileName) throws IOException {
return getPath("chapter5naming" + File.separator + "rule521" + packageName
+ File.separator + fileName);
}
Expand Down
Expand Up @@ -54,6 +54,12 @@ protected static DefaultConfiguration createCheckConfig(Class<?> clazz) {
return new DefaultConfiguration(clazz.getName());
}

/**
* Creates {@link Checker} instance based on the given {@link Configuration} instance.
* @param checkConfig {@link Configuration} instance.
* @return {@link Checker} instance based on the given {@link Configuration} instance.
* @throws Exception if an exception occurs during checker configuration.
*/
public Checker createChecker(Configuration checkConfig)
throws Exception {
final DefaultConfiguration dc = createCheckerConfig(checkConfig);
Expand All @@ -69,6 +75,13 @@ public Checker createChecker(Configuration checkConfig)
return checker;
}

/**
* Creates {@link DefaultConfiguration} for the {@link Checker}
* based on the given {@link Configuration} instance.
* @param config {@link Configuration} instance.
* @return {@link DefaultConfiguration} for the {@link Checker}
* based on the given {@link Configuration} instance.
*/
protected DefaultConfiguration createCheckerConfig(Configuration config) {
final DefaultConfiguration dc =
new DefaultConfiguration("configuration");
Expand All @@ -80,31 +93,81 @@ protected DefaultConfiguration createCheckerConfig(Configuration config) {
return dc;
}

/**
* Returns canonical path for the file with the given file name.
* The path is formed base on the root location.
* This implementation uses 'src/test/resources/com/puppycrawl/tools/checkstyle/'
* as a root location.
* @param filename file name.
* @return canonical path for the file name.
* @throws IOException if I/O exception occurs while forming the path.
*/
protected String getPath(String filename) throws IOException {
return new File("src/test/resources/com/puppycrawl/tools/checkstyle/" + filename)
.getCanonicalPath();
}

/**
* Returns URI-representation of the path for the given file name.
* The path is formed base on the root location.
* This implementation uses 'src/test/resources/com/puppycrawl/tools/checkstyle/'
* as a root location.
* @param filename file name.
* @return URI-representation of the path for the file with the given file name.
*/
protected String getUriString(String filename) {
return new File("src/test/resources/com/puppycrawl/tools/checkstyle/" + filename).toURI()
.toString();
}

/**
* Returns canonical path for the file with the given file name.
* The path is formed base on the sources location.
* This implementation uses 'src/test/java/com/puppycrawl/tools/checkstyle/'
* as a src location.
* @param filename file name.
* @return canonical path for the file with the given file name.
* @throws IOException if I/O exception occurs while forming the path.
*/
protected static String getSrcPath(String filename) throws IOException {
return new File("src/test/java/com/puppycrawl/tools/checkstyle/" + filename)
.getCanonicalPath();
}

/**
* Returns canonical path for the file with the given file name.
* The path is formed base on the non-compilable resources location.
* This implementation uses 'src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/'
* as a non-compilable resource location.
* @param filename file name.
* @return canonical path for the file with the given file name.
* @throws IOException if I/O exception occurs while forming the path.
*/
protected String getNonCompilablePath(String filename) throws IOException {
return new File("src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/"
+ filename).getCanonicalPath();
}

/**
* Performs verification of the given text ast tree representation.
* This implementation uses {@link BaseCheckTestSupport#verifyAst(String, String, boolean)}
* method inside.
* @param expectedTextPrintFileName expected text ast tree representation.
* @param actualJavaFileName actual text ast tree representation.
* @throws Exception if exception occurs during verification.
*/
protected static void verifyAst(String expectedTextPrintFileName, String actualJavaFileName)
throws Exception {
verifyAst(expectedTextPrintFileName, actualJavaFileName, false);
}

/**
* Performs verification of the given text ast tree representation.
* @param expectedTextPrintFileName expected text ast tree representation.
* @param actualJavaFileName actual text ast tree representation.
* @param withComments whether to perform verification of comment nodes in tree.
* @throws Exception if exception occurs during verification.
*/
protected static void verifyAst(String expectedTextPrintFileName, String actualJavaFileName,
boolean withComments) throws Exception {
final String expectedContents = new String(Files.readAllBytes(
Expand All @@ -117,16 +180,49 @@ protected static void verifyAst(String expectedTextPrintFileName, String actualJ
actualContents);
}

/**
* Performs verification of the file with the given file name. Uses specified configuration.
* Expected messages are represented by the array of strings.
* This implementation uses overloaded
* {@link BaseCheckTestSupport#verify(Checker, File[], String, String...)} method inside.
* @param aConfig configuration.
* @param fileName file name to verify.
* @param expected an array of expected messages.
* @throws Exception if exception occurs during verification process.
*/
protected void verify(Configuration aConfig, String fileName, String... expected)
throws Exception {
verify(createChecker(aConfig), fileName, fileName, expected);
}

/**
* Performs verification of the file with the given file name.
* Uses provided {@link Checker} instance.
* Expected messages are represented by the array of strings.
* This implementation uses overloaded
* {@link BaseCheckTestSupport#verify(Checker, String, String, String...)} method inside.
* @param checker {@link Checker} instance.
* @param fileName file name to verify.
* @param expected an array of expected messages.
* @throws Exception if exception occurs during verification process.
*/
protected void verify(Checker checker, String fileName, String... expected)
throws Exception {
verify(checker, fileName, fileName, expected);
}

/**
* Performs verification of the file with the given file name.
* Uses provided {@link Checker} instance.
* Expected messages are represented by the array of strings.
* This implementation uses overloaded
* {@link BaseCheckTestSupport#verify(Checker, File[], String, String...)} method inside.
* @param checker {@link Checker} instance.
* @param processedFilename file name to verify.
* @param messageFileName message file name.
* @param expected an array of expected messages.
* @throws Exception if exception occurs during verification process.
*/
protected void verify(Checker checker,
String processedFilename,
String messageFileName,
Expand Down Expand Up @@ -173,6 +269,13 @@ protected void verify(Checker checker,
checker.destroy();
}

/**
* Performs verification of the given files.
* @param checker {@link Checker} instance
* @param processedFiles files to process.
* @param expectedViolations a map of expected violations per files.
* @throws Exception if exception occurs during verification process.
*/
protected void verify(Checker checker,
File[] processedFiles,
Map<String, List<String>> expectedViolations)
Expand Down
Expand Up @@ -60,7 +60,7 @@ protected String getPath(String filename) throws IOException {
+ "header" + File.separator + filename);
}

protected String getConfigPath(String filename) throws IOException {
private String getConfigPath(String filename) throws IOException {
return super.getPath("configs" + File.separator + filename);
}

Expand Down

0 comments on commit 4019a5b

Please sign in to comment.