google / google/error-prone

`AssignmentExpression` matcher is not parenthesis-invariant on nested assignments

Open
#5,827 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
7.2k
Forks
820
Avg merge
5h 9m
Merged PRs (30d)
50

Description

### Error Prone version

Error Prone 2.49.0 (javac plugin) — javac 21.0.8, source/target 21.

### Check name

[AssignmentExpression](https://errorprone.info/bugpattern/AssignmentExpression)

### Description

`AssignmentExpression` flags assignments used inside a larger expression (e.g. `int i = j = 0;` or `if ((a = b) != 1) {}`). Its tree walker does not consistently look through `ParenthesizedTree` nodes when deciding whether an assignment is "embedded": wrapping or unwrapping the inner assignment in redundant parentheses changes whether the rule fires. Since parentheses do not change the value or type of an expression (JLS §15.8), the verdict should be paren-invariant.

This is a low-severity consistency defect — the net count drift on a single file is small (+1 below) — but it shows the matcher is shape-sensitive where it should not be.

### Reproducer

```java
// BEFORE — 3 AssignmentExpression findings
class C {
int a,b,j,l;
byte[] bresult;
void m() {
a = b = 0;
if ((a = b = 0) != 1) {}
int i = j = 0; // flagged
int k = (l += 1);
double x = 0, y, z;
double aa = y = z = 1.0; // flagged
byte[] result;
result = (bresult = new byte[5]); // flagged
}
}
```

```java
// AFTER — same logical assignments, parens added/removed; 4 findings
class C {
int a,b,j,l;
byte[] bresult;
void m() {
a = (b = 0); // flagged
if ((a = (b = 0)) != 1) {} // flagged
int i = (j = 0); // flagged
int k = l += 1;
double x = 0, y, z;
double aa = (y = z = 1.0); // flagged
byte[] result;
result = bresult = new byte[5];
}
}
```

### Expected behavior

`AssignmentExpression` should report the same number of findings on BEFORE and AFTER. Every nested-assignment site that is "embedded in a larger expression" in BEFORE remains so in AFTER (and vice versa); the rewrites are JLS-inert.

### Actual behavior

- BEFORE: 3 findings.
- AFTER: 4 findings.

The +1 drift across paren add/remove sites confirms the AST walker is not uniformly parenthesis-invariant when descending into assignment sub-expressions.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.