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.

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

Parameter range is incorrect for *args and **kwargs #10282

Closed
GtrMo opened this issue Mar 7, 2024 · 1 comment · Fixed by #10283
Closed

Parameter range is incorrect for *args and **kwargs #10282

GtrMo opened this issue Mar 7, 2024 · 1 comment · Fixed by #10283
Labels
internal An internal refactor or improvement

Comments

@GtrMo
Copy link
Contributor

GtrMo commented Mar 7, 2024

I'm working on a fix for the flake8-unused-arguments (ARG) rules that would delete them.

While working on this, I noticed that the *args and **kwargs Parameter.range does not include the *.
I think that Parameter.range should include * or **, leaving Parameter.name.range for the actual name range if needed.

def f(*args, **kwargs): pass
#      ~~~~    ~~~~~~    <-- actual range
#     ^^^^^  ^^^^^^^^    <-- expected range
See the currently generated AST

vararg: Some(
Parameter {
range: 16..20,
name: Identifier {
id: "args",
range: 16..20,
},
annotation: None,
},
),

Here I would expect the Parameter range to be 1 longer than the Identifier range.

kwarg: Some(
Parameter {
range: 39..45,
name: Identifier {
id: "kwargs",
range: 39..45,
},
annotation: None,
},
),
},

And here I would expect the Parameter range to be 2 longer than the Identifier range.

If this is indeed considered a bug, I have a PR ready to fix it.

@charliermarsh
Copy link
Member

I generally agree, since x: int = 1 is the parameter range for a non-variadic parameter, it makes sense that the * and ** would be included. CPython doesn't seem to include it in the range, but our AST structure is a bit different...

@charliermarsh charliermarsh added the internal An internal refactor or improvement label Mar 7, 2024
nkxxll pushed a commit to nkxxll/ruff that referenced this issue Mar 10, 2024
…#10283)

## Summary

Fix astral-sh#10282 

This PR updates the Python grammar to include the `*` character in
`*args` `**kwargs` in the range of the `Parameter`
```
def f(*args, **kwargs): pass
#      ~~~~    ~~~~~~    <-- range before the PR
#     ^^^^^  ^^^^^^^^    <-- range after
```

The invalid syntax `def f(*, **kwargs): ...` is also now correctly
reported.

## Test Plan

Test cases were added to `function.rs`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants