Skip to content

Commit 9eff6bb

Browse files
authored
Merge pull request #78 from reidbaker/r-add-fix-option
add a --fix flag and --fix-apply flag for fixing some issues
2 parents 777882c + 57f4114 commit 9eff6bb

13 files changed

Lines changed: 671 additions & 21 deletions

‎tool/dart_skills_lint/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ If no directory is specified, it automatically checks `.claude/skills` and `.age
7474
- `--fast-fail`: Halt execution immediately on the error.
7575
- `--ignore-config`: Ignore the YAML configuration file entirely.
7676
- `--[no-]check-trailing-whitespace`: Enable/disable checking for trailing whitespace. (Disabled by default).
77+
- `--fix`: Preview fixes for failing lints (dry run).
78+
- `--fix-apply`: Apply fixes for failing lints.
7779

7880
### 2. As a Command Line Tool with a YAML Configuration File
7981
You can configure the linter using a configuration file (defaulting to `dart_skills_lint.yaml` in the current directory).

‎tool/dart_skills_lint/lib/src/entry_point.dart‎

Lines changed: 164 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,15 @@ import 'package:meta/meta.dart';
1111
import 'package:path/path.dart' as p;
1212

1313
import 'config_parser.dart';
14+
import 'fixable_rule.dart';
1415
import 'models/analysis_severity.dart';
1516
import 'models/check_type.dart';
1617
import 'models/ignore_entry.dart';
18+
import 'models/skill_context.dart';
1719
import 'models/skill_rule.dart';
1820
import 'models/skills_ignores.dart';
1921
import 'models/validation_error.dart';
2022
import 'rule_registry.dart';
21-
2223
import 'skills_ignores_storage.dart';
2324
import 'validator.dart';
2425

@@ -32,6 +33,8 @@ const _skillOption = 'skill';
3233
const _ignoreFileOption = 'ignore-file';
3334
const _ignoreConfigFlag = 'ignore-config';
3435
const _generateBaselineFlag = 'generate-baseline';
36+
const _fixFlag = 'fix';
37+
const _fixApplyFlag = 'fix-apply';
3538

3639
@visibleForTesting
3740
const defaultIgnoreFileName = 'dart_skills_lint_ignore.json';
@@ -93,7 +96,9 @@ Future<void> runApp(List<String> args) async {
9396
negatable: false,
9497
help: 'Write all current errors into $defaultIgnoreFileName to ignore on future runs.')
9598
..addFlag(_ignoreConfigFlag,
96-
negatable: false, help: 'Ignore the YAML configuration file entirely.');
99+
negatable: false, help: 'Ignore the YAML configuration file entirely.')
100+
..addFlag(_fixFlag, negatable: false, help: 'Preview fixes for failing lints (dry run).')
101+
..addFlag(_fixApplyFlag, negatable: false, help: 'Apply fixes for failing lints.');
97102

98103
final ArgResults results;
99104
try {
@@ -143,20 +148,24 @@ Future<void> runApp(List<String> args) async {
143148
final fastFail = results[_fastFailFlag] as bool;
144149
final quiet = results[_quietFlag] as bool;
145150
final generateBaseline = results[_generateBaselineFlag] as bool;
151+
final fix = results[_fixFlag] as bool;
152+
final fixApply = results[_fixApplyFlag] as bool;
146153

147154
String? ignoreFileOverride;
148155
if (results.wasParsed(_ignoreFileOption)) {
149156
ignoreFileOverride = results[_ignoreFileOption] as String?;
150157
}
151158

152-
final bool success = await validateSkills(
159+
final bool success = await validateSkillsInternal(
153160
skillDirPaths: skillDirPaths,
154161
individualSkillPaths: individualSkillPaths,
155162
resolvedRules: resolvedRules,
156163
printWarnings: printWarnings,
157164
fastFail: fastFail,
158165
quiet: quiet,
159166
generateBaseline: generateBaseline,
167+
fix: fix,
168+
fixApply: fixApply,
160169
ignoreFileOverride: ignoreFileOverride,
161170
config: config,
162171
);
@@ -166,6 +175,9 @@ Future<void> runApp(List<String> args) async {
166175

167176
/// Validates skills based on the provided configuration.
168177
///
178+
/// This is the public API for validating skills. It does not support fixing
179+
/// lints as that feature is currently considered internal to the CLI.
180+
///
169181
/// [skillDirPaths] is a list of directories containing multiple skills.
170182
/// [individualSkillPaths] is a list of paths to individual skill directories.
171183
/// [resolvedRules] is a map of rule names to their severity overrides.
@@ -188,6 +200,38 @@ Future<bool> validateSkills({
188200
String? ignoreFileOverride,
189201
Configuration? config,
190202
List<SkillRule> customRules = const [],
203+
}) async {
204+
return validateSkillsInternal(
205+
skillDirPaths: skillDirPaths,
206+
individualSkillPaths: individualSkillPaths,
207+
resolvedRules: resolvedRules,
208+
printWarnings: printWarnings,
209+
fastFail: fastFail,
210+
quiet: quiet,
211+
generateBaseline: generateBaseline,
212+
ignoreFileOverride: ignoreFileOverride,
213+
config: config,
214+
customRules: customRules,
215+
);
216+
}
217+
218+
/// Internal implementation of skill validation that supports fixing.
219+
///
220+
/// Kept internal to avoid exposing experimental fix parameters in the public API.
221+
@visibleForTesting
222+
Future<bool> validateSkillsInternal({
223+
List<String> skillDirPaths = const [],
224+
List<String> individualSkillPaths = const [],
225+
Map<String, AnalysisSeverity> resolvedRules = const {},
226+
bool printWarnings = true,
227+
bool fastFail = false,
228+
bool quiet = false,
229+
bool generateBaseline = false,
230+
bool fix = false,
231+
bool fixApply = false,
232+
String? ignoreFileOverride,
233+
Configuration? config,
234+
List<SkillRule> customRules = const [],
191235
}) async {
192236
config ??= Configuration();
193237
var globalAnyFailed = false;
@@ -231,8 +275,18 @@ Future<bool> validateSkills({
231275
quiet: quiet,
232276
);
233277

278+
final ValidationResult finalResult = await _applyFixesIfNeeded(
279+
skillDir: skillDir,
280+
result: result,
281+
validator: validator,
282+
skillIgnores: skillIgnores,
283+
fix: fix,
284+
fixApply: fixApply,
285+
quiet: quiet,
286+
);
287+
234288
if (generateBaseline) {
235-
await _generateBaselineFile(result, localIgnoreFile, skillDir, skillDir);
289+
await _generateBaselineFile(finalResult, localIgnoreFile, skillDir, skillDir);
236290
}
237291

238292
if (!generateBaseline) {
@@ -245,7 +299,7 @@ Future<bool> validateSkills({
245299
}
246300
}
247301

248-
if (!result.isValid) {
302+
if (!finalResult.isValid) {
249303
globalAnyFailed = true;
250304
if (fastFail) {
251305
break;
@@ -304,11 +358,21 @@ Future<bool> validateSkills({
304358
quiet: quiet,
305359
);
306360

361+
final ValidationResult finalResult = await _applyFixesIfNeeded(
362+
skillDir: entity,
363+
result: result,
364+
validator: validator,
365+
skillIgnores: ignoresMap[p.basename(entity.path)] ?? [],
366+
fix: fix,
367+
fixApply: fixApply,
368+
quiet: quiet,
369+
);
370+
307371
if (generateBaseline) {
308-
await _generateBaselineFile(result, localIgnoreFile, rootDir, entity);
372+
await _generateBaselineFile(finalResult, localIgnoreFile, rootDir, entity);
309373
}
310374

311-
if (!result.isValid) {
375+
if (!finalResult.isValid) {
312376
globalAnyFailed = true;
313377
if (fastFail) {
314378
break;
@@ -339,7 +403,7 @@ Future<bool> validateSkills({
339403
var foundSingleSkillPassedToD = false;
340404
for (final rootPath in skillDirPaths) {
341405
final String expandedRootPath = _expandPath(rootPath);
342-
final skillMdFile = File(p.join(expandedRootPath, 'SKILL.md'));
406+
final skillMdFile = File(p.join(expandedRootPath, SkillContext.skillFileName));
343407
if (skillMdFile.existsSync()) {
344408
_log.severe(
345409
'Directory "$expandedRootPath" appears to be an individual skill. Use --skill / -s instead of -d / --skills-directory.');
@@ -483,6 +547,98 @@ Future<ValidationResult> _validateSingleSkill({
483547
return result;
484548
}
485549

550+
Future<ValidationResult> _applyFixesIfNeeded({
551+
required Directory skillDir,
552+
required ValidationResult result,
553+
required Validator validator,
554+
required List<IgnoreEntry> skillIgnores,
555+
required bool fix,
556+
required bool fixApply,
557+
required bool quiet,
558+
}) async {
559+
if (!fix && !fixApply) {
560+
return result;
561+
}
562+
563+
final SkillContext? context = result.context;
564+
if (context == null) {
565+
return result;
566+
}
567+
568+
final String skillName = p.basename(skillDir.path);
569+
final skillMdFile = File(p.join(skillDir.path, SkillContext.skillFileName));
570+
if (!skillMdFile.existsSync()) {
571+
return result;
572+
}
573+
574+
String currentContent = context.rawContent;
575+
final originalContent = currentContent;
576+
var modified = false;
577+
578+
for (final SkillRule rule in validator.rules) {
579+
if (rule is FixableRule) {
580+
final bool hasErrors =
581+
result.validationErrors.any((e) => e.ruleId == rule.name && !e.isIgnored);
582+
if (hasErrors) {
583+
try {
584+
final String newContent = await (rule as FixableRule)
585+
.fix(SkillContext.skillFileName, currentContent, context.directory);
586+
if (newContent != currentContent) {
587+
currentContent = newContent;
588+
modified = true;
589+
}
590+
} catch (e) {
591+
_log.severe(" Failed to apply fix for rule '${rule.name}': $e");
592+
}
593+
}
594+
}
595+
}
596+
597+
if (modified) {
598+
if (fixApply) {
599+
await skillMdFile.writeAsString(currentContent);
600+
if (!quiet) {
601+
_log.info(' Applied fixes for $skillName');
602+
}
603+
final ValidationResult newResult = await validator.validate(skillDir);
604+
_applyIgnores(newResult, skillIgnores, skillDir);
605+
return newResult;
606+
} else if (fix) {
607+
if (!quiet) {
608+
_log.info(' [Dry Run] Proposed changes for $skillName (SKILL.md):');
609+
_printDiff(originalContent, currentContent);
610+
}
611+
}
612+
}
613+
614+
return result;
615+
}
616+
617+
/// Prints a simple line-by-line diff between [original] and [modified].
618+
///
619+
/// **Limitation**: This naive diff algorithm does not handle line additions or
620+
/// removals well, as it compares lines at the same index. It is sufficient for
621+
/// current fixers that only modify existing lines, but should be replaced with
622+
/// a more robust diffing solution (e.g., `package:diff`) if future fixers
623+
/// add or remove lines.
624+
void _printDiff(String original, String modified) {
625+
final List<String> origLines = original.split('\n');
626+
final List<String> modLines = modified.split('\n');
627+
final int maxLines = origLines.length > modLines.length ? origLines.length : modLines.length;
628+
for (var i = 0; i < maxLines; i++) {
629+
final String orig = i < origLines.length ? origLines[i] : '';
630+
final String mod = i < modLines.length ? modLines[i] : '';
631+
if (orig != mod) {
632+
if (orig.isNotEmpty) {
633+
_log.info('- Line ${i + 1}: $orig');
634+
}
635+
if (mod.isNotEmpty) {
636+
_log.info('+ Line ${i + 1}: $mod');
637+
}
638+
}
639+
}
640+
}
641+
486642
Future<void> _generateBaselineFile(ValidationResult result, String? ignoreFileOverride,
487643
Directory rootDir, Directory skillDir) async {
488644
final String ignorePath = ignoreFileOverride != null
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:io';
6+
7+
/// Interface for rules that support fixes.
8+
/// Kept internal to the package for now.
9+
abstract class FixableRule {
10+
/// Returns the updated content of the file at [filePath].
11+
/// [currentContent] is the content after previous fixes have been applied.
12+
/// If the rule does not support fixing the file at [filePath], it should return [currentContent].
13+
///
14+
/// Rules should rely on [currentContent] for the current state of the file.
15+
/// If a rule needs structured access (like parsed YAML), it should parse
16+
/// [currentContent] itself, as structured data in [SkillContext] may be stale
17+
/// if previous rules applied fixes.
18+
Future<String> fix(String filePath, String currentContent, Directory directory);
19+
}

‎tool/dart_skills_lint/lib/src/models/skill_context.dart‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ class SkillContext {
1010
this.yamlParsingError,
1111
});
1212

13+
/// The required filename for skill documentation.
14+
static const String skillFileName = 'SKILL.md';
15+
16+
/// Regex to match the YAML frontmatter in SKILL.md.
17+
static final RegExp skillStartRegex = RegExp(r'^---\s*\n(.*?)\n---\s*\n', dotAll: true);
18+
1319
final Directory directory;
1420

1521
/// Guaranteed to be non-null because we only run rules if SKILL.md exists.

‎tool/dart_skills_lint/lib/src/rules/absolute_paths_rule.dart‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1+
import 'dart:io';
12
import 'package:path/path.dart';
3+
import '../fixable_rule.dart';
24
import '../models/analysis_severity.dart';
35
import '../models/skill_context.dart';
46
import '../models/skill_rule.dart';
57
import '../models/validation_error.dart';
68

79
/// Enforces that links in SKILL.md do not use absolute paths.
8-
class AbsolutePathsRule extends SkillRule {
10+
class AbsolutePathsRule extends SkillRule implements FixableRule {
911
AbsolutePathsRule({this.severity = defaultSeverity});
1012

1113
static const String ruleName = 'check-absolute-paths';
@@ -18,7 +20,7 @@ class AbsolutePathsRule extends SkillRule {
1820
final AnalysisSeverity severity;
1921

2022
static final _markdownLinkRegex = RegExp(r'\[.*?\]\((.*?)\)');
21-
static const _skillFileName = 'SKILL.md';
23+
static const String _skillFileName = SkillContext.skillFileName;
2224

2325
@override
2426
Future<List<ValidationError>> validate(SkillContext context) async {
@@ -44,4 +46,26 @@ class AbsolutePathsRule extends SkillRule {
4446

4547
return errors;
4648
}
49+
50+
@override
51+
Future<String> fix(String filePath, String currentContent, Directory directory) async {
52+
if (filePath != SkillContext.skillFileName) {
53+
return currentContent;
54+
}
55+
56+
return currentContent.replaceAllMapped(_markdownLinkRegex, (match) {
57+
final String path = match.group(1)!;
58+
if (isAbsolute(path) || windows.isAbsolute(path)) {
59+
final file = File(path);
60+
if (file.existsSync()) {
61+
final String relativePath = relative(path, from: directory.path);
62+
final String posixRelativePath = relativePath.replaceAll(r'\', '/');
63+
final String fullMatch = match.group(0)!;
64+
final int lastParen = fullMatch.lastIndexOf('(');
65+
return '${fullMatch.substring(0, lastParen + 1)}$posixRelativePath)';
66+
}
67+
}
68+
return match.group(0)!;
69+
});
70+
}
4771
}

0 commit comments

Comments
 (0)