Rewrite the preprocessor infrastructure to return events as part of the result.

--
MOS_MIGRATED_REVID=102658808

3 files changed, 32 insertions, 31 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index ffea0bddad..d5239fe814 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -1050,10 +1050,9 @@ public final class PackageFactory {
       throw new BuildFileContainsErrorsException(packageId, "IOException occured");
     }
 
-    StoredEventHandler localReporter = new StoredEventHandler();
     Globber globber = createLegacyGlobber(buildFile.getParentDirectory(), packageId, locator);
     Preprocessor.Result preprocessingResult =
-        preprocess(packageId, buildFile, inputSource, globber, localReporter);
+        preprocess(packageId, buildFile, inputSource, globber);
     ExternalPackage externalPkg =
         new ExternalPackage.Builder(
             buildFile.getRelative("WORKSPACE"), ruleClassProvider.getRunfilesPrefix()).build();
@@ -1064,7 +1063,7 @@ public final class PackageFactory {
                 packageId,
                 buildFile,
                 preprocessingResult,
-                localReporter.getEvents(), /* preprocessingEvents */
+                preprocessingResult.events,
                 ImmutableList.<Statement>of(), /* preludeStatements */
                 ImmutableMap.<PathFragment, SkylarkEnvironment>of(), /* imports */
                 ImmutableList.<Label>of(), /* skylarkFileDependencies */
@@ -1077,19 +1076,21 @@ public final class PackageFactory {
   }
 
   /** Preprocesses the given BUILD file. */
-  // Used outside of bazel!
   public Preprocessor.Result preprocess(
       PackageIdentifier packageId,
       Path buildFile,
-      CachingPackageLocator locator,
-      EventHandler eventHandler) throws InterruptedException {
-    ParserInputSource inputSource = maybeGetParserInputSource(buildFile, eventHandler);
-    if (inputSource == null) {
-      return Preprocessor.Result.transientError(buildFile.asFragment());
+      CachingPackageLocator locator) throws InterruptedException {
+    ParserInputSource inputSource;
+    try {
+      inputSource = ParserInputSource.create(buildFile);
+    } catch (IOException e) {
+      List<Event> events = ImmutableList.of(
+          Event.error(Location.fromFile(buildFile), e.getMessage()));
+      return Preprocessor.Result.transientError(buildFile.asFragment(), events);
     }
     Globber globber = createLegacyGlobber(buildFile.getParentDirectory(), packageId, locator);
     try {
-      return preprocess(packageId, buildFile, inputSource, globber, eventHandler);
+      return preprocess(packageId, buildFile, inputSource, globber);
     } finally {
       globber.onCompletion();
     }
@@ -1103,26 +1104,24 @@ public final class PackageFactory {
       PackageIdentifier packageId,
       Path buildFile,
       ParserInputSource inputSource,
-      Globber globber,
-      EventHandler eventHandler) throws InterruptedException {
+      Globber globber) throws InterruptedException {
     Preprocessor preprocessor = preprocessorFactory.getPreprocessor();
     if (preprocessor == null) {
       return Preprocessor.Result.noPreprocessing(inputSource);
     }
     try {
-      return preprocessor.preprocess(inputSource, packageId.toString(), globber, eventHandler,
+      return preprocessor.preprocess(inputSource, packageId.toString(), globber,
           globalEnv, ruleFactory.getRuleClassNames());
     } catch (IOException e) {
-      eventHandler.handle(Event.error(Location.fromFile(buildFile),
+      List<Event> events = ImmutableList.of(Event.error(Location.fromFile(buildFile),
                      "preprocessing failed: " + e.getMessage()));
-      return Preprocessor.Result.transientError(buildFile.asFragment());
+      return Preprocessor.Result.transientError(buildFile.asFragment(), events);
     } catch (InterruptedException e) {
       globber.onInterrupt();
       throw e;
     }
   }
 
-  // Used outside of bazel!
   public LegacyGlobber createLegacyGlobber(Path packageDirectory, PackageIdentifier packageId,
       CachingPackageLocator locator) {
     return new LegacyGlobber(new GlobCache(packageDirectory, packageId, locator, syscalls,
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java b/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java
index a479ca9f55..babe5ff555 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java
@@ -13,13 +13,15 @@
 // limitations under the License.
 package com.google.devtools.build.lib.packages;
 
-import com.google.devtools.build.lib.events.EventHandler;
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.PackageFactory.Globber;
 import com.google.devtools.build.lib.syntax.Environment;
 import com.google.devtools.build.lib.syntax.ParserInputSource;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
 import java.io.IOException;
+import java.util.List;
 import java.util.Set;
 
 import javax.annotation.Nullable;
@@ -96,19 +98,22 @@ public interface Preprocessor {
     public final boolean preprocessed;
     public final boolean containsErrors;
     public final boolean containsTransientErrors;
+    public final List<Event> events;
 
     private Result(ParserInputSource result,
-        boolean preprocessed, boolean containsPersistentErrors, boolean containsTransientErrors) {
+        boolean preprocessed, boolean containsPersistentErrors, boolean containsTransientErrors,
+        List<Event> events) {
       this.result = result;
       this.preprocessed = preprocessed;
       this.containsErrors = containsPersistentErrors || containsTransientErrors;
       this.containsTransientErrors = containsTransientErrors;
+      this.events = ImmutableList.copyOf(events);
     }
 
     /** Convenience factory for a {@link Result} wrapping non-preprocessed BUILD file contents. */
     public static Result noPreprocessing(ParserInputSource buildFileSource) {
       return new Result(buildFileSource, /*preprocessed=*/false, /*containsErrors=*/false,
-          /*containsTransientErrors=*/false);
+          /*containsTransientErrors=*/false, ImmutableList.<Event>of());
     }
 
     /**
@@ -116,19 +121,20 @@ public interface Preprocessor {
      * read and has valid syntax and was preprocessed. But note that there may have been be errors
      * during preprocessing.
      */
-    public static Result success(ParserInputSource result, boolean containsErrors) {
+    public static Result success(ParserInputSource result, boolean containsErrors,
+        List<Event> events) {
       return new Result(result, /*preprocessed=*/true, /*containsPersistentErrors=*/containsErrors,
-          /*containsTransientErrors=*/false);
+          /*containsTransientErrors=*/false, events);
     }
 
-    public static Result invalidSyntax(PathFragment buildFile) {
+    public static Result invalidSyntax(PathFragment buildFile, List<Event> events) {
       return new Result(ParserInputSource.create(EMPTY_CHARS, buildFile), /*preprocessed=*/true,
-          /*containsPersistentErrors=*/true, /*containsTransientErrors=*/false);
+          /*containsPersistentErrors=*/true, /*containsTransientErrors=*/false, events);
     }
 
-    public static Result transientError(PathFragment buildFile) {
+    public static Result transientError(PathFragment buildFile, List<Event> events) {
       return new Result(ParserInputSource.create(EMPTY_CHARS, buildFile), /*preprocessed=*/false,
-          /*containsPersistentErrors=*/false, /*containsTransientErrors=*/true);
+          /*containsPersistentErrors=*/false, /*containsTransientErrors=*/true, events);
     }
   }
 
@@ -140,7 +146,6 @@ public interface Preprocessor {
    * @param in the BUILD file to be preprocessed.
    * @param packageName the BUILD file's package.
    * @param globber a globber for evaluating globs.
-   * @param eventHandler a eventHandler on which to report warnings/errors.
    * @param globalEnv the GLOBALS Python environment.
    * @param ruleNames the set of names of all rules in the build language.
    * @throws IOException if there was an I/O problem during preprocessing.
@@ -150,7 +155,6 @@ public interface Preprocessor {
       ParserInputSource in,
       String packageName,
       Globber globber,
-      EventHandler eventHandler,
       Environment globalEnv,
       Set<String> ruleNames)
     throws IOException, InterruptedException;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index 9e3b8d516b..5cb76854ff 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -797,12 +797,10 @@ public class PackageFunction implements SkyFunction {
       try {
         Globber globber = packageFactory.createLegacyGlobber(buildFilePath.getParentDirectory(),
             packageId, packageLocator);
-        StoredEventHandler localReporter = new StoredEventHandler();
         Preprocessor.Result preprocessingResult = preprocessCache.getIfPresent(packageId);
         if (preprocessingResult == null) {
           preprocessingResult = replacementSource == null
-              ? packageFactory.preprocess(packageId, buildFilePath, inputSource, globber,
-                  localReporter)
+              ? packageFactory.preprocess(packageId, buildFilePath, inputSource, globber)
               : Preprocessor.Result.noPreprocessing(replacementSource);
           preprocessCache.put(packageId, preprocessingResult);
         }
@@ -821,7 +819,7 @@ public class PackageFunction implements SkyFunction {
         preprocessCache.invalidate(packageId);
 
         pkgBuilder = packageFactory.createPackageFromPreprocessingResult(externalPkg, packageId,
-            buildFilePath, preprocessingResult, localReporter.getEvents(), preludeStatements,
+            buildFilePath, preprocessingResult, preprocessingResult.events, preludeStatements,
             importResult.importMap, importResult.fileDependencies, packageLocator,
             defaultVisibility, globber);
         numPackagesLoaded.incrementAndGet();