Keep some mandatory parens in PatternMatchingInstanceOf

Fixes #4923 PiperOrigin-RevId: 739213124

Author: cushon

core/src/main/java/com/google/errorprone/bugpatterns/PatternMatchingInstanceof.java

@@ -244,10 +244,7 @@ public Void visitTypeCast(TypeCastTree node, Void unused) {
  if (getSymbol(node.getExpression()) instanceof VarSymbol v) {
  if (v.equals(symbol)
  && state.getTypes().isSubtype(targetType, getType(node.getType()))) {
- usages.add(
- getCurrentPath().getParentPath().getLeaf() instanceof ParenthesizedTree
- ? getCurrentPath().getParentPath()
- : getCurrentPath());
+ usages.add(getUsage(getCurrentPath()));
  }
  }
  return super.visitTypeCast(node, null);
@@ -259,6 +256,29 @@ public Void visitTypeCast(TypeCastTree node, Void unused) {
  return usages.build();
  }
 
+ private static TreePath getUsage(TreePath currentPath) {
+ TreePath parentPath = currentPath.getParentPath();
+ return parentPath.getLeaf() instanceof ParenthesizedTree && !requiresParentheses(parentPath)
+ ? parentPath
+ : currentPath;
+ }
+
+ private static boolean requiresParentheses(TreePath path) {
+ // This isn't ASTHelpers.requiresParentheses, because we want to know if parens are needed when
+ // replacing a cast with the cast's expression, i.e. `((Foo) bar)` -> `(bar)`
+ return switch (path.getParentPath().getLeaf().getKind()) {
+ case IDENTIFIER,
+ MEMBER_SELECT,
+ METHOD_INVOCATION,
+ ARRAY_ACCESS,
+ PARENTHESIZED,
+ NEW_CLASS,
+ MEMBER_REFERENCE ->
+ false;
+ default -> true;
+ };
+ }
+
  private static boolean isReassigned(VarSymbol symbol, Iterable<Tree> trees) {
  AtomicBoolean isReassigned = new AtomicBoolean(false);
  new TreeScanner<Void, Void>() {

core/src/test/java/com/google/errorprone/bugpatterns/PatternMatchingInstanceofTest.java

@@ -764,4 +764,70 @@ int f(Object o) {
  .expectUnchanged()
  .doTest();
  }
+
+ // https://github.com/google/error-prone/issues/4923
+ @Test
+ public void requiredParentheses_retainedInFix() {
+ assume().that(Runtime.version().feature()).isAtLeast(21);
+ helper
+ .addInputLines(
+ "Test1.java",
+ """
+ public class Test1 {
+
+ int test_switch() {
+ Object o = 1;
+ if (o instanceof Integer) {
+ // Next line will be turned into "return switch i {".
+ return switch ((Integer) o) {
+ case 0 -> 0;
+ default -> 1;
+ };
+ }
+ return 0;
+ }
+
+ boolean test_if() {
+ Object o = false;
+ if (o instanceof Boolean) {
+ // Next line will be turned into "if b {".
+ if ((Boolean) o) {
+ return (Boolean) o;
+ }
+ }
+ return false;
+ }
+ }
+ """)
+ .addOutputLines(
+ "Test1.java",
+ """
+ public class Test1 {
+
+ int test_switch() {
+ Object o = 1;
+ if (o instanceof Integer i) {
+ // Next line will be turned into "return switch i {".
+ return switch (i) {
+ case 0 -> 0;
+ default -> 1;
+ };
+ }
+ return 0;
+ }
+
+ boolean test_if() {
+ Object o = false;
+ if (o instanceof Boolean b) {
+ // Next line will be turned into "if b {".
+ if (b) {
+ return b;
+ }
+ }
+ return false;
+ }
+ }
+ """)
+ .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
+ }
 }