Skip to content

Commit

Permalink
Various cleanups enabled by bumping minimum Java and Error Prone vers…
Browse files Browse the repository at this point in the history
…ions (#962)

These warning suppressions and compatibility code are no longed needed
given minimum JDK 11 and Error Prone 2.14.0
  • Loading branch information
msridhar authored Jun 4, 2024
1 parent 0ab6c22 commit 2a1e74b
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 83 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,10 @@ private static void copyAndAnnotateJarEntry(
String manifestText = stringBuilder.toString();
// Check for evidence of jar signing, note that lines can be split if too long so regex
// matching line by line will have false negatives.
// NOTE: this code only handles the case where the message digest algorithm used when signing
// was SHA-256. Eventually we may need a more robust solution for other digest algorithms.
// E.g., on JDK 21, the default message digest algorithm is SHA-384, and this code does not
// work for that algorithm (the DIGEST_ENTRY_PATTERN regex is hardcoded for SHA-256)
String manifestMinusDigests = manifestText.replaceAll(DIGEST_ENTRY_PATTERN, "");
if (!manifestText.equals(manifestMinusDigests) && !stripJarSignatures) {
throw new SignedJarException(SIGNED_JAR_ERROR_MESSAGE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,21 @@
import com.sun.tools.javac.main.Main;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.security.KeyStore;
import java.security.KeyStoreException;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.UnrecoverableEntryException;
import java.security.cert.CertificateException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.zip.ZipFile;
import jdk.security.jarsigner.JarSigner;
import org.apache.commons.io.FilenameUtils;
import org.junit.Assert;
import org.junit.Before;
Expand All @@ -61,11 +65,7 @@ public class JarInferTest {
* A dummy checker to allow us to use {@link CompilationTestHelper} to compile Java code for
* testing, as it requires a {@link BugChecker} to run.
*/
@BugPattern(
name = "DummyChecker",
summary = "Dummy checker to use CompilationTestHelper",
severity = WARNING)
@SuppressWarnings("BugPatternNaming") // remove once we require EP 2.11+
@BugPattern(summary = "Dummy checker to use CompilationTestHelper", severity = WARNING)
public static class DummyChecker extends BugChecker {
public DummyChecker() {}
}
Expand Down Expand Up @@ -514,32 +514,23 @@ public void testSignedJars() throws Exception {

/** copy the jar at {@code baseJarPath} to a signed jar at {@code signedJarPath} */
private void copyAndSignJar(String baseJarPath, String signedJarPath)
throws IOException, InterruptedException {
Files.copy(
Paths.get(baseJarPath), Paths.get(signedJarPath), StandardCopyOption.REPLACE_EXISTING);
throws CertificateException,
KeyStoreException,
IOException,
NoSuchAlgorithmException,
UnrecoverableEntryException {
String ksPath =
Thread.currentThread().getContextClassLoader().getResource("testKeyStore.jks").getPath();
// A public API for signing jars was added for Java 9+, but there is only an internal API on
// Java 8. And we need the test code to compile on Java 8. For simplicity and uniformity, we
// just run jarsigner as an executable (which slightly slows down test execution)
String jarsignerExecutable =
String.join(
FileSystems.getDefault().getSeparator(),
new String[] {System.getenv("JAVA_HOME"), "bin", "jarsigner"});
Process process =
Runtime.getRuntime()
.exec(
new String[] {
jarsignerExecutable,
"-keystore",
ksPath,
"-storepass",
"testPassword",
signedJarPath,
"testKeystore"
});
int exitCode = process.waitFor();
Assert.assertEquals("jarsigner process failed", 0, exitCode);
var ksPassword = "testPassword".toCharArray();
var keystore = KeyStore.getInstance(new File(ksPath), ksPassword);
var protParam = new KeyStore.PasswordProtection(ksPassword);
var pkEntry = (KeyStore.PrivateKeyEntry) keystore.getEntry("testKeystore", protParam);

JarSigner signer = new JarSigner.Builder(pkEntry).digestAlgorithm("SHA-256").build();
try (ZipFile in = new ZipFile(baseJarPath);
FileOutputStream out = new FileOutputStream(signedJarPath)) {
signer.sign(in, out);
}
}

private byte[] sha1sum(String path) throws Exception {
Expand Down
31 changes: 2 additions & 29 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.Modifier;
import javax.lang.model.element.ModuleElement;
import javax.lang.model.element.NestingKind;
import javax.lang.model.type.TypeKind;
import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode;
Expand Down Expand Up @@ -151,12 +152,10 @@
*/
@AutoService(BugChecker.class)
@BugPattern(
name = "NullAway",
altNames = {"CheckNullabilityTypes"},
summary = "Nullability type error.",
tags = BugPattern.StandardTags.LIKELY_ERROR,
severity = WARNING)
@SuppressWarnings("BugPatternNaming") // remove once we require EP 2.11+
public class NullAway extends BugChecker
implements BugChecker.MethodInvocationTreeMatcher,
BugChecker.AssignmentTreeMatcher,
Expand Down Expand Up @@ -267,14 +266,6 @@ public Config getConfig() {
*/
private final Map<ExpressionTree, Nullness> computedNullnessMap = new LinkedHashMap<>();

/**
* Used to check if a symbol represents a module in {@link #matchMemberSelect(MemberSelectTree,
* VisitorState)}. We need to use reflection to preserve compatibility with Java 8.
*
* <p>TODO remove this once NullAway requires JDK 11
*/
@Nullable private final Class<?> moduleElementClass;

/**
* Error Prone requires us to have an empty constructor for each Plugin, in addition to the
* constructor taking an ErrorProneFlags object. This constructor should not be used anywhere
Expand All @@ -286,7 +277,6 @@ public NullAway() {
handler = Handlers.buildEmpty();
nonAnnotatedMethod = this::isMethodUnannotated;
errorBuilder = new ErrorBuilder(config, "", ImmutableSet.of());
moduleElementClass = null;
}

@Inject // For future Error Prone versions in which checkers are loaded using Guice
Expand All @@ -295,14 +285,6 @@ public NullAway(ErrorProneFlags flags) {
handler = Handlers.buildDefault(config);
nonAnnotatedMethod = this::isMethodUnannotated;
errorBuilder = new ErrorBuilder(config, canonicalName(), allNames());
Class<?> moduleElementClass = null;
try {
moduleElementClass =
getClass().getClassLoader().loadClass("javax.lang.model.element.ModuleElement");
} catch (ClassNotFoundException e) {
// can occur pre JDK 11
}
this.moduleElementClass = moduleElementClass;
}

private boolean isMethodUnannotated(MethodInvocationNode invocationNode) {
Expand Down Expand Up @@ -586,7 +568,7 @@ public Description matchMemberSelect(MemberSelectTree tree, VisitorState state)
if (symbol == null
|| symbol.getSimpleName().toString().equals("class")
|| symbol.isEnum()
|| isModuleSymbol(symbol)) {
|| symbol instanceof ModuleElement) {
return Description.NO_MATCH;
}

Expand All @@ -602,15 +584,6 @@ public Description matchMemberSelect(MemberSelectTree tree, VisitorState state)
return Description.NO_MATCH;
}

/**
* Checks if {@code symbol} represents a JDK 9+ module using reflection.
*
* <p>TODO just check using instanceof once NullAway requires JDK 11
*/
private boolean isModuleSymbol(Symbol symbol) {
return moduleElementClass != null && moduleElementClass.isAssignableFrom(symbol.getClass());
}

/**
* Look for @NullMarked or @NullUnmarked annotations at the method level and adjust our scan for
* annotated code accordingly (fast scan for a fully annotated/unannotated top-level class or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,27 +266,7 @@ public Type createWithBaseTypeAndTypeArgs(Type baseType, List<Type> typeArgs) {

/** The TypeMetadataBuilder to be used for the current JDK version. */
private static final TypeMetadataBuilder TYPE_METADATA_BUILDER =
getVersion() >= 21
Runtime.version().feature() >= 21
? new JDK21TypeMetadataBuilder()
: new JDK17AndEarlierTypeMetadataBuilder();

/**
* Utility method to get the current JDK version, that works on Java 8 and above.
*
* <p>TODO remove this method once we drop support for Java 8
*
* @return the current JDK version
*/
private static int getVersion() {
String version = System.getProperty("java.version");
if (version.startsWith("1.")) {
version = version.substring(2, 3);
} else {
int dot = version.indexOf(".");
if (dot != -1) {
version = version.substring(0, dot);
}
}
return Integer.parseInt(version);
}
}

0 comments on commit 2a1e74b

Please sign in to comment.