Skip to content
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

Jump to bottom

gh-104683: Argument Clinic: Modernise parse_special_symbol() #106837

Merged
merged 8 commits into from
Jul 17, 2023
Merged

gh-104683: Argument Clinic: Modernise parse_special_symbol() #106837

Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
230b85f
Annotate
erlend-aasland Jul 3, 2023
98273d4
Refactor: extract functions
erlend-aasland Jul 3, 2023
6994d09
Fix mypy warnings
erlend-aasland Jul 17, 2023
8e36b51
Remove parse_special_symbol()
erlend-aasland Jul 17, 2023
b402b29
Pull in main
erlend-aasland Jul 17, 2023
1a08f13
Address code review: exhaustive match
AlexWaygood Jul 17, 2023
deb05a2
Address review
erlend-aasland Jul 17, 2023
b0ae589
Apply suggestions from code review
erlend-aasland Jul 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
136 changes: 80 additions & 56 deletions Tools/clinic/clinic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4864,11 +4864,21 @@ def state_parameter(self, line: str | None) -> None:
self.parameter_continuation = line[:-1]
return

line = line.lstrip()

if line in ('*', '/', '[', ']'):
self.parse_special_symbol(line)
return
func = self.function
match line.lstrip():
case '*':
self.parse_star(func)
case '[':
self.parse_opening_square_bracket(func)
case ']':
self.parse_closing_square_bracket(func)
case '/':
self.parse_slash(func)
case param:
self.parse_parameter(param)

def parse_parameter(self, line: str) -> None:
assert self.function is not None

match self.parameter_state:
case ParamState.START | ParamState.REQUIRED:
Expand Down Expand Up @@ -5146,57 +5156,71 @@ def parse_converter(
"Annotations must be either a name, a function call, or a string."
)

def parse_special_symbol(self, symbol):
if symbol == '*':
if self.keyword_only:
fail("Function " + self.function.name + " uses '*' more than once.")
self.keyword_only = True
elif symbol == '[':
match self.parameter_state:
case ParamState.START | ParamState.LEFT_SQUARE_BEFORE:
self.parameter_state = ParamState.LEFT_SQUARE_BEFORE
case ParamState.REQUIRED | ParamState.GROUP_AFTER:
self.parameter_state = ParamState.GROUP_AFTER
case st:
fail(f"Function {self.function.name} has an unsupported group configuration. (Unexpected state {st}.b)")
self.group += 1
self.function.docstring_only = True
elif symbol == ']':
if not self.group:
fail("Function " + self.function.name + " has a ] without a matching [.")
if not any(p.group == self.group for p in self.function.parameters.values()):
fail("Function " + self.function.name + " has an empty group.\nAll groups must contain at least one parameter.")
self.group -= 1
match self.parameter_state:
case ParamState.LEFT_SQUARE_BEFORE | ParamState.GROUP_BEFORE:
self.parameter_state = ParamState.GROUP_BEFORE
case ParamState.GROUP_AFTER | ParamState.RIGHT_SQUARE_AFTER:
self.parameter_state = ParamState.RIGHT_SQUARE_AFTER
case st:
fail(f"Function {self.function.name} has an unsupported group configuration. (Unexpected state {st}.c)")
elif symbol == '/':
if self.positional_only:
fail("Function " + self.function.name + " uses '/' more than once.")
self.positional_only = True
# REQUIRED and OPTIONAL are allowed here, that allows positional-only without option groups
# to work (and have default values!)
allowed = {
ParamState.REQUIRED,
ParamState.OPTIONAL,
ParamState.RIGHT_SQUARE_AFTER,
ParamState.GROUP_BEFORE,
}
if (self.parameter_state not in allowed) or self.group:
fail(f"Function {self.function.name} has an unsupported group configuration. (Unexpected state {self.parameter_state}.d)")
if self.keyword_only:
fail("Function " + self.function.name + " mixes keyword-only and positional-only parameters, which is unsupported.")
# fixup preceding parameters
for p in self.function.parameters.values():
if p.is_vararg():
continue
if (p.kind != inspect.Parameter.POSITIONAL_OR_KEYWORD and not isinstance(p.converter, self_converter)):
fail("Function " + self.function.name + " mixes keyword-only and positional-only parameters, which is unsupported.")
p.kind = inspect.Parameter.POSITIONAL_ONLY
def parse_star(self, function: Function) -> None:
"""Parse keyword-only parameter marker '*'."""
if self.keyword_only:
fail(f"Function {function.name} uses '*' more than once.")
self.keyword_only = True

def parse_opening_square_bracket(self, function: Function) -> None:
"""Parse opening parameter group symbol '['."""
match self.parameter_state:
case ParamState.START | ParamState.LEFT_SQUARE_BEFORE:
self.parameter_state = ParamState.LEFT_SQUARE_BEFORE
case ParamState.REQUIRED | ParamState.GROUP_AFTER:
self.parameter_state = ParamState.GROUP_AFTER
case st:
fail(f"Function {function.name} has an unsupported group configuration. "
f"(Unexpected state {st}.b)")
self.group += 1
function.docstring_only = True

def parse_closing_square_bracket(self, function: Function) -> None:
"""Parse closing parameter group symbol '['."""
erlend-aasland marked this conversation as resolved.
Show resolved Hide resolved
if not self.group:
fail(f"Function {function.name} has a ] without a matching [.")
if not any(p.group == self.group for p in function.parameters.values()):
fail(f"Function {function.name} has an empty group.\n"
"All groups must contain at least one parameter.")
self.group -= 1
match self.parameter_state:
case ParamState.LEFT_SQUARE_BEFORE | ParamState.GROUP_BEFORE:
self.parameter_state = ParamState.GROUP_BEFORE
case ParamState.GROUP_AFTER | ParamState.RIGHT_SQUARE_AFTER:
self.parameter_state = ParamState.RIGHT_SQUARE_AFTER
case st:
fail(f"Function {function.name} has an unsupported group configuration. "
f"(Unexpected state {st}.c)")

def parse_slash(self, function: Function) -> None:
"""Parse positional-only parameter marker '/'."""
if self.positional_only:
fail(f"Function {function.name} uses '/' more than once.")
self.positional_only = True
# REQUIRED and OPTIONAL are allowed here, that allows positional-only
# without option groups to work (and have default values!)
allowed = {
ParamState.REQUIRED,
ParamState.OPTIONAL,
ParamState.RIGHT_SQUARE_AFTER,
ParamState.GROUP_BEFORE,
}
if (self.parameter_state not in allowed) or self.group:
fail(f"Function {function.name} has an unsupported group configuration. "
f"(Unexpected state {self.parameter_state}.d)")
if self.keyword_only:
fail(f"Function {function.name} mixes keyword-only and "
"positional-only parameters, which is unsupported.")
# fixup preceding parameters
for p in function.parameters.values():
if p.is_vararg():
continue
if (p.kind != inspect.Parameter.POSITIONAL_OR_KEYWORD and
erlend-aasland marked this conversation as resolved.
Show resolved Hide resolved
not isinstance(p.converter, self_converter)
):
fail(f"Function {function.name} mixes keyword-only and "
"positional-only parameters, which is unsupported.")
erlend-aasland marked this conversation as resolved.
Show resolved Hide resolved
p.kind = inspect.Parameter.POSITIONAL_ONLY

def state_parameter_docstring_start(self, line: str | None) -> None:
self.parameter_docstring_indent = len(self.indent.margin)
Expand Down