diff options
5 files changed, 138 insertions, 41 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java index 6c1834e443..cea5ea56bd 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java @@ -295,16 +295,18 @@ public class Environment { this.importedExtensions = importedExtensions; } - public void importSymbol(PathFragment extension, String symbol) + public void importSymbol(PathFragment extension, Identifier symbol, String nameInLoadedFile) throws NoSuchVariableException, LoadFailedException { if (!importedExtensions.containsKey(extension)) { throw new LoadFailedException(extension.toString()); } - Object value = importedExtensions.get(extension).lookup(symbol); + + Object value = importedExtensions.get(extension).lookup(nameInLoadedFile); if (!isSkylarkEnabled()) { value = SkylarkType.convertFromSkylark(value); } - update(symbol, value); + + update(symbol.getName(), value); } /** diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java index 26c5f1557f..ce68d66d60 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java @@ -33,7 +33,7 @@ public final class Identifier extends Expression { public Identifier(String name) { this.name = name; } - + /** * Returns the name of the Identifier. */ @@ -51,15 +51,6 @@ public final class Identifier extends Expression { } @Override - Object eval(Environment env) throws EvalException { - try { - return env.lookup(name); - } catch (Environment.NoSuchVariableException e) { - throw createInvalidIdentifierException(); - } - } - - @Override public boolean equals(@Nullable Object object) { if (object instanceof Identifier) { Identifier that = (Identifier) object; @@ -72,6 +63,15 @@ public final class Identifier extends Expression { public int hashCode() { return name.hashCode(); } + + @Override + Object eval(Environment env) throws EvalException { + try { + return env.lookup(name); + } catch (Environment.NoSuchVariableException e) { + throw createInvalidIdentifierException(); + } + } @Override public void accept(SyntaxTreeVisitor visitor) { 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 0eae19bbc8..8b9547a2c2 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 @@ -15,9 +15,10 @@ package com.google.devtools.build.lib.syntax; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.vfs.PathFragment; -import java.util.List; +import java.util.Map; /** * Syntax node for an import statement. @@ -26,21 +27,28 @@ public final class LoadStatement extends Statement { public static final String PATH_ERROR_MSG = "Path '%s' is not valid. " + "It should either start with a slash or refer to a file in the current directory."; - private final ImmutableList<Identifier> symbols; + private final ImmutableMap<Identifier, String> symbols; + private final ImmutableList<Identifier> cachedSymbols; // to save time private final PathFragment importPath; private final String pathString; /** * Constructs an import statement. + * + * <p>Symbols maps a symbol to its original name under which it was defined in + * the bzl file that should be loaded. + * If aliasing is used, the value differs from it's key's symbol#getName(). + * Otherwise, both values are identical. */ - LoadStatement(String path, List<Identifier> symbols) { - this.symbols = ImmutableList.copyOf(symbols); + LoadStatement(String path, Map<Identifier, String> symbols) { + this.symbols = ImmutableMap.copyOf(symbols); + this.cachedSymbols = ImmutableList.copyOf(symbols.keySet()); this.importPath = new PathFragment(path + ".bzl"); this.pathString = path; } public ImmutableList<Identifier> getSymbols() { - return symbols; + return cachedSymbols; } public PathFragment getImportPath() { @@ -49,18 +57,22 @@ public final class LoadStatement extends Statement { @Override public String toString() { - return String.format("load(\"%s\", %s)", importPath, Joiner.on(", ").join(symbols)); + return String.format("load(\"%s\", %s)", importPath, Joiner.on(", ").join(cachedSymbols)); } @Override void exec(Environment env) throws EvalException, InterruptedException { - for (Identifier i : symbols) { + for (Map.Entry<Identifier, String> entry : symbols.entrySet()) { try { - if (i.isPrivate()) { - throw new EvalException(getLocation(), "symbol '" + i + "' is private and cannot " - + "be imported"); + Identifier current = entry.getKey(); + + if (current.isPrivate()) { + throw new EvalException( + getLocation(), "symbol '" + current + "' is private and cannot be imported"); } - env.importSymbol(getImportPath(), i.getName()); + // The key is the original name that was used to define the symbol + // in the loaded bzl file + env.importSymbol(getImportPath(), current, entry.getValue()); } catch (Environment.NoSuchVariableException | Environment.LoadFailedException e) { throw new EvalException(getLocation(), e.getMessage()); } @@ -74,23 +86,21 @@ public final class LoadStatement extends Statement { @Override void validate(ValidationEnvironment env) throws EvalException { - validateLoadPath(); + validatePath(); if (!importPath.isAbsolute() && importPath.segmentCount() > 1) { throw new EvalException(getLocation(), String.format(PATH_ERROR_MSG, importPath)); } - for (Identifier symbol : symbols) { + for (Identifier symbol : cachedSymbols) { env.declare(symbol.getName(), getLocation()); } } /** - * Throws an exception if the path argument to load() does starts with more than one forward + * Throws an exception if the path argument to load() starts with more than one forward * slash ('/') - * - * @throws EvalException if the path is empty or starts with two forward slashes */ - public void validateLoadPath() throws EvalException { + public void validatePath() throws EvalException { String error = null; if (pathString.isEmpty()) { 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 46e7ca74ce..21eb2a5a80 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 @@ -36,6 +36,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.EnumSet; +import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -1069,7 +1070,7 @@ class Parser { return list; } - // load '(' STRING (COMMA STRING)* COMMA? ')' + // load '(' STRING (COMMA [IDENTIFIER EQUALS] STRING)* COMMA? ')' private void parseLoad(List<Statement> list) { int start = token.left; if (token.kind != TokenKind.STRING) { @@ -1083,20 +1084,16 @@ class Parser { nextToken(); expect(TokenKind.COMMA); - List<Identifier> symbols = new ArrayList<>(); - if (token.kind == TokenKind.STRING) { - symbols.add(new Identifier((String) token.value)); - } - expect(TokenKind.STRING); + Map<Identifier, String> symbols = new HashMap<>(); + parseLoadSymbol(symbols); // At least one symbol is required + while (token.kind != TokenKind.RPAREN && token.kind != TokenKind.EOF) { expect(TokenKind.COMMA); if (token.kind == TokenKind.RPAREN) { break; } - if (token.kind == TokenKind.STRING) { - symbols.add(new Identifier((String) token.value)); - } - expect(TokenKind.STRING); + + parseLoadSymbol(symbols); } expect(TokenKind.RPAREN); @@ -1106,7 +1103,7 @@ class Parser { // this only happens in Skylark. Consequently, we invoke it here to discover // invalid load paths in BUILD mode, too. try { - stmt.validateLoadPath(); + stmt.validatePath(); } catch (EvalException e) { syntaxError(pathToken, e.getMessage()); } @@ -1114,6 +1111,51 @@ class Parser { list.add(setLocation(stmt, start, token.left)); } + /** + * 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. + */ + private void parseLoadSymbol(Map<Identifier, String> symbols) { + Token nameToken, declaredToken; + + if (token.kind == TokenKind.STRING) { + nameToken = token; + declaredToken = nameToken; + } else { + if (token.kind != TokenKind.IDENTIFIER) { + syntaxError(token, "Expected either a literal string or an identifier"); + } + + nameToken = token; + + expect(TokenKind.IDENTIFIER); + expect(TokenKind.EQUALS); + + declaredToken = token; + } + + expect(TokenKind.STRING); + + try { + Identifier identifier = new Identifier(nameToken.value.toString()); + + if (symbols.containsKey(identifier)) { + syntaxError( + nameToken, String.format("Symbol '%s' has already been loaded", identifier.getName())); + } else { + symbols.put( + setLocation(identifier, nameToken.left, token.left), declaredToken.value.toString()); + } + } catch (NullPointerException npe) { + // This means that the value of at least one token is null. In this case, the previous + // expect() call has already logged an error. + } + } + private void parseTopLevelStatement(List<Statement> list) { // In Python grammar, there is no "top-level statement" and imports are // considered as "small statements". We are a bit stricter than Python here. 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 dad2d46e89..93e14a8c56 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 @@ -29,8 +29,10 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import java.util.LinkedList; import java.util.List; + /** * Tests of parser behaviour. */ @@ -1069,6 +1071,47 @@ public class ParserTest extends EvaluationTestCase { } @Test + public void testLoadAlias() throws Exception { + runLoadAliasTestForSymbols("my_alias = 'lawl'", "my_alias"); + } + + @Test + public void testLoadAliasMultiple() throws Exception { + runLoadAliasTestForSymbols( + "my_alias = 'lawl', 'lol', next_alias = 'rofl'", "my_alias", "lol", "next_alias"); + } + + private void runLoadAliasTestForSymbols(String loadSymbolString, String... expectedSymbols) { + List<Statement> statements = + parseFileForSkylark(String.format("load('/foo/bar/file', %s)\n", loadSymbolString)); + LoadStatement stmt = (LoadStatement) statements.get(0); + ImmutableList<Identifier> actualSymbols = stmt.getSymbols(); + + assertThat(actualSymbols).hasSize(expectedSymbols.length); + + List<String> actualSymbolNames = new LinkedList<>(); + + for (Identifier identifier : actualSymbols) { + actualSymbolNames.add(identifier.getName()); + } + + assertThat(actualSymbolNames).containsExactly((Object[]) expectedSymbols); + } + + @Test + public void testLoadAliasSyntaxError() throws Exception { + setFailFast(false); + parseFileForSkylark("load('/foo', test1 = )\n"); + assertContainsEvent("syntax error at ')': expected string"); + + parseFileForSkylark("load('/foo', test2 = 1)\n"); + assertContainsEvent("syntax error at '1': expected string"); + + parseFileForSkylark("load('/foo', test3 = old)\n"); + assertContainsEvent("syntax error at 'old': expected string"); + } + + @Test public void testParseErrorNotComparison() throws Exception { setFailFast(false); parseFile("2 < not 3"); |