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

feat: add support for pattern matching #140

Merged
merged 2 commits into from
Jan 28, 2022

Conversation

theHamsta
Copy link
Contributor

@theHamsta
Copy link
Contributor Author

I need to solve this somehow with patterns (including "or" and "as" patterns)

@theHamsta theHamsta force-pushed the pattern_matching branch 4 times, most recently from 314a28e to 2e38411 Compare December 3, 2021 19:10
@@ -522,6 +534,14 @@ module.exports = grammar({
choice($.identifier, $.keyword_identifier, $.subscript, $.attribute)
),

// Extended patterns (patterns allowed in match statement are far more flexible than simple patterns

as_pattern: $ => prec.left(seq(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or call this alias_expression?

Copy link
Contributor Author

@theHamsta theHamsta Dec 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patterns in a match case are a subset of expressions. I simplified using expression here since I didn't want to replicated all the new patterns that basically follow the same rules as expressions.

@theHamsta
Copy link
Contributor Author

theHamsta commented Dec 3, 2021

Test fail for uses of match not as a keyword (django loves match)

match = 4

Match is a soft keyword, the python parser would backtrack when seeing a : and start parsing a match statement
https://www.python.org/dev/peps/pep-0622/#id69

theHamsta added a commit to theHamsta/nvim-treesitter that referenced this pull request Dec 4, 2021
@theHamsta
Copy link
Contributor Author

theHamsta commented Dec 4, 2021

A possible hacky resolution would be to have a common parent for assignments that assign to "match" and match_statement

        _maybe_match_statement: $ => seq(
            choice($.match_statement,
                alias(seq(alias("match", $.identifier),
                    choice(
                        seq('=', field('right', $._right_hand_side)),
                        seq(':', field('type', $.type)),
                        seq(':', field('type', $.type), '=', field('right', $._right_hand_side))
                    )), $.assignment))),

I can't make maybe_match_statement hidden due to super node requirement in compound statement. _maybe_match_statement can be a third category next to _compound_statement and _simple_statements.

grammar.js Outdated Show resolved Hide resolved
@theHamsta theHamsta force-pushed the pattern_matching branch 2 times, most recently from d253bb8 to d49540f Compare December 4, 2021 17:34
@tmke8
Copy link

tmke8 commented Dec 5, 2021

Maybe there could also be a test case for a variable called match with a type annotation:

match: int = 3

@theHamsta
Copy link
Contributor Author

theHamsta commented Dec 5, 2021

@thomkeh sure, but right now it doesn't parse the simple case correctly, also for match, other = 1, 2

theHamsta added a commit to theHamsta/nvim-treesitter that referenced this pull request Dec 5, 2021
@theHamsta theHamsta force-pushed the pattern_matching branch 7 times, most recently from 55ab5fa to b6e4254 Compare December 5, 2021 17:26
grammar.js Show resolved Hide resolved
@theHamsta theHamsta force-pushed the pattern_matching branch 2 times, most recently from b9526a8 to e2f8f54 Compare December 5, 2021 17:33
grammar.js Outdated Show resolved Hide resolved
@theHamsta theHamsta force-pushed the pattern_matching branch 2 times, most recently from 64f0694 to 471d02b Compare December 6, 2021 11:14
@GBeauregard
Copy link

if you're looking for weird match test cases you can go through the ones in CPython: https://github.com/python/cpython/blob/main/Lib/test/test_patma.py

There's plenty of weird ones https://github.com/python/cpython/blob/eb483c46d62707bdf705491f76cf1fa9642fb47e/Lib/test/test_patma.py#L1205

@theHamsta
Copy link
Contributor Author

theHamsta commented Dec 14, 2021

Thanks @GBeauregard I hope I fixed a few edge cases in test_patma.py . The file now parses successfully.

@theHamsta theHamsta force-pushed the pattern_matching branch 2 times, most recently from 41267b4 to 92f52e9 Compare December 14, 2021 01:01
@GBeauregard
Copy link

Test fail for uses of match not as a keyword (django loves match)

match = 4

Match is a soft keyword, the python parser would backtrack when seeing a : and start parsing a match statement https://www.python.org/dev/peps/pep-0622/#id69

I think you figured it out (and I don't think it mattered for this comment anyway), but to clarify the PEP 622 you linked here is not the accepted pattern matching specification (note the status is superseded); the final/accepted specification differs in a few aspects and was put in PEP 634: https://www.python.org/dev/peps/pep-0634/

Thanks @GBeauregard I hope I fixed a few edge cases in test_patma.py . The file now parses successfully.

no problem, thanks for implementing parsing for the fun new feature :)

@theHamsta theHamsta force-pushed the pattern_matching branch 2 times, most recently from ca50b1f to 3b4246e Compare December 14, 2021 01:32
@GBeauregard
Copy link

GBeauregard commented Dec 17, 2021

One more comment, this and the nvim-treesitter PR seem to be missing the new _ soft keyword. See https://docs.python.org/3.10/reference/lexical_analysis.html#soft-keywords for reference on it being considered a keyword and python/cpython#25851 where the CPython built-in IDLE highlighting considers and highlights this as a soft keyword.

See also https://docs.python.org/3.10/reference/compound_stmts.html#wildcard-patterns

This can be useful for users as well, since the syntax highlighting difference will hint that _ has a different behavior from a capture variable named _ (can be used multiple times, etc etc). (To the extent this isn't obvious is all the more reason to highlight it!)

@theHamsta
Copy link
Contributor Author

This PR just regards all of the patterns introduced by pattern matching as expressions. To treat _ special only in patterns one would have to implement all the introduced patterns. Not sure whether that's worth it since _ is only semantically different from an identifier (it can bind to multiple values). For highlighting, one could just always highlight _ as wildcards.

@bellini666
Copy link

Hey guys. Just wondering when this will get merged :)

Copy link
Contributor

@dcreager dcreager left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, @theHamsta, thanks for taking this on!

You'll need to rebase onto the latest master since I just merged a couple of other PRs. Also, can you add match and case as new keywords in the syntax highlighting file? And ideally some highlighting tests to test/highlight/keywords.py. Then I think this is good to merge!

@theHamsta
Copy link
Contributor Author

@dcreager rebased and added a highlight test

@dcreager dcreager merged commit ed0fe62 into tree-sitter:master Jan 28, 2022
theHamsta added a commit to theHamsta/nvim-treesitter that referenced this pull request Jan 28, 2022
theHamsta added a commit to theHamsta/nvim-treesitter that referenced this pull request Jan 28, 2022
theHamsta added a commit to theHamsta/nvim-treesitter that referenced this pull request Jan 28, 2022
theHamsta added a commit to theHamsta/nvim-treesitter that referenced this pull request Jan 28, 2022
theHamsta added a commit to theHamsta/nvim-treesitter that referenced this pull request Jan 28, 2022
theHamsta added a commit to nvim-treesitter/nvim-treesitter that referenced this pull request Jan 28, 2022
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 this pull request may close these issues.

Support pattern matching in python 3.10
5 participants