clean-ups and finished tests

src/python/pants/engine/internals/parser.py

@@ -103,20 +103,24 @@ def parse(
         except NameError as e:
             valid_symbols = sorted(s for s in global_symbols.keys() if s != "__builtins__")
             original = e.args[0].capitalize()
-            err_string = f"""If you expect to see more symbols activated in the 
-                below list, refer to {docs_url("enabling_backends")} for all available
-                backends to activate."""
+            err_string = (
+                    "If you expect to see more symbols activated in the below list,"
+                    f"refer to {docs_url('enabling_backends')} for all available"
+                    " backends to activate."
+            )
             dym = "Did you mean "
 
+            # note that this only includes 3 matches at the most by default
             candidates = get_close_matches(build_file_content,valid_symbols)
             if len(candidates) == 1:
-                dym += candidates[0] + '?'
+                dym += candidates[0] + '?\n\n'
                 err_string = dym + err_string
             elif len(candidates) > 1:
-                dym += ", ".join(candidates)[:-1]
-                dym += ", or  " + candidates[-1]  # naturally, we use the oxford comma
+                dym += ", ".join(candidates[:-1])
+                # no oxford comma for len==2
+                dym += " or " + candidates[-1] + "?\n\n"
                 err_string = dym + err_string
-            raise ParseError(f"{err_string}\n\n{original}.\n\nAll registered symbols: {valid_symbols}")
+            raise ParseError(f"{original}.\n\n{err_string}\n\nAll registered symbols: {valid_symbols}")
 
         error_on_imports(build_file_content, filepath)
 

src/python/pants/engine/internals/parser_test.py

@@ -3,11 +3,12 @@
 
 import pytest
 
+
 from pants.build_graph.build_file_aliases import BuildFileAliases
 from pants.engine.internals.parser import BuildFilePreludeSymbols, ParseError, Parser
+from pants.util.docutil import docs_url
 from pants.util.frozendict import FrozenDict
 
-
 def test_imports_banned() -> None:
     parser = Parser(target_type_aliases=[], object_aliases=BuildFileAliases())
     with pytest.raises(ParseError) as exc:
@@ -17,18 +18,70 @@ def test_imports_banned() -> None:
     assert "Import used in dir/BUILD at line 4" in str(exc.value)
 
 
-def test_unrecognized_symbol() -> None:
-    parser = Parser(
-        target_type_aliases=["tgt"],
-        object_aliases=BuildFileAliases(
-            objects={"obj": 0},
-            context_aware_object_factories={"caof": lambda parse_context: lambda _: None},
-        ),
+def test_unrecogonized_symbol() -> None:
+    def perform_test(tta: list, good_err: str) -> None:
+        parser = Parser(
+            target_type_aliases=tta,
+            object_aliases=BuildFileAliases(
+                objects={"obj": 0},
+                context_aware_object_factories={"caof": lambda parse_context: lambda _: None},
+            ),
+        )
+        prelude_symbols = BuildFilePreludeSymbols(FrozenDict({"prelude": 0}))
+        with pytest.raises(ParseError) as exc:
+            parser.parse("dir/BUILD", "fake", prelude_symbols)
+        assert (
+            str(exc.value)
+            == good_err
     )
-    prelude_symbols = BuildFilePreludeSymbols(FrozenDict({"prelude": 0}))
-    with pytest.raises(ParseError) as exc:
-        parser.parse("dir/BUILD", "fake", prelude_symbols)
-    assert (
-        str(exc.value)
-        == "Name 'fake' is not defined.\n\nAll registered symbols: ['caof', 'obj', 'prelude', 'tgt']"
+
+    # confirm that it works if there's nothing similar
+    no_match_tta = ["tgt"]
+    no_match_str = ("Name 'fake' is not defined.\n\n"
+                    "If you expect to see more symbols activated in the below list,"
+                    f"refer to {docs_url('enabling_backends')} for all available"
+                    " backends to activate.\n\n"
+                    "All registered symbols: ['caof', 'obj', 'prelude', 'tgt']"
+    )
+
+    # confirm that "did you mean x" works
+    one_match_tta = ["tgt","fake1"]
+    one_match_str = ("Name 'fake' is not defined.\n\n"
+                    "Did you mean fake1?\n\n"
+                    "If you expect to see more symbols activated in the below list,"
+                    f"refer to {docs_url('enabling_backends')} for all available"
+                    " backends to activate.\n\n"
+                    "All registered symbols: ['caof', 'fake1', 'obj', 'prelude', 'tgt']"
+    )
+
+
+    # confirm that "did you mean x or y" works
+    two_match_tta = ["tgt","fake1","fake2"]
+    two_match_str = ("Name 'fake' is not defined.\n\n"
+                    "Did you mean fake2 or fake1?\n\n"
+                    "If you expect to see more symbols activated in the below list,"
+                    f"refer to {docs_url('enabling_backends')} for all available"
+                    " backends to activate.\n\n"
+                    "All registered symbols: ['caof', 'fake1', 'fake2', 'obj', 'prelude', 'tgt']"
     )
+
+
+    # confirm that "did you mean x, y or z" works (limit to 3 match)
+    many_match_tta = ["tgt","fake1","fake2","fake3","fake4","fake5"]
+    many_match_str = ("Name 'fake' is not defined.\n\n"
+                    "Did you mean fake5, fake4 or fake3?\n\n"
+                    "If you expect to see more symbols activated in the below list,"
+                    f"refer to {docs_url('enabling_backends')} for all available"
+                    " backends to activate.\n\n"
+                    "All registered symbols: ['caof', 'fake1', 'fake2',"
+                    " 'fake3', 'fake4', 'fake5', 'obj', 'prelude', 'tgt']"
+    )
+
+
+    perform_test(no_match_tta,no_match_str)
+    perform_test(one_match_tta,one_match_str)
+    perform_test(two_match_tta,two_match_str)
+    perform_test(many_match_tta,many_match_str)
+
+
+