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