fix: the generated ruff config runs the rules it selects - #8
Merged
Conversation
`DTZ` and `ANN` sat in `select` and in `ignore` at once, so a generated project shipped a lint file that promised three timezone rules and the whole annotation family and ran neither. A naive `datetime.now()`, a `utcnow()` and an unannotated def passed `make check` without a word. `DTZ` is the family nothing else covers, and this org computes deadlines, retention windows and TTLs from timestamps, so it comes on rather than out of `select`. Annotations mypy already enforces for the package; keeping `ANN` on adds the coverage mypy has no view of, since it reads the package and never `tests/`. The entries that only ever meant "except in tests" now say so: `S101`, `S105` and `S106` move into a `per-file-ignores` block for `tests/**/*.py`, which leaves library code held to them. `S104` stays global -- library code does bind all interfaces. `ANN401` and `ANN204` stay and finally mean something. Measured against the org's libraries before changing anything: `DTZ` reports nothing in `deadline-budget`, `clientwright`, `servicewright` or `sqlalchemy-foundation-kit`, in library code or tests. The whole cost of the new set is five narrowing asserts in `clientwright` and fifteen missing test annotations across three repositories. CI gets the check that was missing: a rule selected and then ignored looks exactly like a rule that works, so the rendered project is now fed the mistakes those rules exist to catch and has to report them. Closes #7
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.
The problem
[tool.ruff.lint]selectedDTZandANNand then ignored both families. Generating a project and dropping a file into the package shows what that costs:make checkbefore this change:All checks passed!. After it: seven findings —DTZ005,DTZ003,DTZ006, threeANN202, oneANN001. Theselectlist was making a promise the linter did not keep, and every library generated from the template carries the same two lines.The decision
DTZcomes on, not out ofselect. It is the family nothing else covers, and it catches naivenow(),utcnow()and naivefromtimestampin an org whose libraries compute deadlines, retention windows, partition boundaries and idempotency TTLs from timestamps.ANNcomes on too. mypy'sdisallow_untyped_defsalready enforces annotations for the package, soANNadds nothing there — butfiles = ["<package>"]means mypy never readstests/, and ruff is the only thing that can hold that line whereCONTRIBUTING.mdalready claims it ("type hints on all functions and methods, including tests").The entries that only ever meant except in tests now say so:
S104stays global —servicewrightbinds0.0.0.0in library code in four places, so scoping it to tests would be a lie in the other direction.TRY003,RUF012,N802,PERF401andSIM105all fire in library code too.ANN401andANN204stay and finally mean something.What the org's libraries say about it
Measured before writing anything, with ruff 0.16.6
--isolatedoverdeadline-budget,clientwright,servicewrightandsqlalchemy-foundation-kit(read-only; nothing changed in those repositories):deadline-budgetclientwrightS101, 1 ×ANN201in testsservicewrightANN201in testssqlalchemy-foundation-kitANN001/ANN202in testsDTZreports nothing anywhere — library code, tests,scripts/,examples/— and it is a real pass, not a vacuous one:clientwright/core/engine/base.py:32already computes(when - datetime.datetime.now(datetime.UTC)).total_seconds()andsqlalchemy_foundation_kit/base/models.py:30mapsdatetime.datetimetoTIMESTAMP(timezone=True). Nothing to file against those repositories.The five
S101inclientwrightare all type narrowing rather than validation (assert isinstance(response, requests.Response),assert response is not None). That is a decision for that repository when it takes the update, not something to pre-empt here.What I rejected
DTZandANNfromselect. It would make the file honest and throw away the one family nothing else covers.ANNintests/, which the issue suggests. I measured it instead of assuming: fifteen findings across three repositories, against a contributing guide that already asks for annotations in tests. Fifteen annotations is a fair price for the config and the guide agreeing.S104,TRY003,RUF012,N802,PERF401andSIM105all fire in library code in at least one library, so they belong where they are.The test
The failure mode is a config that claims a rule it does not run, and the generated gate stays green either way — which is how this survived. CI now feeds the rendered project a naive
datetime.now()and an unannotated def and fails if ruff stays quiet, plus a bareassertintests/that has to stay accepted so theper-file-ignoresblock is checked too.Against the pre-change config that step fails on the first probe (
::error::DTZ005 never fired); against this one it passes.Verification
Rendered
widget-kitfrom this branch withcopier copy --defaults:Closes #7