Skip to content

Commit

Permalink
[154] Add back the ability to pass -IgnoreJDKClass with an argument. …
Browse files Browse the repository at this point in the history
…Without an argument, this will act as -IgnoreJDKClasses.

One breaking change is renaming of the parameters in the maven plugins. However, this is likely not a big issue.

Signed-off-by: James R. Perkins <[email protected]>
  • Loading branch information
jamezp committed Oct 3, 2024
1 parent 9391829 commit 9631d44
Show file tree
Hide file tree
Showing 14 changed files with 161 additions and 54 deletions.
11 changes: 7 additions & 4 deletions tools/sigtest/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,14 @@ with the action option set to `strictcheck` the plugin will detect any API chang

There are some cases where avoiding the verification of certain JDK classes entirely or their signatures can improve the ability to verify your API on different JDK versions.
The `-IgnoreJDKClasses` option will ignore (all) JDK java.* and javax.* classes during signature verification checking which helps avoid failures caused by
JDK specific signature changes introduced by a later JDK version. As an example, a Signature file with @java.lang.Deprecated annotations from JDK8 may be seeing verification failures on JDK9+
due to `default` fields being added to @Deprecated. With `-IgnoreJDKClasses specified, verification of the @Deprecated will only check that the tested class member has the
@Deprecated class but no verification of the @Deprecated signature will be performed.
JDK specific signature changes introduced by a later JDK version. As an example, a Signature file with `@java.lang.Deprecated` annotations from JDK8 may be seeing verification failures on JDK9+
due to `default` fields being added to `@Deprecated`. With `-IgnoreJDKClasses` specified, verification of the `@Deprecated` will only check that the tested class member has the
`@Deprecated` class but no verification of the `@Deprecated` signature will be performed.

Note that the `-IgnoreJDKClass` option can still be specified only if no JDK Class name is included after the `-IgnoreJDKClass`. You will get a failure if you try to use `-IgnoreJDKClass JDKClassName`.
In 2.4, the `-IgnoreJDKClass` was re-introduced to the previous behavior, with one exception. If you pass `-IgnoreJDKClass` with no argument, it's the same as passing `-IgnoreJDKClasses`.
However, you can now pass class name arguments to the `-IgnoreJDKClass` option like you could in previous version.

These options should likely be used sparingly. Using the `-Release` option the minimum target version of the API is the suggested way to handle forward compatibility for signature tests.

### Specify JDK classes to ignore in Maven plugin
Specify the `-IgnoreJDKClasses` option as shown below:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ protected boolean parseParameters(String[] args) {

parser.addOption(EXCLUDE_JDK_CLASS_OPTION, OptionInfo.optionalFlag(), optionsDecoder);

parser.addOption(LEGACY_EXCLUDE_JDK_CLASS_OPTION, OptionInfo.optionalFlag(), optionsDecoder);
parser.addOption(LEGACY_EXCLUDE_JDK_CLASS_OPTION, OptionInfo.optionVariableParams(0, OptionInfo.UNLIMITED), optionsDecoder);

try {
parser.processArgs(args);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public boolean run(String[] args, PrintWriter log, PrintWriter ref) {
parser.addOption(SigTest.PACKAGE_OPTION, OptionInfo.optionVariableParams(1, OptionInfo.UNLIMITED), optionsDecoder);
parser.addOption(SigTest.FILENAME_OPTION, OptionInfo.option(1), optionsDecoder);
parser.addOption(SigTest.EXCLUDE_JDK_CLASS_OPTION, OptionInfo.optionalFlag(), optionsDecoder);
parser.addOption(LEGACY_EXCLUDE_JDK_CLASS_OPTION, OptionInfo.optionalFlag(), optionsDecoder);
parser.addOption(LEGACY_EXCLUDE_JDK_CLASS_OPTION, OptionInfo.optionVariableParams(0, OptionInfo.UNLIMITED), optionsDecoder);
parser.addOption(SigTest.WITHOUTSUBPACKAGES_OPTION, OptionInfo.optionVariableParams(1, OptionInfo.UNLIMITED), optionsDecoder);
parser.addOption(SigTest.EXCLUDE_OPTION, OptionInfo.optionVariableParams(1, OptionInfo.UNLIMITED), optionsDecoder);
parser.addOption(SigTest.APIVERSION_OPTION, OptionInfo.option(1), optionsDecoder);
Expand Down Expand Up @@ -180,7 +180,11 @@ public void decodeOptions(String optionName, String[] args) {
} else if (optionName.equalsIgnoreCase(SigTest.EXCLUDE_JDK_CLASS_OPTION)) {
addFlag(testOptions, SigTest.EXCLUDE_JDK_CLASS_OPTION);
} else if (optionName.equalsIgnoreCase(LEGACY_EXCLUDE_JDK_CLASS_OPTION)) {
addFlag(testOptions, SigTest.EXCLUDE_JDK_CLASS_OPTION);
if (args == null || args.length == 0) {
addFlag(testOptions, SigTest.EXCLUDE_JDK_CLASS_OPTION);
} else {
addOption(testOptions, SigTest.LEGACY_EXCLUDE_JDK_CLASS_OPTION, args[0]);
}
} else if (optionName.equalsIgnoreCase(SigTest.FILENAME_OPTION) ||
optionName.equalsIgnoreCase(SigTest.PACKAGE_OPTION) ||
optionName.equalsIgnoreCase(SigTest.WITHOUTSUBPACKAGES_OPTION) ||
Expand Down
22 changes: 19 additions & 3 deletions tools/sigtest/src/main/java/com/sun/tdk/signaturetest/SigTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import com.sun.tdk.signaturetest.util.CommandLineParser;
import com.sun.tdk.signaturetest.util.CommandLineParserException;
import com.sun.tdk.signaturetest.util.I18NResourceBundle;
import com.sun.tdk.signaturetest.util.OptionInfo;

import java.io.PrintWriter;
import java.lang.reflect.Constructor;
Expand Down Expand Up @@ -219,6 +218,9 @@ public abstract class SigTest extends Result implements PluginAPI, Log {
private ClassDescriptionLoader loader;
protected boolean reportWarningAsError = false;

private final PackageGroup excludedJdkClasses = new PackageGroup(true);
private JDKExclude jdkExclude = excludedJdkClasses::checkName;

public void initErrors() {
errorMessages.clear();
}
Expand Down Expand Up @@ -287,9 +289,14 @@ protected void decodeCommonOptions(String optionName, String[] args) throws Comm
} else if (optionName.equalsIgnoreCase(CLASSPATH_OPTION)) {
classpathStr = args[0];
} else if (optionName.equalsIgnoreCase(EXCLUDE_JDK_CLASS_OPTION)) {
JDKExclude.enable();
jdkExclude = JDKExclude.JDK_CLASSES;
} else if (optionName.equalsIgnoreCase(LEGACY_EXCLUDE_JDK_CLASS_OPTION)) {
JDKExclude.enable();
final String[] packages = CommandLineParser.parseListOption(args);
if (packages.length == 0) {
jdkExclude = JDKExclude.JDK_CLASSES;
} else {
excludedJdkClasses.addPackages(packages);
}
} else if (optionName.equalsIgnoreCase(USE_BOOT_CP)) {
if (args.length == 0) {
release = Release.BOOT_CLASS_PATH;
Expand Down Expand Up @@ -367,6 +374,15 @@ protected boolean isPackageMember(String name) {
(packages.checkName(name) || purePackages.checkName(name));
}

/**
* Returns the {@link JDKExclude} utility used to determine if JDK classes should be excluded.
*
* @return the JDKExcluded, never {@code null}
*/
protected JDKExclude jdkExclude() {
return jdkExclude;
}

public void setClassDescrLoader(ClassDescriptionLoader loader) {
this.loader = loader;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,14 @@ public class SignatureTest extends SigTest {

protected Exclude exclude;
private int readMode = MultipleFileReader.MERGE_MODE;
private JDKExclude jdkExclude = new DefaultJDKExclude();
/**
* List of names of JDK classes and/or packages to be ignored along with subpackages.
*/
private static final PackageGroup excludedJdkClasses = new PackageGroup(true);

public SignatureTest() {
normalizer = new ThrowsNormalizer();
normalizer = new ThrowsNormalizer(jdkExclude);
}

/**
Expand Down Expand Up @@ -379,7 +384,7 @@ private boolean parseParameters(String[] args) {
parser.addOption(MODE_OPTION, OptionInfo.option(1), optionsDecoder);
parser.addOption(ALLPUBLIC_OPTION, OptionInfo.optionalFlag(), optionsDecoder);
parser.addOption(EXCLUDE_JDK_CLASS_OPTION,OptionInfo.optionalFlag(), optionsDecoder);
parser.addOption(LEGACY_EXCLUDE_JDK_CLASS_OPTION, OptionInfo.optionalFlag(), optionsDecoder);
parser.addOption(LEGACY_EXCLUDE_JDK_CLASS_OPTION, OptionInfo.optionVariableParams(0, OptionInfo.UNLIMITED), optionsDecoder);
parser.addOption(VERBOSE_OPTION, OptionInfo.optionalFlag(), optionsDecoder);

parser.addOption(HELP_OPTION, OptionInfo.optionalFlag(), optionsDecoder);
Expand Down Expand Up @@ -500,9 +505,16 @@ public void decodeOptions(String optionName, String[] args) throws CommandLinePa
} else if (optionName.equalsIgnoreCase(NOMERGE_OPTION)) {
readMode = MultipleFileReader.CLASSPATH_MODE;
} else if (optionName.equalsIgnoreCase(EXCLUDE_JDK_CLASS_OPTION)) {
JDKExclude.enable();
jdkExclude = JDKExclude.JDK_CLASSES;
} else if (optionName.equalsIgnoreCase(LEGACY_EXCLUDE_JDK_CLASS_OPTION)) {
JDKExclude.enable();
final String[] packages = CommandLineParser.parseListOption(args);
// In version 2.3 the -IgnoreJDKClass was changed to not allow parameters. It should be treated like
// -IgnoreJDKClasses
if (packages.length == 0) {
jdkExclude = JDKExclude.JDK_CLASSES;
} else {
excludedJdkClasses.addPackages(packages);
}
} else {
super.decodeCommonOptions(optionName, args);
}
Expand Down Expand Up @@ -720,7 +732,7 @@ private boolean check() {

testableHierarchy = new ClassHierarchyImpl(loader, trackMode);

testableMCBuilder = new MemberCollectionBuilder(this);
testableMCBuilder = new MemberCollectionBuilder(this, jdkExclude);

signatureClassesHierarchy = new ClassHierarchyImpl(in, trackMode);

Expand All @@ -741,7 +753,7 @@ else if ((outFormat != null) && FORMAT_BACKWARD.equals(outFormat))
boolean buildMembers = in.isFeatureSupported(FeaturesHolder.BuildMembers);
MemberCollectionBuilder sigfileMCBuilder = null;
if (buildMembers) {
sigfileMCBuilder = new MemberCollectionBuilder(this);
sigfileMCBuilder = new MemberCollectionBuilder(this, jdkExclude);
}

// Reading the sigfile: main loop.
Expand Down Expand Up @@ -1403,7 +1415,7 @@ private void trackMember(ClassDescription parentReq, ClassDescription parentFou,
}
}

if (!isSupersettingEnabled && found != null && !JDKExclude.isJdkClass(found.getDeclaringClassName())) {
if (!isSupersettingEnabled && found != null && !jdkExclude.isJdkClass(found.getDeclaringClassName())) {
errorManager.addError(MessageType.getAddedMessageType(found.getMemberType()), name, found.getMemberType(), found.toString(), found);
if (logger.isLoggable(Level.FINE)) {
logger.fine("added :-( " + found);
Expand Down Expand Up @@ -1438,15 +1450,15 @@ private void checkAnnotations(MemberDescription required, MemberDescription foun
int requiredPos = 0;
int foundPos = 0;

if (required != null && JDKExclude.isJdkClass(required.getDeclaringClassName())) {
if (required != null && jdkExclude.isJdkClass(required.getDeclaringClassName())) {
// don't check excluded Annotation classes
return;
}

while ((requiredPos < bl) && (foundPos < tl)) {
int comp = 0;
boolean isJDKExcludedClass = (JDKExclude.isJdkClass(baseAnnotList[requiredPos].getName()) ||
JDKExclude.isJdkClass(testAnnotList[foundPos].getName()));
boolean isJDKExcludedClass = (jdkExclude.isJdkClass(baseAnnotList[requiredPos].getName()) ||
jdkExclude.isJdkClass(testAnnotList[foundPos].getName()));
if (isJDKExcludedClass) {
// do comparison with just the JDK annotation name
comp = baseAnnotList[requiredPos].getName().compareTo(testAnnotList[foundPos].getName());
Expand Down Expand Up @@ -1566,4 +1578,13 @@ public String report() {
}

}

static class DefaultJDKExclude implements JDKExclude {

@Override
public boolean isJdkClass(String name) {
return name != null &&
excludedJdkClasses.checkName(name);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public class ClassCorrector implements Transformer {

protected ClassHierarchy classHierarchy = null;
private Log log;
private final JDKExclude jdkExclude;

/**
* Selftracing can be turned on by setting FINER level
Expand All @@ -76,13 +77,25 @@ public class ClassCorrector implements Transformer {


private static I18NResourceBundle i18n = I18NResourceBundle.getBundleForClass(ClassCorrector.class);

public ClassCorrector(Log log) {

public ClassCorrector(Log log, JDKExclude jdkExclude) {
this.jdkExclude = jdkExclude != null ? jdkExclude :
new JDKExclude() {
@Override
public boolean isJdkClass(String name) {
return false;
}
};
this.log = log;
// not configured externally
if(logger.getLevel() == null) {
logger.setLevel(Level.OFF);
}

}

public ClassCorrector(Log log) {
this(log, null);
}


Expand Down Expand Up @@ -180,7 +193,7 @@ private void replaceInvisibleExceptions(MemberDescription mr) throws ClassNotFou
} else
exceptionName = throwables.substring(startPos);

if (!JDKExclude.isJdkClass(exceptionName) && isInvisibleClass(exceptionName)) {
if (!jdkExclude.isJdkClass(exceptionName) && isInvisibleClass(exceptionName)) {
List supers = classHierarchy.getSuperClasses(exceptionName);
exceptionName = findVisibleReplacement(exceptionName, supers, "java.lang.Throwable", true);
mustCorrect = true;
Expand Down Expand Up @@ -700,7 +713,7 @@ private boolean isInvisibleClass(String fqname) {

if (fqname.startsWith("?"))
return false;
if (JDKExclude.isJdkClass(fqname))
if (jdkExclude.isJdkClass(fqname))
return false;
String pname = ClassCorrector.stripArrays(ClassCorrector.stripGenerics(fqname));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,22 @@
*
* @author Scott Marlow
*/
public class JDKExclude {
public interface JDKExclude {

private static PackageGroup excludedJdkClasses = new PackageGroup(true);

public static void enable() {
/**
* A default filter which excludes classes that start with {@code java} and {@code javax}.
*/
JDKExclude JDK_CLASSES = (name) -> {
final PackageGroup excludedJdkClasses = new PackageGroup(true);
excludedJdkClasses.addPackages(new String[] {"java", "javax" });
}
return excludedJdkClasses.checkName(name);
};

/**
* Check for JDK classes that should be excluded from signature testing.
* @param name is class name (with typical dot separators) to check.
* @return true if the class should be excluded from signature testing.
*/
public static boolean isJdkClass(String name) {
return name != null &&
excludedJdkClasses.checkName(name);

}
boolean isJdkClass(String name);

}
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,16 @@ public class MemberCollectionBuilder {
*/
static Logger logger = Logger.getLogger(MemberCollectionBuilder.class.getName());

public MemberCollectionBuilder(Log log, JDKExclude jdkExclude) {
this.cc = jdkExclude == null ? new ClassCorrector(log) : new ClassCorrector(log, jdkExclude);
this.log = log;

// not configured externally
if (logger.getLevel() == null) {
logger.setLevel(Level.OFF);
}
}

public MemberCollectionBuilder(Log log) {
this.cc = new ClassCorrector(log);
this.log = log;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ public class ThrowsNormalizer {
public ThrowsNormalizer() {

}
public ThrowsNormalizer(JDKExclude jdkExclude) {
this.jdkExclude = jdkExclude;
}

public void normThrows(ClassDescription c, boolean removeJLE, boolean onlyRemoveJavaSEThrows) throws ClassNotFoundException {

Expand Down Expand Up @@ -97,7 +100,7 @@ private void normThrows(ClassHierarchy h, MemberDescription mr, boolean removeJL
if (s == null)
continue;

if (JDKExclude.isJdkClass(s) ) {
if (jdkExclude.isJdkClass(s) ) {
xthrows.set(i, null);
superfluousExceptionCount++;
}
Expand Down Expand Up @@ -153,5 +156,10 @@ else if (!onlyRemoveJavaSEThrows && s.charAt(0) != '{' /* if not generic */) {

private List/*String*/ xthrows = new ArrayList();
private StringBuffer sb = new StringBuffer();

private JDKExclude jdkExclude = new JDKExclude() {
@Override
public boolean isJdkClass(String name) {
return false;
}
};
}
9 changes: 6 additions & 3 deletions tools/sigtest/src/main/java/org/netbeans/apitest/Sigtest.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ public final class Sigtest extends Task {
String failureProperty;
String release;

Boolean jdkExcludeEnabled = Boolean.FALSE;

public void setFileName(File f) {
fileName = f;
}
Expand Down Expand Up @@ -178,7 +176,12 @@ protected Integer getRelease() {

@Override
protected boolean isJDKExcludeEnabled() {
return jdkExcludeEnabled.booleanValue();
return false;
}

@Override
protected String[] getIgnoreJDKClassEntries() {
return null;
}
};
int returnCode;
Expand Down
Loading

0 comments on commit 9631d44

Please sign in to comment.