diff options
4 files changed, 29 insertions, 22 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java index 3e35a9de33..e2d2a486cc 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.syntax.AssignmentStatement; import com.google.devtools.build.lib.syntax.BuildFileAST; import com.google.devtools.build.lib.syntax.Environment.Extension; import com.google.devtools.build.lib.syntax.EvalException; +import com.google.devtools.build.lib.syntax.Identifier; import com.google.devtools.build.lib.syntax.LoadStatement; import com.google.devtools.build.lib.syntax.Mutability; import com.google.devtools.build.lib.syntax.SkylarkImport; @@ -417,14 +418,14 @@ public class SkylarkImportLookupFunction implements SkyFunction { return; } AssignmentStatement assignmentStatement = (AssignmentStatement) statement; - ImmutableSet<String> boundNames = assignmentStatement.getLValue().boundNames(); - for (String name : boundNames) { - Object lookup = extensionEnv.lookup(name); + ImmutableSet<Identifier> boundIdentifiers = assignmentStatement.getLValue().boundIdentifiers(); + for (Identifier ident : boundIdentifiers) { + Object lookup = extensionEnv.lookup(ident.getName()); if (lookup instanceof SkylarkExportable) { try { SkylarkExportable exportable = (SkylarkExportable) lookup; if (!exportable.isExported()) { - exportable.export(extensionLabel, name); + exportable.export(extensionLabel, ident.getName()); } } catch (EvalException e) { eventHandler.handle(Event.error(e.getLocation(), e.getMessage())); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java b/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java index 5278c80aef..9c0b4959e6 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java @@ -291,8 +291,8 @@ public abstract class AbstractComprehension extends Expression { // Check if a loop variable conflicts with another local variable. LValue lvalue = clause.getLValue(); if (lvalue != null) { - for (String name : lvalue.boundNames()) { - env.removeLocalBinding(name); + for (Identifier ident : lvalue.boundIdentifiers()) { + env.removeLocalBinding(ident.getName()); } } } 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 b964a2750f..6fbe08d652 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 @@ -196,30 +196,32 @@ public final class LValue extends ASTNode { } /** - * Returns all names bound by this LValue. + * Returns all names bound by this LValue. * - * Examples: - * <ul> - * <li><{@code x = ...} binds x.</li> - * <li><{@code x, [y,z] = ..} binds x, y, z.</li> - * <li><{@code x[5] = ..} does not bind any names.</li> - * </ul> + * <p>Examples: + * + * <ul> + * <li><{@code x = ...} binds x. + * <li><{@code x, [y,z] = ..} binds x, y, z. + * <li><{@code x[5] = ..} does not bind any names. + * </ul> */ - public ImmutableSet<String> boundNames() { - ImmutableSet.Builder<String> result = ImmutableSet.builder(); - collectBoundNames(expr, result); + public ImmutableSet<Identifier> boundIdentifiers() { + ImmutableSet.Builder<Identifier> result = ImmutableSet.builder(); + collectBoundIdentifiers(expr, result); return result.build(); } - private static void collectBoundNames(Expression lhs, ImmutableSet.Builder<String> result) { + private static void collectBoundIdentifiers( + Expression lhs, ImmutableSet.Builder<Identifier> result) { if (lhs instanceof Identifier) { - result.add(((Identifier) lhs).getName()); + result.add((Identifier) lhs); return; } if (lhs instanceof ListLiteral) { ListLiteral variables = (ListLiteral) lhs; for (Expression expression : variables.getElements()) { - collectBoundNames(expression, result); + collectBoundIdentifiers(expression, result); } } } diff --git a/src/test/java/com/google/devtools/build/lib/syntax/LValueBoundNamesTest.java b/src/test/java/com/google/devtools/build/lib/syntax/LValueBoundNamesTest.java index b2ae351475..53750c021a 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/LValueBoundNamesTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/LValueBoundNamesTest.java @@ -15,11 +15,13 @@ package com.google.devtools.build.lib.syntax; import com.google.common.truth.Truth; import java.util.Arrays; +import java.util.Set; +import java.util.stream.Collectors; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -/** A test for {@link LValue#boundNames()}. */ +/** A test for {@link LValue#boundIdentifiers}()}. */ @RunWith(JUnit4.class) public class LValueBoundNamesTest { @@ -48,10 +50,12 @@ public class LValueBoundNamesTest { assertBoundNames("[[x], y], [z, w[1]] = 1", "x", "y", "z"); } - private static void assertBoundNames(String assignment, String... boundNames) { + private static void assertBoundNames(String assignment, String... expectedBoundNames) { BuildFileAST buildFileAST = BuildFileAST .parseSkylarkString(Environment.FAIL_FAST_HANDLER, assignment); LValue lValue = ((AssignmentStatement) buildFileAST.getStatements().get(0)).getLValue(); - Truth.assertThat(lValue.boundNames()).containsExactlyElementsIn(Arrays.asList(boundNames)); + Set<String> boundNames = + lValue.boundIdentifiers().stream().map(Identifier::getName).collect(Collectors.toSet()); + Truth.assertThat(boundNames).containsExactlyElementsIn(Arrays.asList(expectedBoundNames)); } } |