Skip to content
This repository was archived by the owner on Jul 27, 2023. It is now read-only.

Add Decorator node #7

Merged
merged 1 commit into from
Jun 8, 2023
Merged

Add Decorator node #7

merged 1 commit into from
Jun 8, 2023

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Jun 6, 2023

This PR introduces a new Decorator node. The node wraps an expression and its range includes the @.

Having a dedicated node for Decorators simplifies the implementation of the Formatter when handling leading comments:

@deco1
# leading 2
@deco2(with_args=True)
# leading 3
@deco3

# leading 4 that already has an empty line
def decorated_with_split_leading_comments():
    pass

The leading comment should be attached to the whole decorator and not just the expression coming after the @. It's also necessary that we distinguish between comments attached to the decorator vs comments attached to the decorator's expression:

@(
    # leading 3
    deco3
)

This is straightforward when the decorator and the expression are different nodes.

This change has the added benefit that the Linter now gets the full range of the decorator. For example, we have fixes that replace remove the whole decorator but they incorrectly assume that its safe to subtract 1 from the start (offset of the @). However, this doesn't account for:

@         deco2(with_args=True)
def decorated1():
    pass
    

which is valid python

@MichaReiser
Copy link
Member Author

MichaReiser commented Jun 6, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@MichaReiser MichaReiser changed the base branch from named-expr-range to include-parentheses-in-arguments-range June 6, 2023 12:39
@MichaReiser MichaReiser marked this pull request as ready for review June 6, 2023 12:46
@MichaReiser MichaReiser requested a review from charliermarsh June 7, 2023 13:48
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Great change.

Base automatically changed from include-parentheses-in-arguments-range to main June 7, 2023 20:26
@MichaReiser MichaReiser merged commit 0dc8fdf into main Jun 8, 2023
@MichaReiser MichaReiser deleted the decorator-node branch June 8, 2023 05:44
charliermarsh pushed a commit that referenced this pull request Jun 19, 2023
zanieb added a commit that referenced this pull request Jul 10, 2023
zanieb added a commit that referenced this pull request Jul 17, 2023
zanieb pushed a commit that referenced this pull request Jul 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants