Skip to content

Commit

Permalink
Batch SJD errors for an entire compilation
Browse files Browse the repository at this point in the history 
instead of emitting them one file at a time. This provides users
with a single add_dep command instead of one-per-file.

PiperOrigin-RevId: 180979982
  • Loading branch information
@cushon
cushon authored and Copybara-Service committed Jan 5, 2018
1 parent 13b000c commit 11389d5
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 18 deletions.
1 change: 1 addition & 0 deletions src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ java_library(
":plugins",
"//src/java_tools/buildjar/java/com/google/devtools/build/buildjar:JarOwner",
"//src/main/protobuf:deps_java_proto",
"//third_party:auto_value",
"//third_party:guava",
"//third_party:tomcat_annotations_api",
"//third_party/java/jdk/langtools:javac",
Expand Down
66 changes: 48 additions & 18 deletions ...ava/com/google/devtools/build/buildjar/javac/plugins/dependency/StrictJavaDepsPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule.StrictJavaDeps.ERROR;

import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Ordering;
import com.google.devtools.build.buildjar.JarOwner;
Expand All @@ -42,8 +43,10 @@
import java.io.PrintWriter;
import java.nio.file.Path;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
Expand Down Expand Up @@ -74,12 +77,26 @@ public final class StrictJavaDepsPlugin extends BlazeJavaCompilerPlugin {
private final Set<JCTree> trees;
/** Computed missing dependencies */
private final Set<JarOwner> missingTargets;
/** Strict deps diagnostics. */
private final List<SjdDiagnostic> diagnostics;

private static Properties targetMap;


private PrintWriter errWriter;

@AutoValue
abstract static class SjdDiagnostic {
abstract int pos();

abstract String message();

abstract JavaFileObject source();

static SjdDiagnostic create(int pos, String message, JavaFileObject source) {
return new AutoValue_StrictJavaDepsPlugin_SjdDiagnostic(pos, message, source);
}
}

/**
* On top of javac, we keep Blaze-specific information in the form of two maps. Both map jars
* (exactly as they appear on the classpath) to target names, one is used for direct dependencies,
Expand All @@ -96,6 +113,7 @@ public StrictJavaDepsPlugin(DependencyModule dependencyModule) {
trees = new HashSet<>();
targetMap = new Properties();
missingTargets = new HashSet<>();
diagnostics = new ArrayList<>();
}

@Override
Expand All @@ -111,7 +129,7 @@ public void init(Context context, Log log, JavaCompiler compiler) {
if (checkingTreeScanner == null) {
Set<Path> platformJars = dependencyModule.getPlatformJars();
checkingTreeScanner =
new CheckingTreeScanner(dependencyModule, log, missingTargets, platformJars);
new CheckingTreeScanner(dependencyModule, diagnostics, missingTargets, platformJars);
context.put(CheckingTreeScanner.class, checkingTreeScanner);
}
initTargetMap();
Expand All @@ -123,7 +141,7 @@ private void initTargetMap() {
targetMap.load(is);
}
} catch (IOException ex) {
log.warning("Error loading Strict Java Deps mapping file: " + targetMapping, ex);
throw new AssertionError("Error loading Strict Java Deps mapping file: " + targetMapping, ex);
}
}

Expand All @@ -133,18 +151,18 @@ private void initTargetMap() {
*/
@Override
public void postAttribute(Env<AttrContext> env) {
JavaFileObject previousSource =
log.useSource(
env.enclClass.sym.sourcefile != null
? env.enclClass.sym.sourcefile
: env.toplevel.sourcefile);
JavaFileObject previousSource = checkingTreeScanner.source;
boolean previousExemption = checkingTreeScanner.isStrictDepsExempt;
try {
ProcessorDependencyMode mode = isAnnotationProcessorExempt(env.toplevel);
if (mode == ProcessorDependencyMode.EXEMPT_NORECORD) {
return;
}
checkingTreeScanner.isStrictDepsExempt |= mode == ProcessorDependencyMode.EXEMPT_RECORD;
checkingTreeScanner.source =
env.enclClass.sym.sourcefile != null
? env.enclClass.sym.sourcefile
: env.toplevel.sourcefile;
if (trees.add(env.tree)) {
checkingTreeScanner.scan(env.tree);
}
Expand All @@ -155,14 +173,27 @@ public void postAttribute(Env<AttrContext> env) {
}
} finally {
checkingTreeScanner.isStrictDepsExempt = previousExemption;
log.useSource(previousSource);
checkingTreeScanner.source = previousSource;
}
}

@Override
public void finish() {
implicitDependencyExtractor.accumulate(context, checkingTreeScanner.getSeenClasses());

for (SjdDiagnostic diagnostic : diagnostics) {
JavaFileObject prev = log.useSource(diagnostic.source());
try {
if (dependencyModule.getStrictJavaDeps() == ERROR) {
log.error(diagnostic.pos(), "proc.messager", diagnostic.message());
} else {
log.warning(diagnostic.pos(), "proc.messager", diagnostic.message());
}
} finally {
log.useSource(prev);
}
}

if (!missingTargets.isEmpty()) {
String canonicalizedLabel =
dependencyModule.getTargetLabel() == null
Expand Down Expand Up @@ -198,8 +229,8 @@ private static class CheckingTreeScanner extends TreeScanner {
/** Lookup for jars coming from transitive dependencies */
private final Map<Path, JarOwner> indirectJarsToTargets;

/** All error reporting is done through javac's log, */
private final Log log;
/** Strict deps diagnostics. */
private final List<SjdDiagnostic> diagnostics;

/** The strict_java_deps mode */
private final StrictJavaDeps strictJavaDepsMode;
Expand All @@ -221,14 +252,17 @@ private static class CheckingTreeScanner extends TreeScanner {
/** Was the node being visited generated by an exempt annotation processor? */
private boolean isStrictDepsExempt = false;

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

public CheckingTreeScanner(
DependencyModule dependencyModule,
Log log,
List<SjdDiagnostic> diagnostics,
Set<JarOwner> missingTargets,
Set<Path> platformJars) {
this.indirectJarsToTargets = dependencyModule.getIndirectMapping();
this.strictJavaDepsMode = dependencyModule.getStrictJavaDeps();
this.log = log;
this.diagnostics = diagnostics;
this.missingTargets = missingTargets;
this.directDependenciesMap = dependencyModule.getExplicitDependenciesMap();
this.platformJars = platformJars;
Expand Down Expand Up @@ -275,11 +309,7 @@ private void collectExplicitDependency(Path jarPath, JCTree node, Symbol sym) {
? "package " + sym.getEnclosingElement()
: "type " + sym;
String message = MessageFormat.format(TRANSITIVE_DEP_MESSAGE, used, toolInfo);
if (strictJavaDepsMode == ERROR) {
log.error(node.pos, "proc.messager", message);
} else {
log.warning(node.pos, "proc.messager", message);
}
diagnostics.add(SjdDiagnostic.create(node.pos, message, source));
}
}

Expand Down

0 comments on commit 11389d5

Please sign in to comment.