Add 'did you mean' suggestion when accessing a struct field -- PiperOrigin-RevId: 143380643 MOS_MIGRATED_REVID=143380643
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java index dc84c5d..8296c36 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Package.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -540,12 +540,7 @@ suffix = "; however, a source file of this name exists. (Perhaps add " + "'exports_files([\"" + targetName + "\"])' to " + name + "/BUILD?)"; } else { - String suggestion = SpellChecker.suggest(targetName, targets.keySet()); - if (suggestion != null) { - suffix = " (did you mean '" + suggestion + "'?)"; - } else { - suffix = ""; - } + suffix = SpellChecker.didYouMean(targetName, targets.keySet()); } throw makeNoSuchTargetException(targetName, suffix);
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 5d94498..ec5a8a8 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
@@ -20,6 +20,7 @@ import com.google.devtools.build.lib.syntax.compiler.ByteCodeUtils; import com.google.devtools.build.lib.syntax.compiler.DebugInfo; import com.google.devtools.build.lib.syntax.compiler.VariableScope; +import com.google.devtools.build.lib.util.SpellChecker; import java.util.ArrayList; import java.util.List; import net.bytebuddy.implementation.bytecode.ByteCodeAppender; @@ -64,19 +65,22 @@ */ public static Object checkResult(Object objValue, Object result, String name, Location loc) throws EvalException { - if (result == null) { - if (objValue instanceof ClassObject) { - String customErrorMessage = ((ClassObject) objValue).errorMessage(name); - if (customErrorMessage != null) { - throw new EvalException(loc, customErrorMessage); - } - } - throw new EvalException( - loc, - Printer.format( - "object of type '%s' has no field %r", EvalUtils.getDataTypeName(objValue), name)); + if (result != null) { + return result; } - return result; + String suffix = ""; + if (objValue instanceof ClassObject) { + String customErrorMessage = ((ClassObject) objValue).errorMessage(name); + if (customErrorMessage != null) { + throw new EvalException(loc, customErrorMessage); + } + suffix = SpellChecker.didYouMean(name, ((ClassObject) objValue).getKeys()); + } + throw new EvalException( + loc, + String.format( + "object of type '%s' has no field '%s'%s", + EvalUtils.getDataTypeName(objValue), name, suffix)); } /** @@ -102,9 +106,10 @@ } } - Iterable<MethodDescriptor> methods = objValue instanceof Class<?> - ? FuncallExpression.getMethods((Class<?>) objValue, name, loc) - : FuncallExpression.getMethods(objValue.getClass(), name, loc); + Iterable<MethodDescriptor> methods = + objValue instanceof Class<?> + ? FuncallExpression.getMethods((Class<?>) objValue, name) + : FuncallExpression.getMethods(objValue.getClass(), name); if (methods != null) { methods =
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java index 30b9417..e3468da 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -279,15 +279,14 @@ } /** - * Returns the list of Skylark callable Methods of objClass with the given name - * and argument number. + * Returns the list of Skylark callable Methods of objClass with the given name and argument + * number. */ - public static List<MethodDescriptor> getMethods(Class<?> objClass, String methodName, - Location loc) throws EvalException { + public static List<MethodDescriptor> getMethods(Class<?> objClass, String methodName) { try { return methodCache.get(objClass).get(methodName); } catch (ExecutionException e) { - throw new EvalException(loc, "method invocation failed: " + e); + throw new IllegalStateException("method invocation failed: " + e); } } @@ -295,8 +294,12 @@ * Returns a set of the Skylark name of all Skylark callable methods for object of type {@code * objClass}. */ - public static Set<String> getMethodNames(Class<?> objClass) throws ExecutionException { - return methodCache.get(objClass).keySet(); + public static Set<String> getMethodNames(Class<?> objClass) { + try { + return methodCache.get(objClass).keySet(); + } catch (ExecutionException e) { + throw new IllegalStateException("method invocation failed: " + e); + } } static Object callMethod(MethodDescriptor methodDescriptor, String methodName, Object obj, @@ -357,7 +360,7 @@ Class<?> objClass, String methodName, List<Object> args, Map<String, Object> kwargs) throws EvalException { Pair<MethodDescriptor, List<Object>> matchingMethod = null; - List<MethodDescriptor> methods = getMethods(objClass, methodName, getLocation()); + List<MethodDescriptor> methods = getMethods(objClass, methodName); ArgumentListConversionResult argumentListConversionResult = null; if (methods != null) { for (MethodDescriptor method : methods) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java index 83335e2..a6c1708 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
@@ -95,14 +95,7 @@ if (name.equals("$error$")) { return new EvalException(getLocation(), "contains syntax error(s)", true); } - - String suggestion = SpellChecker.suggest(name, symbols); - if (suggestion == null) { - suggestion = ""; - } else { - suggestion = " (did you mean '" + suggestion + "'?)"; - } - + String suggestion = SpellChecker.didYouMean(name, symbols); return new EvalException(getLocation(), "name '" + name + "' is not defined" + suggestion); }
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 415d427..572b281 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
@@ -39,7 +39,6 @@ import java.util.NoSuchElementException; import java.util.Set; import java.util.TreeSet; -import java.util.concurrent.ExecutionException; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -1960,12 +1959,7 @@ return true; } - try { - return FuncallExpression.getMethodNames(obj.getClass()).contains(name); - } catch (ExecutionException e) { - // This shouldn't happen - throw new EvalException(loc, e.getMessage()); - } + return FuncallExpression.getMethodNames(obj.getClass()).contains(name); } @SkylarkSignature( @@ -1988,12 +1982,7 @@ fields.addAll(((ClassObject) object).getKeys()); } fields.addAll(Runtime.getFunctionNames(object.getClass())); - try { - fields.addAll(FuncallExpression.getMethodNames(object.getClass())); - } catch (ExecutionException e) { - // This shouldn't happen - throw new EvalException(loc, e.getMessage()); - } + fields.addAll(FuncallExpression.getMethodNames(object.getClass())); return new MutableList(fields, env); } };
diff --git a/src/main/java/com/google/devtools/build/lib/util/SpellChecker.java b/src/main/java/com/google/devtools/build/lib/util/SpellChecker.java index b8fda8f..2671a90 100644 --- a/src/main/java/com/google/devtools/build/lib/util/SpellChecker.java +++ b/src/main/java/com/google/devtools/build/lib/util/SpellChecker.java
@@ -94,4 +94,17 @@ } return best; } + + /** + * Return a string to be used at the end of an error message. It is either an empty string, or a + * spelling suggestion, e.g. " (did you mean 'x'?)". + */ + public static String didYouMean(String input, Iterable<String> words) { + String suggestion = suggest(input, words); + if (suggestion == null) { + return ""; + } else { + return " (did you mean '" + suggestion + "'?)"; + } + } }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java index 38874b8b..92d962a 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -212,7 +212,7 @@ case "field": return "a"; case "nset": return NestedSetBuilder.stableOrder().build(); } - throw new IllegalStateException(); + return null; } @Override @@ -822,7 +822,16 @@ public void testStructAccessOfMethod() throws Exception { new SkylarkTest() .update("mock", new Mock()) - .testIfExactError("object of type 'Mock' has no field \"function\"", "v = mock.function"); + .testIfExactError("object of type 'Mock' has no field 'function'", "v = mock.function"); + } + + @Test + public void testStructAccessTypo() throws Exception { + new SkylarkTest() + .update("mock", new MockClassObject()) + .testIfExactError( + "object of type 'MockClassObject' has no field 'fild' (did you mean 'field'?)", + "mock.fild"); } @Test @@ -926,7 +935,7 @@ @Test public void testDotExpressionOnNonStructObject() throws Exception { new SkylarkTest() - .testIfExactError("object of type 'string' has no field \"field\"", "x = 'a'.field"); + .testIfExactError("object of type 'string' has no field 'field'", "x = 'a'.field"); } @Test