gh-115077: Argument Clinic: generate better error messages when parsing function declaration by erlend-aasland · Pull Request #115555 · python/cpython · GitHub
Skip to content

gh-115077: Argument Clinic: generate better error messages when parsing function declaration #115555

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
erlend-aasland wants to merge 8 commits into from
+171 −70
Closed

gh-115077: Argument Clinic: generate better error messages when parsing function declaration #115555

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fe3b8ca
Use a lexer to generate better error messages for invalid syntax
erlend-aasland Feb 15, 2024
c926abc
Extend test suite
erlend-aasland Feb 16, 2024
33c21e7
Validate cloned name post parsing
erlend-aasland Feb 16, 2024
60553b7
Remove now obsoleted comment
erlend-aasland Feb 16, 2024
9b93771
Use regex instead; compromise by not detecting some edge cases
erlend-aasland Feb 16, 2024
1cc7248
Add parser.py
erlend-aasland Feb 16, 2024
e8c1de3
Pull in main
erlend-aasland Feb 16, 2024
3c83c5d
Detect more cases of invalid syntax
erlend-aasland Feb 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 81 additions & 30 deletions Lib/test/test_clinic.py
View file
Open in desktop
123 changes: 83 additions & 40 deletions Tools/clinic/clinic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
Any,
Final,
Literal,
NamedTuple,
NoReturn,
Protocol,
TypeVar,
Expand All @@ -52,6 +51,12 @@
import libclinic
import libclinic.cpp
from libclinic import ClinicError
from libclinic.parser import (
RE_CLONE,
RE_C_BASENAME,
RE_FULLNAME,
RE_RETURNS,
)


# TODO:
Expand Down Expand Up @@ -4757,11 +4762,6 @@ class ParamState(enum.IntEnum):
RIGHT_SQUARE_AFTER = 6


class FunctionNames(NamedTuple):
full_name: str
c_basename: str


class DSLParser:
function: Function | None
state: StateKeeper
Expand Down Expand Up @@ -5047,25 +5047,6 @@ def state_dsl_start(self, line: str) -> None:

self.next(self.state_modulename_name, line)

def parse_function_names(self, line: str) -> FunctionNames:
left, as_, right = line.partition(' as ')
full_name = left.strip()
c_basename = right.strip()
if as_ and not c_basename:
fail("No C basename provided after 'as' keyword")
if not c_basename:
fields = full_name.split(".")
if fields[-1] == '__new__':
fields.pop()
c_basename = "_".join(fields)
if not libclinic.is_legal_py_identifier(full_name):
fail(f"Illegal function name: {full_name!r}")
if not libclinic.is_legal_c_identifier(c_basename):
fail(f"Illegal C basename: {c_basename!r}")
names = FunctionNames(full_name=full_name, c_basename=c_basename)
self.normalize_function_kind(names.full_name)
return names

def normalize_function_kind(self, fullname: str) -> None:
# Fetch the method name and possibly class.
fields = fullname.split('.')
Expand All @@ -5089,7 +5070,7 @@ def normalize_function_kind(self, fullname: str) -> None:
self.kind = METHOD_INIT

def resolve_return_converter(
self, full_name: str, forced_converter: str
self, full_name: str, forced_converter: str | None
) -> CReturnConverter:
if forced_converter:
if self.kind in {GETTER, SETTER}:
Expand All @@ -5115,8 +5096,12 @@ def resolve_return_converter(
return init_return_converter()
return CReturnConverter()

def parse_cloned_function(self, names: FunctionNames, existing: str) -> None:
full_name, c_basename = names
def parse_cloned_function(
self,
full_name: str,
c_basename: str,
existing: str
) -> None:
fields = [x.strip() for x in existing.split('.')]
function_name = fields.pop()
module, cls = self.clinic._module_and_class(fields)
Expand Down Expand Up @@ -5158,6 +5143,70 @@ def parse_cloned_function(self, names: FunctionNames, existing: str) -> None:
(cls or module).functions.append(function)
self.next(self.state_function_docstring)

@staticmethod
def generate_c_basename(full_name: str) -> str:
fields = full_name.split(".")
if fields[-1] == '__new__':
fields.pop()
return "_".join(fields)

def parse_declaration(
self, line: str
) -> tuple[str, str, str | None, str | None]:
cloned = None
returns = None

def invalid_syntax(msg: str | None = None) -> NoReturn:
preamble = "Invalid syntax"
if msg:
preamble += f" ({msg})"
fail(f"{preamble}: {line!r}\n\n"
"Allowed syntax:\n"
"[module.[submodule.]][class.]func [as c_name] [-> return_annotation]\n\n"
"Nested submodules are allowed.")

m = RE_FULLNAME.match(line)
assert m
full_name = m[1]
if not libclinic.is_legal_py_identifier(full_name):
fail(f"Illegal function name: {full_name!r}")
pos = m.end()

m = RE_C_BASENAME.match(line, pos)
if m:
if not m[1]:
fail(f"No C basename provided for {full_name!r} after 'as' keyword")
c_basename = m[1]
if not libclinic.is_legal_c_identifier(c_basename):
fail(f"Illegal C basename: {c_basename!r}")
pos = m.end()
else:
c_basename = self.generate_c_basename(full_name)

m = RE_CLONE.match(line, pos)
if m:
if not m[1]:
fail(f"No source function provided for {full_name!r} after '=' keyword")
cloned = m[1]
if not libclinic.is_legal_py_identifier(cloned):
fail(f"Illegal source function name: {cloned!r}")
pos = m.end()

m = RE_RETURNS.match(line, pos)
if m:
if cloned:
invalid_syntax()
if not m[1]:
fail(f"No return annotation provided for {full_name!r} after '->' keyword")
returns = m[1].strip()
Comment thread
erlend-aasland marked this conversation as resolved.
pos = m.end()

if pos != len(line):
invalid_syntax()

self.normalize_function_kind(full_name)
return full_name, c_basename, cloned, returns

def state_modulename_name(self, line: str) -> None:
# looking for declaration, which establishes the leftmost column
# line should be
Expand All @@ -5178,20 +5227,14 @@ def state_modulename_name(self, line: str) -> None:
assert self.valid_line(line)
self.indent.infer(line)

# are we cloning?
before, equals, existing = line.rpartition('=')
if equals:
existing = existing.strip()
if libclinic.is_legal_py_identifier(existing):
# we're cloning!
names = self.parse_function_names(before)
return self.parse_cloned_function(names, existing)

line, _, returns = line.partition('->')
returns = returns.strip()
full_name, c_basename = self.parse_function_names(line)
full_name, c_basename, cloned, returns = self.parse_declaration(line)

if cloned:
return self.parse_cloned_function(full_name, c_basename, cloned)

return_converter = self.resolve_return_converter(full_name, returns)

# Split out function name, and determine module and class.
fields = [x.strip() for x in full_name.split('.')]
function_name = fields.pop()
module, cls = self.clinic._module_and_class(fields)
Expand Down
7 changes: 7 additions & 0 deletions Tools/clinic/libclinic/parser.py
View file
Open in desktop