Add typo detection when lookups on SkylarkModules fail.

Also consolidate code with getattr so getattr now also gets typo detection.

PiperOrigin-RevId: 197612666

2 files changed, 46 insertions, 31 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
index 4a1507bbfa..ed45d6079f 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
@@ -63,19 +63,54 @@ public final class DotExpression extends Expression {
     if (result != null) {
       return result;
     }
+    throw getMissingFieldException(objValue, name, loc, "field");
+  }
+
+  static EvalException getMissingFieldException(
+      Object objValue, String name, Location loc, String accessName) {
     String suffix = "";
+    EvalException toSuppress = null;
     if (objValue instanceof ClassObject) {
       String customErrorMessage = ((ClassObject) objValue).getErrorMessageForUnknownField(name);
       if (customErrorMessage != null) {
-        throw new EvalException(loc, customErrorMessage);
+        return new EvalException(loc, customErrorMessage);
       }
-      suffix = SpellChecker.didYouMean(name, ((ClassObject) objValue).getFieldNames());
+      try {
+        suffix = SpellChecker.didYouMean(name, ((ClassObject) objValue).getFieldNames());
+      } catch (EvalException ee) {
+        toSuppress = ee;
+      }
+    } else {
+      suffix =
+          SpellChecker.didYouMean(
+              name,
+              FuncallExpression.getStructFieldNames(
+                  objValue instanceof Class ? (Class<?>) objValue : objValue.getClass()));
+    }
+    if (suffix.isEmpty() && hasMethod(objValue, name)) {
+      // If looking up the field failed, then we know that this method must have struct_field=false
+      suffix = ", however, a method of that name exists";
     }
-    throw new EvalException(
-        loc,
-        String.format(
-            "object of type '%s' has no field '%s'%s",
-            EvalUtils.getDataTypeName(objValue), name, suffix));
+    EvalException ee =
+        new EvalException(
+            loc,
+            String.format(
+                "object of type '%s' has no %s '%s'%s",
+                EvalUtils.getDataTypeName(objValue), accessName, name, suffix));
+    if (toSuppress != null) {
+      ee.addSuppressed(toSuppress);
+    }
+    return ee;
+  }
+
+  /** Returns whether the given object has a method with the given name. */
+  static boolean hasMethod(Object obj, String name) {
+    Class<?> cls = obj instanceof Class ? (Class<?>) obj : obj.getClass();
+    if (Runtime.getBuiltinRegistry().getFunctionNames(cls).contains(name)) {
+      return true;
+    }
+
+    return FuncallExpression.getMethodNames(cls).contains(name);
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
index 97c582ad8d..7ab54a1abd 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -707,12 +707,12 @@ public class MethodLibrary {
   private static final BuiltinFunction hasattr =
       new BuiltinFunction("hasattr") {
         @SuppressWarnings("unused")
-        public Boolean invoke(Object obj, String name, Environment env)
-            throws EvalException {
+        public Boolean invoke(Object obj, String name, Environment env) throws EvalException {
           if (obj instanceof ClassObject && ((ClassObject) obj).getValue(name) != null) {
             return true;
           }
-          return hasMethod(obj, name);
+          // shouldn't this filter things with struct_field = false?
+          return DotExpression.hasMethod(obj, name);
         }
       };
 
@@ -748,35 +748,15 @@ public class MethodLibrary {
             throws EvalException, InterruptedException {
           Object result = DotExpression.eval(obj, name, loc, env);
           if (result == null) {
-            // 'Real' describes methods with structField() == false. Because DotExpression.eval
-            // returned null in this case, we know that structField() cannot return true.
-            boolean isRealMethod = hasMethod(obj, name);
             if (defaultValue != Runtime.UNBOUND) {
               return defaultValue;
             }
-            throw new EvalException(
-                loc,
-                Printer.format(
-                    "object of type '%s' has no attribute %r%s",
-                    EvalUtils.getDataTypeName(obj),
-                    name,
-                    isRealMethod ? ", however, a method of that name exists" : ""));
+            throw DotExpression.getMissingFieldException(obj, name, loc, "attribute");
           }
           return result;
         }
       };
 
-  /**
-   * Returns whether the given object has a method with the given name.
-   */
-  private static boolean hasMethod(Object obj, String name) throws EvalException {
-    if (Runtime.getBuiltinRegistry().getFunctionNames(obj.getClass()).contains(name)) {
-      return true;
-    }
-
-    return FuncallExpression.getMethodNames(obj.getClass()).contains(name);
-  }
-
   @SkylarkSignature(
     name = "dir",
     returnType = MutableList.class,