Sync lenstringzero across linter docs and spec contract (21 → 22)#37751
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
lenstringzero across linter docs and spec contract (21 → 22)
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #37751 does not have the 'implementation' label (has_implementation_label=false) and has only 14 new lines in business logic directories (default_business_additions=14, threshold=100). Neither Condition A nor Condition B is met. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
There was a problem hiding this comment.
Pull request overview
This PR brings the public-facing linter documentation and the spec-enforcing tests back in sync with the actual registered analyzer set by adding the already-implemented lenstringzero analyzer to the documented/spec surface, updating the documented analyzer count from 21 to 22.
Changes:
- Update
pkg/linters/doc.goto reflect 22 active analyzers and includelenstringzeroin the canonical list. - Update
pkg/linters/README.mdto documentlenstringzeroacross the overview list, Public API table, usage example, and dependency list. - Update
pkg/linters/spec_test.goto includelenstringzeroin the documented analyzer contract (imports +documentedAnalyzers()+ count comment).
Show a summary per file
| File | Description |
|---|---|
pkg/linters/spec_test.go |
Extends the README-backed spec contract to include lenstringzero and updates the documented analyzer count to 22. |
pkg/linters/README.md |
Documents lenstringzero consistently across the overview, public API, usage example, and dependency listing. |
pkg/linters/doc.go |
Updates package docs to list 22 analyzers and adds the lenstringzero entry. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 0
|
🧪 Test Quality Sentinel completed test quality analysis. |
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /grill-with-docs — changes look correct and complete. Approving.
📋 Analysis Highlights
What was checked
- Descriptions match implementation: The
lenstringzeropackage doc,Analyzer.Doc, README overview, and Public API table all accurately describe the analyzer behaviour (flagginglen(s) == 0/len(s) != 0on strings). - Alphabetical ordering preserved:
largefunc<lenstringzero<manualmutexunlock✅ across all four README sections,doc.go, andspec_test.go. - All four README surfaces updated: overview list, Public API → Subpackages table, usage import/analyzer example, and internal dependency listing.
- Spec contract correctly tightened:
documentedAnalyzers()entry and the inline comment enumeration both updated; count 21 → 22 is consistent acrossdoc.goandspec_test.go. - Naming convention followed: description label
len-string-zeroin the internal deps section matches the hyphenation pattern used by peers (large-func,fprintln-sprintf).
Positive Highlights
✅ PR description is thorough — clearly explains which surfaces were stale and why
✅ README uses s == "" (more explicit than the bare == "" in Analyzer.Doc) — a clearer read for users
✅ No implementation logic touched; a focused, low-risk sync
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 126.8 AIC · ⌖ 13.4 AIC
🧪 Test Quality Sentinel Report✅ Test Quality Score: 100/100 — Excellent
📊 Metrics & Test Classification (1 test scenario analyzed)
Test Classification Details
Nature of ChangeNo new test functions were added. The PR modifies the
Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References: §27118693886
|
There was a problem hiding this comment.
✅ Test Quality Sentinel: 100/100. Test quality is excellent — 0% of new test scenarios are implementation tests (threshold: 30%). The lenstringzero entry correctly extends three existing table-driven spec tests with a design-level behavioral contract check.
There was a problem hiding this comment.
REQUEST_CHANGES — one blocking regression in the workflow step; the lenstringzero registry additions are correct.
🔴 Blocking issue: `set -e` + grep without `|| true`
The Extract designer file metadata step removes || true from two grep commands inside { ... } command groups. GitHub Actions runs run: steps with bash -eo pipefail; a zero-match grep returns exit code 1, which set -e treats as a fatal error. The step then aborts before fully writing its output files, breaking the drift-audit agent that depends on them.
This is a latent fragility: it works today only because both files happen to contain .github/aw/ references, but will silently break the workflow whenever either file is refactored.
See inline comments for the specific lines.
✅ Looks fine: `lenstringzero` additions
The README, doc.go, and spec_test.go changes correctly register the new linter: count updated (21→22), alphabetical ordering maintained in all lists, import and blank-identifier entries added. No issues.
🔎 Code quality review by PR Code Quality Reviewer · ⌖ 13.8 AIC
lenstringzerowas registered in the analyzer driver but missing from the documented/public spec surface, leaving docs stale at 21 analyzers and omitting spec coverage for the new analyzer entrypoint. This PR aligns package docs, README spec, and spec-test expectations to the 22-analyzer set.Documentation surface (
pkg/linters/doc.go)lenstringzerorule description to the canonical analyzer listREADME spec surface (
pkg/linters/README.md)lenstringzeroto the overview subpackage listlenstringzerorow in Public API → Subpackageslenstringzeroin the usage import/analyzer examplelenstringzeroto internal dependency listinglargefunc, beforemanualmutexunlock)Spec contract enforcement (
pkg/linters/spec_test.go)lenstringzeroimport{"lenstringzero", lenstringzero.Analyzer}todocumentedAnalyzers(){"largefunc", largefunc.Analyzer}, {"lenstringzero", lenstringzero.Analyzer}, {"manualmutexunlock", manualmutexunlock.Analyzer},