coderabbit review suggestions

msridhar · msridhar · commit 75c613f91ebf · 2025-09-04T10:26:19.000-07:00
diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
@@ -666,8 +666,7 @@ private Type substituteInferredNullabilityForTypeVariables(
         for (Type.TypeVar tv : tvc.getMatches()) {
           typeVars.append(tv);
           inferredTypes.append(
-              TypeSubstitutionUtils.typeWithAnnot(
-                  tv, GenericsChecks.getSyntheticNullAnnotType(state)));
+              TypeSubstitutionUtils.typeWithAnnot(tv, getSyntheticNullAnnotType(state)));
         }
       }
     }
@@ -1449,6 +1448,8 @@ public boolean isNullableAnnotated(Type type) {
     return Nullness.hasNullableAnnotation(type.getAnnotationMirrors().stream(), config);
   }
 
+  private @Nullable Type syntheticNullAnnotType;
+
   /**
    * Returns a "fake" {@link Type} object representing a synthetic {@code @Nullable} annotation.
    *
@@ -1462,12 +1463,15 @@ public boolean isNullableAnnotated(Type type) {
    *     Symtab}.
    * @return a fake {@code Type} for a synthetic {@code @Nullable} annotation.
    */
-  public static Type getSyntheticNullAnnotType(VisitorState state) {
-    Names names = Names.instance(state.context);
-    Symtab symtab = Symtab.instance(state.context);
-    Name name = names.fromString("nullaway.synthetic");
-    Symbol.PackageSymbol packageSymbol = new Symbol.PackageSymbol(name, symtab.noSymbol);
-    Name simpleName = names.fromString("Nullable");
-    return new Type.ErrorType(simpleName, packageSymbol, Type.noType);
+  public Type getSyntheticNullAnnotType(VisitorState state) {
+    if (syntheticNullAnnotType == null) {
+      Names names = Names.instance(state.context);
+      Symtab symtab = Symtab.instance(state.context);
+      Name name = names.fromString("nullaway.synthetic");
+      Symbol.PackageSymbol packageSymbol = new Symbol.PackageSymbol(name, symtab.noSymbol);
+      Name simpleName = names.fromString("Nullable");
+      syntheticNullAnnotType = new Type.ErrorType(simpleName, packageSymbol, Type.noType);
+    }
+    return syntheticNullAnnotType;
   }
 }
diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/TypeVarWithSymbolCollector.java b/nullaway/src/main/java/com/uber/nullaway/generics/TypeVarWithSymbolCollector.java
@@ -30,9 +30,12 @@ public TypeVarWithSymbolCollector(Element symbol) {
     this.symbol = symbol;
   }
 
+  private final Set<Type> seen =
+      java.util.Collections.newSetFromMap(new java.util.IdentityHashMap<>());
+
   /** Walk a (possibly null) type. Safe to call multiple times on different roots. */
   private void scan(@Nullable Type t) {
-    if (t != null) {
+    if (t != null && seen.add(t)) {
       t.accept(this, null);
     }
   }

Original file line number	Diff line number	Diff line change
`@@ -666,8 +666,7 @@ private Type substituteInferredNullabilityForTypeVariables(`
`666`	`666`	`for (Type.TypeVar tv : tvc.getMatches()) {`
`667`	`667`	`typeVars.append(tv);`
`668`	`668`	`inferredTypes.append(`
`669`		`- TypeSubstitutionUtils.typeWithAnnot(`
`670`		`- tv, GenericsChecks.getSyntheticNullAnnotType(state)));`
	`669`	`+ TypeSubstitutionUtils.typeWithAnnot(tv, getSyntheticNullAnnotType(state)));`
`671`	`670`	`}`
`672`	`671`	`}`
`673`	`672`	`}`
`@@ -1449,6 +1448,8 @@ public boolean isNullableAnnotated(Type type) {`
`1449`	`1448`	`return Nullness.hasNullableAnnotation(type.getAnnotationMirrors().stream(), config);`
`1450`	`1449`	`}`
`1451`	`1450`
	`1451`	`+ private @Nullable Type syntheticNullAnnotType;`
	`1452`	`+`
`1452`	`1453`	`/**`
`1453`	`1454`	`* Returns a "fake" {@link Type} object representing a synthetic {@code @Nullable} annotation.`
`1454`	`1455`	`*`
`@@ -1462,12 +1463,15 @@ public boolean isNullableAnnotated(Type type) {`
`1462`	`1463`	`* Symtab}.`
`1463`	`1464`	`* @return a fake {@code Type} for a synthetic {@code @Nullable} annotation.`
`1464`	`1465`	`*/`
`1465`		`- public static Type getSyntheticNullAnnotType(VisitorState state) {`
`1466`		`- Names names = Names.instance(state.context);`
`1467`		`- Symtab symtab = Symtab.instance(state.context);`
`1468`		`- Name name = names.fromString("nullaway.synthetic");`
`1469`		`- Symbol.PackageSymbol packageSymbol = new Symbol.PackageSymbol(name, symtab.noSymbol);`
`1470`		`- Name simpleName = names.fromString("Nullable");`
`1471`		`- return new Type.ErrorType(simpleName, packageSymbol, Type.noType);`
	`1466`	`+ public Type getSyntheticNullAnnotType(VisitorState state) {`
	`1467`	`+ if (syntheticNullAnnotType == null) {`
	`1468`	`+ Names names = Names.instance(state.context);`
	`1469`	`+ Symtab symtab = Symtab.instance(state.context);`
	`1470`	`+ Name name = names.fromString("nullaway.synthetic");`
	`1471`	`+ Symbol.PackageSymbol packageSymbol = new Symbol.PackageSymbol(name, symtab.noSymbol);`
	`1472`	`+ Name simpleName = names.fromString("Nullable");`
	`1473`	`+ syntheticNullAnnotType = new Type.ErrorType(simpleName, packageSymbol, Type.noType);`
	`1474`	`+ }`
	`1475`	`+ return syntheticNullAnnotType;`
`1472`	`1476`	`}`
`1473`	`1477`	`}`
Original file line number	Diff line number	Diff line change
`@@ -30,9 +30,12 @@ public TypeVarWithSymbolCollector(Element symbol) {`
`30`	`30`	`this.symbol = symbol;`
`31`	`31`	`}`
`32`	`32`
	`33`	`+ private final Set<Type> seen =`
	`34`	`+ java.util.Collections.newSetFromMap(new java.util.IdentityHashMap<>());`
	`35`	`+`
`33`	`36`	`/** Walk a (possibly null) type. Safe to call multiple times on different roots. */`
`34`	`37`	`private void scan(@Nullable Type t) {`
`35`		`- if (t != null) {`
	`38`	`+ if (t != null && seen.add(t)) {`
`36`	`39`	`t.accept(this, null);`
`37`	`40`	`}`
`38`	`41`	`}`