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.