Renamed Ident to Identifier, added some helper methods and refactored two methods.
--
MOS_MIGRATED_REVID=98922811
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index 210c68b..76f0bc0 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -42,7 +42,7 @@
import com.google.devtools.build.lib.syntax.FuncallExpression;
import com.google.devtools.build.lib.syntax.FunctionSignature;
import com.google.devtools.build.lib.syntax.GlobList;
-import com.google.devtools.build.lib.syntax.Ident;
+import com.google.devtools.build.lib.syntax.Identifier;
import com.google.devtools.build.lib.syntax.Label;
import com.google.devtools.build.lib.syntax.ParserInputSource;
import com.google.devtools.build.lib.syntax.SkylarkEnvironment;
@@ -1340,10 +1340,10 @@
for (Statement stmt : ast.getStatements()) {
if (stmt instanceof AssignmentStatement) {
Expression lvalue = ((AssignmentStatement) stmt).getLValue().getExpression();
- if (!(lvalue instanceof Ident)) {
+ if (!(lvalue instanceof Identifier)) {
continue;
}
- String target = ((Ident) lvalue).getName();
+ String target = ((Identifier) lvalue).getName();
if (pkgEnv.lookup(target, null) != null) {
eventHandler.handle(Event.error(stmt.getLocation(), "Reassignment of builtin build "
+ "function '" + target + "' not permitted"));
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
index e5b06a9..bb0e8ed 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
@@ -27,9 +27,9 @@
private final Expression obj;
- private final Ident field;
+ private final Identifier field;
- public DotExpression(Expression obj, Ident field) {
+ public DotExpression(Expression obj, Identifier field) {
this.obj = obj;
this.field = field;
}
@@ -38,7 +38,7 @@
return obj;
}
- public Ident getField() {
+ public Identifier getField() {
return field;
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 21df356..a448325 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -169,7 +169,7 @@
private final Expression obj;
- private final Ident func;
+ private final Identifier func;
private final List<Argument.Passed> args;
@@ -182,7 +182,7 @@
* arbitrary expressions. In any case, the "func" expression is always
* evaluated, so functions and variables share a common namespace.
*/
- public FuncallExpression(Expression obj, Ident func,
+ public FuncallExpression(Expression obj, Identifier func,
List<Argument.Passed> args) {
this.obj = obj;
this.func = func;
@@ -197,7 +197,7 @@
* arbitrary expressions. In any case, the "func" expression is always
* evaluated, so functions and variables share a common namespace.
*/
- public FuncallExpression(Ident func, List<Argument.Passed> args) {
+ public FuncallExpression(Identifier func, List<Argument.Passed> args) {
this(null, func, args);
}
@@ -217,7 +217,7 @@
/**
* Returns the function expression.
*/
- public Ident getFunction() {
+ public Identifier getFunction() {
return func;
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
index dce9e9c..f9e26ed 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
@@ -24,11 +24,11 @@
*/
public class FunctionDefStatement extends Statement {
- private final Ident ident;
+ private final Identifier ident;
private final FunctionSignature.WithValues<Expression, Expression> args;
private final ImmutableList<Statement> statements;
- public FunctionDefStatement(Ident ident,
+ public FunctionDefStatement(Identifier ident,
FunctionSignature.WithValues<Expression, Expression> args,
Collection<Statement> statements) {
this.ident = ident;
@@ -59,7 +59,7 @@
return "def " + ident + "(" + args + "):\n";
}
- public Ident getIdent() {
+ public Identifier getIdent() {
return ident;
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Ident.java b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
similarity index 61%
rename from src/main/java/com/google/devtools/build/lib/syntax/Ident.java
rename to src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
index 9bc58b1..26c5f15 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Ident.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
@@ -14,30 +14,36 @@
package com.google.devtools.build.lib.syntax;
+import javax.annotation.Nullable;
+
// TODO(bazel-team): for extra performance:
// (1) intern the strings, so we can use == to compare, and have .equals use the assumption.
-// Then have Argument and Parameter use Ident again instead of String as keys.
-// (2) Use Ident, not String, as keys in the Environment, which will be cleaner.
+// Then have Argument and Parameter use Identifier again instead of String as keys.
+// (2) Use Identifier, not String, as keys in the Environment, which will be cleaner.
// (3) For performance, avoid doing HashMap lookups at runtime, and compile local variable access
// into array reference with a constant index. Variable lookups are currently a speed bottleneck,
// as previously measured in an experiment.
/**
* Syntax node for an identifier.
*/
-public final class Ident extends Expression {
+public final class Identifier extends Expression {
private final String name;
- public Ident(String name) {
+ public Identifier(String name) {
this.name = name;
}
/**
- * Returns the name of the Ident.
+ * Returns the name of the Identifier.
*/
public String getName() {
return name;
}
+
+ public boolean isPrivate() {
+ return name.startsWith("_");
+ }
@Override
public String toString() {
@@ -49,15 +55,25 @@
try {
return env.lookup(name);
} catch (Environment.NoSuchVariableException e) {
- if (name.equals("$error$")) {
- throw new EvalException(getLocation(), "contains syntax error(s)", true);
- } else {
- throw new EvalException(getLocation(), "name '" + name + "' is not defined");
- }
+ throw createInvalidIdentifierException();
}
}
@Override
+ public boolean equals(@Nullable Object object) {
+ if (object instanceof Identifier) {
+ Identifier that = (Identifier) object;
+ return this.name.equals(that.name);
+ }
+ return false;
+ }
+
+ @Override
+ public int hashCode() {
+ return name.hashCode();
+ }
+
+ @Override
public void accept(SyntaxTreeVisitor visitor) {
visitor.visit(this);
}
@@ -65,11 +81,13 @@
@Override
void validate(ValidationEnvironment env) throws EvalException {
if (!env.hasSymbolInEnvironment(name)) {
- if (name.equals("$error$")) {
- throw new EvalException(getLocation(), "contains syntax error(s)", true);
- } else {
- throw new EvalException(getLocation(), "name '" + name + "' is not defined");
- }
+ throw createInvalidIdentifierException();
}
}
+
+ private EvalException createInvalidIdentifierException() {
+ return name.equals("$error$")
+ ? new EvalException(getLocation(), "contains syntax error(s)", true)
+ : new EvalException(getLocation(), "name '" + name + "' is not defined");
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LValue.java b/src/main/java/com/google/devtools/build/lib/syntax/LValue.java
index 2e9bf26..095a803 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/LValue.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/LValue.java
@@ -49,8 +49,8 @@
private static void assign(Environment env, Location loc, Expression lvalue, Object result)
throws EvalException, InterruptedException {
- if (lvalue instanceof Ident) {
- assign(env, loc, (Ident) lvalue, result);
+ if (lvalue instanceof Identifier) {
+ assign(env, loc, (Identifier) lvalue, result);
return;
}
@@ -77,7 +77,7 @@
/**
* Assign value to a single variable.
*/
- private static void assign(Environment env, Location loc, Ident ident, Object result)
+ private static void assign(Environment env, Location loc, Identifier ident, Object result)
throws EvalException, InterruptedException {
Preconditions.checkNotNull(result, "trying to assign null to %s", ident);
@@ -103,8 +103,8 @@
private static void validate(ValidationEnvironment env, Location loc, Expression expr)
throws EvalException {
- if (expr instanceof Ident) {
- Ident ident = (Ident) expr;
+ if (expr instanceof Identifier) {
+ Identifier ident = (Identifier) expr;
env.declare(ident.getName(), loc);
return;
}
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 106b348..0eae19b 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
@@ -26,20 +26,20 @@
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<Ident> symbols;
+ private final ImmutableList<Identifier> symbols;
private final PathFragment importPath;
private final String pathString;
/**
* Constructs an import statement.
*/
- LoadStatement(String path, List<Ident> symbols) {
+ LoadStatement(String path, List<Identifier> symbols) {
this.symbols = ImmutableList.copyOf(symbols);
this.importPath = new PathFragment(path + ".bzl");
this.pathString = path;
}
- public ImmutableList<Ident> getSymbols() {
+ public ImmutableList<Identifier> getSymbols() {
return symbols;
}
@@ -54,9 +54,9 @@
@Override
void exec(Environment env) throws EvalException, InterruptedException {
- for (Ident i : symbols) {
+ for (Identifier i : symbols) {
try {
- if (i.getName().startsWith("_")) {
+ if (i.isPrivate()) {
throw new EvalException(getLocation(), "symbol '" + i + "' is private and cannot "
+ "be imported");
}
@@ -79,7 +79,7 @@
if (!importPath.isAbsolute() && importPath.segmentCount() > 1) {
throw new EvalException(getLocation(), String.format(PATH_ERROR_MSG, importPath));
}
- for (Ident symbol : symbols) {
+ for (Identifier symbol : symbols) {
env.declare(symbol.getName(), getLocation());
}
}
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 efd40b4..46e7ca7 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
@@ -388,8 +388,8 @@
}
// create an error expression
- private Ident makeErrorExpression(int start, int end) {
- return setLocation(new Ident("$error$"), start, end);
+ private Identifier makeErrorExpression(int start, int end) {
+ return setLocation(new Identifier("$error$"), start, end);
}
// Convenience wrapper around ASTNode.setLocation that returns the node.
@@ -410,7 +410,7 @@
}
// create a funcall expression
- private Expression makeFuncallExpression(Expression receiver, Ident function,
+ private Expression makeFuncallExpression(Expression receiver, Identifier function,
List<Argument.Passed> args,
int start, int end) {
if (function.getLocation() == null) {
@@ -474,21 +474,21 @@
int start = token.left;
if (token.kind == TokenKind.STAR_STAR) { // kwarg
nextToken();
- Ident ident = parseIdent();
+ Identifier ident = parseIdent();
return setLocation(new Parameter.StarStar<Expression, Expression>(
ident.getName()), start, ident);
} else if (token.kind == TokenKind.STAR) { // stararg
int end = token.right;
nextToken();
if (token.kind == TokenKind.IDENTIFIER) {
- Ident ident = parseIdent();
+ Identifier ident = parseIdent();
return setLocation(new Parameter.Star<Expression, Expression>(ident.getName()),
start, ident);
} else {
return setLocation(new Parameter.Star<Expression, Expression>(null), start, end);
}
} else {
- Ident ident = parseIdent();
+ Identifier ident = parseIdent();
if (token.kind == TokenKind.EQUALS) { // there's a default value
nextToken();
Expression expr = parseNonTupleExpression();
@@ -502,7 +502,7 @@
}
// funcall_suffix ::= '(' arg_list? ')'
- private Expression parseFuncallSuffix(int start, Expression receiver, Ident function) {
+ private Expression parseFuncallSuffix(int start, Expression receiver, Identifier function) {
List<Argument.Passed> args = Collections.emptyList();
expect(TokenKind.LPAREN);
int end;
@@ -522,7 +522,7 @@
private Expression parseSelectorSuffix(int start, Expression receiver) {
expect(TokenKind.DOT);
if (token.kind == TokenKind.IDENTIFIER) {
- Ident ident = parseIdent();
+ Identifier ident = parseIdent();
if (token.kind == TokenKind.LPAREN) {
return parseFuncallSuffix(start, receiver, ident);
} else {
@@ -599,7 +599,7 @@
new StringLiteral(labelName, '"')), location));
args.add(setLocation(new Argument.Positional(
new StringLiteral(file, '"')), location));
- Ident mockIdent = setLocation(new Ident("mocksubinclude"), location);
+ Identifier mockIdent = setLocation(new Identifier("mocksubinclude"), location);
Expression funCall = new FuncallExpression(null, mockIdent, args);
return setLocation(new ExpressionStatement(funCall), location);
}
@@ -680,7 +680,7 @@
return literal;
}
case IDENTIFIER: {
- Ident ident = parseIdent();
+ Identifier ident = parseIdent();
if (token.kind == TokenKind.LPAREN) { // it's a function application
return parseFuncallSuffix(start, null, ident);
} else {
@@ -720,7 +720,7 @@
List<Argument.Passed> args = new ArrayList<>();
Expression expr = parsePrimaryWithSuffix();
args.add(setLocation(new Argument.Positional(expr), start, expr));
- return makeFuncallExpression(null, new Ident("-"), args,
+ return makeFuncallExpression(null, new Identifier("-"), args,
start, token.right);
}
default: {
@@ -765,7 +765,7 @@
// This is a dictionary access
if (token.kind == TokenKind.RBRACKET) {
expect(TokenKind.RBRACKET);
- return makeFuncallExpression(receiver, new Ident("$index"), args,
+ return makeFuncallExpression(receiver, new Identifier("$index"), args,
start, token.right);
}
// This is a slice (or substring)
@@ -779,7 +779,7 @@
expect(TokenKind.RBRACKET);
args.add(setLocation(new Argument.Positional(endExpr), loc2, endExpr));
- return makeFuncallExpression(receiver, new Ident("$slice"), args,
+ return makeFuncallExpression(receiver, new Identifier("$slice"), args,
start, token.right);
}
@@ -933,12 +933,12 @@
return makeErrorExpression(start, end);
}
- private Ident parseIdent() {
+ private Identifier parseIdent() {
if (token.kind != TokenKind.IDENTIFIER) {
expect(TokenKind.IDENTIFIER);
return makeErrorExpression(token.left, token.right);
}
- Ident ident = new Ident(((String) token.value));
+ Identifier ident = new Identifier(((String) token.value));
setLocation(ident, token.left, token.right);
nextToken();
return ident;
@@ -1083,9 +1083,9 @@
nextToken();
expect(TokenKind.COMMA);
- List<Ident> symbols = new ArrayList<>();
+ List<Identifier> symbols = new ArrayList<>();
if (token.kind == TokenKind.STRING) {
- symbols.add(new Ident((String) token.value));
+ symbols.add(new Identifier((String) token.value));
}
expect(TokenKind.STRING);
while (token.kind != TokenKind.RPAREN && token.kind != TokenKind.EOF) {
@@ -1094,7 +1094,7 @@
break;
}
if (token.kind == TokenKind.STRING) {
- symbols.add(new Ident((String) token.value));
+ symbols.add(new Identifier((String) token.value));
}
expect(TokenKind.STRING);
}
@@ -1122,7 +1122,7 @@
// Check if there is an include
if (token.kind == TokenKind.IDENTIFIER) {
Token identToken = token;
- Ident ident = parseIdent();
+ Identifier ident = parseIdent();
if (ident.getName().equals("include")
&& token.kind == TokenKind.LPAREN
@@ -1200,7 +1200,8 @@
Expression rvalue = parseExpression();
if (expression instanceof FuncallExpression) {
FuncallExpression func = (FuncallExpression) expression;
- if (func.getFunction().getName().equals("$index") && func.getObject() instanceof Ident) {
+ if (func.getFunction().getName().equals("$index")
+ && func.getObject() instanceof Identifier) {
// Special casing to translate 'ident[key] = value' to 'ident = ident + {key: value}'
// Note that the locations of these extra expressions are fake.
Preconditions.checkArgument(func.getArguments().size() == 1);
@@ -1274,7 +1275,7 @@
private void parseFunctionDefStatement(List<Statement> list) {
int start = token.left;
expect(TokenKind.DEF);
- Ident ident = parseIdent();
+ Identifier ident = parseIdent();
expect(TokenKind.LPAREN);
FunctionSignature.WithValues<Expression, Expression> args = parseFunctionSignature();
expect(TokenKind.RPAREN);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java b/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
index 3d699ff..6769061 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
@@ -55,7 +55,7 @@
visitAll(node.getArguments());
}
- public void visit(Ident node) {
+ public void visit(Identifier node) {
}
public void visit(ListComprehension node) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
index 5bb9019..4a5968c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
@@ -25,7 +25,7 @@
private final ImmutableList<Statement> statements;
private final SkylarkEnvironment definitionEnv;
- protected UserDefinedFunction(Ident function,
+ protected UserDefinedFunction(Identifier function,
FunctionSignature.WithValues<Object, SkylarkType> signature,
ImmutableList<Statement> statements, SkylarkEnvironment definitionEnv) {
super(function.getName(), signature, function.getLocation());
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 de59d2a..dad2d46 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
@@ -145,7 +145,7 @@
public void testFuncallExpr() throws Exception {
FuncallExpression e = (FuncallExpression) parseExpression("foo(1, 2, bar=wiz)");
- Ident ident = e.getFunction();
+ Identifier ident = e.getFunction();
assertEquals("foo", ident.getName());
assertThat(e.getArguments()).hasSize(3);
@@ -159,7 +159,7 @@
Argument.Passed arg2 = e.getArguments().get(2);
assertEquals("bar", arg2.getName());
- Ident arg2val = (Ident) arg2.getValue();
+ Identifier arg2val = (Identifier) arg2.getValue();
assertEquals("wiz", arg2val.getName());
}
@@ -168,7 +168,7 @@
FuncallExpression e =
(FuncallExpression) parseExpression("foo.foo(1, 2, bar=wiz)");
- Ident ident = e.getFunction();
+ Identifier ident = e.getFunction();
assertEquals("foo", ident.getName());
assertThat(e.getArguments()).hasSize(3);
@@ -182,7 +182,7 @@
Argument.Passed arg2 = e.getArguments().get(2);
assertEquals("bar", arg2.getName());
- Ident arg2val = (Ident) arg2.getValue();
+ Identifier arg2val = (Identifier) arg2.getValue();
assertEquals("wiz", arg2val.getName());
}
@@ -191,7 +191,7 @@
FuncallExpression e =
(FuncallExpression) parseExpression("foo.replace().split(1)");
- Ident ident = e.getFunction();
+ Identifier ident = e.getFunction();
assertEquals("split", ident.getName());
assertThat(e.getArguments()).hasSize(1);
@@ -205,7 +205,7 @@
public void testPropRefExpr() throws Exception {
DotExpression e = (DotExpression) parseExpression("foo.foo");
- Ident ident = e.getField();
+ Identifier ident = e.getField();
assertEquals("foo", ident.getName());
}
@@ -213,7 +213,7 @@
public void testStringMethExpr() throws Exception {
FuncallExpression e = (FuncallExpression) parseExpression("'foo'.foo()");
- Ident ident = e.getFunction();
+ Identifier ident = e.getFunction();
assertEquals("foo", ident.getName());
assertThat(e.getArguments()).isEmpty();
@@ -280,7 +280,7 @@
// Test that the actual parameters are: (1, $error$, 3):
- Ident ident = e.getFunction();
+ Identifier ident = e.getFunction();
assertEquals("f", ident.getName());
assertThat(e.getArguments()).hasSize(3);
@@ -290,7 +290,7 @@
assertEquals(1, (int) arg0.getValue());
Argument.Passed arg1 = e.getArguments().get(1);
- Ident arg1val = ((Ident) arg1.getValue());
+ Identifier arg1val = ((Identifier) arg1.getValue());
assertEquals("$error$", arg1val.getName());
assertLocation(5, 29, arg1val.getLocation());
@@ -658,7 +658,7 @@
assertThat(clauses.get(0).getLValue().getExpression().toString()).isEqualTo("x");
assertThat(clauses.get(0).getExpression()).isInstanceOf(ListLiteral.class);
assertThat(clauses.get(1).getLValue().getExpression().toString()).isEqualTo("y");
- assertThat(clauses.get(1).getExpression()).isInstanceOf(Ident.class);
+ assertThat(clauses.get(1).getExpression()).isInstanceOf(Identifier.class);
}
@Test
