From 3a19ec90e4e7d0033207b95e4fe26b054de63399 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Fri, 30 Mar 2018 17:53:59 +0200 Subject: [PATCH 1/2] When signatures are using classes that are not found on classpath, the option to ignore those warnings is no longer so noisy: It only lists all failed signatures separately where methods/fields do not exit, but the missing classes are reported only with a single line. --- .../de/thetaphi/forbiddenapis/Checker.java | 61 ++++++++++++++----- src/test/antunit/TestInlineSignatures.xml | 4 +- src/test/antunit/TestMavenMojo.xml | 2 +- 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/src/main/java/de/thetaphi/forbiddenapis/Checker.java b/src/main/java/de/thetaphi/forbiddenapis/Checker.java index d01945f7..7a78cfb9 100644 --- a/src/main/java/de/thetaphi/forbiddenapis/Checker.java +++ b/src/main/java/de/thetaphi/forbiddenapis/Checker.java @@ -94,25 +94,30 @@ public static enum Option { final Set suppressAnnotations = new LinkedHashSet(); private static enum UnresolvableReporting { - FAIL() { + FAIL(true) { @Override public void parseFailed(Logger logger, String message, String signature) throws ParseException { throw new ParseException(String.format(Locale.ENGLISH, "%s while parsing signature: %s", message, signature)); } }, - WARNING() { + WARNING(false) { @Override public void parseFailed(Logger logger, String message, String signature) throws ParseException { logger.warn(String.format(Locale.ENGLISH, "%s while parsing signature: %s [signature ignored]", message, signature)); } }, - SILENT() { + SILENT(true) { @Override public void parseFailed(Logger logger, String message, String signature) throws ParseException { // keep silent } }; + private UnresolvableReporting(boolean reportClassNotFound) { + this.reportClassNotFound = reportClassNotFound; + } + + public final boolean reportClassNotFound; public abstract void parseFailed(Logger logger, String message, String signature) throws ParseException; } @@ -348,7 +353,7 @@ public ClassSignature lookupRelatedClass(String internalName) { } /** Adds the method signature to the list of disallowed methods. The Signature is checked against the given ClassLoader. */ - private void addSignature(final String line, final String defaultMessage, final UnresolvableReporting report) throws ParseException,IOException { + private void addSignature(final String line, final String defaultMessage, final UnresolvableReporting report, final Set missingClasses) throws ParseException,IOException { final String clazz, field, signature; String message = null; final Method method; @@ -360,6 +365,9 @@ private void addSignature(final String line, final String defaultMessage, final signature = line; message = defaultMessage; } + if (line.isEmpty()) { + throw new ParseException("Empty signature"); + } p = signature.indexOf('#'); if (p >= 0) { clazz = signature.substring(0, p); @@ -401,7 +409,11 @@ private void addSignature(final String line, final String defaultMessage, final try { c = getClassFromClassLoader(clazz); } catch (ClassNotFoundException cnfe) { - report.parseFailed(logger, String.format(Locale.ENGLISH, "Class '%s' not found on classpath", cnfe.getMessage()), signature); + if (report.reportClassNotFound) { + report.parseFailed(logger, String.format(Locale.ENGLISH, "Class '%s' not found on classpath", cnfe.getMessage()), signature); + } else { + missingClasses.add(clazz); + } return; } if (method != null) { @@ -433,10 +445,25 @@ private void addSignature(final String line, final String defaultMessage, final } } } + + private void reportMissingSignatureClasses(Set missingClasses) { + if (!missingClasses.isEmpty()) { + final StringBuilder sb = new StringBuilder("Some signatures were ignored because the following classes were not found on classpath: "); + boolean comma = false; + for (String s : missingClasses) { + if (comma) sb.append(", "); + comma = true; + sb.append(s); + } + logger.warn(sb.toString()); + } + } /** Reads a list of bundled API signatures from classpath. */ public void addBundledSignatures(String name, String jdkTargetVersion) throws IOException,ParseException { - addBundledSignatures(name, jdkTargetVersion, true); + final Set missingClasses = new TreeSet(); + addBundledSignatures(name, jdkTargetVersion, true, missingClasses); + reportMissingSignatureClasses(missingClasses); } public static String fixTargetVersion(String name) throws ParseException { @@ -465,7 +492,7 @@ public static String fixTargetVersion(String name) throws ParseException { return name; } - private void addBundledSignatures(String name, String jdkTargetVersion, boolean logging) throws IOException,ParseException { + private void addBundledSignatures(String name, String jdkTargetVersion, boolean logging, Set missingClasses) throws IOException,ParseException { if (!name.matches("[A-Za-z0-9\\-\\.]+")) { throw new ParseException("Invalid bundled signature reference: " + name); } @@ -487,13 +514,15 @@ private void addBundledSignatures(String name, String jdkTargetVersion, boolean throw new FileNotFoundException("Bundled signatures resource not found: " + name); } if (logging) logger.info("Reading bundled API signatures: " + name); - parseSignaturesFile(in, true); + parseSignaturesStream(in, true, missingClasses); } /** Reads a list of API signatures. Closes the Reader when done (on Exception, too)! */ public void parseSignaturesFile(InputStream in, String name) throws IOException,ParseException { logger.info("Reading API signatures: " + name); - parseSignaturesFile(in, false); + final Set missingClasses = new TreeSet(); + parseSignaturesStream(in, false, missingClasses); + reportMissingSignatureClasses(missingClasses); } /** Reads a list of API signatures from the given URL. */ @@ -509,18 +538,20 @@ public void parseSignaturesFile(File f) throws IOException,ParseException { /** Reads a list of API signatures from a String. */ public void parseSignaturesString(String signatures) throws IOException,ParseException { logger.info("Reading inline API signatures..."); - parseSignaturesFile(new StringReader(signatures), false); + final Set missingClasses = new TreeSet(); + parseSignaturesFile(new StringReader(signatures), false, missingClasses); + reportMissingSignatureClasses(missingClasses); } - private void parseSignaturesFile(InputStream in, boolean allowBundled) throws IOException,ParseException { - parseSignaturesFile(new InputStreamReader(in, "UTF-8"), allowBundled); + private void parseSignaturesStream(InputStream in, boolean allowBundled, Set missingClasses) throws IOException,ParseException { + parseSignaturesFile(new InputStreamReader(in, "UTF-8"), allowBundled, missingClasses); } private static final String BUNDLED_PREFIX = "@includeBundled "; private static final String DEFAULT_MESSAGE_PREFIX = "@defaultMessage "; private static final String IGNORE_UNRESOLVABLE_LINE = "@ignoreUnresolvable"; - private void parseSignaturesFile(Reader reader, boolean isBundled) throws IOException,ParseException { + private void parseSignaturesFile(Reader reader, boolean isBundled, Set missingClasses) throws IOException,ParseException { final BufferedReader r = new BufferedReader(reader); try { String line, defaultMessage = null; @@ -532,7 +563,7 @@ private void parseSignaturesFile(Reader reader, boolean isBundled) throws IOExce if (line.startsWith("@")) { if (isBundled && line.startsWith(BUNDLED_PREFIX)) { final String name = line.substring(BUNDLED_PREFIX.length()).trim(); - addBundledSignatures(name, null, false); + addBundledSignatures(name, null, false, missingClasses); } else if (line.startsWith(DEFAULT_MESSAGE_PREFIX)) { defaultMessage = line.substring(DEFAULT_MESSAGE_PREFIX.length()).trim(); if (defaultMessage.length() == 0) defaultMessage = null; @@ -542,7 +573,7 @@ private void parseSignaturesFile(Reader reader, boolean isBundled) throws IOExce throw new ParseException("Invalid line in signature file: " + line); } } else { - addSignature(line, defaultMessage, reporter); + addSignature(line, defaultMessage, reporter, missingClasses); } } } finally { diff --git a/src/test/antunit/TestInlineSignatures.xml b/src/test/antunit/TestInlineSignatures.xml index 555b7661..7e08525d 100644 --- a/src/test/antunit/TestInlineSignatures.xml +++ b/src/test/antunit/TestInlineSignatures.xml @@ -97,7 +97,7 @@ java.lang.String#forbiddenFoobarField @ should be ignored java.awt.Color @ Color is disallowed, thats not bad, because ANT has no colors... (this was just added to don't fail because of missing signatures) - + @@ -110,7 +110,7 @@ java.lang.String#forbiddenFoobarField @ should be ignored java.awt.Color @ Color is disallowed, thats not bad, because ANT has no colors... (this was just added to don't fail because of missing signatures) - + diff --git a/src/test/antunit/TestMavenMojo.xml b/src/test/antunit/TestMavenMojo.xml index 5297fcae..aa9d995b 100644 --- a/src/test/antunit/TestMavenMojo.xml +++ b/src/test/antunit/TestMavenMojo.xml @@ -84,7 +84,7 @@ - + From 63bae01452285ccf8935417fdf3296e6c09a4d7f Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Fri, 30 Mar 2018 20:29:07 +0200 Subject: [PATCH 2/2] Limit line length / number of missing classes to be reported on signature parsing --- .../de/thetaphi/forbiddenapis/Checker.java | 21 ++++++++++++------- src/test/antunit/TestInlineSignatures.xml | 6 ++++-- src/test/antunit/TestMavenMojo.xml | 3 ++- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/main/java/de/thetaphi/forbiddenapis/Checker.java b/src/main/java/de/thetaphi/forbiddenapis/Checker.java index 7a78cfb9..30906dd2 100644 --- a/src/main/java/de/thetaphi/forbiddenapis/Checker.java +++ b/src/main/java/de/thetaphi/forbiddenapis/Checker.java @@ -447,16 +447,21 @@ private void addSignature(final String line, final String defaultMessage, final } private void reportMissingSignatureClasses(Set missingClasses) { - if (!missingClasses.isEmpty()) { - final StringBuilder sb = new StringBuilder("Some signatures were ignored because the following classes were not found on classpath: "); - boolean comma = false; - for (String s : missingClasses) { - if (comma) sb.append(", "); - comma = true; - sb.append(s); + if (missingClasses.isEmpty()) { + return; + } + logger.warn("Some signatures were ignored because the following classes were not found on classpath:"); + final StringBuilder sb = new StringBuilder(); + int count = 0; + for (String s : missingClasses) { + sb.append(count == 0 ? " " : ", ").append(s); + count++; + if (sb.length() >= 70) { + sb.append(",... (and ").append(missingClasses.size() - count).append(" more)."); + break; } - logger.warn(sb.toString()); } + logger.warn(sb.toString()); } /** Reads a list of bundled API signatures from classpath. */ diff --git a/src/test/antunit/TestInlineSignatures.xml b/src/test/antunit/TestInlineSignatures.xml index 7e08525d..84a34b29 100644 --- a/src/test/antunit/TestInlineSignatures.xml +++ b/src/test/antunit/TestInlineSignatures.xml @@ -97,7 +97,8 @@ java.lang.String#forbiddenFoobarField @ should be ignored java.awt.Color @ Color is disallowed, thats not bad, because ANT has no colors... (this was just added to don't fail because of missing signatures) - + + @@ -110,7 +111,8 @@ java.lang.String#forbiddenFoobarField @ should be ignored java.awt.Color @ Color is disallowed, thats not bad, because ANT has no colors... (this was just added to don't fail because of missing signatures) - + + diff --git a/src/test/antunit/TestMavenMojo.xml b/src/test/antunit/TestMavenMojo.xml index aa9d995b..a2957dbd 100644 --- a/src/test/antunit/TestMavenMojo.xml +++ b/src/test/antunit/TestMavenMojo.xml @@ -84,7 +84,8 @@ - + +