Author: rwhitcomb Date: Fri Aug 27 06:38:53 2021 New Revision: 1892628 URL: http://svn.apache.org/viewvc?rev=1892628&view=rev Log: PIVOT-1032: In the StyleChecks program, make sure the file names for the "-f" parameter actually exist, and that the categories for "-c" are actually tested in our style file. Also fix a bug due to duplicate file names in different subtrees that caused counts to be off between the top list and the final counts.
Modified: pivot/trunk/StyleChecks.java Modified: pivot/trunk/StyleChecks.java URL: http://svn.apache.org/viewvc/pivot/trunk/StyleChecks.java?rev=1892628&r1=1892627&r2=1892628&view=diff ============================================================================== --- pivot/trunk/StyleChecks.java (original) +++ pivot/trunk/StyleChecks.java Fri Aug 27 06:38:53 2021 @@ -19,6 +19,7 @@ import java.io.File; import java.io.FileReader; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -150,10 +151,18 @@ public final class StyleChecks { String getName() { return fileName; } + /** @return The name only in this object. */ + String getNameOnly() { + return fileNameOnly(fileName); + } /** @return The count for this object. */ int getCount() { return count; } + @Override + public String toString() { + return fileName; + } } /** @@ -174,6 +183,46 @@ public final class StyleChecks { }; /** + * Our file name lists have "short" names that are all relative to + * the {@link ORG_APACHE_PIVOT} path. But to report names properly + * that may be duplicates in the name, but not in the full path + * we need to decide if there are any such duplicates before beginning + * to print. That's what we do here. + * + * @param strings The set of strings we are preparing to print. + * @return Whether there are any duplicates in name only. + */ + private static boolean anyDuplicateNames(final Iterable<String> strings) { + Set<String> nonDuplicateNames = new HashSet<>(); + // The strategy is to take the name only and put into the non-duplicate + // set, which will eliminate duplicate names. If the final size of that + // set is less than the original list size, then there are duplicate + // names in the original list. + int count = 0; + for (String s : strings) { + count++; + nonDuplicateNames.add(fileNameOnly(s)); + } + return nonDuplicateNames.size() < count; + } + + /** + * Search for duplicate names in a list of {@link FileInfo} objects. + * + * @param infos The set of info objects. + * @return Whether or not there are duplicate names. + */ + private static boolean anyDuplicateInfos(final Iterable<FileInfo> infos) { + Set<String> nonDuplicateNames = new HashSet<>(); + int count = 0; + for (FileInfo info : infos) { + count++; + nonDuplicateNames.add(fileNameOnly(info.getName())); + } + return nonDuplicateNames.size() < count; + } + + /** * Get a list of strings as a parenthesized list. * @param strings The set of strings to traverse. * @return A nicely formatted list. @@ -191,6 +240,90 @@ public final class StyleChecks { return buf.toString(); } + /** + * Get a list of the short form of strings as a parenthesized list, + * using the names only if there are no duplicates, otherwise show + * the full names to disambiguate the duplicates. + * + * @param strings The set of strings to traverse. + * @return A nicely formatted list. + */ + private static String listShortName(final Iterable<String> strings) { + boolean duplicates = anyDuplicateNames(strings); + StringBuilder buf = new StringBuilder("("); + int i = 0; + for (String s : strings) { + if (i++ > 0) { + buf.append(", "); + } + buf.append(duplicates ? s : fileNameOnly(s)); + } + buf.append(')'); + return buf.toString(); + } + + /** + * Find a file by name if it exists starting at the given directory location. + * <p> This is used to ensure that the <code>"-f"</code> file names actually exist, + * and aren't not found because the name is a typo. + * + * @param file The starting directory to search. + * @param search The file name to search for. + * @return {@code true} or {@code false} depending on whether the file is + * found anywhere in the directory hierarchy. + */ + private static boolean findFile(final File file, final String search) { + if (file.isDirectory()) { + for (File f : file.listFiles()) { + if (findFile(f, search)) { + return true; + } + } + } else { + if (search.equals(file.getName())) { + return true; + } + } + return false; + } + + /** + * Look for the file starting under the {@link #CURRENT_DIR_FILE} and if found, add it to + * the file name filter list, otherwise print an error. + * + * @param fileName The file name to potentially add to the file name filter list. + * @return Whether or not the file name was valid. + * @see #filterFileNames + */ + private static boolean addToFileNameList(final String fileName) { + if (findFile(CURRENT_DIR_FILE, fileName)) { + filterFileNames.add(fileName); + return true; + } else { + error("The '%1$s' file wasn't found!", fileName); + return false; + } + } + + /** + * Lookup the given category name in the list of valid ones and add to the category filter + * list if found, otherwise print an error. + * + * @param categoryName The category to potentially add to the filter list. + * @return Whether or not the category name was valid. + * @see #filterCategories + */ + private static boolean addToCategoriesList(final String categoryName) { + if (Arrays.binarySearch(VALID_CATEGORIES, categoryName.toLowerCase()) >= 0) { + filterCategories.add(categoryName); + return true; + } else { + error("The '[%1$s]' category wasn't found!", categoryName); + return false; + } + } + + /** Default name of the input file if none is given on the command line. */ private static final String DEFAULT_INPUT_FILE = "style_checks.log"; /** Pattern used to parse each input line. */ @@ -231,6 +364,8 @@ public final class StyleChecks { private static final String CATEGORY = "----------------------------"; /** Format string for the file vs problem count report. */ private static final String FORMAT4 = " %1$-42s %2$5d%n"; + /** Alternate format string for the file vs problem count report. */ + private static final String FORMAT4A = " %1$-48s %2$5d%n"; /** The set of unique file names found in the list. */ private static Set<String> fileNameSet = new HashSet<>(); /** The set of unique file names with warnings in the list. */ @@ -253,8 +388,10 @@ public final class StyleChecks { private static final boolean ON_WINDOWS = System.getProperty("os.name").startsWith("Windows"); /** The system file separator string. */ private static final String SEPARATOR = System.getProperty("file.separator"); + /** The starting directory location. */ + private static final File CURRENT_DIR_FILE = new File(System.getProperty("user.dir")); /** The starting directory (used to strip off the leading part of the file paths). */ - private static final String CURRENT_DIR = new File(System.getProperty("user.dir")).getPath() + SEPARATOR; + private static final String CURRENT_DIR = CURRENT_DIR_FILE.getPath() + SEPARATOR; /** Our package name prefix. */ private static final String PACKAGE_PREFIX = "org.apache.pivot"; /** Whether to report all the file problem counts, or just the least/most. */ @@ -289,6 +426,33 @@ public final class StyleChecks { /** The list of filter packages converted to regular expression patterns for matching. */ private static List<Pattern> packageFilterPatterns; + /** The standard path. */ + private static final String ORG_APACHE_PIVOT = "org/apache/pivot/"; + + /** + * A list of the possible checkstyle categories we can filter on. + * <p> Note: these are the "module" entries in our "pivot_style_checks.xml". + */ + private static final String[] VALID_CATEGORIES = { + "arraytypestyle", "avoidinlineconditionals", "avoidnestedblocks", + "avoidstarimport", "constantname", "designforextension", "emptyblock", + "emptyforiteratorpad", "emptystatement", "equalshashcode", "finalclass", + "finalparameters", "genericwhitespace", "hiddenfield", + "hideutilityclassconstructor", "illegalimport", "illegalinstantiation", + "innerassignment", "interfaceistype", "javadocmethod", "javadocstyle", + "javadoctype", "javadocvariable", "leftcurly", "linelength", + "localfinalvariablename", "localvariablename", "magicnumber", "membername", + "methodlength", "methodname", "methodparampad", "missingswitchdefault", + "modifierorder", "needbraces", "nowhitespaceafter", "nowhitespacebefore", + "operatorwrap", "packagename", "parametername", "parameternumber", "parenpad", + "redundantimport", "redundantmodifier", "rightcurly", + "simplifybooleanexpression", "simplifybooleanreturn", "staticvariablename", + "todocomment", "typename", "typecastparenpad", "unusedimports", "upperell", + "visibilitymodifier", "whitespaceafter", "whitespacearound", "filelength", + "filetabcharacter", "javadocpackage", "newlineatendoffile", "regexpsingleline", + "translation" + }; + /** * Output a formatted error message to {@link System#err}. * @param format The format string as input to {@link String#format}. @@ -365,7 +529,7 @@ public final class StyleChecks { info.getCount(), size); } else { System.out.format(FORMAT2, num, info.getSeverity(), info.getProblemCategory(), - info.getCount(), list(fileSet)); + info.getCount(), listShortName(fileSet)); } } @@ -400,9 +564,13 @@ public final class StyleChecks { /** * Process all the command-line arguments. * @param files The list of actual files to process (to add to). - * @param args The command line arguments to process. + * @param args The command line arguments to process. + * @return <code>true</code> if everything was fine, but <code>false</code> + * if there was a problem with the arguments. */ - private static void processArguments(final List<File> files, final String[] args) { + private static boolean processArguments(final List<File> files, final String[] args) { + boolean result; + // Process options and save the straight file names for (String arg : args) { if (arg.startsWith("--")) { @@ -417,11 +585,14 @@ public final class StyleChecks { String[] values = arg.split("[;,]"); for (String value : values) { if (value.endsWith(".java")) { - filterFileNames.add(value); + result = addToFileNameList(value); } else if (value.indexOf(".") >= 0) { - filterFileNames.add(value); + result = addToFileNameList(value); } else { - filterFileNames.add(value + ".java"); + result = addToFileNameList(value + ".java"); + } + if (!result) { + return false; } } filter = false; @@ -430,9 +601,12 @@ public final class StyleChecks { String[] values = arg.split("[;,]"); for (String value : values) { if (value.startsWith("[") && value.endsWith("]")) { - filterCategories.add(value.substring(1, value.length() - 1)); + result = addToCategoriesList(value.substring(1, value.length() - 1)); } else { - filterCategories.add(value); + result = addToCategoriesList(value); + } + if (!result) { + return false; } } category = false; @@ -462,6 +636,8 @@ public final class StyleChecks { fileNameFilterPatterns = makePatterns(filterFileNames); categoryFilterPatterns = makePatterns(filterCategories); packageFilterPatterns = makePatterns(filterPackages); + + return true; } /** @@ -540,6 +716,35 @@ public final class StyleChecks { } /** + * Get a unique, but short file name with only the part after <code>org/apache/pivot</code> saved. + * + * @param fullFileName The full (but still relative to the base directory) + * file name (in OS-specific form). + * @return The abbreviated form, which should be unique. + */ + private static String shortFileName(final String fullFileName) { + // First, translate the path separators to a canonical form + String canonicalName = fullFileName.replaceAll("[\\\\/]", "/"); + int ix = canonicalName.lastIndexOf(ORG_APACHE_PIVOT); + if (ix > 0) { + return canonicalName.substring(ix + ORG_APACHE_PIVOT.length()); + } else { + return canonicalName; + } + } + + /** + * Get the file name only from a short file name. + * + * @param shortFileName The result of {@link #shortFileName}. + * @return The name-only part. + */ + private static String fileNameOnly(final String shortFileName) { + int ix = shortFileName.lastIndexOf('/'); + return ix < 0 ? shortFileName : shortFileName.substring(ix + 1); + } + + /** * The main method, executed from the command line, which reads through each file * and processes it. * @param args The command line arguments. @@ -547,7 +752,9 @@ public final class StyleChecks { public static void main(final String[] args) { List<File> files = new ArrayList<>(args.length); - processArguments(files, args); + if (!processArguments(files, args)) { + System.exit(1); + } // Try to process the default error log if none was specified on the command line if (files.isEmpty()) { @@ -599,11 +806,12 @@ public final class StyleChecks { } String relativeFileName = f.getPath().replace(CURRENT_DIR, ""); fileNameSet.add(relativeFileName); + String shortFileName = shortFileName(relativeFileName); Info info = workingSet.get(problemCategory); if (info == null) { - workingSet.put(problemCategory, new Info(problemCategory, severity, nameOnly)); + workingSet.put(problemCategory, new Info(problemCategory, severity, shortFileName)); } else { - info.addFile(nameOnly); + info.addFile(shortFileName); } total++; if (severity.equals(SEV_ERROR)) { @@ -614,13 +822,13 @@ public final class StyleChecks { fileNameWarnSet.add(relativeFileName); totalWarn++; } - if (nameOnly.equals(currentFileName)) { + if (shortFileName.equals(currentFileName)) { totalForThisFile++; } else { if (currentFileName != null) { fileCounts.put(currentFileName, Integer.valueOf(totalForThisFile)); } - currentFileName = nameOnly; + currentFileName = shortFileName; totalForThisFile = 1; } } else if (line.equals("Starting audit...") || line.equals("Audit done.")) { @@ -651,18 +859,21 @@ public final class StyleChecks { StringBuilder buf = new StringBuilder("No results"); boolean anyFilters = false; if (filteredByFile) { - buf.append(anyFilters ? " or " : " for "); - buf.append("the filtered files ").append(list(filterFileNames)); + buf.append(anyFilters ? " or" : " for").append(" the filtered "); + buf.append(filterFileNames.size() == 1 ? "file " : "files "); + buf.append(list(filterFileNames)); anyFilters = true; } if (filteredByCategory) { - buf.append(anyFilters ? " or " : " for "); - buf.append("the filtered categories ").append(list(filterCategories)); + buf.append(anyFilters ? " or" : " for").append(" the filtered "); + buf.append(filterCategories.size() == 1 ? "category " : "categories "); + buf.append(list(filterCategories)); anyFilters = true; } if (filteredByPackage) { - buf.append(anyFilters ? " or " : " for "); - buf.append("the filtered packages ").append(list(filterPackages)); + buf.append(anyFilters ? " or" : " for").append(" the filtered "); + buf.append(filterPackages.size() == 1 ? "package " : "packages "); + buf.append(list(filterPackages)); anyFilters = true; } if (!anyFilters) { @@ -697,6 +908,9 @@ public final class StyleChecks { } if (fileCountList.size() > 1) { + boolean duplicates = anyDuplicateInfos(fileCountList); + String format4 = duplicates ? FORMAT4A : FORMAT4; + int remaining = fileCountList.size() - NUMBER_OF_FILES_TO_REPORT; int leastRemaining = Math.min(remaining, NUMBER_OF_FILES_TO_REPORT); boolean twoReports = !verbose && remaining > NUMBER_OF_FILES_TO_REPORT; @@ -707,7 +921,8 @@ public final class StyleChecks { System.out.println(twoReports ? "-----------------------------" : "--------------------"); int num = 1; for (FileInfo info : fileCountList) { - System.out.format(FORMAT4, info.getName(), info.getCount()); + String name = duplicates ? info.getName() : info.getNameOnly(); + System.out.format(format4, name, info.getCount()); if (twoReports && num++ >= NUMBER_OF_FILES_TO_REPORT) { break; } @@ -722,7 +937,8 @@ public final class StyleChecks { System.out.println("-------------------------------"); for (int i = leastRemaining; i > 0; i--) { FileInfo info = fileCountList.get(i - 1); - System.out.format(FORMAT4, info.getName(), info.getCount()); + String name = duplicates ? info.getName() : info.getNameOnly(); + System.out.format(format4, name, info.getCount()); } System.out.println(); }