Skip to content

suppress: avoid orphaning foreign linter pragmas#3454

Open
nitishagar wants to merge 1 commit into
facebook:mainfrom
nitishagar:nitishagar/issue-3373-suppress-pragma-conflict
Open

suppress: avoid orphaning foreign linter pragmas#3454
nitishagar wants to merge 1 commit into
facebook:mainfrom
nitishagar:nitishagar/issue-3373-suppress-pragma-conflict

Conversation

@nitishagar
Copy link
Copy Markdown

Summary

pyrefly suppress (default LineBefore mode) inserts # pyrefly: ignore [...] on the line above a target. When the line directly above already contains a non-pyrefly linter pragma — # noqa, # nosemgrep, # pyright: ignore, # pylint:, # nosec, # ruff:, # flake8:, foreign-tool # type: ignore — the new pyrefly comment slides between them, breaking the foreign pragma's adjacency to its target line and silently disabling it.

This PR detects that case and appends # pyrefly: ignore [...] to the end of the target line instead, leaving the foreign pragma untouched. Behavior for plain comments and other situations is unchanged.

  • Adds has_foreign_linter_pragma(line) helper (uses the existing find_comment_start_in_line so string literals don't false-match).
  • Wires it into add_suppressions's LineBefore branch via a force_inline flag.
  • 6 new tests covering noqa, nosemgrep, pyright, plain-comment-unchanged, string-not-comment, and the helper itself.

Fixes #3373

Test plan

  • cargo test -p pyrefly --lib — 5161/5161 passing locally (includes the 6 new tests).
  • Existing test_add_suppressions_existing_comment (where the line above is a plain comment, not a pragma) still passes — confirms behavior is unchanged for non-pragma comments.
  • CI green.

@meta-cla
Copy link
Copy Markdown

meta-cla Bot commented May 18, 2026

Hi @nitishagar!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@rchen152
Copy link
Copy Markdown
Contributor

rchen152 commented May 19, 2026

Hi @nitishagar - thanks for the PR! We can review it (and any other PRs from you) as soon as you sign the CLA (see the instructions here).

@nitishagar
Copy link
Copy Markdown
Author

@rchen152 I've signed the CLA. Let me know if you have any questions or anything I can clarify on the above changes.

@meta-cla
Copy link
Copy Markdown

meta-cla Bot commented May 19, 2026

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@meta-cla meta-cla Bot added the cla signed label May 19, 2026
@meta-cla
Copy link
Copy Markdown

meta-cla Bot commented May 19, 2026

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@rchen152
Copy link
Copy Markdown
Contributor

Thanks! @yangdanny97 would you mind taking a look at this one, since it looks like you had started thinking about #3373?

When the line above a target already contains a non-pyrefly linter pragma
(noqa, nosemgrep, pyright:, pylint:, etc.), `pyrefly suppress` was inserting
a `# pyrefly: ignore [...]` line between them, which silently disabled the
foreign pragma. Detect that case and append inline at the end of the target
line instead.

Fixes facebook#3373
@nitishagar nitishagar force-pushed the nitishagar/issue-3373-suppress-pragma-conflict branch from ef4617a to c0f67a5 Compare May 20, 2026 09:41
@github-actions github-actions Bot added size/l and removed size/l labels May 20, 2026
@nitishagar
Copy link
Copy Markdown
Author

Rebased onto latest main and resolved the conflicts (Cargo.lock + suppress.rs) — ready for review whenever convenient. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pyrefly suppress disables line-ignores for other linters

3 participants