ASTHelpers.getSymbol returns null for a parenthesized expression, while javac's TreeInfo.symbol returns the symbol inside

Inizia da ASTHelpers.getSymbol(Tree) in check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java ed esamina come gestisce i tipi di albero e le dichiarazioni. Aggiungi o aggiorna test mirati per le espressioni tra parentesi, incluso il caso del lock synchronized, quindi esegui i test di check_api; il lavoro è concluso quando getSymbol restituisce il simbolo dell’espressione interna per le parentesi e i test esistenti passano.

A check that looks up the symbol of an operand stops working as soon as the user writes that operand in redundant parentheses: ASTHelpers.getSymbol returns null for (x), for (this.x), and for the (lock) of every synchronized statement, where javac's own TreeInfo.symbol returns the field. The null reads as "not a variable", so the check neither crashes nor reports; it silently does nothing.

Versions: Error Prone 2.50.0 (error_prone_check_api-2.50.0.jar) on OpenJDK 21.0.9, and master at 0a9708634f56 built locally, on OpenJDK 25.0.4; both print the output below.

What happens today

The program below compiles a class with javac and, for every ParenthesizedTree, prints ASTHelpers.getSymbol of the tree, getSymbol of the stripped tree, and TreeInfo.symbol of the tree.

import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.CompilationUnitTree;
import com.sun.source.tree.ParenthesizedTree;
import com.sun.source.tree.SynchronizedTree;
import com.sun.source.util.JavacTask;
import com.sun.source.util.TreeScanner;
import com.sun.tools.javac.tree.JCTree;
import com.sun.tools.javac.tree.TreeInfo;
import java.net.URI;
import java.util.List;
import javax.tools.JavaFileObject;
import javax.tools.SimpleJavaFileObject;
import javax.tools.ToolProvider;

public class GetSymbolParens {
  public static void main(String[] args) throws Exception {
    String src = "class T { int x; final Object lock = new Object(); int f() { return (x) + (this.x); } void g() { synchronized (lock) {} } }";
    JavaFileObject file =
        new SimpleJavaFileObject(URI.create("string:///T.java"), JavaFileObject.Kind.SOURCE) {
          @Override
          public CharSequence getCharContent(boolean ignoreEncodingErrors) {
            return src;
          }
        };
    JavacTask task =
        (JavacTask)
            ToolProvider.getSystemJavaCompiler().getTask(null, null, null, null, null, List.of(file));
    Iterable<? extends CompilationUnitTree> units = task.parse();
    task.analyze();
    for (CompilationUnitTree unit : units) {
      new TreeScanner<Void, Void>() {
        @Override
        public Void visitParenthesized(ParenthesizedTree tree, Void unused) {
          System.out.printf(
              "%-10s getSymbol=%s  getSymbol(stripParentheses)=%s  TreeInfo.symbol=%s%n",
              tree,
              ASTHelpers.getSymbol(tree),
              ASTHelpers.getSymbol(ASTHelpers.stripParentheses(tree)),
              TreeInfo.symbol((JCTree) tree));
          return super.visitParenthesized(tree, unused);
        }

        @Override
        public Void visitSynchronized(SynchronizedTree tree, Void unused) {
          System.out.printf(
              "synchronized: getExpression() is %s%n", tree.getExpression().getKind());
          return super.visitSynchronized(tree, unused);
        }
      }.scan(unit, null);
    }
  }
}

Run with error_prone_check_api-2.50.0.jar, guava-33.5.0-jre.jar, error_prone_annotation-2.50.0.jar, and caffeine-3.3.0.jar on the class path, and --add-exports jdk.compiler/com.sun.tools.javac.{tree,code,util,api,parser,comp,main,model,processing,file}=ALL-UNNAMED:

(x)        getSymbol=null  getSymbol(stripParentheses)=x  TreeInfo.symbol=x
(this.x)   getSymbol=null  getSymbol(stripParentheses)=x  TreeInfo.symbol=x
synchronized: getExpression() is PARENTHESIZED
(lock)     getSymbol=null  getSymbol(stripParentheses)=lock  TreeInfo.symbol=lock

Expected

ASTHelpers.getSymbol(tree) for a ParenthesizedTree returns the same symbol as getSymbol of the expression inside it, so in the output above the getSymbol column reads x, x, lock, the same as the TreeInfo.symbol column.

Grounding:

• JLS §15.8.5 (https://docs.oracle.com/javase/specs/jls/se21/html/jls-15.html#jls-15.8.5): if the contained expression denotes a variable, the parenthesized expression denotes that variable too.
• javac's TreeInfo.symbol calls skipParens before its switch: https://github.com/openjdk/jdk/blob/d63b981776ec2258023f54d8c5bfdbf52b074f4f/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java#L1059-L1077
• ASTHelpers.getSymbol(Tree) has no case for ParenthesizedTree and falls through to getDeclaredSymbol, which returns null for anything that is not a declaration: https://github.com/google/error-prone/blob/d802cff975605b2c76f262eaf28d22bf39534f78/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java#L212-L270

Who else hits this

Every caller has to remember stripParentheses, and the ones that forget fail silently.

• Error Prone itself wraps the call in four checks on master: UnnecessaryBoxedVariable, CanIgnoreReturnValueSuggester, StaticGuardedByInstance, and SynchronizeOnNonFinalField. The last two read the expression of a synchronized statement, which javac always parenthesizes, as the output above shows.
• The open PR #5889 adds a fifth wrapper, in LoopConditionChecker, for the (i)++ false positive reported in #6213 (split out of #5845).
• NullAway, a plugin built on Error Prone, has added the same wrapper twice: in uber/NullAway#2002 (merged) for filter((Objects::nonNull)), and in uber/NullAway#2010 (open) for if (h.containsKey((K))) { take(h.get(K)); }, which reported passing @Nullable parameter 'h.get(K)' where @NonNull is required until the key was stripped. A metamorphic test that wraps expressions in redundant parentheses (uber/NullAway#2009) found both.

Scope

This asks only about the symbol lookup. It does not cover matchers that compare trees or test instanceof on an operand, which #5845 and its split-out issues track; a parenthesized operand breaks those for a different reason. A cast is not a parenthesized expression and stays out of scope: getSymbol((Object) x) returning null is not part of this request.

Compatibility

A caller that relies on getSymbol returning null for a parenthesized tree would change behavior. To look for one, I applied the shape below to master (0a9708634f56) and ran mvn test -pl check_api,core on OpenJDK 25.0.4: 189 tests in check_api and 7447 in core pass, 135 skipped, none failed.

The tests do not exercise every one of the roughly 1000 getSymbol calls in check_api and core, so I also read the three scanners in core that call getSymbol on every node they visit, where a parenthesized identifier would now yield its symbol twice, once for the parentheses and once for the identifier. MockNotUsedInProduction merges the deletion ranges it collects, so a range found twice is deleted once. UnnecessaryBoxedVariable stops scanning at the parentheses instead of at the identifier inside them. UnusedTypeParameter counts only type variables, and a type variable is never a parenthesized expression. I did not read the other call sites one by one.

One possible shape

Not the requirement, only an illustration: a case at the top of getSymbol(Tree).

if (tree instanceof ParenthesizedTree parenthesizedTree) {
  return getSymbol(parenthesizedTree.getExpression());
}

A separate method such as getSymbolSkippingParentheses would leave every existing caller to opt in, which is the step the current callers already forget.

Prior reports

Searched the tracker, open and closed, issues and PRs, for getSymbol parentheses, getSymbol ParenthesizedTree, getSymbol stripParentheses, getSymbol parenthesized, parentheses ASTHelpers, parenthesized expression symbol, and stripParentheses. The nearest hits are #5845 (checks that are not invariant under parentheses, split into #6213 to #6218) and PR #5889 (per-check fixes). Both fix individual checks; this issue asks for the fix in ASTHelpers.getSymbol, so that checks written later get it too.