Skip to content

Let # pyrefly: ignore exclude symbols from pyrefly coverage#3919

Open
jorenham wants to merge 1 commit into
facebook:mainfrom
jorenham:coverage/pyrefly-ignore
Open

Let # pyrefly: ignore exclude symbols from pyrefly coverage#3919
jorenham wants to merge 1 commit into
facebook:mainfrom
jorenham:coverage/pyrefly-ignore

Conversation

@jorenham

Copy link
Copy Markdown
Contributor

Summary

The pyrefly coverage commands now respects # pyrefly: ignore. Because, well, they didn't before... An ignored symbol was still reported and still counted against the percentage. If unused ignores were configured, then pyrefly check would also complain about them.

Now, we can do things like

def legacy(x):  # pyrefly: ignore[coverage-missing]
    ...

which will cause legacy to not be reported as a warning anymore by coverage check, and will no longer affect the total coverage percentages of coverage check and coverage report (counted as 0 typables).

Specifically ignoring [coverage-partial] and a bare # pyrefly: ignore work too.
However, I chose to not include # type: ignore, even if explicitly configured. That's because those have always been intended for type errors, and also because Pyrefly is the only type-coverage checker out there, so it's probably safe to assume that a # type: ignore wasn't intended to ignore coverage.

A coverage ignore that matches nothing is flagged as unused by coverage check, and plain pyrefly check no longer reports coverage-* codes as unused (they're only for coverage check).

For decorated symbols like a @property, the # pyrefly: ignore should be placed on the decorator line (that's where the coverage diagnostic starts).

Test Plan

Added tests for exclusion (incl. properties), unused/out-of-scope ignores, the type: ignore boundary, and check / report consistency.

@github-actions

Copy link
Copy Markdown

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@NathanTempest NathanTempest left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the well-thought-out feature, @jorenham and for the real test of the design choice (zeroing slots so excluded symbols drop out of the percentage, not just the output).

Plain pyrefly check no longer treating coverage-* codes as unused , a small but real correctness fix; nice that you covered the mixed-code case (bad-override still flagged, coverage-missing silently ignored).

The refactor in state/errors.rs (extracting Error::unused_ignore + Error::unused_pyrefly_ignore) is a net win ~50 lines of repeated range-building boilerplate gone. And confirmed the new exclusion pass is safe under the parallel collect_one from #3825 since each closure owns its ModuleSymbols. LGTM and importing. Thanks for another contribution to Pyrefly!

@meta-codesync

meta-codesync Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

@NathanTempest has imported this pull request. If you are a Meta employee, you can view this in D109597080.

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.

2 participants