Skip to content

Commit 12bfbaf

Browse files
uschindleruschindler
authored
Fix severity override in Ant task by using child elements (see #253) (#262)
1 parent d7f276f commit 12bfbaf

File tree

7 files changed

+161
-41
lines changed

7 files changed

+161
-41
lines changed

‎src/main/docs/ant-task.html

+5-14
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,6 @@ <h2>Parameters</h2>
7373
<td>Name of a <a href="bundled-signatures.html">built-in signatures</a> file.</td>
7474
</tr>
7575

76-
<tr>
77-
<td>signaturesWithSeveritySuppress</td>
78-
<td><code>String</code></td>
79-
<td></td>
80-
<td>A forbidden API signature for which violations should not be reported at all (i.e. neither fail the build nor appear in the logs). This takes precedence over<code>failOnViolation</code> and <code>signaturesWithSeverityWarn</code>.</td>
81-
</tr>
82-
83-
<tr>
84-
<td>signaturesWithSeverityWarn</td>
85-
<td><code>String</code></td>
86-
<td></td>
87-
<td>A forbidden API signature for which violations should be reported as warnings (i.e. not fail the build). This takes precedence over<code>failOnViolation</code>.</td>
88-
</tr>
89-
9076
<tr>
9177
<td>classpath</td>
9278
<td><code>Path</code></td>
@@ -217,6 +203,11 @@ <h2>Parameters specified as nested elements</h2>
217203
<code>RetentionPolicy#CLASS</code>. They can be applied to classes, their methods, or fields. By default, <code>@de.thetaphi.forbiddenapis.SuppressForbidden</code>
218204
can always be used, but needs the <code>forbidden-apis.jar</code> file in classpath of compiled project, which may not be wanted. Instead of a full class name, a glob
219205
pattern may be used (e.g., <code>**.SuppressForbidden</code>).</p>
206+
207+
<p>You can include multiple <code>&lt;severityOverride severity=&quot;(ERROR|WARNING|INFO|DEBUG|SUPPRESS)&quot;&gt;...signature...&lt;severityOverride&gt;</code> elements to
208+
override problem severity of certain signatures (i.e. not fail the build). This takes precedence over <code>failOnViolation</code>. If several signatures should get the same
209+
severity you can separate them by newlines. It is also possible to separate them into different elements. Each signature must be specified in exactly the same way like in the
210+
original signatures files!</p>
220211

221212
</body>
222213
</html>

‎src/main/java/de/thetaphi/forbiddenapis/Checker.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public static enum Option {
6060
}
6161

6262
public enum ViolationSeverity {
63-
ERROR, WARNING, INFO, DEBUG, SUPPRESS
63+
ERROR, WARNING, INFO, DEBUG, SUPPRESS
6464
}
6565

6666
public final boolean isSupportedJDK;

‎src/main/java/de/thetaphi/forbiddenapis/Signatures.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ private void addSignature(final String line, final String defaultMessage, final
173173
}
174174
}
175175

176-
private Collection<String> getKeys(final UnresolvableReporting report, final boolean localIgnoreMissingClasses, final Set<String> missingClasses,
176+
private Collection<String> getKeys(final UnresolvableReporting report, final boolean localIgnoreMissingClasses, final Set<String> missingClasses,
177177
final String signature) throws ParseException, IOException {
178178
final String clazz;
179179
final String field;
@@ -374,9 +374,9 @@ public boolean noSignaturesFilesParsed() {
374374
return numberOfFiles == 0;
375375
}
376376

377-
public void setSignaturesSeverity(Collection<String> signature, ViolationSeverity severity) throws ParseException, IOException {
378-
logger.info("Adjusting severity to " + severity + " for signatures...");
379-
for (String s : signature) {
377+
public void setSignaturesSeverity(Collection<String> signatures, ViolationSeverity severity) throws ParseException, IOException {
378+
logger.info("Adjusting severity to " + severity + " for " + signatures.size() + " signatures...");
379+
for (String s : signatures) {
380380
setSignatureSeverity(s, severity);
381381
}
382382
}

‎src/main/java/de/thetaphi/forbiddenapis/ant/AntTask.java

+20-22
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@
2323
import java.io.File;
2424
import java.io.IOException;
2525
import java.io.InputStream;
26+
import java.util.ArrayList;
2627
import java.util.Collection;
2728
import java.util.EnumSet;
2829
import java.util.Iterator;
29-
import java.util.LinkedHashSet;
3030
import java.util.Locale;
3131

3232
import org.apache.tools.ant.AntClassLoader;
@@ -60,10 +60,9 @@ public class AntTask extends Task implements Constants {
6060

6161
private final Union classFiles = new Union();
6262
private final Union apiSignatures = new Union();
63-
private final Collection<BundledSignaturesType> bundledSignatures = new LinkedHashSet<>();
64-
private final Collection<SuppressAnnotationType> suppressAnnotations = new LinkedHashSet<>();
65-
private final Collection<String> signaturesWithSeverityWarn = new LinkedHashSet<>();;
66-
private final Collection<String> signaturesWithSeveritySuppress = new LinkedHashSet<>();;
63+
private final Collection<BundledSignaturesType> bundledSignatures = new ArrayList<>();
64+
private final Collection<SuppressAnnotationType> suppressAnnotations = new ArrayList<>();
65+
private final Collection<SeverityOverrideType> severityOverrides = new ArrayList<>();
6766

6867
private Path classpath = null;
6968

@@ -178,11 +177,12 @@ public void debug(String msg) {
178177
checker.parseSignaturesFile(r.getInputStream(), r.toString());
179178
}
180179
}
181-
if (!signaturesWithSeverityWarn.isEmpty()) {
182-
checker.setSignaturesSeverity(signaturesWithSeverityWarn, Checker.ViolationSeverity.WARNING);
183-
}
184-
if (!signaturesWithSeveritySuppress.isEmpty()) {
185-
checker.setSignaturesSeverity(signaturesWithSeveritySuppress, Checker.ViolationSeverity.SUPPRESS);
180+
181+
for (SeverityOverrideType override : severityOverrides) {
182+
if (override.severity == null) {
183+
throw new BuildException("Severity must be given as argument of <severityOverride/> element.");
184+
}
185+
checker.setSignaturesSeverity(override.getSignatures(), override.severity);
186186
}
187187
} catch (IOException ioe) {
188188
throw new BuildException("IO problem while reading files with API signatures: " + ioe.getMessage(), ioe);
@@ -302,20 +302,18 @@ public void setBundledSignatures(String name) {
302302
}
303303

304304
/**
305-
* A list of forbidden API signatures for which violations should not be reported at all (i.e. neither fail the build nor appear in the logs). This takes precedence over {@link #failOnViolation} and {@link #signaturesWithSeverityWarn}.
306-
* In order to be effective the signature must be given in either {@link #bundledSignatures}, {@link #signaturesFiles}, {@link #signaturesArtifacts}, or {@link #signatures}.
305+
* Adds an override for forbidden API signatures for which violations should not be reported at all (i.e. neither fail the build nor appear in the logs).
306+
* Each signature must be listed in a separate XML element (in text) and a {@code priority} attribute.
307+
* This takes precedence over {@link #failOnViolation}.
308+
* In order to be effective the signature must be given in one of the signatures elements (e.g., it can be used to disable a bundled signature which
309+
* is not yet ready to be enforced).
307310
* @since 3.9
308311
*/
309-
public void setSignaturesWithSeverityWarn(String signature) {
310-
signaturesWithSeverityWarn.add(signature);
311-
}
312-
313-
/** A list of forbidden API signatures for which violations should not be reported at all (i.e. neither fail the build nor appear in the logs). This takes precedence over {@link #failOnViolation} and {@link #signaturesWithSeverityWarn}.
314-
* In order to be effective the signature must be given in either {@link #bundledSignatures}, {@link #signaturesFiles}, {@link #signaturesArtifacts}, or {@link #signatures}.
315-
* @since 3.9
316-
*/
317-
public void setSignaturesWithSeveritySuppress(String signature) {
318-
signaturesWithSeveritySuppress.add(signature);
312+
public SeverityOverrideType createSeverityOverride() {
313+
final SeverityOverrideType s = new SeverityOverrideType();
314+
s.setProject(getProject());
315+
severityOverrides.add(s);
316+
return s;
319317
}
320318

321319
/** Creates a instance of an annotation class name that suppresses error reporting in classes/methods/fields. */

‎src/main/java/de/thetaphi/forbiddenapis/ant/SeverityOverrideType.java

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* (C) Copyright Uwe Schindler (Generics Policeman) and others.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package de.thetaphi.forbiddenapis.ant;
18+
19+
import java.util.Arrays;
20+
import java.util.List;
21+
import java.util.Locale;
22+
23+
import org.apache.tools.ant.BuildException;
24+
import org.apache.tools.ant.ProjectComponent;
25+
26+
import de.thetaphi.forbiddenapis.Checker.ViolationSeverity;
27+
28+
public final class SeverityOverrideType extends ProjectComponent {
29+
30+
private final StringBuilder signatures = new StringBuilder();
31+
32+
ViolationSeverity severity = null;
33+
34+
List<String> getSignatures() {
35+
return Arrays.asList(signatures.toString().trim().split("\\s*[\r\n]+\\s*"));
36+
}
37+
38+
public void addText(String signature) {
39+
this.signatures.append('\n').append(signature);
40+
}
41+
42+
public void setSeverity(String severity) {
43+
try {
44+
this.severity = ViolationSeverity.valueOf(severity.toUpperCase(Locale.ROOT));
45+
} catch (IllegalArgumentException iae) {
46+
throw new BuildException("Severity is not a valid enum value, use one of " + Arrays.toString(ViolationSeverity.values()));
47+
}
48+
}
49+
50+
}

‎src/main/java/de/thetaphi/forbiddenapis/ant/SignaturesResources.java

+5
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,9 @@ public BundledSignaturesType createBundled() {
3131
return task.createBundledSignatures();
3232
}
3333

34+
// we also allow to add a severity override in the <signatures/> element. Cool ne?
35+
public SeverityOverrideType createSeverityOverride() {
36+
return task.createSeverityOverride();
37+
}
38+
3439
}

‎src/test/antunit/TestSeverityOverrides.xml

+76
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
* (C) Copyright Uwe Schindler (Generics Policeman) and others.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
-->
17+
<project xmlns:au="antlib:org.apache.ant.antunit">
18+
19+
<fileset id="main.classes" dir="${antunit.main.classes}"/>
20+
21+
<target name="testOverrideGeneral">
22+
<forbiddenapis classpathref="path.all" targetVersion="${jdk.version}">
23+
<fileset refid="main.classes"/>
24+
<bundledSignatures name="jdk-unsafe"/>
25+
java.util.Locale#ENGLISH @ We are speaking chinese here!
26+
java.lang.** @ You are crazy that you disallow all java.lang
27+
java.io.** @ You are crazy that you disallow all java.io
28+
<severityOverride severity="warning">java.util.Locale#ENGLISH</severityOverride>
29+
<severityOverride severity="debug">
30+
java.lang.**
31+
java.io.**
32+
</severityOverride>
33+
</forbiddenapis>
34+
<au:assertLogContains level="info" text="Reading bundled API signatures: jdk-unsafe-${jdk.version}"/>
35+
<au:assertLogContains level="info" text="Adjusting severity to WARNING for 1 signatures..."/>
36+
<au:assertLogContains level="info" text="Adjusting severity to DEBUG for 2 signatures..."/>
37+
<au:assertLogContains level="warning" text="java.util.Locale#ENGLISH [We are speaking chinese here!]"/>
38+
<au:assertLogContains level="debug" text="java.lang.String [You are crazy that you disallow all java.lang]"/>
39+
<au:assertLogContains level="debug" text="java.lang.StringBuilder [You are crazy that you disallow all java.lang]"/>
40+
<au:assertLogContains level="debug" text="java.io.InputStream [You are crazy that you disallow all java.io]"/>
41+
<au:assertLogContains level="info" text=" 0 error(s)."/>
42+
</target>
43+
44+
<target name="testOverrideInSignaturesElement">
45+
<forbiddenapis classpathref="path.all" targetVersion="${jdk.version}">
46+
<fileset refid="main.classes"/>
47+
<signatures>
48+
<string>java.util.Locale#ENGLISH @ We are speaking chinese here!</string>
49+
<string>java.lang.** @ You are crazy that you disallow all java.lang</string>
50+
<bundled name="jdk-unsafe"/>
51+
<severityOverride severity="warning">java.util.Locale#ENGLISH</severityOverride>
52+
<severityOverride severity="debug">java.lang.**</severityOverride>
53+
</signatures>
54+
</forbiddenapis>
55+
<au:assertLogContains level="info" text="Reading bundled API signatures: jdk-unsafe-${jdk.version}"/>
56+
<au:assertLogContains level="info" text="Adjusting severity to WARNING for 1 signatures..."/>
57+
<au:assertLogContains level="info" text="Adjusting severity to DEBUG for 1 signatures..."/>
58+
<au:assertLogContains level="warning" text="java.util.Locale#ENGLISH [We are speaking chinese here!]"/>
59+
<au:assertLogContains level="debug" text="java.lang.String [You are crazy that you disallow all java.lang]"/>
60+
<au:assertLogContains level="debug" text="java.lang.StringBuilder [You are crazy that you disallow all java.lang]"/>
61+
<au:assertLogContains level="info" text=" 0 error(s)."/>
62+
</target>
63+
64+
<target name="testDoNothingOverride">
65+
<au:expectfailure expectedMessage="Check for forbidden API calls failed, see log">
66+
<forbiddenapis classpathref="path.all">
67+
<fileset refid="main.classes"/>
68+
java.lang.String#substring(int,int) @ You are crazy that you disallow substrings
69+
<severityOverride severity="error">java.lang.String#substring(int,int)</severityOverride>
70+
</forbiddenapis>
71+
</au:expectfailure>
72+
<au:assertLogContains level="info" text="Adjusting severity to ERROR for 1 signatures..."/>
73+
<au:assertLogContains level="error" text="java.lang.String#substring(int,int) [You are crazy that you disallow substrings]"/>
74+
</target>
75+
76+
</project>

0 commit comments

Comments
 (0)