Fix a hypothetical issue with 'buildfiles' and 'loadfiles' with duplicate targets.

These two functions don't use a Uniquifier internally so if the same bzl file foo.bzl is loaded multiple ways in 'e' in 'loadfiles(e)' then 'f' in 'f(loadfiles(e))' will observe foo.bzl multiple times.

This isn't observable in practice (i.e. I cannot write a black-box query test that would fail without this CL) because:
(1) BlazeQueryEnvironment#eval(QueryExpression, VariableContext<Target>, Callback<Target>) uses an intermediate aggregating callback and thus doesn't use the streaming query evaluation model. This means that the internal deduping in BlazeQueryEnvironment#getBuildFiles is sufficient.
(2) SkyQueryEnvironment uses an outer BatchStreamedCallback which internally uses a Uniquifier.

Still, this CL is useful for SkyQuery because without we're doing wasted work (e.g. in my example above, 'f' will wastefully do duplicate work with the .bzl files from the inner loadfiles(e)) because the deduping happens on the final query result, not on the intermediate result from 'buildfiles'/'loadfiles'.

For 'buildfiles', there's an additional wart with (1) above because FakeSubincludeTarget doesn't override Object#equals/hashCode.

--
PiperOrigin-RevId: 147656183
MOS_MIGRATED_REVID=147656183

2 files changed, 6 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 0e65cbdf05..d2a2eb0fa8 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
@@ -49,6 +49,7 @@ class BuildFilesFunction implements QueryFunction {
       List<Argument> args,
       final Callback<T> callback)
       throws QueryException, InterruptedException {
+    final Uniquifier<T> uniquifier = env.createUniquifier();
     env.eval(
         args.get(0).getExpression(),
         context,
@@ -58,9 +59,9 @@ class BuildFilesFunction implements QueryFunction {
               throws QueryException, InterruptedException {
             Set<T> result = CompactHashSet.create();
             Iterables.addAll(result, partialResult);
-            callback.process(
+            callback.process(uniquifier.unique(
                 env.getBuildFiles(
-                    expression, result, /* BUILD */ true, /* subinclude */ true, /* load */ true));
+                    expression, result, /* BUILD */ true, /* subinclude */ true, /* load */ true)));
           }
         });
   }
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 6b42ba4f37..311a6afff5 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
@@ -45,6 +45,7 @@ class LoadFilesFunction implements QueryEnvironment.QueryFunction {
       List<QueryEnvironment.Argument> args,
       final Callback<T> callback)
       throws QueryException, InterruptedException {
+    final Uniquifier<T> uniquifier = env.createUniquifier();
     env.eval(
         args.get(0).getExpression(),
         context,
@@ -54,13 +55,13 @@ class LoadFilesFunction implements QueryEnvironment.QueryFunction {
               throws QueryException, InterruptedException {
             Set<T> result = CompactHashSet.create();
             Iterables.addAll(result, partialResult);
-            callback.process(
+            callback.process(uniquifier.unique(
                 env.getBuildFiles(
                     expression,
                     result,
                     /* BUILD */ false,
                     /* subinclude */ false,
-                    /* load */ true));
+                    /* load */ true)));
           }
         });
   }