Display parsing errors for WORKSPACE files

Previously, for some types of errors, you'd just see "WORKSPACE file
could not be parsed" if there was a syntax error (instead of what the
syntax error was).

Change-Id: Ia278cf23b79f8daba02917cdfb4cc515a87080b1

--
Change-Id: Ia278cf23b79f8daba02917cdfb4cc515a87080b1
Reviewed-on: https://github.com/bazelbuild/bazel/pull/774
MOS_MIGRATED_REVID=112190322

3 files changed, 89 insertions, 1 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
index 15c2e82332..23c92dcd5d 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -133,7 +133,7 @@ public class WorkspaceFactory {
     buildFileAST = BuildFileAST.parseBuildFile(source, localReporter, false);
     if (buildFileAST.containsErrors()) {
       environmentBuilder = null;
-      throw new IOException("WORKSPACE file could not be parsed");
+      throw new IOException("Failed to parse " + source.getPath());
     }
     environmentBuilder = Environment.builder(mutability)
         .setGlobals(Environment.BUILD)
@@ -305,4 +305,11 @@ public class WorkspaceFactory {
   public static ClassObject newNativeModule(RuleClassProvider ruleClassProvider) {
     return newNativeModule(createWorkspaceFunctions(ruleClassProvider));
   }
+
+  /**
+   * @return a list of events that have occurred while parsing the WORKSPACE file.
+   */
+  public ImmutableList<Event> getEvents() {
+    return localReporter.getEvents();
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
index 08514baad9..65060a29e1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.skyframe;
 
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.Package.LegacyBuilder;
 import com.google.devtools.build.lib.packages.PackageFactory;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
@@ -94,6 +95,9 @@ public class WorkspaceFileFunction implements SkyFunction {
       } catch (PackageFunctionException e) {
         throw new WorkspaceFileFunctionException(e, Transience.PERSISTENT);
       } catch (IOException e) {
+        for (Event event : parser.getEvents()) {
+          env.getListener().handle(event);
+        }
         throw new WorkspaceFileFunctionException(e, Transience.TRANSIENT);
       }
     }
diff --git a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
new file mode 100644
index 0000000000..5cdd3cb22e
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
@@ -0,0 +1,77 @@
+// Copyright 2015 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.packages;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.packages.Package.LegacyBuilder;
+import com.google.devtools.build.lib.syntax.Mutability;
+import com.google.devtools.build.lib.syntax.ParserInputSource;
+import com.google.devtools.build.lib.testutil.Scratch;
+import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
+import com.google.devtools.build.lib.vfs.Path;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+import java.io.IOException;
+import java.util.List;
+
+/**
+ * Tests for WorkspaceFactory.
+ */
+@RunWith(JUnit4.class)
+public class WorkspaceFactoryTest {
+
+  private Scratch scratch;
+  private Path root;
+
+  @Before
+  public void setUpFileSystem() throws Exception {
+    scratch = new Scratch("/");
+    root = scratch.dir("/workspace");
+  }
+
+  @Test
+  public void testLoadError() throws Exception {
+    // WS with a syntax error: '//a' should end with .bzl.
+    Path workspaceFilePath = scratch.file("/workspace/WORKSPACE", "load('//a', 'a')");
+    LegacyBuilder builder = Package.newExternalPackageBuilder(workspaceFilePath, "");
+    WorkspaceFactory factory =
+        new WorkspaceFactory(
+            builder,
+            TestRuleClassProvider.getRuleClassProvider(),
+            ImmutableList.<PackageFactory.EnvironmentExtension>of(),
+            Mutability.create("test"),
+            root,
+            root);
+    try {
+      factory.parse(ParserInputSource.create(workspaceFilePath));
+      fail("Parsing " + workspaceFilePath + " should have failed");
+    } catch (IOException e) {
+      assertThat(e.getMessage()).contains("Failed to parse " + workspaceFilePath);
+    }
+    List<Event> events = factory.getEvents();
+    assertEquals(1, events.size());
+    assertThat(events.get(0).getMessage())
+        .contains("The label must reference a file with extension '.bzl'");
+  }
+}