Content-Length: 263962 | pFad | http://github.com/astral-sh/ruff/pull/16747

72 [internal] Use `ruff_python_ast::OperatorPrecedence` in Parser (`ruff_python_parser`) by junhsonjb · Pull Request #16747 · astral-sh/ruff · GitHub
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

[internal] Use ruff_python_ast::OperatorPrecedence in Parser (ruff_python_parser) #16747

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

junhsonjb
Copy link
Contributor

Summary

This change continues to resolve #16071 (and continues the work started in #16162). Specifically, this PR changes the code in the parser so that it uses the OperatorPrecedence struct from ruff_python_ast instead of its own version. This is part of an effort to get rid of the redundant definitions of OperatorPrecedence throughout the codebase.

Note that this PR only makes this change for ruff_python_parser -- we still want to make a similar change for the formatter (namely the OperatorPrecedence defined in the expression part of the formatter, the pattern one is different). I separated the work to keep the PRs small and easily reviewable.

Test Plan

Because this is an internal change, I didn't add any additional tests. Existing tests do pass.

It's worth noting however, that this change did have a slight impact on snapshot results. The removed defintion of OperatorPrecedence had separate enum variants BitOr and BitXor. This differs from the newer version of OperatorPrecedence which keeps both operators on the same level with BitXorOr. As expected, this resulted in a slightly different snapshot. After inspecting both the old and new versions, however, I think that the new one is acceptable. Simply put, the old snapshot prioritized a BitXor operation over a BitOr and the new snapshot does not.

The snapshot only had to be updated for the following expression:

1 | 2 & 3 ^ 4 + 5 @ 6 << 7 // 8 >> 9

The old snapshot resolved to a precedence of:

1 | ((2 & 3) ^ ((4 + (5 @ 6)) << (7 // 8)) >> 9)

which ultimately boils down to an expression of the form A | B where A = 1 and B = ((2 & 3) ^ ((4 + (5 @ 6)) << (7 // 8)) >> 9).

Meanwhile, the new snapshot resolved to a precedence of:

(1 | (2 & 3)) ^ (((4 + (5 @ 6)) << (7 // 8)) >> 9)

which boils down to an expression of the form A ^ B where A = (1 | (2 & 3)) and B = (((4 + (5 @ 6)) << (7 // 8)) >> 9).

And since the new OperatorPrecedence states that ^ and | have the same precedence, BitXorOr, both of the resulting expressions have equal precedence. It was based on this logic that I approved the new snapshot.

All other tests passed.

Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@MichaReiser MichaReiser added internal An internal refactor or improvement parser Related to the parser labels Mar 14, 2025
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 parser Related to the parser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unify OperatorPrecedence enums
2 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/astral-sh/ruff/pull/16747

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy