Fix output=locations display source file location

This PR is a patch to fix this [commit](f00f911) after being brought up [here](#12322 (comment)). This aims to provide a fix for #8900.

The goal of this PR is to align the output of `--output=location` with its current definition in the [documentation](https://docs.bazel.build/versions/master/query.html#output-location). With the flag `--output=location`, source files are supposed to output "the location of line 1 of the actual file" but only displayed its location in the BUILD file prior to the commit.

**Expected behavior by definition:**
```
bazel query bazeltest:main.py --output=location
/home/tanzhengwei/bazeltest/main.py:1:1: source file //bazeltest:main.py
```

The previous [commit](f00f911) wrongly modified the output to the following (with the `--incompatible_display_source_file_location` flag):
```
bazel query bazeltest:main.py --output=location --incompatible_display_source_file_location
/home/tanzhengwei/bazeltest/BUILD:1:10: source file /home/tanzhengwei/bazeltest/main.py:1:1
```

This PR fixes it to output to the correct implementation (with the `--incompatible_display_source_file_location` flag):
```
bazel query bazeltest:main.py --output=location --incompatible_display_source_file_location
/home/tanzhengwei/bazeltest/main.py:1:1: source file //bazeltest:main.py
```

Closes #12329.

PiperOrigin-RevId: 338620611

src/main/java/com/google/devtools/build/lib/query2/query/output/FormatUtils.java

@@ -59,6 +59,27 @@ public int compare(Target o1, Target o2) {
    */
   static String getLocation(Target target, boolean relative) {
     Location loc = target.getLocation();
+
+    if (relative) {
+      loc = getRootRelativeLocation(loc, target.getPackage());
+    }
+    return loc.toString();
+  }
+
+  /**
+   * Returns the target location string, optionally relative to its package's source root directory
+   * and optionally to display the location of source files.
+   *
+   * @param relative flag to display the location relative to its package's source root directory.
+   * @param displaySourceFileLocation flag to display the location of line 1 of the actual source
+   *     file instead of its location in the BUILD file.
+   */
+  static String getLocation(Target target, boolean relative, boolean displaySourceFileLocation) {
+    Location loc = target.getLocation();
+    if (target instanceof InputFile && displaySourceFileLocation) {
+      PathFragment packageDir = target.getPackage().getPackageDirectory().asFragment();
+      loc = Location.fromFileLineColumn(packageDir.getRelative(target.getName()).toString(), 1, 1);
+    }
     if (relative) {
       loc = getRootRelativeLocation(loc, target.getPackage());
     }
@@ -81,30 +102,4 @@ static Location getRootRelativeLocation(Location location, @Nullable Package bas
     }
     return location;
   }
-
-  /**
-   * Returns the target label string. For {@link InputFile} targets, displays the location of line 1
-   * of the actual source file by default.
-   *
-   * @param target the target to get the label from
-   * @param displaySourceFileLocation displays the location of line 1 of the actual source file
-   *     instead of its target if true.
-   * @param relativeLocations displays the location of the source file relative to its package's
-   *     source root directory
-   */
-  static String getLabel(
-      Target target, boolean displaySourceFileLocation, boolean relativeLocations) {
-    if (!(target instanceof InputFile) || !displaySourceFileLocation) {
-      return target.getLabel().getDefaultCanonicalForm();
-    }
-
-    // Default behaviour for source files without the incompatible_display_source_file_location flag
-    PathFragment packageDir = target.getPackage().getPackageDirectory().asFragment();
-    Location sourceFileLoc =
-        Location.fromFileLineColumn(packageDir.getRelative(target.getName()).toString(), 1, 1);
-    if (relativeLocations) {
-      sourceFileLoc = getRootRelativeLocation(sourceFileLoc, target.getPackage());
-    }
-    return sourceFileLoc.toString();
-  }
 }

src/main/java/com/google/devtools/build/lib/query2/query/output/LocationOutputFormatter.java

@@ -83,11 +83,11 @@ public void processOutput(Iterable<Target> partialResult) throws IOException {
         final String lineTerm = options.getLineTerminator();
         for (Target target : partialResult) {
           writer
-              .append(FormatUtils.getLocation(target, relativeLocations))
+              .append(FormatUtils.getLocation(target, relativeLocations, displaySourceFileLocation))
               .append(": ")
               .append(target.getTargetKind())
               .append(" ")
-              .append(FormatUtils.getLabel(target, displaySourceFileLocation, relativeLocations))
+              .append(target.getLabel().getDefaultCanonicalForm())
               .append(lineTerm);
         }
       }

src/test/shell/integration/bazel_query_test.sh

@@ -497,7 +497,7 @@ EOF
   expect_log "//foo:foo"
 }
 
-test_location_output_source_files() {
+function test_location_output_source_files() {
   rm -rf foo
   mkdir -p foo
   cat > foo/BUILD <<EOF
@@ -514,38 +514,41 @@ EOF
     --output=location \
     --incompatible_display_source_file_location \
     '//foo:main.py' >& $TEST_log || fail "Expected success"
-  expect_log "source file ${TEST_TMPDIR}/.*/foo/main.py:1:1"
-  expect_not_log "source file //foo:main.py"
+  expect_log "source file //foo:main.py"
+  expect_log "^${TEST_TMPDIR}/.*/foo/main.py:1:1"
+  expect_not_log "^${TEST_TMPDIR}/.*/foo/BUILD:[0-9]*:[0-9]*"
 
-  # Default behavior overridden by noincompatible_display_source_file_location
-  # flag to display the source file target instead
+  # The noincompatible_display_source_file_location flag displays its location
+  # in the BUILD file
   bazel query \
     --output=location \
     --noincompatible_display_source_file_location \
     '//foo:main.py' >& $TEST_log || fail "Expected success"
   expect_log "source file //foo:main.py"
-  expect_not_log "source file ${TEST_TMPDIR}/.*/foo/main.py:1:1"
+  expect_log "^${TEST_TMPDIR}/.*/foo/BUILD:[0-9]*:[0-9]*"
+  expect_not_log "^${TEST_TMPDIR}/.*/foo/main.py:1:1"
 
-  # Adding relative_locations flag should modify default behavior and
-  # make location of source file be relative
+  # The incompatible_display_source_file_location should still be affected by
+  # relative_locations flag to display the relative location of the source file
   bazel query \
     --output=location \
     --relative_locations \
     --incompatible_display_source_file_location \
     '//foo:main.py' >& $TEST_log || fail "Expected success"
-  expect_log "source file foo/main.py:1:1"
-  expect_not_log "source file ${TEST_TMPDIR}/.*/foo/main.py:1:1"
+  expect_log "source file //foo:main.py"
+  expect_log "^foo/main.py:1:1"
+  expect_not_log "^${TEST_TMPDIR}/.*/foo/main.py:1:1"
 
-  # Adding noincompatible_display_source_file_location flag should still
-  # override default behaviour regardless of relative_locations to
-  # display the source file target
+  # The noincompatible_display_source_file_location flag should still be
+  # affected by relative_locations flag to display the relative location of
+  # the BUILD file.
   bazel query --output=location \
     --relative_locations \
     --noincompatible_display_source_file_location \
     '//foo:main.py' >& $TEST_log || fail "Expected success"
   expect_log "source file //foo:main.py"
-  expect_not_log "source file foo/main.py:1:1"
-  expect_not_log "source file ${TEST_TMPDIR}/.*/foo/main.py:1:1"
+  expect_log "^foo/BUILD:[0-9]*:[0-9]*"
+  expect_not_log "^${TEST_TMPDIR}/.*/foo/BUILD:[0-9]*:[0-9]*"
 }
 
 function test_subdirectory_named_external() {