From 4019a5bbf8353922cacf3e18398439fa9a7397d4 Mon Sep 17 00:00:00 2001 From: Andrei Selkin Date: Sat, 22 Oct 2016 23:53:36 +0300 Subject: [PATCH] Issue #3102: Fix DesignForExtension violations on Checkstyle's source code --- config/checkstyle_checks.xml | 6 +- .../test/base/BaseCheckTestSupport.java | 68 ++++++++++++ .../rule521packagenames/PackageNameTest.java | 2 +- .../checkstyle/BaseCheckTestSupport.java | 103 ++++++++++++++++++ .../checks/header/HeaderCheckTest.java | 2 +- 5 files changed, 174 insertions(+), 7 deletions(-) diff --git a/config/checkstyle_checks.xml b/config/checkstyle_checks.xml index 226add9d5f07..9f6d7289ce65 100644 --- a/config/checkstyle_checks.xml +++ b/config/checkstyle_checks.xml @@ -157,11 +157,7 @@ - - + diff --git a/src/it/java/com/google/checkstyle/test/base/BaseCheckTestSupport.java b/src/it/java/com/google/checkstyle/test/base/BaseCheckTestSupport.java index 177dc1e163fa..908a2d5d330c 100644 --- a/src/it/java/com/google/checkstyle/test/base/BaseCheckTestSupport.java +++ b/src/it/java/com/google/checkstyle/test/base/BaseCheckTestSupport.java @@ -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( @@ -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); @@ -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"); @@ -100,11 +124,32 @@ 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), @@ -112,6 +157,15 @@ protected void verify(Configuration config, String fileName, String[] expected, 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, @@ -192,6 +246,13 @@ protected String getCheckMessage(Map 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()) { @@ -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 result = new ArrayList<>(); try (BufferedReader br = new BufferedReader(new InputStreamReader( diff --git a/src/it/java/com/google/checkstyle/test/chapter5naming/rule521packagenames/PackageNameTest.java b/src/it/java/com/google/checkstyle/test/chapter5naming/rule521packagenames/PackageNameTest.java index da574aef26b7..14f85e66e5e4 100644 --- a/src/it/java/com/google/checkstyle/test/chapter5naming/rule521packagenames/PackageNameTest.java +++ b/src/it/java/com/google/checkstyle/test/chapter5naming/rule521packagenames/PackageNameTest.java @@ -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); } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/BaseCheckTestSupport.java b/src/test/java/com/puppycrawl/tools/checkstyle/BaseCheckTestSupport.java index 612b8094f2e2..6565747b059d 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/BaseCheckTestSupport.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/BaseCheckTestSupport.java @@ -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); @@ -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"); @@ -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( @@ -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, @@ -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> expectedViolations) diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/header/HeaderCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/header/HeaderCheckTest.java index 0d21f98a596d..d27dc1a095b6 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/header/HeaderCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/header/HeaderCheckTest.java @@ -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); }