Skip to content

Commit

Permalink
Internal build change.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 587885671
Change-Id: I426b7349412df400b0d2c149da5ff2dd47050e39
  • Loading branch information
cpovirk authored and copybara-github committed Dec 5, 2023
1 parent 96b3612 commit aa70121
Show file tree
Hide file tree
Showing 2 changed files with 210 additions and 46 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
load("@rules_java//java:defs.bzl", "java_library")
load("//tools/build_rules:java_rules_skylark.bzl", "bootstrap_java_library")
load("@rules_java//java:defs.bzl", "java_library")

# Description:
# Plugins for the Java library builders, which are used by Bazel to
Expand Down Expand Up @@ -30,6 +30,7 @@ java_library(
javacopts = [
"--add-exports=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.resources=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@

import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.devtools.build.buildjar.javac.plugins.dependency.StrictJavaDepsPlugin.NonPlatformJar.Kind.FOR_JSPECIFY_FROM_PLATFORM;
import static com.google.devtools.build.buildjar.javac.plugins.dependency.StrictJavaDepsPlugin.NonPlatformJar.Kind.IN_CLASSPATH;
import static java.lang.Boolean.parseBoolean;
import static javax.tools.StandardLocation.CLASS_PATH;

import com.google.auto.value.AutoOneOf;
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.buildjar.JarOwner;
Expand All @@ -41,13 +46,15 @@
import com.sun.tools.javac.util.Log;
import com.sun.tools.javac.util.Log.WriterKind;
import com.sun.tools.javac.util.Name;
import com.sun.tools.javac.util.Names;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.UncheckedIOException;
import java.lang.reflect.Method;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
Expand All @@ -59,7 +66,9 @@
import java.util.jar.Manifest;
import javax.lang.model.element.AnnotationValue;
import javax.lang.model.util.SimpleAnnotationValueVisitor8;
import javax.tools.JavaFileManager;
import javax.tools.JavaFileObject;
import javax.tools.JavaFileObject.Kind;

/**
* A plugin for BlazeJavaCompiler that checks for types referenced directly in the source, but
Expand Down Expand Up @@ -133,9 +142,14 @@ public void init(
dependencyModule.getPlatformJars());
checkingTreeScanner = context.get(CheckingTreeScanner.class);
if (checkingTreeScanner == null) {
Set<Path> platformJars = dependencyModule.getPlatformJars();
checkingTreeScanner =
new CheckingTreeScanner(dependencyModule, diagnostics, missingTargets, platformJars);
new CheckingTreeScanner(
dependencyModule,
diagnostics,
missingTargets,
dependencyModule.getPlatformJars(),
context.get(JavaFileManager.class),
Names.instance(context));
context.put(CheckingTreeScanner.class, checkingTreeScanner);
}
}
Expand Down Expand Up @@ -235,24 +249,41 @@ private static class CheckingTreeScanner extends TreeScanner {

private final Set<JarOwner> seenTargets = new HashSet<>();

private final Set<Path> seenStrictDepsViolatingJars = new HashSet<>();
private final Set<NonPlatformJar> seenStrictDepsViolatingJars = new HashSet<>();

/** The set of jars on the compilation bootclasspath. */
private final Set<Path> platformJars;

private final JavaFileManager fileManager;

/** The current source, for diagnostics. */
private JavaFileObject source = null;

/** Cache of classpath (not platform) jars in which given symbols can be found. */
private final Map<ClassSymbol, Optional<Path>> classpathOnlyDepPaths = new HashMap<>();

private final Name jspecifyAnnotationsPackage;
private final Name jspecifyNullnessPackage;

/* TODO(b/297254214): Remove this flag after the depot is clean. */
private final boolean hidePlatformJspecify;

public CheckingTreeScanner(
DependencyModule dependencyModule,
List<SjdDiagnostic> diagnostics,
Set<JarOwner> missingTargets,
Set<Path> platformJars) {
Set<Path> platformJars,
JavaFileManager fileManager,
Names names) {
this.directJars = dependencyModule.directJars();
this.diagnostics = diagnostics;
this.missingTargets = missingTargets;
this.directDependenciesMap = dependencyModule.getExplicitDependenciesMap();
this.platformJars = platformJars;
this.fileManager = fileManager;
jspecifyAnnotationsPackage = names.fromString("org.jspecify.annotations");
jspecifyNullnessPackage = names.fromString("org.jspecify.nullness");
hidePlatformJspecify = parseBoolean(System.getProperty("hidePlatformJspecify", "true"));
}

Set<ClassSymbol> getSeenClasses() {
Expand All @@ -264,26 +295,26 @@ private void checkTypeLiteral(JCTree node, Symbol sym) {
if (sym == null || sym.kind != Kinds.Kind.TYP) {
return;
}
Path jarPath = getJarPath(sym.enclClass(), platformJars);
NonPlatformJar jar = getNonPlatformJar(sym.enclClass(), platformJars);

// If this type symbol comes from a class file loaded from a jar, check
// whether that jar was a direct dependency and error out otherwise.
if (jarPath != null && seenClasses.add(sym.enclClass())) {
collectExplicitDependency(jarPath, node, sym);
if (jar != null && seenClasses.add(sym.enclClass())) {
collectExplicitDependency(jar, node, sym);
}
}

/**
* Marks the provided dependency as a direct/explicit dependency. Additionally, if
* strict_java_deps is enabled, it emits a [strict] compiler warning/error.
*/
private void collectExplicitDependency(Path jarPath, JCTree node, Symbol sym) {
private void collectExplicitDependency(NonPlatformJar jar, JCTree node, Symbol sym) {
// Does it make sense to emit a warning/error for this pair of (type, owner)?
// We want to emit only one error/warning per owner.
if (!directJars.contains(jarPath) && seenStrictDepsViolatingJars.add(jarPath)) {
if (!directJars.contains(jar.pathOrEmpty()) && seenStrictDepsViolatingJars.add(jar)) {
// IO cost here is fine because we only hit this path for an explicit dependency
// _not_ in the direct jars, i.e. an error
JarOwner owner = readJarOwnerFromManifest(jarPath);
JarOwner owner = readJarOwnerFromManifest(jar);
if (seenTargets.add(owner)) {
// owner is of the form "//label/of:rule <Aspect name>" where <Aspect name> is
// optional.
Expand All @@ -309,22 +340,28 @@ private void collectExplicitDependency(Path jarPath, JCTree node, Symbol sym) {
}
}

if (!directDependenciesMap.containsKey(jarPath)) {
if (!directDependenciesMap.containsKey(jar.pathOrEmpty())) {
// Also update the dependency proto
Dependency dep =
Dependency.newBuilder()
// Path.toString uses the platform separator (`\` on Windows) which may not
// match the format in params files (which currently always use `/`, see
// bazelbuild/bazel#4108). JavaBuilder should always parse Path strings into
// java.nio.file.Paths before comparing them.
.setPath(jarPath.toString())
//
// An empty path is OK in the cases we produce it. See readJarOwnerFromManifest.
.setPath(jar.pathOrEmpty().toString())
.setKind(Dependency.Kind.EXPLICIT)
.build();
directDependenciesMap.put(jarPath, dep);
directDependenciesMap.put(jar.pathOrEmpty(), dep);
}
}

private static JarOwner readJarOwnerFromManifest(Path jarPath) {
private JarOwner readJarOwnerFromManifest(NonPlatformJar jar) {
if (jar.getKind() == FOR_JSPECIFY_FROM_PLATFORM) {
return JSPECIFY_JAR_OWNER;
}
Path jarPath = jar.inClasspath();
try (JarFile jarFile = new JarFile(jarPath.toFile())) {
Manifest manifest = jarFile.getManifest();
if (manifest == null) {
Expand Down Expand Up @@ -423,6 +460,110 @@ public void visitPackageDef(JCTree.JCPackageDecl tree) {
scan(tree.annotations);
checkTypeLiteral(tree, tree.packge.package_info);
}

/**
* Returns the name of the <i>classpath</i> jar from which the given class symbol was loaded
* (with an exception for the JSpecify annotations) or else {@code null}.
*
* <p>If the symbol came from the platform (i.e., system modules/bootclasspath), rather than
* from the classpath, this method <i>usually</i> returns {@code null}. The exception is for
* JSpecify-annotation symbols that are read from the platform: For such a symbol, this method
* still returns the first <i>classpath</i> jar that contains the symbol, or, if no classpath
* jar contains the symbol, it returns {@code FOR_JSPECIFY_FROM_PLATFORM}. (The calling code
* later converts that to {@code JSPECIFY_JAR_OWNER}, which will lead to a strict-deps error,
* since that jar clearly isn't a direct dependency.) In this way, we pretend that the
* JSpecify-annotation symbols <i>aren't</i> part of the platform. That's important because in
* fact they aren't part of it <i>at runtime</i> and so we want to force users of those classes
* to declare a dependency on them.
*
* <p>This behavior is mildly unfortunate in the unusual situation that a project normally reads
* a JSpecify-annotations class from an uber-jar, rather than from the normal JSpecify target.
* In that case, we claim that the class is being loaded from the normal target. That is not the
* target that the project's developers are likely to want. It's even possible that the class
* isn't present on the reduced classpath but <i>would</i> be present (via the uber-jar) if only
* we compiled with the full classpath. The full-classpath compilation would still produce a
* strict-deps error, but it would produce one that recommends the correct jar/dependency. But
* as this code is, we fail with a suggestion of the normal JSpecify target, and we may or may
* not fall back to the full classpath.
*
* <p>OK, arguably it's unfortunate that <i>ever</i> we suggest that the normal JSpecify target
* is on the classpath when it isn't really. However, the most common result of that is going to
* be that we produce a more convenient error message. That convenience helps to offset any
* confusion that we produce. Still, we won't introduce similar behavior for other classes
* lightly.
*
* @param platformJars jars on javac's bootclasspath
*/
private NonPlatformJar getNonPlatformJar(ClassSymbol classSymbol, Set<Path> platformJars) {
if (classSymbol == null) {
return null;
}

// Ignore symbols that appear in the sourcepath:
if (haveSourceForSymbol(classSymbol)) {
return null;
}

JavaFileObject classfile = classSymbol.classfile;

Path path = ImplicitDependencyExtractor.getJarPath(classfile);
// Filter out classes from the system modules and bootclasspath
if (path == null || platformJars.contains(path)) {
// ...except the JSpecify annotations, which we treat specially.
if (hidePlatformJspecify
&& (classSymbol.packge().fullname.equals(jspecifyAnnotationsPackage)
|| classSymbol.packge().fullname.equals(jspecifyNullnessPackage))) {
Path classpathJar = findLookingOnlyInClasspath(classSymbol);
return classpathJar != null
? NonPlatformJar.forClasspathJar(classpathJar)
: NonPlatformJar.FOR_JSPECIFY_FROM_PLATFORM;
}
return null;
}

return NonPlatformJar.forClasspathJar(path);
}

/**
* Returns the first jar file in the classpath (not system modules, not bootclasspath) that
* contains the given class or {@code null} if no such jar is available.
*/
private Path findLookingOnlyInClasspath(ClassSymbol sym) {
/*
* computeIfAbsent doesn't cache null results, so we store Optional instances instead.
*
* In practice, that won't normally matter much: The only case in which our computation
* function runs once per usage of a JSpecify-annotation class is the failing-build case—that
* is, when the class is not on the classpath.
*/
return classpathOnlyDepPaths
.computeIfAbsent(
sym,
(unused) -> {
try {
for (JavaFileObject file :
fileManager.list(
CLASS_PATH,
sym.packge().fullname.toString(),
ImmutableSet.of(Kind.CLASS),
false /* do not return classes in subpackages */)) {
/*
* The query above returns all classpath classes from the given package. We can
* imagine situations in which only *some* JSpecify annotations are present in a
* given classpath jar (an uber-jar with unused classes removed?), so we want to
* make sure that we found the class we want.
*/
if (file.isNameCompatible(sym.getSimpleName().toString(), Kind.CLASS)) {
return Optional.of(ImplicitDependencyExtractor.getJarPath(file));
}
}
} catch (IOException e) {
throw new UncheckedIOException(e);
}
return Optional.empty();
})
.orElse(null);
}
}

/**
Expand Down Expand Up @@ -507,37 +648,6 @@ static String canonicalizeTarget(String target) {
return target;
}

/**
* Returns the name of the jar file from which the given class symbol was loaded, if available,
* and null otherwise. Implicitly filters out jars from the compilation bootclasspath.
*
* @param platformJars jars on javac's bootclasspath
*/
public static Path getJarPath(ClassSymbol classSymbol, Set<Path> platformJars) {
if (classSymbol == null) {
return null;
}

// Ignore symbols that appear in the sourcepath:
if (haveSourceForSymbol(classSymbol)) {
return null;
}

JavaFileObject classfile = classSymbol.classfile;

Path path = ImplicitDependencyExtractor.getJarPath(classfile);
if (path == null) {
return null;
}

// Filter out classes on bootclasspath
if (platformJars.contains(path)) {
return null;
}

return path;
}

/** Returns true if the given classSymbol corresponds to one of the sources being compiled. */
private static boolean haveSourceForSymbol(ClassSymbol classSymbol) {
if (classSymbol.sourcefile == null) {
Expand All @@ -559,4 +669,57 @@ private static boolean haveSourceForSymbol(ClassSymbol classSymbol) {
public boolean runOnAttributionErrors() {
return true;
}

/**
* Either a jar in the classpath or the well-known jar that contains the classes that are present
* in the platform at compile time but not runtime.
*/
@AutoOneOf(NonPlatformJar.Kind.class)
abstract static class NonPlatformJar {
enum Kind {
IN_CLASSPATH,
FOR_JSPECIFY_FROM_PLATFORM,
}

abstract Kind getKind();

abstract Path inClasspath();

abstract Placeholder forJspecifyFromPlatform();

final Path pathOrEmpty() {
return getKind() == IN_CLASSPATH ? inClasspath() : EMPTY_PATH;
}

static NonPlatformJar forClasspathJar(Path s) {
return AutoOneOf_StrictJavaDepsPlugin_NonPlatformJar.inClasspath(s);
}

static final NonPlatformJar FOR_JSPECIFY_FROM_PLATFORM =
AutoOneOf_StrictJavaDepsPlugin_NonPlatformJar.forJspecifyFromPlatform(Placeholder.INSTANCE);
}

enum Placeholder {
INSTANCE
}

private static final Path EMPTY_PATH = Path.of("");

/**
* A special-purpose {@link JarOwner} instance that points to the main JSpecify target but is used
* when the JSpecify annotations are instead read from the platform.
*
* <p>We use this instance to force users to add the explicit JSpecify dependency—again, even
* though the annotations are present in the compile-time platform (i.e., bootclasspath or system
* modules). We require users to add the dependency because the annotations are <i>not</i> present
* in the <i>runtime</i> platform.
*
* <p>The {@link Path} argument that we pass to this instance doesn't matter because the build is
* usually going to fail. (Or, if a strict-deps enforcement is disabled, the user can't reasonably
* expect fully accurate dependency information, and our tools should be mostly resilient to an
* empty path.)
*/
private static final JarOwner JSPECIFY_JAR_OWNER =
JarOwner.create(
EMPTY_PATH, "//third_party/java/jspecify_annotations", /* aspect= */ Optional.empty());
}

0 comments on commit aa70121

Please sign in to comment.