feat(ignore): add invalid-ignore-comment and ignore-without-code rules#3832
Open
mikeleppane wants to merge 1 commit into
Open
feat(ignore): add invalid-ignore-comment and ignore-without-code rules#3832mikeleppane wants to merge 1 commit into
mikeleppane wants to merge 1 commit into
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3752
Closes #3450
Two new ignore-comment lint rules, mirroring ty's
invalid-ignore-commentand basedpyright's
reportIgnoreCommentWithoutRule.What & why
invalid-ignore-comment(defaultwarn). A comment that looks like anignore 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:
ignore-without-code(defaultignore, opt-in). A bare# pyrefly: ignoresuppresses any error on its line, so it can silently swallow a later, unrelated
error. Enable the rule to require an explicit code:
Key design property: purely additive
invalid-ignore-commentchanges no suppression behavior. The ignore-commentparser now returns a single recognizer result with three disjoint outcomes
(
Valid,Malformed { suppression, .. },NotADirective), so exactly one firesper comment, and a non-directive such as
# type: list[int](which never reachesthe
ignorestem) can never be flagged. AMalformedresult still carriesthe exact suppression it produced before, so the diagnostic is layered on top:
[: keeps the lenient parse, still suppresses[]: still suppresses for tools whose codes aren't checked(
# type: ignore[]remains a blanket suppression), a no-op for PyreflyHow
crates/pyrefly_python/src/ignore.rs:parse_ignore_commentreturns thenew
ParsedIgnoreenum;Ignorerecords aVec<MalformedIgnore>alongsideits suppression map.
ignore-errors/ignore-all-errorsare excluded via aword-boundary check so genuine file-level directives aren't flagged.
crates/pyrefly_config/src/error_kind.rs: two newErrorKinds inlexicographic position (
InvalidIgnoreComment=warn,IgnoreWithoutCode=ignore) plusErrorKind::is_unsuppressable(), the shared predicate for thethree 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.
error/collector.rsso theignore-all, per-line, and f-string paths are covered uniformly; also excluded
from
--suppress-errorsso it can't write a comment that could never takeeffect.
website/docs/error-kinds.mdxgains both rule sections.Presets are unchanged:
allpromotes both automatically;strictis left as-isto keep
ignore-without-codeopt-in.Test plan
ignore::tests::test_parse_ignore_commentignore::tests::test_malformed_ignores_are_additivestate::errors::tests::test_invalid_ignore_comment_{emitted,variants}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}# type: ignorenot (Pyrefly-scoped)test/ignores.md(scrut)pyrefly_config219;clippyandfmtclean.mypy_primerover 56 real-world projects (mypy, sphinx, scrapy, pydantic,cryptography, and more): zero diagnostic diff, confirming no existing
diagnostic changes on real code.