Improve BanUnresolvedType conformance error message

The conformance error will contain the actual name of the forward declared type rather than the string "NoResolvedType" in most cases.

There are some remaining cases where the error still says "NoResolvedType". Fixing those would require tweaking the "greatest common subtype" & "least common supertype" operations on NoResolvedTypes, which is probably tricky.

PiperOrigin-RevId: 483981189

Author: lauraharker

src/com/google/javascript/jscomp/ConformanceRules.java

@@ -1712,7 +1712,7 @@ protected ConformanceResult checkConformance(NodeTraversal t, Node n) {
       if (n.isGetProp()) {
         Node target = n.getFirstChild();
         JSType type = target.getJSType();
-        JSType nonConformingPart = getNonConformingPart(type);
+        String nonConformingPart = getNonConformingPart(type);
         if (nonConformingPart != null && !isTypeImmediatelyTightened(n)) {
           return new ConformanceResult(
               ConformanceLevel.VIOLATION,
@@ -1722,21 +1722,28 @@ protected ConformanceResult checkConformance(NodeTraversal t, Node n) {
       return ConformanceResult.CONFORMANCE;
     }
 
-    private static @Nullable JSType getNonConformingPart(JSType type) {
+    private static @Nullable String getNonConformingPart(JSType type) {
       if (type == null) {
         return null;
       }
       if (type.isUnionType()) {
         // unwrap union types which might contain unresolved type name
         // references for example {Foo|undefined}
         for (JSType part : type.getUnionMembers()) {
-          JSType nonConformingPart = getNonConformingPart(part);
+          String nonConformingPart = getNonConformingPart(part);
           if (nonConformingPart != null) {
             return nonConformingPart;
           }
         }
       } else if (type.isNoResolvedType()) {
-        return type;
+        ObjectType noResolvedType = type.toObjectType();
+        // Whenever possible, return the 'reference name' of the NoResolvedType (i.e. the exact
+        // forward declared name). Some NoResolvedTypes do not have reference names because they
+        // were created as the product of an operation on NoResolvedTypes, e.g. consider
+        // 'noResolvedTypeA.getGreatestSubtype(noResolvedTypeB)'
+        return noResolvedType.getReferenceName() != null
+            ? noResolvedType.getReferenceName()
+            : "NoResolvedType";
       }
       return null;
     }
@@ -1751,7 +1758,7 @@ public StrictBanUnresolvedType(AbstractCompiler compiler, Requirement requiremen
 
     @Override
     protected ConformanceResult checkConformance(NodeTraversal t, Node n) {
-      JSType nonConformingPart = BanUnresolvedType.getNonConformingPart(n.getJSType());
+      String nonConformingPart = BanUnresolvedType.getNonConformingPart(n.getJSType());
       if (nonConformingPart != null && !isTypeImmediatelyTightened(n)) {
         return new ConformanceResult(
             ConformanceLevel.VIOLATION,

src/com/google/javascript/rhino/jstype/JSType.java

@@ -1168,12 +1168,15 @@ static JSType getGreatestSubtype(JSType thisType, JSType thatType) {
   }
 
   /**
-   * When computing infima, we may get a situation like
-   * inf(Type1, Type2)
-   * where both types are unresolved, so they're technically
-   * subtypes of one another.
+   * When computing infima, we may get a situation like inf(Type1, Type2) where both types are
+   * unresolved, so they're technically subtypes of one another.
    *
-   * If this happens, filter them down to NoResolvedType.
+   * <p>If this happens, filter them down to NoResolvedType.
+   *
+   * <p>TODO(lharker): this filtering sometimes leads to conformance errors with unhelpful error
+   * messages: "Unfulfilled forward declaration 'NoResolvedType'". Is there something else we could
+   * do to preserve both type names, e.g. a NoResolvedType with the string name "Type1|Type2"?
+   * Related issue: b/112425334.
    */
   static JSType filterNoResolvedType(JSType type) {
     if (type.isNoResolvedType()) {

test/com/google/javascript/jscomp/CheckConformanceTest.java

@@ -2726,6 +2726,25 @@ public void testCustomBanUnresolvedType() {
         CheckConformance.CONFORMANCE_VIOLATION,
         "Violation: BanUnresolvedType Message\nReference to type 'Foo' never resolved.");
 
+    testWarning(
+        "goog.forwardDeclare('Foo'); /** @type {Foo} */ var f = makeFoo(); f.foo();",
+        CheckConformance.CONFORMANCE_VIOLATION,
+        "Violation: BanUnresolvedType Message\nReference to type 'Foo' never resolved.");
+
+    test(
+        srcs(
+            lines(
+                "goog.forwardDeclare('Foo');",
+                "goog.forwardDeclare('Bar');",
+                "/** @param {?Foo|Bar} foobar */",
+                "function f(foobar) {",
+                "  return foobar.m();",
+                "}")),
+        warning(CheckConformance.CONFORMANCE_VIOLATION)
+            .withMessage(
+                // TODO(lharker): instead of 'Foo' could we print something like 'Foo|Bar'?
+                "Violation: BanUnresolvedType Message\nReference to type 'Foo' never resolved."));
+
     testNoWarning(
         lines(
             "/**",
@@ -2760,6 +2779,41 @@ public void testCustomStrictBanUnresolvedType() {
             "goog.forwardDeclare('Foo'); /** @return {!Foo} */ var f;", //
             "f();"),
         CheckConformance.CONFORMANCE_VIOLATION);
+
+    test(
+        srcs(
+            lines(
+                "goog.forwardDeclare('Foo');",
+                "goog.forwardDeclare('Bar');",
+                "/**",
+                " * @param {?Foo} foo",
+                " * @param {?Bar} bar",
+                " */",
+                "function f(foo, bar) {",
+                "  return foo || bar;",
+                "}")),
+        warning(CheckConformance.CONFORMANCE_VIOLATION)
+            .withMessage(
+                "Violation: StrictBanUnresolvedType Message\n"
+                    + "Reference to type 'Foo' never resolved."),
+        warning(CheckConformance.CONFORMANCE_VIOLATION)
+            .withMessage(
+                "Violation: StrictBanUnresolvedType Message\n"
+                    + "Reference to type 'Bar' never resolved."),
+        warning(CheckConformance.CONFORMANCE_VIOLATION)
+            .withMessage(
+                "Violation: StrictBanUnresolvedType Message\n"
+                    + "Reference to type 'Foo' never resolved."),
+        warning(CheckConformance.CONFORMANCE_VIOLATION)
+            .withMessage(
+                // TODO(lharker): instead of 'NoResolvedType', could we print something like
+                // 'Foo|Bar' ? We'd need to modify anything calling JSType.filterNoResolvedType.
+                "Violation: StrictBanUnresolvedType Message\n"
+                    + "Reference to type 'NoResolvedType' never resolved."),
+        warning(CheckConformance.CONFORMANCE_VIOLATION)
+            .withMessage(
+                "Violation: StrictBanUnresolvedType Message\n"
+                    + "Reference to type 'Bar' never resolved."));
   }
 
   @Test