diff options
author | Taras Tsugrii <ttsugrii@fb.com> | 2018-07-11 05:32:24 -0700 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-07-11 05:33:46 -0700 |
commit | feb8050e553712f4d689d9e84e624abc0f4c2fa8 (patch) | |
tree | 0e2119c13cbf4d82c957222d07fd92ffcbd5929f /src | |
parent | a3320b4ba0628af8fa7ee2ba2cf40d72efbbc6f7 (diff) |
[Skylint] Fix a crash on analyzing augmented assignment to IndexExpression.
Fixes #5534.
Skylint was operating under assumption that all lvalues have a single
bound identifier, which is not true for `IndexExpression`s like
```
d["foo"] += "bar"
```
As a result, Skylint would crash. This change makes it handle cases without
bound identifiers gracefully.
Ideally `IndexExpression` assigments should be analyzed too, but it's a more
involved change.
Closes #5535.
PiperOrigin-RevId: 204109060
Diffstat (limited to 'src')
-rw-r--r-- | src/tools/skylark/java/com/google/devtools/skylark/skylint/BadOperationChecker.java | 8 | ||||
-rw-r--r-- | src/tools/skylark/javatests/com/google/devtools/skylark/skylint/BadOperationCheckerTest.java | 5 |
2 files changed, 12 insertions, 1 deletions
diff --git a/src/tools/skylark/java/com/google/devtools/skylark/skylint/BadOperationChecker.java b/src/tools/skylark/java/com/google/devtools/skylark/skylint/BadOperationChecker.java index 09df8ff7fe..cb9460ad01 100644 --- a/src/tools/skylark/java/com/google/devtools/skylark/skylint/BadOperationChecker.java +++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/BadOperationChecker.java @@ -137,7 +137,13 @@ public class BadOperationChecker extends AstVisitorWithNameResolution { @Override public void visit(AugmentedAssignmentStatement node) { super.visit(node); - Identifier ident = Iterables.getOnlyElement(node.getLValue().boundIdentifiers()); + ImmutableSet<Identifier> lvalues = node.getLValue().boundIdentifiers(); + if (lvalues.size() != 1) { + // assignment visitor does not track information about assignments to IndexExpressions like + // kwargs["name"] += "foo" so nothing to do here until that changes + return; + } + Identifier ident = Iterables.getOnlyElement(lvalues); NodeType identType = getInferredTypeOrNull(ident); NodeType expressionType = getInferredTypeOrNull(node.getExpression()); if (identType == NodeType.DICT || expressionType == NodeType.DICT) { diff --git a/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/BadOperationCheckerTest.java b/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/BadOperationCheckerTest.java index 27c1063147..20cd662de7 100644 --- a/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/BadOperationCheckerTest.java +++ b/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/BadOperationCheckerTest.java @@ -152,4 +152,9 @@ public class BadOperationCheckerTest { .contains("1:1-1:5: '/' operator is deprecated"); Truth.assertThat(findIssues("5 // 2")).isEmpty(); } + + @Test + public void augmentedAssignmentOperator() { + Truth.assertThat(findIssues("kwargs['name'] += 'foo'")).isEmpty(); + } } |