diff_cover icon indicating copy to clipboard operation
diff_cover copied to clipboard

Plugins are being given options that aren't in hookspec; plugin guide example fails

Open bpcreech opened this issue 6 months ago • 1 comments

The diff-quality plugin feature is super cool! I'm using it to create a linter for an esoteric use case my team has.

One thing I noticed when following the directions... If you follow the directions and make a hookimpl this way:

@diff_cover_hookimpl  # type: ignore[misc]
def diff_cover_report_quality() -> BaseViolationReporter:
    return ExplainIgnoresViolationReporter()

... Then it fails, because diff_cover/diff_quality_tool.py currently passes in two kwargs, reports and options, resulting in this error:

% diff-quality --violations explainignores --compare-branch origin/main
Traceback (most recent call last):
  File "/Users/ben.creech/workspace/mypy_diff/.venv/bin/diff-quality", line 10, in <module>
    sys.exit(main())
             ~~~~^^
  File "/Users/ben.creech/workspace/mypy_diff/.venv/lib/python3.13/site-packages/diff_cover/diff_quality_tool.py", line 352, in main
    reporter = reporter_factory_fn(
        reports=input_reports, options=user_options
    )
TypeError: diff_cover_report_quality() got an unexpected keyword argument 'reports'

If you add these as positional args, you get an error from pluggy about the hookspec not matching the hookimpl (which is... correct!):

@diff_cover_hookimpl  # type: ignore[misc]
def diff_cover_report_quality(reports: list[IO[bytes]], options: str | None) -> BaseViolationReporter:
    del reports, options
    return ExplainIgnoresViolationReporter()
pluggy._manager.PluginValidationError: Plugin 'explainignores' for hook 'diff_cover_report_quality'
hookimpl definition: diff_cover_report_quality(reports, options) -> 'BaseViolationReporter'
Argument(s) {'reports', 'options'} are declared in the hookimpl but can not be found in the hookspec

The correct formulation is (type annotations, obviously, optional):

@diff_cover_hookimpl  # type: ignore[misc]
def diff_cover_report_quality(*, reports: list[IO[bytes]], options: str | None) -> BaseViolationReporter:
    del reports, options
    return ExplainIgnoresViolationReporter()

I may or may not make a PR for this; I figured at least filing this issue might help someone find it...

BTW on that note, in case it helps anyone, if you've modernized your project to pyproject.toml instead of setup.py, here is the hook entrypoint syntax:

[project.entry-points.diff_cover]
yourtool = "yourpkg.yourmodule"

bpcreech avatar Oct 05 '25 20:10 bpcreech

Note the above "correct formulation" is working around a bug, I think... probably the right thing would be to add reports and options as kwargs to the diff_cover hookspec, something like, in diff_cover/hookspecs.py:

import pluggy

hookspec = pluggy.HookspecMarker("diff_cover")


@hookspec
def diff_cover_report_quality(*, reports, options):
    """
    Return a 2-part tuple:
    - Quality plugin name
    - Object that implements the BaseViolationReporter protocol
    """

bpcreech avatar Oct 05 '25 20:10 bpcreech