Skip to content

Commit 3570ab9

Browse files
committed
Improve /validate skill and analyze script
SKILL.md: write JSON output to CWD instead of temp dir, recommend --details-only flag, expand "What to look for" with modifier-sensitive, context-sensitive, and concentration patterns. analyze.py: add --details-only flag (pipe-friendly, no summary), add 6 new suspicious pattern detectors: const field init, struct fields, digit- suffixed type params, naming in interop files, suggestion rule outliers, concentrated violations. 🤖 Co-Authored-By: Claude Code <noreply@anthropic.com>
1 parent c700999 commit 3570ab9

File tree

2 files changed

+122
-7
lines changed

2 files changed

+122
-7
lines changed

.claude/skills/validate/SKILL.md

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,17 @@ If the path is ambiguous, ask the user to clarify before running.
3535
Run CsLint with `--format json`, save the output, then run the analysis script:
3636

3737
```bash
38-
dotnet run --project src/CsLint.Cli -- --format json <path> [--exclude <exclude-pattern>] 2>/dev/null > "$TEMP/cslint-validate.json"
38+
dotnet run --project src/CsLint.Cli -- --format json <path> [--exclude <exclude-pattern>] 2>/dev/null > cslint-validate.json
3939
```
4040

41+
Write the JSON output to the current directory (not `$TEMP` — that path doesn't work reliably on Windows with Git Bash). Clean up `cslint-validate.json` when done.
42+
4143
CsLint exits 0 (clean), 1 (violations found), or 2 (error). Exit code 1 is expected — it means violations were found, which is what we want to analyze. If exit code is 2, report the error and stop.
4244

4345
Then run the bundled analysis script to get a summary and flag suspicious patterns:
4446

4547
```bash
46-
python "${SKILL_DIR}/scripts/analyze.py" "$TEMP/cslint-validate.json"
48+
python "${SKILL_DIR}/scripts/analyze.py" cslint-validate.json
4749
```
4850

4951
`${SKILL_DIR}` is the directory containing this SKILL.md file.
@@ -56,10 +58,10 @@ The script prints:
5658
To get file:line details for specific rules (useful for investigation):
5759

5860
```bash
59-
python "${SKILL_DIR}/scripts/analyze.py" "$TEMP/cslint-validate.json" --details CSLINT210 CSLINT104
61+
python "${SKILL_DIR}/scripts/analyze.py" cslint-validate.json --details-only CSLINT210 CSLINT104
6062
```
6163

62-
Use `--details` with no rule IDs to print all violations, or pass specific rule IDs to filter.
64+
Use `--details-only` (not `--details`) to skip the summary and print only file:line violations — this works well with piping and `head`. Pass rule IDs to filter, or omit them to print all violations.
6365

6466
## Step 2 — Investigate suspicious patterns
6567

@@ -74,10 +76,23 @@ Use `--details` on rules that look suspicious from the summary, then **read the
7476
- Fields in interop/P/Invoke structs — names must match native APIs
7577
- Local constants flagged by the class-level constant rule
7678

79+
**Modifier-sensitive rules (rules that should behave differently based on modifiers):**
80+
- `const` fields flagged by rules that only apply to mutable fields (e.g., unnecessary initialization — constants *require* an initializer)
81+
- `static readonly` fields flagged by instance-only rules
82+
- Fields in structs flagged by class-only rules (e.g., "field should be private" — struct fields are commonly public for data carriers, interop, etc.)
83+
84+
**Context-sensitive rules (rules that should consider the containing type/scope):**
85+
- Fields in `[StructLayout]` interop structs — must be public for marshaling
86+
- Members in test classes or test harnesses — may follow different conventions
87+
- Members in nested private types — encapsulation is already provided by the outer type
88+
7789
**Style rules (CSLINT200+):**
7890
- Rule suggestion doesn't apply to the actual code pattern (e.g., suggesting `??` on a ternary that returns different types)
7991
- Extremely high violation counts for a single rule vs others (outlier)
8092

93+
**Concentration pattern:**
94+
- If >80% of a rule's violations come from <3 files, it often indicates a context the rule doesn't handle (interop files, generated code, lookup tables with alignment whitespace, etc.)
95+
8196
Group confirmed false positives by root cause. A single bug in CsLint can produce many false positives.
8297

8398
## Step 3 — Report

.claude/skills/validate/scripts/analyze.py

Lines changed: 103 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
"""Analyze CsLint JSON output for false positive patterns.
22
33
Usage:
4-
python analyze.py <results.json> # summary + suspicious patterns
5-
python analyze.py <results.json> --details # also print file:line for every violation
6-
python analyze.py <results.json> --details CSLINT210 CSLINT104 # details for specific rules only
4+
python analyze.py <results.json> # summary + suspicious patterns
5+
python analyze.py <results.json> --details # summary + file:line for every violation
6+
python analyze.py <results.json> --details CSLINT210 # summary + file:line for specific rules
7+
python analyze.py <results.json> --details-only CSLINT210 # file:line ONLY (no summary, pipe-friendly)
78
"""
89

910
import argparse
@@ -98,6 +99,86 @@ def find_suspicious(data: list[dict]) -> dict[str, list[dict]]:
9899
if any(p in fp for p in (".g.cs", ".Generated.", "AssemblyInfo.cs", ".designer.cs")):
99100
suspicious["generated_file"].append(d)
100101

102+
# CSLINT238: "Do not initialize field" on const fields (constants require initializers)
103+
if rid == "CSLINT238" and "Do not initialize field" in msg:
104+
suspicious["possible_const_field_init"].append(d)
105+
106+
# CSLINT251: "Field should be private" — may be in struct/interop context
107+
if rid == "CSLINT251":
108+
suspicious["possible_struct_field"].append(d)
109+
110+
# CSLINT106: digit-suffixed type params (T0, T1, T2) are valid convention
111+
if rid == "CSLINT106":
112+
name_match = re.search(r"'(T\d+)'", msg)
113+
if name_match:
114+
suspicious["digit_suffixed_type_param"].append(d)
115+
116+
# Build per-file and per-rule indexes for cross-cutting checks
117+
by_file: dict[str, list[dict]] = defaultdict(list)
118+
by_rule: dict[str, list[dict]] = defaultdict(list)
119+
for d in data:
120+
by_file[d["filePath"]].append(d)
121+
by_rule[d["ruleId"]].append(d)
122+
123+
# Naming violations in interop files (files containing DllImport/LibraryImport/StructLayout)
124+
interop_files: set[str] = set()
125+
for fp, items in by_file.items():
126+
# Check if any violation in this file hints at interop context
127+
# (we can't read source, but file names and other rule hits are clues)
128+
has_interop_hint = False
129+
for d in items:
130+
# CSLINT251 in a file strongly hints at structs with public fields
131+
if d["ruleId"] == "CSLINT251":
132+
has_interop_hint = True
133+
break
134+
# Field naming violations mentioning ALL_CAPS or native-style names
135+
if d["ruleId"] == "CSLINT104" and d["message"]:
136+
name_match = re.search(r"'(\w+)'", d["message"])
137+
if name_match:
138+
name = name_match.group(1)
139+
# Names with underscores or ALL_CAPS suggest native/interop
140+
if "_" in name or (name.isupper() and len(name) > 2):
141+
has_interop_hint = True
142+
break
143+
if has_interop_hint:
144+
interop_files.add(fp)
145+
146+
# Flag naming rule violations (CSLINT100-106) co-located in interop files
147+
naming_rules = {"CSLINT100", "CSLINT101", "CSLINT102", "CSLINT103", "CSLINT104", "CSLINT105", "CSLINT106"}
148+
for fp in interop_files:
149+
for d in by_file[fp]:
150+
if d["ruleId"] in naming_rules and d not in suspicious.get("verbatim_identifier", []):
151+
suspicious["naming_in_interop_file"].append(d)
152+
153+
# Suggestion rules with outlier counts (5x+ median suggests rule is too aggressive)
154+
suggestion_rules = {
155+
"CSLINT200", "CSLINT201", "CSLINT208", "CSLINT209", "CSLINT210",
156+
"CSLINT216", "CSLINT218", "CSLINT220", "CSLINT222", "CSLINT306",
157+
}
158+
suggestion_counts = {rid: len(items) for rid, items in by_rule.items() if rid in suggestion_rules and len(items) > 0}
159+
if len(suggestion_counts) >= 3:
160+
median_count = sorted(suggestion_counts.values())[len(suggestion_counts) // 2]
161+
if median_count > 0:
162+
for rid, count in suggestion_counts.items():
163+
if count >= median_count * 5:
164+
for d in by_rule[rid]:
165+
suspicious["suggestion_rule_outlier"].append(d)
166+
167+
# Concentration check: rules where >80% of violations come from <3 files
168+
by_rule: dict[str, list[dict]] = defaultdict(list)
169+
for d in data:
170+
by_rule[d["ruleId"]].append(d)
171+
172+
for rid, items in by_rule.items():
173+
if len(items) < 5:
174+
continue
175+
file_counts = Counter(d["filePath"] for d in items)
176+
top_files = file_counts.most_common(3)
177+
top_count = sum(c for _, c in top_files)
178+
if top_count / len(items) > 0.8 and len(file_counts) <= 3:
179+
for d in items:
180+
suspicious["concentrated_violations"].append(d)
181+
101182
return suspicious
102183

103184

@@ -116,6 +197,12 @@ def print_suspicious(suspicious: dict[str, list[dict]]) -> None:
116197
"empty_identifier": "Empty identifier names — CsLint reading wrong token",
117198
"pascal_case_parameter": "PascalCase parameters — may be primary ctor / record params",
118199
"generated_file": "Violations in generated/auto-generated files",
200+
"possible_const_field_init": "Unnecessary init on possible const fields — constants require initializers",
201+
"possible_struct_field": "Field visibility in possible structs — struct fields are commonly public",
202+
"digit_suffixed_type_param": "Digit-suffixed type parameters (T0, T1) — valid naming convention",
203+
"naming_in_interop_file": "Naming violations in likely interop files — names must match native APIs",
204+
"suggestion_rule_outlier": "Suggestion rule with 5x+ median count — rule may be too aggressive",
205+
"concentrated_violations": "Highly concentrated violations (>80% in ≤3 files) — check for unhandled context",
119206
}
120207

121208
for pattern, items in suspicious.items():
@@ -155,6 +242,13 @@ def main() -> None:
155242
metavar="RULE_ID",
156243
help="Print file:line for every violation. Optionally filter by rule IDs (e.g. --details CSLINT210 CSLINT104)",
157244
)
245+
parser.add_argument(
246+
"--details-only",
247+
nargs="*",
248+
default=None,
249+
metavar="RULE_ID",
250+
help="Print ONLY file:line details (no summary). Pipe-friendly. Optionally filter by rule IDs.",
251+
)
158252
args = parser.parse_args()
159253

160254
data = load_results(args.results)
@@ -163,6 +257,12 @@ def main() -> None:
163257
print("No violations found. CsLint reported a clean run.")
164258
sys.exit(0)
165259

260+
# --details-only: skip summary, print only file:line details
261+
if args.details_only is not None:
262+
rule_filter = args.details_only if args.details_only else None
263+
print_details(data, rule_filter)
264+
return
265+
166266
print_summary(data)
167267
suspicious = find_suspicious(data)
168268
print_suspicious(suspicious)

0 commit comments

Comments
 (0)