Ban the combination of buildfiles/loadfiles and --output=location.

RELNOTES[INC]: The --output=location flag to 'bazel query' cannot be used with query expressions that involve the 'buildfiles' or 'loadfiles' operators. This also applies to 'genquery' rules.

PiperOrigin-RevId: 159259061

6 files changed, 84 insertions, 4 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/query2/engine/BuildFilesFunction.java b/src/main/java/com/google/devtools/build/lib/query2/engine/BuildFilesFunction.java
index cbc0ae8d38..c64cb6c5fa 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/engine/BuildFilesFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/engine/BuildFilesFunction.java
@@ -31,7 +31,7 @@ import java.util.Set;
  *
  * <pre>expr ::= BUILDFILES '(' expr ')'</pre>
  */
-class BuildFilesFunction implements QueryFunction {
+public class BuildFilesFunction implements QueryFunction {
   BuildFilesFunction() {
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/query2/engine/LoadFilesFunction.java b/src/main/java/com/google/devtools/build/lib/query2/engine/LoadFilesFunction.java
index 80b912f6d7..dd25f15aa2 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/engine/LoadFilesFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/engine/LoadFilesFunction.java
@@ -28,7 +28,7 @@ import java.util.Set;
  *
  * <pre>expr ::= LOADFILES '(' expr ')'</pre>
  */
-class LoadFilesFunction implements QueryEnvironment.QueryFunction {
+public class LoadFilesFunction implements QueryEnvironment.QueryFunction {
   LoadFilesFunction() {}
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java
index 78231f9d8c..d504437c22 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java
@@ -33,8 +33,16 @@ import com.google.devtools.build.lib.packages.RawAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.TriState;
+import com.google.devtools.build.lib.query2.AbstractBlazeQueryEnvironment;
+import com.google.devtools.build.lib.query2.engine.BuildFilesFunction;
+import com.google.devtools.build.lib.query2.engine.FunctionExpression;
+import com.google.devtools.build.lib.query2.engine.LoadFilesFunction;
 import com.google.devtools.build.lib.query2.engine.OutputFormatterCallback;
 import com.google.devtools.build.lib.query2.engine.QueryEnvironment;
+import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction;
+import com.google.devtools.build.lib.query2.engine.QueryException;
+import com.google.devtools.build.lib.query2.engine.QueryExpression;
+import com.google.devtools.build.lib.query2.engine.QueryExpressionMapper;
 import com.google.devtools.build.lib.query2.engine.SynchronizedDelegatingOutputFormatterCallback;
 import com.google.devtools.build.lib.query2.engine.ThreadSafeOutputFormatterCallback;
 import com.google.devtools.build.lib.query2.output.QueryOptions.OrderOutput;
@@ -55,6 +63,7 @@ import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
 import javax.annotation.Nullable;
 
 /**
@@ -156,6 +165,10 @@ public abstract class OutputFormatter implements Serializable {
             : DependencyFilter.NO_IMPLICIT_DEPS);
   }
 
+  public void verifyCompatible(QueryEnvironment<?> env, QueryExpression expr)
+      throws QueryException {
+  }
+
   /**
    * Format the result (a set of target nodes implicitly ordered according to
    * the graph maintained by the QueryEnvironment), and print it to "out".
@@ -370,6 +383,32 @@ public abstract class OutputFormatter implements Serializable {
     }
 
     @Override
+    public void verifyCompatible(QueryEnvironment<?> env, QueryExpression expr)
+        throws QueryException {
+      if (!(env instanceof AbstractBlazeQueryEnvironment)) {
+        return;
+      }
+      final AtomicBoolean found = new AtomicBoolean(false);
+      QueryExpressionMapper noteBuildFilesAndLoadLilesMapper = new QueryExpressionMapper() {
+        @Override
+        public QueryExpression map(FunctionExpression functionExpression) {
+          QueryFunction queryFunction = functionExpression.getFunction();
+          if (queryFunction instanceof LoadFilesFunction
+              || queryFunction instanceof BuildFilesFunction) {
+            found.set(true);
+          }
+          return super.map(functionExpression);
+        }
+      };
+      expr.getMapped(noteBuildFilesAndLoadLilesMapper);
+      if (found.get()) {
+        throw new QueryException(
+            "Query expressions involving 'buildfiles' or 'loadfiles' cannot be used with "
+            + "--output=location");
+      }
+    }
+
+    @Override
     public OutputFormatterCallback<Target> createPostFactoStreamCallback(
         OutputStream out, final QueryOptions options) {
       return new TextOutputFormatterCallback<Target>(out) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
index d044aafed3..f5abf25019 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
@@ -55,6 +55,7 @@ import com.google.devtools.build.lib.query2.engine.DigraphQueryEvalResult;
 import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction;
 import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Setting;
 import com.google.devtools.build.lib.query2.engine.QueryException;
+import com.google.devtools.build.lib.query2.engine.QueryExpression;
 import com.google.devtools.build.lib.query2.engine.QueryUtil;
 import com.google.devtools.build.lib.query2.engine.QueryUtil.AggregateAllOutputFormatterCallback;
 import com.google.devtools.build.lib.query2.engine.SkyframeRestartQueryException;
@@ -300,7 +301,7 @@ public class GenQuery implements RuleConfiguredTargetFactory {
           ruleContext.getAnalysisEnvironment().getSkyframeEnv());
       // This is a precomputed value so it should have been injected by the rules module by the
       // time we get there.
-      formatter =  OutputFormatter.getFormatter(
+      formatter = OutputFormatter.getFormatter(
           Preconditions.checkNotNull(outputFormatters), queryOptions.outputFormat);
       // All the packages are already loaded at this point, so there is no need
       // to start up many threads. 4 are started up to make good use of multiple
@@ -324,7 +325,9 @@ public class GenQuery implements RuleConfiguredTargetFactory {
                   ImmutableList.<QueryFunction>of(),
                   /*packagePath=*/ null,
                   /*blockUniverseEvaluationErrors=*/ false);
-      queryResult = (DigraphQueryEvalResult<Target>) queryEnvironment.evaluateQuery(query, targets);
+      QueryExpression expr = QueryExpression.parse(query, queryEnvironment);
+      formatter.verifyCompatible(queryEnvironment, expr);
+      queryResult = queryEnvironment.evaluateQuery(expr, targets);
     } catch (SkyframeRestartQueryException e) {
       // Do not emit errors for skyframe restarts. They make output of the ConfiguredTargetFunction
       // inconsistent from run to run, and make detecting legitimate errors more difficult.
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java
index 47571ae9fd..8881db18ba 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java
@@ -147,6 +147,14 @@ public final class QueryCommand implements BlazeCommand {
           .handle(Event.error(null, "Error while parsing '" + query + "': " + e.getMessage()));
       return ExitCode.COMMAND_LINE_ERROR;
     }
+
+    try {
+      formatter.verifyCompatible(queryEnv, expr);
+    } catch (QueryException e) {
+      env.getReporter().handle(Event.error(e.getMessage()));
+      return ExitCode.COMMAND_LINE_ERROR;
+    }
+
     expr = queryEnv.transformParsedQuery(expr);
 
     OutputStream out = env.getReporter().getOutErr().getOutputStream();
diff --git a/src/test/shell/integration/bazel_query_test.sh b/src/test/shell/integration/bazel_query_test.sh
index b009adb147..0342370dfd 100755
--- a/src/test/shell/integration/bazel_query_test.sh
+++ b/src/test/shell/integration/bazel_query_test.sh
@@ -257,4 +257,34 @@ EOF
   expect_log "//peach:harken"
 }
 
+function test_location_output_not_allowed_with_buildfiles_or_loadfiles() {
+  mkdir foo
+  cat > foo/bzl.bzl <<EOF
+x = 2
+EOF
+  cat > foo/BUILD <<EOF
+load('//foo:bzl.bzl', 'x')
+sh_library(name='foo')
+EOF
+
+  bazel query 'buildfiles(//foo)' >& $TEST_log || fail "Expected success"
+  expect_log "//foo:bzl.bzl"
+  bazel query 'loadfiles(//foo)' >& $TEST_log || fail "Expected success"
+  expect_log "//foo:bzl.bzl"
+  bazel query --output=location '//foo' >& $TEST_log || fail "Expected success"
+  expect_log "//foo:foo"
+
+  local expected_error_msg="Query expressions involving 'buildfiles' or 'loadfiles' cannot be used with --output=location"
+  local expected_exit_code=2
+  for query_string in 'buildfiles(//foo)' 'loadfiles(//foo)'
+  do
+    bazel query --output=location "$query_string" >& $TEST_log \
+        && fail "Expected failure"
+    exit_code=$?
+    expect_log "$expected_error_msg"
+    assert_equals "$expected_exit_code" "$exit_code"
+  done
+}
+
+
 run_suite "${PRODUCT_NAME} query tests"