ruff
ruff copied to clipboard
Support pattern matching
As I am learning the source code, it seems to me that python's pattern matching is not supported at all.
We have a visitor ready: https://github.com/charliermarsh/ruff/blob/c7349b69c12b7511d48fc45390eac8d7282062a5/src/ast/visitor.rs#L527-L571
But, that's about it.
Things to support (feel free to update):
- [ ] Unused variables check
- [ ] Variable names check (
MatchAs
pattern) - [ ] Custom rules, like do not match
float
s
Yeah it makes some appearances in the RustPython AST but it doesn't seem to be supported yet by the actual parser.
The parser in question is rustpython-parser
, correct? No promises, but I can take a look at its current state and see whether I can contribute support for the match
syntax.
Oh, looks like someone beat me to it :slightly_smiling_face: https://github.com/RustPython/RustPython/pull/4323
Yeah that's the one! There's been a little bit of progress in that PR. There's also someone who's been looking to migrate the entire parser to rust-peg
(https://github.com/RustPython/RustPython/pull/4423) which would be a much bigger change. Then @andersk had augmented the parser to support parenthesized context managers (https://github.com/RustPython/RustPython/pull/4352) and had mentioned perhaps looking into match statements next.
I'd love to sponsor someone (financially, that is) to work on this issue (in RustPython). If you're interested, shoot me a DM on Twitter or email me at [email protected] :)
Actually when there is a match
expression in a python module, ruff fails silently?
I spent an hour debugging why ruff isn't sorting my imports even though the iSort rules are enabled, when I commented out the match expression ruff was able to sort again, is that expected?
@JadHADDAD92 - So sorry that you lost time to this. It is the expected behavior, because syntax errors have their own error code (E999
). So if you don't have that code enabled (e.g., if you do ruff foo --select I
), you won't see those errors at all. It's consistent with how Ruff works, but I agree it's confusing. (Flake8 does have the same behavior.)
A few alternatives:
- Always log an error (separate from a lint violation) if we fail to parse a file. It's hard to imagine this being unhelpful ever.
- Always include
E999
even if it's not explicitly enabled. This is tricky and complicates the rule-selection semantics.
I find the first alternative to be the better behavior, because I presume all ruff rules depend on a successful parse of RustPython (right?) therefore if we lint a module which contains a syntax error (failed to parse), and ruff returns 0 as if the file is well linted, it brings no clarity of what happened.
Not all rules depend on a successful parse. Some can work off the token stream, or even partial stream. Some rules only look at raw lines (like the line-length violations). And some work off the filesystem (e.g., the rule to detect missing __init__.py
files). The majority do, but some rules can complete without one.
We could: always log an error
message if a file fails to parse + exit 1.
@charliermarsh is there an issue for this (logging an error when failing to parse) that I can subscribe to?
@JadHADDAD92 - No but thank you for the reminder -- I just created #2473, hopefully I can knock it out today.
What direction are we going in order to support this?
I'm working on it here: https://github.com/RustPython/RustPython/pull/4519. There are a few problems to solve but that implementation already supports many of the variants.
I'm making good progress on this (I think?). If anyone wants to link to an OSS codebase that makes good use of match statements (for testing), it'd be appreciated.
@charliermarsh there are good match examples in the 2nd edition Fluent Python book. This snippet looks like it would catch some edge cases. In fact, if you search in that repo for the word “match” you’ll get examples of that word being used as a keyword as well as a normal variable.
Pyright has some good examples.
https://github.com/microsoft/pyright/blob/main/packages/pyright-internal/src/tests/samples/match1.py for example or any other match*.py
file in that directory.
Also test if match := re.search(...):
Python's own test suite for pattern matching seems like a good litmus test: https://github.com/python/cpython/blob/main/Lib/test/test_patma.py
https://github.com/brandtbucher/patmaperformance also has some examples.
Parser looking good so far. I got it to parse Black's fixtures which cover a lot of tricky cases.
(It also handles soft keywords as pointed out by @ofek. I've also run it over Hatch, Airflow, and Pandas, and confirmed that the output is unchanged, so it's not incorrectly detecting any match
statements from e.g. variable assignments.)
(Oh, I guess the Black test suite is a subet of test_patma.py
:))
I expect this to go out some time this week, hopefully in the next few days. The PR is up as #3047 (plus depends on https://github.com/RustPython/RustPython/pull/4519 in RustPython).
Thank you!
Heads up for anyone else using the VS code extension you will need to set your ruff path:
"ruff.path": ["./.venv/bin/ruff"]
Looks like the extension has it's own ruff executable, which has already been updated to use the latest ruff but hasn't been released yet.
@danstewart most likely you want this instead:
"ruff.importStrategy": "fromEnvironment"
which will find ruff
in whatever the venv associated with the vscode workspace is, without having to hardcode the path.
Thanks, that's much better.
@danstewart - I cut a new pre-release last night that includes v0.0.251, so you can pull it in if you switch to the pre-release channel. I'll probably cut a main-channel release today or tomorrow.