commit 59c16f6a2730a45d47546b6c69c3f4f5ba152db9
author nharmata <nharmata@google.com> Mon Aug 07 19:49:09 2017 +0200
committer Marcel Hlopko <hlopko@google.com> Tue Aug 08 13:02:21 2017 +0200
tree d32ddba8124ed2b4100c73e6801160c483f35c5c
parent 2f6284268d4f05bb47bdffa106d3d110d5160393

Clean up Label validation, and introduce a factory method for constructing a Label without validation.

RELNOTES: None
PiperOrigin-RevId: 164479651

src/main/java/com/google/devtools/build/lib/packages/Package.java

diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index ec58068..9e4ab19 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -444,7 +444,7 @@
     return events;
   }
 
-  /** Returns an (immutable, unordered) view of all the targets belonging to this package. */
+  /** Returns an (immutable, ordered) view of all the targets belonging to this package. */
   public ImmutableSortedKeyMap<String, Target> getTargets() {
     return targets;
   }
@@ -457,7 +457,7 @@
   }
 
   /**
-   * Returns a (read-only, unordered) iterator of all the targets belonging
+   * Returns a (read-only, ordered) iterable of all the targets belonging
    * to this package which are instances of the specified class.
    */
   public <T extends Target> Iterable<T> getTargets(Class<T> targetClass) {
@@ -1312,12 +1312,14 @@
       // current instance here.
       buildFile = (InputFile) Preconditions.checkNotNull(targets.get(buildFileLabel.getName()));
 
-      List<Rule> rules = Lists.newArrayList(getTargets(Rule.class));
+      // The Iterable returned by getTargets is sorted, so when we build up the list of tests by
+      // processing it in order below, that list will be sorted too.
+      Iterable<Rule> sortedRules = Lists.newArrayList(getTargets(Rule.class));
 
       if (discoverAssumedInputFiles) {
         // All labels mentioned in a rule that refer to an unknown target in the
         // current package are assumed to be InputFiles, so let's create them:
-        for (final Rule rule : rules) {
+        for (final Rule rule : sortedRules) {
           AggregatingAttributeMapper.of(rule).visitLabels(new AcceptsLabelAttribute() {
             @Override
             public void acceptLabelAttribute(Label label, Attribute attribute) {
@@ -1332,18 +1334,17 @@
       // Note, we implement this here when the Package is fully constructed,
       // since clearly this information isn't available at Rule construction
       // time, as forward references are permitted.
-      List<Label> allTests = new ArrayList<>();
-      for (Rule rule : rules) {
+      List<Label> sortedTests = new ArrayList<>();
+      for (Rule rule : sortedRules) {
         if (TargetUtils.isTestRule(rule) && !TargetUtils.hasManualTag(rule)) {
-          allTests.add(rule.getLabel());
+          sortedTests.add(rule.getLabel());
         }
       }
-      Collections.sort(allTests);
-      for (Rule rule : rules) {
+      for (Rule rule : sortedRules) {
         AttributeMap attributes = NonconfigurableAttributeMapper.of(rule);
         if (rule.getRuleClass().equals("test_suite")
             && attributes.get("tests", BuildType.LABEL_LIST).isEmpty()) {
-          rule.setAttributeValueByName("$implicit_tests", allTests);
+          rule.setAttributeValueByName("$implicit_tests", sortedTests);
         }
       }
       return this;