Skip to content

feat(ignore): add invalid-ignore-comment and ignore-without-code rules#3832

Open
mikeleppane wants to merge 1 commit into
facebook:mainfrom
mikeleppane:feat/ignore-comment-lints-3752-3450
Open

feat(ignore): add invalid-ignore-comment and ignore-without-code rules#3832
mikeleppane wants to merge 1 commit into
facebook:mainfrom
mikeleppane:feat/ignore-comment-lints-3752-3450

Conversation

@mikeleppane

@mikeleppane mikeleppane commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Closes #3752
Closes #3450

Two new ignore-comment lint rules, mirroring ty's
invalid-ignore-comment
and basedpyright's
reportIgnoreCommentWithoutRule.

What & why

invalid-ignore-comment (default warn). A comment that looks like an
ignore directive but is malformed used to be silently discarded: it produced
neither a suppression nor any warning, so you never learned it was wrong. Three
shapes are now flagged:

x: int = "oops"  # pyrefly: ignoree              # typo'd keyword
x: int = "oops"  # pyrefly: ignore[bad-assignment # unterminated bracket
x: int = "oops"  # pyrefly: ignore[]             # empty code list

ignore-without-code (default ignore, opt-in). A bare # pyrefly: ignore
suppresses any error on its line, so it can silently swallow a later, unrelated
error. Enable the rule to require an explicit code:

# flagged when enabled:
x: int = "oops"  # pyrefly: ignore
# fix:
x: int = "oops"  # pyrefly: ignore[bad-assignment]

Key design property: purely additive

invalid-ignore-comment changes no suppression behavior. The ignore-comment
parser now returns a single recognizer result with three disjoint outcomes
(Valid, Malformed { suppression, .. }, NotADirective), so exactly one fires
per comment, and a non-directive such as # type: list[int] (which never reaches
the ignore stem) can never be flagged. A Malformed result still carries
the exact suppression it produced before, so the diagnostic is layered on top:

  • typo: suppresses nothing, as before
  • unterminated [: keeps the lenient parse, still suppresses
  • empty []: still suppresses for tools whose codes aren't checked
    (# type: ignore[] remains a blanket suppression), a no-op for Pyrefly

How

  • crates/pyrefly_python/src/ignore.rs: parse_ignore_comment returns the
    new ParsedIgnore enum; Ignore records a Vec<MalformedIgnore> alongside
    its suppression map. ignore-errors / ignore-all-errors are excluded via a
    word-boundary check so genuine file-level directives aren't flagged.
  • crates/pyrefly_config/src/error_kind.rs: two new ErrorKinds in
    lexicographic position (InvalidIgnoreComment = warn, IgnoreWithoutCode =
    ignore) plus ErrorKind::is_unsuppressable(), the shared predicate for the
    three ignore-comment diagnostics.
  • pyrefly/lib/state/errors.rs: collect_ignore_comment_lint_errors()
    emits both diagnostics (gated on the tool being enabled), folded into the
    ignore-diagnostics display path so configured severity is respected.
  • Unsuppressable enforcement: centralized in error/collector.rs so the
    ignore-all, per-line, and f-string paths are covered uniformly; also excluded
    from --suppress-errors so it can't write a comment that could never take
    effect.
  • Docs: website/docs/error-kinds.mdx gains both rule sections.

Presets are unchanged: all promotes both automatically; strict is left as-is
to keep ignore-without-code opt-in.

Test plan

Test Protects
ignore::tests::test_parse_ignore_comment recognizer: all three malformed shapes plus valid/non-directive negatives
ignore::tests::test_malformed_ignores_are_additive malformed comments still suppress exactly as before
state::errors::tests::test_invalid_ignore_comment_{emitted,variants} diagnostic emitted for typo / unterminated / empty; real error still fires
state::errors::tests::test_empty_bracket_ignore_is_additive # type: ignore[] still suppresses (no regression for non-Pyrefly tools)
state::errors::tests::test_ignore_without_code_{emitted,scoped_to_pyrefly} bare ignore flagged; # type: ignore not (Pyrefly-scoped)
test/ignores.md (scrut) end-to-end: warn-on-by-default and additive, opt-in behavior, never-flag cases
  • Full lib suite: 5641 pass / 0 fail; pyrefly_config 219; clippy and fmt clean.
  • mypy_primer over 56 real-world projects (mypy, sphinx, scrapy, pydantic,
    cryptography, and more): zero diagnostic diff, confirming no existing
    diagnostic changes on real code.

Malformed ignore comments were silently discarded: a typo'd directive
(`# pyrefly: ignoree`), an unterminated `[`, or an empty `[]` produced
neither a suppression nor any warning, so a developer never learned the
comment was wrong. Separately, a bare `# pyrefly: ignore` suppresses any
error on its line, which can silently swallow a later, unrelated error.

Turn the ignore-comment parser into a single recognizer with three
disjoint outcomes (valid / malformed / not-a-directive) so exactly one
fires and a non-directive such as `# type: list[int]`, which never
reaches the `ignore` stem, can never be flagged. A malformed result
still carries the suppression it produced before, so the new
invalid-ignore-comment diagnostic (default `warn`) is purely additive
and no existing suppression behavior changes. ignore-without-code is
opt-in (default off) and scoped to the Pyrefly tool, the only one whose
per-line codes Pyrefly honors. Both kinds are unsuppressable, enforced
centrally so an ignore comment cannot silence the lint about itself.

Refs: facebook#3752, facebook#3450
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.

new rule - invalid ignore comment option to report an error on # pyrefly: ignore comments without an error code

1 participant