Skip to content

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

Merged
MichaReiser merged 1 commit intomainfrom
micha/fix-script-comment-era001
Sep 9, 2024
Merged

ERA001: Ignore script-comments with multiple end-tags#13283
MichaReiser merged 1 commit intomainfrom
micha/fix-script-comment-era001

Conversation

@MichaReiser
Copy link
Copy Markdown
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
@github-actions
Copy link
Copy Markdown
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
Copy Markdown
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
@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