Fix 43 ErrorProneStyle findings:
* Overloads should be grouped together, even when modifiers such as static or private differ between the methods; found ungrouped overloads of 'visit' on line(s): 21
(see []
* Overloads should be grouped together, even when modifiers such as static or private differ between the methods; found ungrouped overloads of 'visit' on line(s): 462, 836, 847
(see []
* Overloads should be grouped together, even when modifiers such as static or private differ between the methods; found ungrouped overloads of 'visit' on line(s): 462, 565, 573, 622, 638, 646, 654, 660, 698, 709
(see []
* Overloads should be grouped together, even when modifiers such as static or private differ between the methods; found ungrouped overloads of 'visit' on line(s): 48, 53, 60, 65, 70, 75, 77, 88, 93, 97, 103, 109, 113, 115, 117, 119, 124, 128, 136, 142, 148, 150, 154, 159, 163, 168, 173, 178, 191, 193
(see []
* Overloads should be grouped together, even when modifiers such as static or private differ between the methods; found ungrouped overloads of 'visit' on line(s): 565, 573, 622, 638, 646, 654, 660, 698, 709, 836, 847
(see []
PiperOrigin-RevId: 438845959
diff --git a/src/main/java/net/starlark/java/syntax/NodeVisitor.java b/src/main/java/net/starlark/java/syntax/NodeVisitor.java
index cf9ac69..096f950 100644
--- a/src/main/java/net/starlark/java/syntax/NodeVisitor.java
+++ b/src/main/java/net/starlark/java/syntax/NodeVisitor.java
@@ -23,25 +23,6 @@
node.accept(this);
}
- // methods dealing with sequences of nodes
- public void visitAll(List<? extends Node> nodes) {
- for (Node node : nodes) {
- visit(node);
- }
- }
-
- /**
- * Visit a sequence ("block") of statements (e.g. an if branch, for block, function block etc.)
- *
- * This method allows subclasses to handle statement blocks more easily, like doing an action
- * after every statement in a block without having to override visit(...) for all statements.
- *
- * @param statements list of statements in the block
- */
- public void visitBlock(List<Statement> statements) {
- visitAll(statements);
- }
-
// node-specific visit methods
// All four subclasses of Argument are handled together.
@@ -197,4 +178,23 @@
visit(node.getElseCase());
}
}
+
+ // methods dealing with sequences of nodes
+ public void visitAll(List<? extends Node> nodes) {
+ for (Node node : nodes) {
+ visit(node);
+ }
+ }
+
+ /**
+ * Visit a sequence ("block") of statements (e.g. an if branch, for block, function block etc.)
+ *
+ * This method allows subclasses to handle statement blocks more easily, like doing an action
+ * after every statement in a block without having to override visit(...) for all statements.
+ *
+ * @param statements list of statements in the block
+ */
+ public void visitBlock(List<Statement> statements) {
+ visitAll(statements);
+ }
}
diff --git a/src/main/java/net/starlark/java/syntax/Resolver.java b/src/main/java/net/starlark/java/syntax/Resolver.java
index 6986e08..6aa9b96 100644
--- a/src/main/java/net/starlark/java/syntax/Resolver.java
+++ b/src/main/java/net/starlark/java/syntax/Resolver.java
@@ -468,100 +468,6 @@
}
}
- // Resolves a non-binding identifier to an existing binding, or null.
- private Binding use(Identifier id) {
- String name = id.getName();
-
- // Locally defined in this function, comprehension,
- // or file block, or an enclosing one?
- Binding bind = lookupLexical(name, locals);
- if (bind != null) {
- return bind;
- }
-
- // Defined at toplevel (global, predeclared, universal)?
- bind = toplevel.get(name);
- if (bind != null) {
- return bind;
- }
- Scope scope;
- try {
- scope = module.resolve(name);
- } catch (Resolver.Module.Undefined ex) {
- if (!Identifier.isValid(name)) {
- // If Identifier was created by Parser.makeErrorExpression, it
- // contains misparsed text. Ignore ex and report an appropriate error.
- errorf(id, "contains syntax errors");
- } else if (ex.candidates != null) {
- // Exception provided toplevel candidates.
- // Show spelling suggestions of all symbols in scope,
- String suggestion = SpellChecker.didYouMean(name, getAllSymbols(ex.candidates));
- errorf(id, "%s%s", ex.getMessage(), suggestion);
- } else {
- errorf(id, "%s", ex.getMessage());
- }
- return null;
- }
- switch (scope) {
- case GLOBAL:
- bind = new Binding(scope, globals.size(), id);
- // Accumulate globals in module.
- globals.add(name);
- break;
- case PREDECLARED:
- case UNIVERSAL:
- bind = new Binding(scope, 0, id); // index not used
- break;
- default:
- throw new IllegalStateException("bad scope: " + scope);
- }
- toplevel.put(name, bind);
- return bind;
- }
-
- // lookupLexical finds a lexically enclosing local binding of the name,
- // plumbing it through enclosing functions as needed.
- private static Binding lookupLexical(String name, Block b) {
- Binding bind = b.bindings.get(name);
- if (bind != null) {
- return bind;
- }
-
- if (b.parent != null) {
- bind = lookupLexical(name, b.parent);
- if (bind != null) {
- // If a local binding was found in a parent block,
- // and this block is a function, then it is a free variable
- // of this function and must be plumbed through.
- // Add an implicit FREE binding (a hidden parameter) to this function,
- // and record the outer binding that will supply its value when
- // we construct the closure.
- // Also, mark the outer LOCAL as a CELL: a shared, indirect local.
- // (For a comprehension block there's nothing to do,
- // because it's part of the same frame as the enclosing block.)
- //
- // This step may occur many times if the lookupLexical
- // recursion returns through many functions.
- if (b.syntax instanceof DefStatement || b.syntax instanceof LambdaExpression) {
- Scope scope = bind.getScope();
- if (scope == Scope.LOCAL || scope == Scope.FREE || scope == Scope.CELL) {
- if (scope == Scope.LOCAL) {
- bind.scope = Scope.CELL;
- }
- int index = b.freevars.size();
- b.freevars.add(bind);
- bind = new Binding(Scope.FREE, index, bind.first);
- }
- }
-
- // Memoize, to avoid duplicate free vars and repeated walks.
- b.bindings.put(name, bind);
- }
- }
-
- return bind;
- }
-
@Override
public void visit(ReturnStatement node) {
if (locals.syntax instanceof StarlarkFile) {
@@ -717,6 +623,126 @@
ImmutableList.of(ReturnStatement.make(expr.getBody()))));
}
+ @Override
+ public void visit(IfStatement node) {
+ if (locals.syntax instanceof StarlarkFile) {
+ errorf(
+ node,
+ "if statements are not allowed at the top level. You may move it inside a function "
+ + "or use an if expression (x if condition else y).");
+ }
+ super.visit(node);
+ }
+
+ @Override
+ public void visit(AssignmentStatement node) {
+ visit(node.getRHS());
+
+ // Disallow: [e, ...] += rhs
+ // Other bad cases are handled in assign.
+ if (node.isAugmented() && node.getLHS() instanceof ListExpression) {
+ errorf(
+ node.getOperatorLocation(),
+ "cannot perform augmented assignment on a list or tuple expression");
+ }
+
+ assign(node.getLHS());
+ }
+
+ // Resolves a non-binding identifier to an existing binding, or null.
+ private Binding use(Identifier id) {
+ String name = id.getName();
+
+ // Locally defined in this function, comprehension,
+ // or file block, or an enclosing one?
+ Binding bind = lookupLexical(name, locals);
+ if (bind != null) {
+ return bind;
+ }
+
+ // Defined at toplevel (global, predeclared, universal)?
+ bind = toplevel.get(name);
+ if (bind != null) {
+ return bind;
+ }
+ Scope scope;
+ try {
+ scope = module.resolve(name);
+ } catch (Resolver.Module.Undefined ex) {
+ if (!Identifier.isValid(name)) {
+ // If Identifier was created by Parser.makeErrorExpression, it
+ // contains misparsed text. Ignore ex and report an appropriate error.
+ errorf(id, "contains syntax errors");
+ } else if (ex.candidates != null) {
+ // Exception provided toplevel candidates.
+ // Show spelling suggestions of all symbols in scope,
+ String suggestion = SpellChecker.didYouMean(name, getAllSymbols(ex.candidates));
+ errorf(id, "%s%s", ex.getMessage(), suggestion);
+ } else {
+ errorf(id, "%s", ex.getMessage());
+ }
+ return null;
+ }
+ switch (scope) {
+ case GLOBAL:
+ bind = new Binding(scope, globals.size(), id);
+ // Accumulate globals in module.
+ globals.add(name);
+ break;
+ case PREDECLARED:
+ case UNIVERSAL:
+ bind = new Binding(scope, 0, id); // index not used
+ break;
+ default:
+ throw new IllegalStateException("bad scope: " + scope);
+ }
+ toplevel.put(name, bind);
+ return bind;
+ }
+
+ // lookupLexical finds a lexically enclosing local binding of the name,
+ // plumbing it through enclosing functions as needed.
+ private static Binding lookupLexical(String name, Block b) {
+ Binding bind = b.bindings.get(name);
+ if (bind != null) {
+ return bind;
+ }
+
+ if (b.parent != null) {
+ bind = lookupLexical(name, b.parent);
+ if (bind != null) {
+ // If a local binding was found in a parent block,
+ // and this block is a function, then it is a free variable
+ // of this function and must be plumbed through.
+ // Add an implicit FREE binding (a hidden parameter) to this function,
+ // and record the outer binding that will supply its value when
+ // we construct the closure.
+ // Also, mark the outer LOCAL as a CELL: a shared, indirect local.
+ // (For a comprehension block there's nothing to do,
+ // because it's part of the same frame as the enclosing block.)
+ //
+ // This step may occur many times if the lookupLexical
+ // recursion returns through many functions.
+ if (b.syntax instanceof DefStatement || b.syntax instanceof LambdaExpression) {
+ Scope scope = bind.getScope();
+ if (scope == Scope.LOCAL || scope == Scope.FREE || scope == Scope.CELL) {
+ if (scope == Scope.LOCAL) {
+ bind.scope = Scope.CELL;
+ }
+ int index = b.freevars.size();
+ b.freevars.add(bind);
+ bind = new Binding(Scope.FREE, index, bind.first);
+ }
+ }
+
+ // Memoize, to avoid duplicate free vars and repeated walks.
+ b.bindings.put(name, bind);
+ }
+ }
+
+ return bind;
+ }
+
// Common code for def, lambda.
private Function resolveFunction(
Node syntax, // DefStatement or LambdaExpression
@@ -833,32 +859,6 @@
params.add(param);
}
- @Override
- public void visit(IfStatement node) {
- if (locals.syntax instanceof StarlarkFile) {
- errorf(
- node,
- "if statements are not allowed at the top level. You may move it inside a function "
- + "or use an if expression (x if condition else y).");
- }
- super.visit(node);
- }
-
- @Override
- public void visit(AssignmentStatement node) {
- visit(node.getRHS());
-
- // Disallow: [e, ...] += rhs
- // Other bad cases are handled in assign.
- if (node.isAugmented() && node.getLHS() instanceof ListExpression) {
- errorf(
- node.getOperatorLocation(),
- "cannot perform augmented assignment on a list or tuple expression");
- }
-
- assign(node.getLHS());
- }
-
/**
* Process a binding use of a name by adding a binding to the current block if not already bound,
* and associate the identifier with it. Reports whether the name was already bound in this block.
