Add missing handling of binding variables in the visitors.

rluble · copybara-github · commit df8cded565e2 · 2025-10-25T22:27:13.000-07:00
PiperOrigin-RevId: 824071484
diff --git a/translator/src/main/java/com/google/devtools/j2objc/ast/InstanceofExpression.java b/translator/src/main/java/com/google/devtools/j2objc/ast/InstanceofExpression.java
@@ -83,6 +83,7 @@ protected void acceptInner(TreeVisitor visitor) {
     if (visitor.visit(this)) {
       leftOperand.accept(visitor);
       rightOperand.accept(visitor);
+      pattern.accept(visitor);
     }
     visitor.endVisit(this);
   }
diff --git a/translator/src/main/java/com/google/devtools/j2objc/translate/InstanceOfPatternRewriter.java b/translator/src/main/java/com/google/devtools/j2objc/translate/InstanceOfPatternRewriter.java
@@ -31,8 +31,6 @@
 import com.google.devtools.j2objc.types.GeneratedVariableElement;
 import java.util.ArrayDeque;
 import java.util.Deque;
-import java.util.HashSet;
-import java.util.Set;
 import javax.lang.model.element.VariableElement;
 
 /**
@@ -50,8 +48,6 @@ public class InstanceOfPatternRewriter extends UnitTreeVisitor {
   private int tempCount;
   private int patternCount;
 
-  private Set<VariableElement> patternVariables = new HashSet<>();
-
   public InstanceOfPatternRewriter(CompilationUnit unit) {
     super(unit);
   }
@@ -67,14 +63,6 @@ public void endVisit(Block node) {
     enclosingScopes.pop();
   }
 
-  @Override
-  public void endVisit(SimpleName node) {
-    if (patternVariables.contains(node.getElement())) {
-      // Rename all existing access to the variable to the new name.
-      node.setIdentifier(nameTable.getVariableShortName((VariableElement) node.getElement()));
-    }
-  }
-
   @Override
   public void endVisit(InstanceofExpression node) {
     if (node.getPattern() == null) {
@@ -89,7 +77,6 @@ public void endVisit(InstanceofExpression node) {
     nameTable.setVariableName(
         patternVariable,
         nameTable.getVariableShortName(patternVariable) + "$pattern$" + patternCount++);
-    patternVariables.add(patternVariable);
 
     // Generate a temporary variable to preserve evaluation semantics.
     VariableElement tempVariable =
diff --git a/translator/src/main/java/com/google/devtools/j2objc/util/ElementUtil.java b/translator/src/main/java/com/google/devtools/j2objc/util/ElementUtil.java
@@ -369,9 +369,13 @@ private static boolean hasModifier(Element element, Modifier modifier) {
 
   public static boolean isVariable(Element element) {
     ElementKind kind = element.getKind();
-    return kind == ElementKind.FIELD || kind == ElementKind.LOCAL_VARIABLE
-        || kind == ElementKind.PARAMETER || kind == ElementKind.EXCEPTION_PARAMETER
-        || kind == ElementKind.RESOURCE_VARIABLE || kind == ElementKind.ENUM_CONSTANT;
+    return kind == ElementKind.FIELD
+        || kind == ElementKind.LOCAL_VARIABLE
+        || kind == ElementKind.PARAMETER
+        || kind == ElementKind.EXCEPTION_PARAMETER
+        || kind == ElementKind.RESOURCE_VARIABLE
+        || kind == ElementKind.ENUM_CONSTANT
+        || kind == ElementKind.BINDING_VARIABLE;
   }
 
   public static boolean isField(Element element) {
diff --git a/translator/src/test/java/com/google/devtools/j2objc/gen/StatementGeneratorTest.java b/translator/src/test/java/com/google/devtools/j2objc/gen/StatementGeneratorTest.java
@@ -2072,8 +2072,9 @@ public void testASTConversionSimpleSwitchExpressionWithPatternAndGuard() throws
               ast,
               "java.lang.String test(  java.lang.String str){",
               "  java.lang.String msg;",
-              "  if (JreIndexOfStr(str, {}, 0) instanceof java.lang.String "
-                  + "&& ((java.lang.String)JreIndexOfStr(str, {}, 0)).length() > 10) {",
+              "  if (JreIndexOfStr(str, {}, 0) instanceof java.lang.String &&"
+                  + " ((java.lang.String)nil_chk(((java.lang.String)JreIndexOfStr(str, {},"
+                  + " 0)))).length() > 10) {",
               "    msg=JreStrcat($$, \"Long string: \", ((java.lang.String)JreIndexOfStr(str, {},"
                   + " 0)));",
               "  }",
diff --git a/translator/src/test/java/com/google/devtools/j2objc/translate/InstanceOfPatternRewriterTest.java b/translator/src/test/java/com/google/devtools/j2objc/translate/InstanceOfPatternRewriterTest.java
@@ -93,11 +93,11 @@ public void testIssue2580() throws IOException {
         "id y = @\"Hello\";",
         "if (x != nil && (tmp$0 = y, s$pattern$0 = [tmp$0 isKindOfClass:[NSString class]] ?"
             + " (NSString *) tmp$0 : nil, !JreStringEqualsEquals(s$pattern$0, nil))) {",
-        "int32_t i = [s$pattern$0 java_length];",
+        "int32_t i = [((NSString *) nil_chk(s$pattern$0)) java_length];",
         "}",
         "if (x != nil && ((tmp$1 = y, s$pattern$1 = [tmp$1 isKindOfClass:[NSString class]] ?"
             + " (NSString *) tmp$1 : nil, !JreStringEqualsEquals(s$pattern$1, nil)))) {",
-        " int32_t i = [s$pattern$1 java_length];",
+        " int32_t i = [((NSString *) nil_chk(s$pattern$1)) java_length];",
         "}");
   }
 }
diff --git a/translator/src/test/java/com/google/devtools/j2objc/translate/SwitchCaseRewriterTest.java b/translator/src/test/java/com/google/devtools/j2objc/translate/SwitchCaseRewriterTest.java
@@ -57,41 +57,42 @@ public void testSwitchCaseWithInstanceOf() throws IOException {
   public void testSwitchCaseWithOverlappingGuards() throws IOException {
     testOnJava21OrAbove(
         () -> {
-          String source = 
+          String source =
               "class Test {\n"
-              + "  void checkObject(Object obj) {\n"
-              + "    switch (obj) {\n"
-              // The > 25 test must still be tested before > 5.
-              + "      case String s when s.length() > 25 -> log(\"Long string\");\n"
-              + "      case String s when s.length() > 5 -> log(\"Medium string\");\n"
-              // Test without guard.
-              + "      case String s -> log(\"Short string\");\n"
-              // Test with different pattern, no guard.
-              + "      case Integer i -> log(\"An integer: \" + i);\n"
-              + "      default -> log(\"Something else\");\n"
-              + "    }\n"
-              + "  }\n"
-              + "  private static void log(String s) {\n"
-              + "    System.out.println(s);\n"
-              + "  }\n"
-              + "}\n";
+                  + "  void checkObject(Object obj) {\n"
+                  + "    switch (obj) {\n"
+                  // The > 25 test must still be tested before > 5.
+                  + "      case String s when s.length() > 25 -> log(\"Long string\");\n"
+                  + "      case String s when s.length() > 5 -> log(\"Medium string\");\n"
+                  // Test without guard.
+                  + "      case String s -> log(\"Short string\");\n"
+                  // Test with different pattern, no guard.
+                  + "      case Integer i -> log(\"An integer: \" + i);\n"
+                  + "      default -> log(\"Something else\");\n"
+                  + "    }\n"
+                  + "  }\n"
+                  + "  private static void log(String s) {\n"
+                  + "    System.out.println(s);\n"
+                  + "  }\n"
+                  + "}\n";
           String translation = translateSourceFile(source, "Test", "Test.m");
-          assertTranslatedLines(translation,
-            "- (void)checkObjectWithId:(id)obj {",
+          assertTranslatedLines(
+              translation,
+              "- (void)checkObjectWithId:(id)obj {",
               "  if ([obj isKindOfClass:[NSString class]] && "
-              + "[((NSString *) obj) java_length] > 25) {",
+                  + "[((NSString *) nil_chk(((NSString *) obj))) java_length] > 25) {",
               "    Test_logWithNSString_(@\"Long string\");",
               "  }",
               "  else if ([obj isKindOfClass:[NSString class]] && "
-              + "[((NSString *) obj) java_length] > 5) {",
+                  + "[((NSString *) nil_chk(((NSString *) obj))) java_length] > 5) {",
               "    Test_logWithNSString_(@\"Medium string\");",
               "  }",
               "  else if ([obj isKindOfClass:[NSString class]]) {",
               "    Test_logWithNSString_(@\"Short string\");",
               "  }",
               "  else if ([obj isKindOfClass:[JavaLangInteger class]]) {",
               "    Test_logWithNSString_(JreStrcat(\"$@\", "
-              + "@\"An integer: \", ((JavaLangInteger *) obj)));",
+                  + "@\"An integer: \", ((JavaLangInteger *) obj)));",
               "  }",
               "  else {",
               "    Test_logWithNSString_(@\"Something else\");",
@@ -124,14 +125,16 @@ public void testSwitchCaseWithStatements() throws IOException {
               "NSString *s$pattern$0;",
               "id tmp$0;",
               "id o2 = @\"\";",
-              "if ([o isKindOfClass:[JavaLangInteger class]] && [((JavaLangInteger *) o) intValue]"
-                  + " == 0 && [((JavaLangInteger *) o) intValue] < 1 && (tmp$0 = o2, s$pattern$0 ="
-                  + " [tmp$0 isKindOfClass:[NSString class]] ? (NSString *) tmp$0 : nil,"
-                  + " !JreStringEqualsEquals(s$pattern$0, nil))) {",
+              "if ([o isKindOfClass:[JavaLangInteger class]] && [((JavaLangInteger *)"
+                  + " nil_chk(((JavaLangInteger *) o))) intValue] == 0 && [((JavaLangInteger *) o)"
+                  + " intValue] < 1 && (tmp$0 = o2, s$pattern$0 = [tmp$0 isKindOfClass:[NSString"
+                  + " class]] ? (NSString *) tmp$0 : nil, !JreStringEqualsEquals(s$pattern$0,"
+                  + " nil))) {",
               "  return @\"true\";",
               "}",
-              "else if ([o isKindOfClass:[JavaLangInteger class]] && [((JavaLangInteger *) o)"
-                  + " intValue] == 0 || [((JavaLangInteger *) o) intValue] > 1) {",
+              "else if ([o isKindOfClass:[JavaLangInteger class]] && [((JavaLangInteger *)"
+                  + " nil_chk(((JavaLangInteger *) o))) intValue] == 0 || [((JavaLangInteger *) o)"
+                  + " intValue] > 1) {",
               "  return @\"second\";",
               "}",
               "else if ([o isKindOfClass:[NSObject class]]) {",

Original file line number	Diff line number	Diff line change
`@@ -83,6 +83,7 @@ protected void acceptInner(TreeVisitor visitor) {`
`83`	`83`	`if (visitor.visit(this)) {`
`84`	`84`	`leftOperand.accept(visitor);`
`85`	`85`	`rightOperand.accept(visitor);`
	`86`	`+ pattern.accept(visitor);`
`86`	`87`	`}`
`87`	`88`	`visitor.endVisit(this);`
`88`	`89`	`}`
Original file line number	Diff line number	Diff line change
`@@ -93,11 +93,11 @@ public void testIssue2580() throws IOException {`
`93`	`93`	`"id y = @\"Hello\";",`
`94`	`94`	`"if (x != nil && (tmp$0 = y, s$pattern$0 = [tmp$0 isKindOfClass:[NSString class]] ?"`
`95`	`95`	`+ " (NSString *) tmp$0 : nil, !JreStringEqualsEquals(s$pattern$0, nil))) {",`
`96`		`- "int32_t i = [s$pattern$0 java_length];",`
	`96`	`+ "int32_t i = [((NSString *) nil_chk(s$pattern$0)) java_length];",`
`97`	`97`	`"}",`
`98`	`98`	`"if (x != nil && ((tmp$1 = y, s$pattern$1 = [tmp$1 isKindOfClass:[NSString class]] ?"`
`99`	`99`	`+ " (NSString *) tmp$1 : nil, !JreStringEqualsEquals(s$pattern$1, nil)))) {",`
`100`		`- " int32_t i = [s$pattern$1 java_length];",`
	`100`	`+ " int32_t i = [((NSString *) nil_chk(s$pattern$1)) java_length];",`
`101`	`101`	`"}");`
`102`	`102`	`}`
`103`	`103`	`}`