Remove Preprocessor.Result

RELNOTES: None.
PiperOrigin-RevId: 152965874

3 files changed, 44 insertions, 93 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 cd832a91bb..e237d9af8e 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
@@ -1260,43 +1260,43 @@ public final class PackageFactory {
     };
   }
 
-  /****************************************************************************
+  /**
    * Package creation.
    */
 
   /**
-   * Loads, scans parses and evaluates the build file at "buildFile", and
-   * creates and returns a Package builder instance capable of building a package identified by
-   * "packageId".
+   * Loads, scans parses and evaluates the build file at "buildFile", and creates and returns a
+   * Package builder instance capable of building a package identified by "packageId".
    *
    * <p>This method returns a builder to allow the caller to do additional work, if necessary.
    *
-   * <p>This method assumes "packageId" is a valid package name according to the
-   * {@link LabelValidator#validatePackageName} heuristic.
+   * <p>This method assumes "packageId" is a valid package name according to the {@link
+   * LabelValidator#validatePackageName} heuristic.
    *
    * <p>See {@link #evaluateBuildFile} for information on AST retention.
    *
-   * <p>Executes {@code globber.onCompletion()} on completion and executes
-   * {@code globber.onInterrupt()} on an {@link InterruptedException}.
+   * <p>Executes {@code globber.onCompletion()} on completion and executes {@code
+   * globber.onInterrupt()} on an {@link InterruptedException}.
    */
   // Used outside of bazel!
-  public Package.Builder createPackageFromPreprocessingResult(
+  public Package.Builder createPackage(
       String workspaceName,
       PackageIdentifier packageId,
       Path buildFile,
-      Preprocessor.Result preprocessingResult,
+      ParserInputSource input,
       List<Statement> preludeStatements,
       Map<String, Extension> imports,
       ImmutableList<Label> skylarkFileDependencies,
       RuleVisibility defaultVisibility,
-      Globber globber) throws InterruptedException {
+      Globber globber)
+      throws InterruptedException {
     StoredEventHandler localReporterForParsing = new StoredEventHandler();
     // Run the lexer and parser with a local reporter, so that errors from other threads do not
     // show up below.
-    BuildFileAST buildFileAST = parseBuildFile(packageId, preprocessingResult.result,
-        preludeStatements, localReporterForParsing);
-    AstAfterPreprocessing astAfterPreprocessing = new AstAfterPreprocessing(preprocessingResult,
-        buildFileAST, localReporterForParsing);
+    BuildFileAST buildFileAST =
+        parseBuildFile(packageId, input, preludeStatements, localReporterForParsing);
+    AstAfterPreprocessing astAfterPreprocessing =
+        new AstAfterPreprocessing(buildFileAST, localReporterForParsing);
     return createPackageFromPreprocessingAst(
         workspaceName,
         packageId,
@@ -1407,19 +1407,20 @@ public final class PackageFactory {
     }
 
     Globber globber = createLegacyGlobber(buildFile.getParentDirectory(), packageId, locator);
-    Preprocessor.Result preprocessingResult =
-        Preprocessor.Result.noPreprocessing(buildFile.asFragment(), buildFileBytes);
+    ParserInputSource input =
+        ParserInputSource.create(
+            FileSystemUtils.convertFromLatin1(buildFileBytes), buildFile.asFragment());
 
     Package result =
-        createPackageFromPreprocessingResult(
+        createPackage(
                 externalPkg.getWorkspaceName(),
                 packageId,
                 buildFile,
-                preprocessingResult,
-                /*preludeStatements=*/ImmutableList.<Statement>of(),
-                /*imports=*/ImmutableMap.<String, Extension>of(),
-                /*skylarkFileDependencies=*/ImmutableList.<Label>of(),
-                /*defaultVisibility=*/ConstantRuleVisibility.PUBLIC,
+                input,
+                /*preludeStatements=*/ ImmutableList.<Statement>of(),
+                /*imports=*/ ImmutableMap.<String, Extension>of(),
+                /*skylarkFileDependencies=*/ ImmutableList.<Label>of(),
+                /*defaultVisibility=*/ ConstantRuleVisibility.PUBLIC,
                 globber)
             .build();
     Event.replayEventsOn(eventHandler, result.getEvents());
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 e6469f21b1..c415f79919 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
@@ -16,69 +16,16 @@ package com.google.devtools.build.lib.packages;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.syntax.BuildFileAST;
-import com.google.devtools.build.lib.syntax.ParserInputSource;
-import com.google.devtools.build.lib.vfs.FileSystemUtils;
-import com.google.devtools.build.lib.vfs.Path;
-import com.google.devtools.build.lib.vfs.PathFragment;
-import java.io.IOException;
-import java.util.Set;
 
 /** A Preprocessor is an interface to implement generic text-based preprocessing of BUILD files. */
 public interface Preprocessor {
-  /**
-   * A (result, success) tuple indicating the outcome of preprocessing.
-   */
-  static class Result {
-    public final ParserInputSource result;
-
-    private Result(ParserInputSource result) {
-      this.result = result;
-    }
-
-    public static Result noPreprocessing(PathFragment buildFilePathFragment,
-        byte[] buildFileBytes) {
-      return noPreprocessing(ParserInputSource.create(
-          FileSystemUtils.convertFromLatin1(buildFileBytes), buildFilePathFragment));
-    }
-
-    /** Convenience factory for a {@link Result} wrapping non-preprocessed BUILD file contents. */
-    public static Result noPreprocessing(ParserInputSource buildFileSource) {
-      return new Result(buildFileSource);
-    }
-  }
-
-  /**
-   * Returns a Result resulting from applying Python preprocessing to the contents of "in". If
-   * errors happen, they must be reported both as an event on eventHandler and in the function
-   * return value. An IOException is only thrown when preparing for preprocessing. Once
-   * preprocessing actually begins, any I/O problems encountered will be reflected in the return
-   * value, not manifested as exceptions.
-   *
-   * @param buildFilePath the BUILD file to be preprocessed.
-   * @param buildFileBytes the raw contents of the BUILD file to be preprocessed.
-   * @param packageName the BUILD file's package.
-   * @param globber a globber for evaluating globs.
-   * @param globals the global bindings for the Python environment.
-   * @param ruleNames the set of names of all rules in the build language.
-   * @throws IOException if there was an I/O problem preparing for preprocessing.
-   * @return a pair of the ParserInputSource and a map of subincludes seen during the evaluation
-   */
-  Result preprocess(
-      Path buildFilePath,
-      byte[] buildFileBytes,
-      String packageName,
-      Globber globber,
-      Set<String> ruleNames)
-    throws IOException, InterruptedException;
-
   /** The result of parsing a preprocessed BUILD file. */
   static class AstAfterPreprocessing {
     public final BuildFileAST ast;
     public final boolean containsAstParsingErrors;
     public final Iterable<Event> allEvents;
 
-    public AstAfterPreprocessing(Result preprocessingResult, BuildFileAST ast,
-        StoredEventHandler astParsingEventHandler) {
+    public AstAfterPreprocessing(BuildFileAST ast, StoredEventHandler astParsingEventHandler) {
       this.ast = ast;
       this.containsAstParsingErrors = astParsingEventHandler.hasErrors();
       this.allEvents = astParsingEventHandler.getEvents();
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 115c343f22..f848a90325 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
@@ -38,7 +38,6 @@ import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.PackageFactory;
 import com.google.devtools.build.lib.packages.PackageFactory.LegacyGlobber;
-import com.google.devtools.build.lib.packages.Preprocessor;
 import com.google.devtools.build.lib.packages.Preprocessor.AstAfterPreprocessing;
 import com.google.devtools.build.lib.packages.RuleVisibility;
 import com.google.devtools.build.lib.packages.Target;
@@ -1137,38 +1136,42 @@ public class PackageFunction implements SkyFunction {
               buildFilePath.getParentDirectory(), packageId, packageLocator);
           SkyframeHybridGlobber skyframeGlobber = new SkyframeHybridGlobber(packageId, packageRoot,
               env, legacyGlobber);
-          Preprocessor.Result preprocessingResult;
+          ParserInputSource input;
           if (replacementContents == null) {
             Preconditions.checkNotNull(buildFileValue, packageId);
-            byte[] buildFileBytes;
             try {
-              buildFileBytes = buildFileValue.isSpecialFile()
-                  ? FileSystemUtils.readContent(buildFilePath)
-                  : FileSystemUtils.readWithKnownFileSize(buildFilePath, buildFileValue.getSize());
+              byte[] buildFileBytes =
+                  buildFileValue.isSpecialFile()
+                      ? FileSystemUtils.readContent(buildFilePath)
+                      : FileSystemUtils.readWithKnownFileSize(
+                          buildFilePath, buildFileValue.getSize());
+              input =
+                  ParserInputSource.create(
+                      FileSystemUtils.convertFromLatin1(buildFileBytes),
+                      buildFilePath.asFragment());
             } catch (IOException e) {
               // Note that we did this work, so we should conservatively report this error as
               // transient.
               throw new PackageFunctionException(new BuildFileContainsErrorsException(
                   packageId, e.getMessage()), Transience.TRANSIENT);
             }
-            preprocessingResult =
-                Preprocessor.Result.noPreprocessing(buildFilePath.asFragment(), buildFileBytes);
           } else {
-            ParserInputSource replacementSource =
-                ParserInputSource.create(replacementContents, buildFilePath.asFragment());
-            preprocessingResult = Preprocessor.Result.noPreprocessing(replacementSource);
+            input = ParserInputSource.create(replacementContents, buildFilePath.asFragment());
           }
           StoredEventHandler astParsingEventHandler = new StoredEventHandler();
-          BuildFileAST ast = PackageFactory.parseBuildFile(packageId, preprocessingResult.result,
-              preludeStatements, astParsingEventHandler);
+          BuildFileAST ast =
+              PackageFactory.parseBuildFile(
+                  packageId, input, preludeStatements, astParsingEventHandler);
           // If no globs were fetched during preprocessing, then there's no need to reuse the
           // legacy globber instance during BUILD file evaluation since the performance argument
           // below does not apply.
           Set<SkyKey> globDepsRequested = skyframeGlobber.getGlobDepsRequested();
           LegacyGlobber legacyGlobberToStore = globDepsRequested.isEmpty() ? null : legacyGlobber;
-          astCacheEntry = new CacheEntryWithGlobDeps<>(
-              new AstAfterPreprocessing(preprocessingResult, ast, astParsingEventHandler),
-              globDepsRequested, legacyGlobberToStore);
+          astCacheEntry =
+              new CacheEntryWithGlobDeps<>(
+                  new AstAfterPreprocessing(ast, astParsingEventHandler),
+                  globDepsRequested,
+                  legacyGlobberToStore);
           astCache.put(packageId, astCacheEntry);
         }
         AstAfterPreprocessing astAfterPreprocessing = astCacheEntry.value;