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

ERA001: Ignore script-comments with multiple end-tags #13283

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

MichaReiser
Copy link
Member

Summary

A script-comment according to PEP 723 starts with # /// script and spans all lines up to # ///.
However, the ´# /// isn't considered the end-tag if the next line is a valid content line according to PEP 723 (# or #<space>).

This PR fixes the handling of script-comments with multipe end-tags.

Fixes #13278

Discussion

This PR changes the rule's semantics to not skip over invalid script comments. Any code in invaid-script comments will get flagged.

I'm torn if that's the right behavior. Arguably, detecting invalid-script-comments should be its own rule. However,
the spec is very cleary about unclosed blocks:

Unclosed blocks MUST be ignored.

Test Plan

Added and update tests

@MichaReiser MichaReiser added rule Implementing or modifying a lint rule bug Something isn't working labels Sep 8, 2024
@MichaReiser MichaReiser force-pushed the micha/fix-script-comment-era001 branch from f718e9a to ac8b453 Compare September 8, 2024 16:46
Copy link
Contributor

github-actions bot commented Sep 8, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

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.

We also have an implementation for this in uv. You could consider copying that over, but this is also fine. Thanks!

@MichaReiser MichaReiser merged commit ac720cd into main Sep 9, 2024
20 checks passed
@MichaReiser MichaReiser deleted the micha/fix-script-comment-era001 branch September 9, 2024 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ERA001 stops parsing an inline script metadata block at the first # /// line
2 participants