fix(lint): reach every const/vendor __init__.py in isort and drop five stale pylint flags

[JarryShaw]

Please follow the guide below

• Searched for similar pull requests
• Followed the coding style (make pylint, make mypy, make isort)
• make test passes, and a test case covers the change
• Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible -- N/A — changelog centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

• fix — corrects a defect

---

Description of your pull request and other information

Fixes #765.
Fixes #767.

Makefile:126's isort glob stopped at a fixed two levels, so #754's split left
pcapkit/const/reg/apptype/__init__.py (and its vendor counterpart) reached
by no make isort line at all. Two intermediate fixes each traded one gap
for a narrower one (a bash-version dependency, then a partial-failure gap in
a merged isort call); the const/vendor line is now split into two calls,
one per directory, matching cron-vendor.yml's own two calls exactly.

tests/project/test_isort_clean.py is restructured for the resulting four
lines, and its own naming is fixed along with it: it used to key its
target-resolution functions on line numbers, which drifted the moment this
same PR's Makefile comment expanded above the target -- caught by a
reviewer's measurement, not by the test. Functions are now named for what
they target, MAKEFILE_LINES pairs flags with targets positionally rather
than by a line-number key, and _isort_recipe_line_numbers() reads
Makefile and resolves those numbers fresh every run (verified by shifting
the target in a scratch copy and confirming the resolver's output moves with
it). That resolver also treats a blank or column-0 comment line inside the
recipe as transparent, matching make's own behaviour there (make -n isort still reports four commands with either dropped between two recipe
lines; the resolver used to undercount both). Every prose reference to a
specific line number in the module docstring is swept to a descriptive name
for the same reason.

PYLINT_FLAGS named pylint.extensions.emptystring, which pylint removed at
3.0 (E0013 every run), plus four --disable= entries for checks pylint has
since removed or left unloaded -- five stale entries, dropped. This is flag
hygiene, not enablement: compare-to-empty-string reaches the core
refactoring checker's use-implicit-booleaness-not-comparison-to-string
(C1804) via a backward-compatible alias regardless of the plugin's own load
failure -- confirmed with --list-msgs-enabled, identical before and after.
lint.yml's header is corrected to match, re-pinned to 1a85269, and its
R/total range is restated on a consistent basis with the tree it was
measured on.

[JarryShaw]

Second cross-review at f31e5114f: NEEDS CHANGES. The find substitution is equivalent and genuinely safe, the R-range arithmetic is exact and the basis shift is properly stated, and the merge with 83b58ebda is clean and green. But the PR introduces a regression while asserting twice that it cannot.

The Makefile still needs bash ≥ 4.0. shopt -s globstar is what Makefile:126 now depends on, and SHELL := $(shell command -v bash …) returns whatever bash is first on PATH — on macOS without a brew bash that is /bin/bash 3.2.57, and globstar arrived in bash 4.0. Measured:

new glob, globstar unavailable (= bash 3.2) :  12 files
new glob, globstar available                : 276 files
old two-level glob (worked fine on 3.2)     : 264 files
depth-2 __init__.py that ONLY line 126 covers: 34     (line 125 skips every __init__.py)

So on a macOS contributor's machine this PR takes line 126 from 264 files to 12, and those 34 __init__.py are sorted by nothing — the same shape of hole as #765, one level up. And it is silent: a failed shopt prints one stderr line, the following command still runs, and make exits 0.

$ make probe
/bin/bash: line 0: shopt: globstar_not_real: invalid shell option name
second command ran
  make exit: 0

make isort is in no workflow, so CI never sees it — but CONTRIBUTING.md:152 and .github/PULL_REQUEST_TEMPLATE.md:8 both tell contributors to run it. Two files now assert the opposite: cron-vendor.yml says the Makefile's SHELL is "confirmed (elsewhere) to pick up a modern one", and test_isort_clean.py says cron-vendor is "not provably a globstar-capable one the way the Makefile's is". Neither is true, and the Makefile's own SHELL comment names the case — "Homebrew, Linuxbrew and system installs all put it somewhere different" — while claiming only brace expansion, which is 3.2-safe.

The fix is already written in this PR: use find pcapkit/const -mindepth 2 -name '*.py' in the Makefile too. No globstar, all three call sites consistent, and both false claims disappear rather than needing correction.

Two smaller ones. The basis correction is 3 in R and 5 in the total, not 2 — the old R figures already included their three R0022, so old 645-680 becomes 642-677 and 6044-6079 becomes 6039-6074. The floors then coincide exactly (645-3 = 642, 6044-5 = 6039), which is worth saying: the pre- and post-#767 floors are the same tree state, and the range only looks lower because of the three deleted messages. And the post-#767 range is the one figure in that header with no tree label — on the merge with #769, R0801 duplicate-code moves 400 → 573, taking R to 823 and the total to 6220, so "R now runs 642-662" is already superseded on main. Label it [1a852698b] like everything around it.

Confirmed good: find and glob agree at 276 by md5 on both the head and the merge; -mindepth 2 reproduces the old floor exactly while removing the ceiling; the degenerate cases all fail loudly, because isort with no paths exits 1 rather than reading stdin. The pinned E 90 / W 4765 / C 542 survive #769 intact, and E1136 39 / E1101 31 are unchanged, so the narrowed "36/31/87" aside is unaffected.

[JarryShaw]

Third cross-review at f29df2c8b: NEEDS CHANGES — two prose corrections. The executable change is correct and complete: the bash-version dependency is genuinely gone, make propagates the failure, the basis arithmetic is exact, and the label and the fourth comment group are done.

1. A measured number in a permanent comment is wrong, and it is my number used wrongly. The new test prose says the degradation sorts "none of the 34 depth-two __init__.py files it used to cover". Measured:

__init__.py reached by line 126 :  34 total — 32 at depth two, 2 at depth three
degraded glob still reaches     :  const/reg/apptype/__init__.py, vendor/reg/apptype/__init__.py
therefore lost                  :  32

34 is the all-depth total; I supplied it and it was written up as the loss. The figure is 32, and the "depth-two" label is then correct. Both halves need fixing together.

2. The R0801 caveat is unattributed. "R0801 duplicate-code alone moves 400 → 573, taking R to 823 and the total to 6220" is exactly right, but carries no provenance while the paragraph above it carefully distinguishes "two runs here gave" from "a cross-review measured five more". In a header whose whole discipline is provenance that reads as the author's own measurement when it is cited — and it was measured on merge(f31e5114f, 83b58ebda) rather than on main itself. The number is still true of main since this PR touches no pcapkit/ file, but say which. One clause fixes both.

A correction the reviewer made against itself, which matters beyond this PR. Last pass it cleared the space-in-path case as failing loudly — "word-splits into two nonexistent paths → isort errors". Measured, that is false: isort prints Broken 1 paths and exits 0. I reproduced it. That clearance also covered cron-vendor.yml, so both files are less loud than previously reported.

Related, and worth knowing even though neither case is reachable today: the Makefile merges both finds into one isort call, so a single missing directory yields exit 0 with half the tree silently unsorted, where cron-vendor.yml keeps them as two calls and fails loudly. Only the fully-empty expansion is loud in the Makefile (measured: make exit 2). No path under const/vendor contains whitespace and neither directory can be absent where make isort runs, so this is not a blocker — but splitting line 126 into two invocations would close it and make the two files identical in shape.

Three minor points, non-blocking: the same paragraph says the empty case "fails the build under set -e", and there is no set -e in a make recipe — make fails on the command's own non-zero status; "degrades loudly rather than quietly" holds for the empty expansion but not the partial one; and Makefile:8-10 still justifies the bash pin as "Recipes below use bash features (brace expansion)" when no brace expansion survives this PR.

The no-rebase choice is right and should stand. Rebasing onto 83b58ebda would make the parent a tree whose pcapkit/ differs, so [1a852698b] would name neither the base nor the measured tree — recreating the defect pass 1 raised. As it stands the pin names the PR's own merge-base and is directly reproducible. Merge with 83b58ebda is clean: 241 passed / 1819 subtests, find target set still 276 on the merge.

[JarryShaw]

Fourth pass at 497666577: NEEDS CHANGES — one defect, newly introduced this round, and it is this PR's own subject matter.

Every _line_NNN_targets name is now wrong by 8, and one names a line with no isort on it.

main (83b58ebda)  isort lines at : 125, 126, 127
PR head           isort lines at : 133, 134, 135, 136
Makefile length                  : 174 -> 197   (+23; the target moved down 8)

The functions are _line_125_targets, _line_126_targets, _line_127_targets, _line_128_targets; the real lines are 133-136. Expanding the Makefile:8 comment this round is what pushed them down — so the names were correct when written and are wrong now, which is precisely the failure this PR exists to fix, in the file that fixes it.

The report claims "all four physical line numbers stable, verified with make -n isort". make -n prints the commands, not their line numbers, so that check could not have tested the claim it supports.

Do not renumber to 133-136. That re-arms the same trap — the next comment added above the target breaks them again, and the encoded number is load-bearing in six docstring cross-references. Key the names on their target instead: _pcapkit_tree_targets, _const_targets, _vendor_targets, _util_examples_targets, with MAKEFILE_LINES carrying the line number as data if it is wanted at all. That is the same lesson #756 landed — cite the class statement, not the line number — and the same reasoning that made endswith right for #769's branch check.

Everything else this round is correct. The count is fixed to 32 with the "depth-two" label kept, which I verified: 32 at depth two, plus the two depth-three apptype files the degraded glob still reached, is the 34 all-depth total. The split of line 126 into per-directory calls closes the partial-failure gap outright rather than caveating it, matches cron-vendor.yml's shape, and was the right call — both const and vendor are now independent invocations, so a single missing directory fails loudly instead of silently sorting half the tree. The R0801 caveat is attributed and scoped to merge(f31e5114f, 83b58ebda). The set -e mechanism is corrected to make's own non-zero-exit behaviour. Makefile:8 no longer claims brace expansion as the reason for the bash pin, while keeping the pin.

Four isort lines, four subtests, make -n isort renders, and a real make isort through a shim exits 0 on all four.

[JarryShaw]

Round-six review at 67035d94a: NEEDS CHANGES — narrow, and in the new resolver rather than in the isort fix. Everything asked about passes.

Confirmed. Makefile, lint.yml and cron-vendor.yml are byte-identical to 497666577, so rounds 2-5 stand untouched; the only change is tests/project/test_isort_clean.py (+84/−34). The rename is complete, the resolver genuinely self-corrects (.PHONY above the target and an isort-check: target before isort: both come out right, which a startswith match would have failed), the length assertion is correctly placed before the zip, and all four subtests fail independently with the correct physical line number and no cross-contamination — so splitting the const and vendor halves bought real coverage rather than tidiness.

On reordering, the answer is better than I feared: a reorder is not caught, but it structurally cannot mismatch flags to targets, because each MAKEFILE_LINES entry carries its own (flags, resolver) pair and never consults the Makefile. Only a subTest's label can be wrong.

The two hostile inputs I named both make the parser disagree with make. Reproduced independently, with make -n isort as the oracle:

baseline                      make: 4 cmds    resolver: [133, 134, 135, 136]
+ column-0 comment in recipe  make: 4 cmds    resolver: [133, 134]
+ blank line in recipe        make: 4 cmds    resolver: [133, 134]

make ignores blank lines and column-0 comment lines among recipe lines — they do not terminate the rule — whereas the resolver's rule is "the first following line that is not tab-indented ends the target". The docstring states its own rule accurately, so nothing in the file is false; the problem is that the rule is not make's, and the resulting message prescribes the wrong repair — "MAKEFILE_LINES needs an entry added or removed to match", when the correct action is to teach the parser. In a Makefile whose house style is heavy commenting — this PR added a 14-line block above PYLINT_FLAGS — dropping an explanatory comment between two isort lines is a realistic edit that would land a contributor on a red test with a misleading diagnosis. Two lines fix it: continue on not text.strip() and on text.lstrip().startswith('#') rather than breaking.

One silent-wrong-set path, and it is the only one constructible: a define block whose body contains a bare isort: at column 0 before the real target, with four or more tab-indented lines, returns four numbers pointing inside the block — the length assertion passes and the test proceeds citing wrong lines. grep -nE '^define|^endef' Makefile returns nothing, so it is hypothetical; isort::, isort:=, #isort: and a tab-indented isort: all fail the equality and fall through to a loud empty result. One sentence naming define as the assumption closes it honestly.

Blast radius, stated because it bounds all of the above: flags and target expressions come from MAKEFILE_LINES, never from the parsed text, so a wrong line number cannot change which isort checks run. Only labels and hints are corruptible — the verdict is safe.

One pre-existing gap worth a clause rather than a fix: the resolver reads line numbers only, while flags and target expressions stay hardcoded, so changing -l100 to -l120 in the Makefile goes unnoticed. The MAKEFILE_LINES comment puts "a flag changed in one place and not the other is exactly how 'clean locally, red in CI' starts" in the same sentence as the length check, which reads as more coverage than exists.

Merge with 83b58ebda clean: 241 passed / 1820 subtests, resolver [133,134,135,136] on the merge, and const 138 + vendor 138 = 276, matching the pre-split single line exactly.

[JarryShaw]

Round-seven cross-review at ce21aceb6: GOOD TO GO.

The risk I briefed it on did not materialise. text.startswith('\t') is evaluated before the new elif not text.strip() or text.lstrip().startswith('#'), so the new branches are unreachable for tab-indented lines and cannot swallow one. Sixteen cases re-run against make's real command count rather than its printed-line count:

• the two that were wrong (col-0 #, blank line in recipe) now return [133, 135, 136, 137] — right line numbers, not just the right count;
• all four loud cases stayed loud, the \ continuation included: tab-indented 2nd line over-counts to 5, un-indented under-counts to 2, both false alarms rather than silent wrong answers;
• one case it had not constructed before — a tab line after a blank and a comment — was a silent wrong answer on 67035d94a (make ran 5, resolver said 4) and now agrees at 5. The continue branches fixed more than the two they were written for;
• the scan still breaks at the next target header, so it does not bleed past the recipe.

One residual, pre-existing and disclosed: a recipe line commented out with a tab is still counted, so the length guard passes while make isort runs three invocations. Benign in direction — the vendor cleanliness check still runs from MAKEFILE_LINES; only the audit that the Makefile still does the sorting is lost — and it sits inside the bound the file now documents. The define limitation is stated honestly as undefended, and grep -nE '^define|^endef' Makefile confirms the premise.

Merged into 83b58ebda in a scratch worktree: no conflicts, resolver still [133, 134, 135, 136], 241 passed / 1820 subtests. Four legs of CI still outstanding on this head.