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

Support pattern matching #282

Closed
3 tasks
sobolevn opened this issue Sep 29, 2022 · 26 comments · Fixed by #3047
Closed
3 tasks

Support pattern matching #282

sobolevn opened this issue Sep 29, 2022 · 26 comments · Fixed by #3047

Comments

@sobolevn
Copy link
Contributor

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 floats
@charliermarsh
Copy link
Member

Yeah it makes some appearances in the RustPython AST but it doesn't seem to be supported yet by the actual parser.

@woodruffw
Copy link

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.

@woodruffw
Copy link

Oh, looks like someone beat me to it 🙂 RustPython/RustPython#4323

@charliermarsh
Copy link
Member

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 (RustPython/RustPython#4423) which would be a much bigger change. Then @andersk had augmented the parser to support parenthesized context managers (RustPython/RustPython#4352) and had mentioned perhaps looking into match statements next.

@charliermarsh
Copy link
Member

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 charlie.r.marsh@gmail.com :)

@JadHADDAD92
Copy link

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?

@charliermarsh
Copy link
Member

@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:

  1. 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.
  2. Always include E999 even if it's not explicitly enabled. This is tricky and complicates the rule-selection semantics.

@JadHADDAD92
Copy link

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.

@charliermarsh
Copy link
Member

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.

YDX-2147483647 added a commit to YDX-2147483647/watch-exams that referenced this issue Feb 2, 2023
Warning: match-case isn't support by ruff yet.
astral-sh/ruff#282
@charliermarsh
Copy link
Member

@JadHADDAD92 - No but thank you for the reminder -- I just created #2473, hopefully I can knock it out today.

@ofek
Copy link
Contributor

ofek commented Feb 11, 2023

What direction are we going in order to support this?

@charliermarsh
Copy link
Member

I'm working on it here: RustPython/RustPython#4519. There are a few problems to solve but that implementation already supports many of the variants.

@charliermarsh
Copy link
Member

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.

@phillipuniverse
Copy link
Contributor

phillipuniverse commented Feb 19, 2023

@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.

@SRv6d
Copy link

SRv6d commented Feb 19, 2023

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.

@ofek
Copy link
Contributor

ofek commented Feb 19, 2023

Also test if match := re.search(...):

@henribru
Copy link

henribru commented Feb 19, 2023

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.

@charliermarsh
Copy link
Member

Parser looking good so far. I got it to parse Black's fixtures which cover a lot of tricky cases.

@charliermarsh
Copy link
Member

(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.)

@charliermarsh
Copy link
Member

(Oh, I guess the Black test suite is a subet of test_patma.py :))

@charliermarsh
Copy link
Member

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 RustPython/RustPython#4519 in RustPython).

@danstewart
Copy link

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.

@mikeroll
Copy link

@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.

@danstewart
Copy link

Thanks, that's much better.

@charliermarsh
Copy link
Member

@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.

gertvdijk added a commit to gertvdijk/purepythonmilter that referenced this issue Feb 23, 2023
woodruffw added a commit to woodruffw/autopost that referenced this issue Mar 8, 2023
* pyproject, Makefile: use ruff

This can't be merged until ruff supports pattern
matching; see astral-sh/ruff#282.

* Remove more flake8 refs
YDX-2147483647 added a commit to YDX-2147483647/ruff that referenced this issue Mar 13, 2023
Structural pattern matching ([PEP 622](https://peps.python.org/pep-0622/), tracked by astral-sh#282) was implemented in astral-sh#3047, [v0.0.250](https://github.com/charliermarsh/ruff/releases/tag/v0.0.250).

Relates-to: astral-sh#3091 where the compatibility was (partially) re-added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants