Skip to content

Commit

Permalink
Improve error message for unrecognized BUILD file symbols (pantsbuild…
Browse files Browse the repository at this point in the history
…#11564)

Improved error message for unrecognized symbol (Closes pantsbuild#11258) and associated test cases.

In particular, error messages are updated to include a "Did you mean...?" message along with a reference to the docs to show potential backends.

The "Did you mean...?" component is dependent on the number of potential symbols that could be what the user intended. This component is excluded if there are no potential matches, includes the singular match if there is one, includes "x or y" for two, and has "x,y or z" for multiple - essentially, they're comma-delimited without an Oxford comma for the final. 

This follows the original "Name ____ is not defined" message if present. The error component linking to the docs ("If you expect to see more symbols activated in the below list, refer to {doc link about enabling backends} for all available backends to activate") follows this, and is finally followed by the original list of all registered symbols.

[ci skip-rust]
  • Loading branch information
wilsonliam authored Feb 18, 2021
1 parent ef8d4d6 commit c9d6826
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 16 deletions.
21 changes: 20 additions & 1 deletion src/python/pants/engine/internals/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import os.path
import tokenize
from dataclasses import dataclass
from difflib import get_close_matches
from io import StringIO
from typing import Any, Dict, Iterable, List, Tuple, cast

Expand Down Expand Up @@ -90,6 +91,7 @@ def parse(
# those extra symbols will not see our target aliases etc. This also means that if multiple
# prelude files are present, they probably cannot see each others' symbols. We may choose
# to change this at some point.

global_symbols = dict(self._symbols)
for k, v in extra_symbols.symbols.items():
if hasattr(v, "__globals__"):
Expand All @@ -101,7 +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()
raise ParseError(f"{original}.\n\nAll registered symbols: {valid_symbols}")
help_str = (
"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."
)

candidates = get_close_matches(build_file_content, valid_symbols)
if candidates:
if len(candidates) == 1:
formatted_candidates = candidates[0]
elif len(candidates) == 2:
formatted_candidates = " or ".join(candidates)
else:
formatted_candidates = f"{', '.join(candidates[:-1])}, or {candidates[-1]}"
help_str = f"Did you mean {formatted_candidates}?\n\n" + help_str
raise ParseError(
f"{original}.\n\n{help_str}\n\nAll registered symbols: {valid_symbols}"
)

error_on_imports(build_file_content, filepath)

Expand Down
49 changes: 34 additions & 15 deletions src/python/pants/engine/internals/parser_test.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import annotations

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


Expand All @@ -17,18 +20,34 @@ 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},
),
)
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']"
)
def test_unrecogonized_symbol() -> None:
def perform_test(extra_targets: list[str], dym: str) -> None:

parser = Parser(
target_type_aliases=["tgt", *extra_targets],
object_aliases=BuildFileAliases(
objects={"obj": 0},
context_aware_object_factories={"caof": lambda parse_context: lambda _: None},
),
)
prelude_symbols = BuildFilePreludeSymbols(FrozenDict({"prelude": 0}))
fmt_extra_sym = str(extra_targets)[1:-1] + (", ") if len(extra_targets) != 0 else ""
with pytest.raises(ParseError) as exc:
parser.parse("dir/BUILD", "fake", prelude_symbols)
assert str(exc.value) == (
f"Name 'fake' is not defined.\n\n{dym}"
"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"
f"All registered symbols: ['caof', {fmt_extra_sym}'obj', 'prelude', 'tgt']"
)

test_targs = ["fake1", "fake2", "fake3", "fake4", "fake5"]

perform_test([], "")
dym_one = "Did you mean fake1?\n\n"
perform_test(test_targs[:1], dym_one)
dym_two = "Did you mean fake2 or fake1?\n\n"
perform_test(test_targs[:2], dym_two)
dym_many = "Did you mean fake5, fake4, or fake3?\n\n"
perform_test(test_targs, dym_many)

0 comments on commit c9d6826

Please sign in to comment.