Fix severity override in Ant task by using child elements (see #253)

src/main/docs/ant-task.html

@@ -73,20 +73,6 @@ <h2>Parameters</h2>
   <td>Name of a <a href="bundled-signatures.html">built-in signatures</a> file.</td>
 </tr>
 
-<tr>
-  <td>signaturesWithSeveritySuppress</td>
-  <td><code>String</code></td>
-  <td></td>
-  <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>
-</tr>
-
-<tr>
-  <td>signaturesWithSeverityWarn</td>
-  <td><code>String</code></td>
-  <td></td>
-  <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>
-</tr>
-
 <tr>
   <td>classpath</td>
   <td><code>Path</code></td>
@@ -217,6 +203,11 @@ <h2>Parameters specified as nested elements</h2>
 <code>RetentionPolicy#CLASS</code>. They can be applied to classes, their methods, or fields. By default, <code>@de.thetaphi.forbiddenapis.SuppressForbidden</code>
 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
   pattern may be used (e.g., <code>**.SuppressForbidden</code>).</p>
+
+<p>You can include multiple <code>&lt;severityOverride severity=&quot;(ERROR|WARNING|INFO|DEBUG|SUPPRESS)&quot;&gt;...signature...&lt;severityOverride&gt;</code> elements to
+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
+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
+original signatures files!</p>
 
 </body>
 </html>

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

@@ -60,7 +60,7 @@ public static enum Option {
   }
 
   public enum ViolationSeverity {
-      ERROR, WARNING, INFO, DEBUG, SUPPRESS
+    ERROR, WARNING, INFO, DEBUG, SUPPRESS
   }
 
   public final boolean isSupportedJDK;

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

@@ -173,7 +173,7 @@ private void addSignature(final String line, final String defaultMessage, final
     }
   }
 
-private Collection<String> getKeys(final UnresolvableReporting report, final boolean localIgnoreMissingClasses, final Set<String> missingClasses,
+  private Collection<String> getKeys(final UnresolvableReporting report, final boolean localIgnoreMissingClasses, final Set<String> missingClasses,
         final String signature) throws ParseException, IOException {
     final String clazz;
     final String field;
@@ -374,9 +374,9 @@ public boolean noSignaturesFilesParsed() {
     return numberOfFiles == 0;
   }
 
-  public void setSignaturesSeverity(Collection<String> signature, ViolationSeverity severity) throws ParseException, IOException {
-    logger.info("Adjusting severity to " + severity + " for signatures...");
-    for (String s : signature) {
+  public void setSignaturesSeverity(Collection<String> signatures, ViolationSeverity severity) throws ParseException, IOException {
+    logger.info("Adjusting severity to " + severity + " for " + signatures.size() + " signatures...");
+    for (String s : signatures) {
       setSignatureSeverity(s, severity);
     }
   }

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

@@ -23,10 +23,10 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.EnumSet;
 import java.util.Iterator;
-import java.util.LinkedHashSet;
 import java.util.Locale;
 
 import org.apache.tools.ant.AntClassLoader;
@@ -60,10 +60,9 @@ public class AntTask extends Task implements Constants {
 
   private final Union classFiles = new Union();
   private final Union apiSignatures = new Union();
-  private final Collection<BundledSignaturesType> bundledSignatures = new LinkedHashSet<>();
-  private final Collection<SuppressAnnotationType> suppressAnnotations = new LinkedHashSet<>();
-  private final Collection<String> signaturesWithSeverityWarn = new LinkedHashSet<>();;
-  private final Collection<String> signaturesWithSeveritySuppress = new LinkedHashSet<>();;
+  private final Collection<BundledSignaturesType> bundledSignatures = new ArrayList<>();
+  private final Collection<SuppressAnnotationType> suppressAnnotations = new ArrayList<>();
+  private final Collection<SeverityOverrideType> severityOverrides = new ArrayList<>();
 
   private Path classpath = null;
 
@@ -178,11 +177,12 @@ public void debug(String msg) {
             checker.parseSignaturesFile(r.getInputStream(), r.toString());
           }
         }
-        if (!signaturesWithSeverityWarn.isEmpty()) {
-          checker.setSignaturesSeverity(signaturesWithSeverityWarn, Checker.ViolationSeverity.WARNING);
-        }
-        if (!signaturesWithSeveritySuppress.isEmpty()) {
-          checker.setSignaturesSeverity(signaturesWithSeveritySuppress, Checker.ViolationSeverity.SUPPRESS);
+
+        for (SeverityOverrideType override : severityOverrides) {
+          if (override.severity == null) {
+            throw new BuildException("Severity must be given as argument of <severityOverride/> element.");
+          }
+          checker.setSignaturesSeverity(override.getSignatures(), override.severity);
         }
       } catch (IOException ioe) {
         throw new BuildException("IO problem while reading files with API signatures: " + ioe.getMessage(), ioe);
@@ -302,20 +302,18 @@ public void setBundledSignatures(String name) {
   }
 
   /** 
-   * 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}.
-   * In order to be effective the signature must be given in either {@link #bundledSignatures}, {@link #signaturesFiles}, {@link #signaturesArtifacts}, or {@link #signatures}.
+   * 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).
+   * Each signature must be listed in a separate XML element (in text) and a {@code priority} attribute.
+   * This takes precedence over {@link #failOnViolation}.
+   * 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
+   * is not yet ready to be enforced).
    * @since 3.9
    */
-  public void setSignaturesWithSeverityWarn(String signature) {
-      signaturesWithSeverityWarn.add(signature);
-  }
-
-  /** 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}.
-   * In order to be effective the signature must be given in either {@link #bundledSignatures}, {@link #signaturesFiles}, {@link #signaturesArtifacts}, or {@link #signatures}.
-   * @since 3.9
-   */
-  public void setSignaturesWithSeveritySuppress(String signature) {
-    signaturesWithSeveritySuppress.add(signature);
+  public SeverityOverrideType createSeverityOverride() {
+    final SeverityOverrideType s = new SeverityOverrideType();
+    s.setProject(getProject());
+    severityOverrides.add(s);
+    return s;
   }
 
   /** 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

@@ -0,0 +1,50 @@
+/*
+ * (C) Copyright Uwe Schindler (Generics Policeman) and others.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package de.thetaphi.forbiddenapis.ant;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Locale;
+
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.ProjectComponent;
+
+import de.thetaphi.forbiddenapis.Checker.ViolationSeverity;
+
+public final class SeverityOverrideType extends ProjectComponent {
+
+  private final StringBuilder signatures = new StringBuilder();
+
+  ViolationSeverity severity = null;
+
+  List<String> getSignatures() {
+    return Arrays.asList(signatures.toString().trim().split("\\s*[\r\n]+\\s*"));
+  }
+
+  public void addText(String signature) {
+    this.signatures.append('\n').append(signature);
+  }
+
+  public void setSeverity(String severity) {
+    try {
+      this.severity = ViolationSeverity.valueOf(severity.toUpperCase(Locale.ROOT));
+    } catch (IllegalArgumentException iae) {
+      throw new BuildException("Severity is not a valid enum value, use one of " + Arrays.toString(ViolationSeverity.values()));
+    }
+  }
+
+}

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

@@ -31,4 +31,9 @@ public BundledSignaturesType createBundled() {
     return task.createBundledSignatures();
   }
 
+  // we also allow to add a severity override in the <signatures/> element. Cool ne?
+  public SeverityOverrideType createSeverityOverride() {
+    return task.createSeverityOverride();
+  }
+
 }

src/test/antunit/TestSeverityOverrides.xml

@@ -0,0 +1,76 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ * (C) Copyright Uwe Schindler (Generics Policeman) and others.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+-->
+<project xmlns:au="antlib:org.apache.ant.antunit">
+
+  <fileset id="main.classes" dir="${antunit.main.classes}"/>
+
+  <target name="testOverrideGeneral">
+    <forbiddenapis classpathref="path.all" targetVersion="${jdk.version}">
+      <fileset refid="main.classes"/>
+      <bundledSignatures name="jdk-unsafe"/>
+      java.util.Locale#ENGLISH @ We are speaking chinese here!
+      java.lang.** @ You are crazy that you disallow all java.lang
+      java.io.** @ You are crazy that you disallow all java.io
+      <severityOverride severity="warning">java.util.Locale#ENGLISH</severityOverride>
+      <severityOverride severity="debug">
+        java.lang.**
+        java.io.**
+      </severityOverride>
+    </forbiddenapis>
+    <au:assertLogContains level="info" text="Reading bundled API signatures: jdk-unsafe-${jdk.version}"/>
+    <au:assertLogContains level="info" text="Adjusting severity to WARNING for 1 signatures..."/>
+    <au:assertLogContains level="info" text="Adjusting severity to DEBUG for 2 signatures..."/>
+    <au:assertLogContains level="warning" text="java.util.Locale#ENGLISH [We are speaking chinese here!]"/>
+    <au:assertLogContains level="debug" text="java.lang.String [You are crazy that you disallow all java.lang]"/> 
+    <au:assertLogContains level="debug" text="java.lang.StringBuilder [You are crazy that you disallow all java.lang]"/> 
+    <au:assertLogContains level="debug" text="java.io.InputStream [You are crazy that you disallow all java.io]"/> 
+    <au:assertLogContains level="info" text=" 0 error(s)."/>
+  </target>
+
+  <target name="testOverrideInSignaturesElement">
+    <forbiddenapis classpathref="path.all" targetVersion="${jdk.version}">
+      <fileset refid="main.classes"/>
+      <signatures>
+        <string>java.util.Locale#ENGLISH @ We are speaking chinese here!</string>
+        <string>java.lang.** @ You are crazy that you disallow all java.lang</string>
+        <bundled name="jdk-unsafe"/>
+        <severityOverride severity="warning">java.util.Locale#ENGLISH</severityOverride>
+        <severityOverride severity="debug">java.lang.**</severityOverride>
+      </signatures>
+    </forbiddenapis>
+    <au:assertLogContains level="info" text="Reading bundled API signatures: jdk-unsafe-${jdk.version}"/>
+    <au:assertLogContains level="info" text="Adjusting severity to WARNING for 1 signatures..."/>
+    <au:assertLogContains level="info" text="Adjusting severity to DEBUG for 1 signatures..."/>
+    <au:assertLogContains level="warning" text="java.util.Locale#ENGLISH [We are speaking chinese here!]"/>
+    <au:assertLogContains level="debug" text="java.lang.String [You are crazy that you disallow all java.lang]"/> 
+    <au:assertLogContains level="debug" text="java.lang.StringBuilder [You are crazy that you disallow all java.lang]"/> 
+    <au:assertLogContains level="info" text=" 0 error(s)."/>
+  </target>
+
+  <target name="testDoNothingOverride">
+    <au:expectfailure expectedMessage="Check for forbidden API calls failed, see log">
+      <forbiddenapis classpathref="path.all">
+        <fileset refid="main.classes"/>
+        java.lang.String#substring(int,int) @ You are crazy that you disallow substrings
+        <severityOverride severity="error">java.lang.String#substring(int,int)</severityOverride>
+      </forbiddenapis>
+    </au:expectfailure>
+    <au:assertLogContains level="info" text="Adjusting severity to ERROR for 1 signatures..."/>
+    <au:assertLogContains level="error" text="java.lang.String#substring(int,int) [You are crazy that you disallow substrings]"/> 
+  </target>
+
+</project>