Skip to content

Find noqa comments on the right line after form feeds and other separators - #366

Merged
hakancelikdev merged 1 commit into
claude/integrationfrom
fix/noqa-line-split
Sep 25, 2026
Merged

hakancelikdev merged 1 commit into
claude/integrationfrom
fix/noqa-line-split

Conversation

@hakancelikdev

Copy link
Copy Markdown
Owner

From the core-dev review of claude/integration. The bug also exists on main.

Problem

skip_import took the import's lines from self.source.splitlines(). str.splitlines() also splits on \f, \v, \x1c–\x1e, \x85, U+2028 and U+2029, but the parser, and therefore AST line numbers, only ends lines on \r\n, \r and \n. One of those characters inside a string literal shifted every following line:

x = "a\fb"
import os  # noqa      <- the comment was looked up on the wrong line, so os was removed

Fix

Split on \r\n|\r|\n only.

Tests

test_skip_comment_line_after_other_line_separators is parametrized over 5 separators, and all 5 fail before this change. Full suite passes on 3.9 / 3.12 / 3.14, and pre-commit passes.

🤖 Generated with Claude Code

https://claude.ai/code/session_019P9bvuGwAyVCNUdsAYPB1V


Generated by Claude Code

…ators

skip_import cut the import's lines out of str.splitlines(), which also splits
on \f, \v, \x1c, \x85, U+2028 and others. One of those inside a string
literal shifted every following line, so a `# noqa` import could be removed.
Only \r\n, \r and \n end a line for the parser.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019P9bvuGwAyVCNUdsAYPB1V
@hakancelikdev
hakancelikdev merged commit 5c6e631 into claude/integration Sep 25, 2026
40 checks passed
hakancelikdev pushed a commit that referenced this pull request Oct 2, 2026
- Bump the version to 1.5.0 in the package, action.yml and the GitHub
  Action docs.
- Date the changelog section and add entries for #362, #366 and #367.
- Use an SPDX license expression (MIT) and license-files instead of the
  deprecated license table and classifier; requires setuptools>=77.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019P9bvuGwAyVCNUdsAYPB1V
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.

2 participants