Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for issue #83: Tone down warnings by missing classes in signatures files #140

Merged
merged 2 commits into from
Mar 30, 2018
Merged

Fix for issue #83: Tone down warnings by missing classes in signatures files #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a19ec9
When signatures are using classes that are not found on classpath, th…
uschindler Mar 30, 2018
63bae01
Limit line length / number of missing classes to be reported on signa…
uschindler Mar 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 51 additions & 15 deletions src/main/java/de/thetaphi/forbiddenapis/Checker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,25 +94,30 @@ public static enum Option {
final Set<String> suppressAnnotations = new LinkedHashSet<String>();

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;
}

Expand Down Expand Up @@ -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<String> missingClasses) throws ParseException,IOException {
final String clazz, field, signature;
String message = null;
final Method method;
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -433,10 +445,30 @@ private void addSignature(final String line, final String defaultMessage, final
}
}
}

private void reportMissingSignatureClasses(Set<String> missingClasses) {
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());
}

/** 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<String> missingClasses = new TreeSet<String>();
addBundledSignatures(name, jdkTargetVersion, true, missingClasses);
reportMissingSignatureClasses(missingClasses);
}

public static String fixTargetVersion(String name) throws ParseException {
Expand Down Expand Up @@ -465,7 +497,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<String> missingClasses) throws IOException,ParseException {
if (!name.matches("[A-Za-z0-9\\-\\.]+")) {
throw new ParseException("Invalid bundled signature reference: " + name);
}
Expand All @@ -487,13 +519,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<String> missingClasses = new TreeSet<String>();
parseSignaturesStream(in, false, missingClasses);
reportMissingSignatureClasses(missingClasses);
}

/** Reads a list of API signatures from the given URL. */
Expand All @@ -509,18 +543,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<String> missingClasses = new TreeSet<String>();
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<String> 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<String> missingClasses) throws IOException,ParseException {
final BufferedReader r = new BufferedReader(reader);
try {
String line, defaultMessage = null;
Expand All @@ -532,7 +568,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;
Expand All @@ -542,7 +578,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 {
Expand Down
6 changes: 4 additions & 2 deletions src/test/antunit/TestInlineSignatures.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
</forbiddenapis>
<au:assertLogContains level="warning" text="Class 'foo.bar.ForbiddenApis' not found on classpath while parsing signature: foo.bar.ForbiddenApis#testMethod() [signature ignored]"/>
<au:assertLogContains level="warning" text="Some signatures were ignored because the following classes were not found on classpath:"/>
<au:assertLogContains level="warning" text=" foo.bar.ForbiddenApis"/>
<au:assertLogContains level="warning" text="Method not found while parsing signature: java.lang.String#forbiddenFoobarMethod() [signature ignored]"/>
<au:assertLogContains level="warning" text="Field not found while parsing signature: java.lang.String#forbiddenFoobarField [signature ignored]"/>
</target>
Expand All @@ -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)
</forbiddenapis>
<au:assertLogContains level="warning" text="Class 'foo.bar.ForbiddenApis' not found on classpath while parsing signature: foo.bar.ForbiddenApis#testMethod() [signature ignored]"/>
<au:assertLogContains level="warning" text="Some signatures were ignored because the following classes were not found on classpath:"/>
<au:assertLogContains level="warning" text=" foo.bar.ForbiddenApis"/>
<au:assertLogContains level="warning" text="Method not found while parsing signature: java.lang.String#forbiddenFoobarMethod() [signature ignored]"/>
<au:assertLogContains level="warning" text="Field not found while parsing signature: java.lang.String#forbiddenFoobarField [signature ignored]"/>
</target>
Expand Down
3 changes: 2 additions & 1 deletion src/test/antunit/TestMavenMojo.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@
<sysproperty key="antunit.signatures" value="foo.bar.ForbiddenApis#testMethod()&#10;java.lang.String#forbiddenFoobarMethod()&#10;java.lang.String#forbiddenFoobarField"/>
<sysproperty key="antunit.failOnUnresolvableSignatures" value="false"/>
</artifact:mvn>
<au:assertLogContains text="Class 'foo.bar.ForbiddenApis' not found on classpath while parsing signature: foo.bar.ForbiddenApis#testMethod() [signature ignored]"/>
<au:assertLogContains text="Some signatures were ignored because the following classes were not found on classpath:"/>
<au:assertLogContains text=" foo.bar.ForbiddenApis"/>
<au:assertLogContains text="Method not found while parsing signature: java.lang.String#forbiddenFoobarMethod() [signature ignored]"/>
<au:assertLogContains text="Field not found while parsing signature: java.lang.String#forbiddenFoobarField [signature ignored]"/>
</target>
Expand Down