Fix private symbols, clean up load parsing

--
MOS_MIGRATED_REVID=128699330
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java
index 2bc348a..b1e6c15 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java
@@ -72,15 +72,16 @@
   void doExec(Environment env) throws EvalException, InterruptedException {
     for (Map.Entry<Identifier, String> entry : symbols.entrySet()) {
       try {
-        Identifier current = entry.getKey();
+        Identifier name = entry.getKey();
+        Identifier declared = new Identifier(entry.getValue());
 
-        if (current.isPrivate()) {
-          throw new EvalException(
-              getLocation(), "symbol '" + current + "' is private and cannot be imported.");
+        if (declared.isPrivate()) {
+          throw new EvalException(getLocation(),
+              "symbol '" + declared.getName() + "' is private and cannot be imported.");
         }
         // The key is the original name that was used to define the symbol
         // in the loaded bzl file.
-        env.importSymbol(imp.getImportString(), current, entry.getValue());
+        env.importSymbol(imp.getImportString(), name, declared.getName());
       } catch (Environment.NoSuchVariableException | Environment.LoadFailedException e) {
         throw new EvalException(getLocation(), e.getMessage());
       }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index a5ca8bd..dbc16cd 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -1089,9 +1089,9 @@
    * Parses the next symbol argument of a load statement and puts it into the output map.
    *
    * <p> The symbol is either "name" (STRING) or name = "declared" (IDENTIFIER EQUALS STRING).
-   * "Declared" refers to the original name in the bazel file that should be loaded.
-   * Moreover, it will be the key of the entry in the map.
-   * If no alias is used, "name" and "declared" will be identical.
+   * If no alias is used, "name" and "declared" will be identical. "Declared" refers to the
+   * original name in the Bazel file that should be loaded, while "name" will be the key of the
+   * entry in the map.
    */
   private void parseLoadSymbol(Map<Identifier, String> symbols) {
     Token nameToken, declaredToken;
@@ -1119,10 +1119,12 @@
 
       if (symbols.containsKey(identifier)) {
         syntaxError(
-            nameToken, String.format("Symbol '%s' has already been loaded", identifier.getName()));
+            nameToken, String.format("Identifier '%s' is used more than once",
+                identifier.getName()));
       } else {
         symbols.put(
-            setLocation(identifier, nameToken.left, token.left), declaredToken.value.toString());
+            setLocation(identifier, nameToken.left, nameToken.right),
+            declaredToken.value.toString());
       }
     } catch (NullPointerException npe) {
       // This means that the value of at least one token is null. In this case, the previous
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
index 235eb39..9ae4e22 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
@@ -1129,7 +1129,7 @@
   private void invalidImportTest(String importString, String expectedMsg) {
     setFailFast(false);
     parseFileForSkylark("load('" + importString + "', 'fun_test')\n"); 
-    assertContainsError(expectedMsg);    
+    assertContainsError(expectedMsg);
   }
 
   @Test
@@ -1244,6 +1244,11 @@
     LoadStatement stmt = (LoadStatement) statements.get(0);
     assertEquals("/foo/bar/file", stmt.getImport().getImportString());
     assertThat(stmt.getSymbols()).hasSize(1);
+    Identifier sym = stmt.getSymbols().get(0);
+    int startOffset = sym.getLocation().getStartOffset();
+    int endOffset = sym.getLocation().getEndOffset();
+    assertThat(startOffset).named("getStartOffset()").isEqualTo(22);
+    assertThat(endOffset).named("getEndOffset()").isEqualTo(startOffset + 10);
   }
 
   @Test
@@ -1287,7 +1292,18 @@
 
   @Test
   public void testLoadAlias() throws Exception {
-    runLoadAliasTestForSymbols("my_alias = 'lawl'", "my_alias");
+    List<Statement> statements = parseFileForSkylark(
+        "load('/foo/bar/file', my_alias = 'lawl')\n");
+    LoadStatement stmt = (LoadStatement) statements.get(0);
+    ImmutableList<Identifier> actualSymbols = stmt.getSymbols();
+
+    assertThat(actualSymbols).hasSize(1);
+    Identifier sym = actualSymbols.get(0);
+    assertThat(sym.getName()).isEqualTo("my_alias");
+    int startOffset = sym.getLocation().getStartOffset();
+    int endOffset = sym.getLocation().getEndOffset();
+    assertThat(startOffset).named("getStartOffset()").isEqualTo(22);
+    assertThat(endOffset).named("getEndOffset()").isEqualTo(startOffset + 8);
   }
 
   @Test