diff --git a/tool/dart_skills_lint/README.md b/tool/dart_skills_lint/README.md index 1d27af8..becf309 100644 --- a/tool/dart_skills_lint/README.md +++ b/tool/dart_skills_lint/README.md @@ -74,6 +74,8 @@ If no directory is specified, it automatically checks `.claude/skills` and `.age - `--fast-fail`: Halt execution immediately on the error. - `--ignore-config`: Ignore the YAML configuration file entirely. - `--[no-]check-trailing-whitespace`: Enable/disable checking for trailing whitespace. (Disabled by default). +- `--fix`: Preview fixes for failing lints (dry run). +- `--fix-apply`: Apply fixes for failing lints. ### 2. As a Command Line Tool with a YAML Configuration File You can configure the linter using a configuration file (defaulting to `dart_skills_lint.yaml` in the current directory). diff --git a/tool/dart_skills_lint/lib/src/entry_point.dart b/tool/dart_skills_lint/lib/src/entry_point.dart index b2ea0bd..301530f 100644 --- a/tool/dart_skills_lint/lib/src/entry_point.dart +++ b/tool/dart_skills_lint/lib/src/entry_point.dart @@ -11,14 +11,15 @@ import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; import 'config_parser.dart'; +import 'fixable_rule.dart'; import 'models/analysis_severity.dart'; import 'models/check_type.dart'; import 'models/ignore_entry.dart'; +import 'models/skill_context.dart'; import 'models/skill_rule.dart'; import 'models/skills_ignores.dart'; import 'models/validation_error.dart'; import 'rule_registry.dart'; - import 'skills_ignores_storage.dart'; import 'validator.dart'; @@ -32,6 +33,8 @@ const _skillOption = 'skill'; const _ignoreFileOption = 'ignore-file'; const _ignoreConfigFlag = 'ignore-config'; const _generateBaselineFlag = 'generate-baseline'; +const _fixFlag = 'fix'; +const _fixApplyFlag = 'fix-apply'; @visibleForTesting const defaultIgnoreFileName = 'dart_skills_lint_ignore.json'; @@ -93,7 +96,9 @@ Future runApp(List args) async { negatable: false, help: 'Write all current errors into $defaultIgnoreFileName to ignore on future runs.') ..addFlag(_ignoreConfigFlag, - negatable: false, help: 'Ignore the YAML configuration file entirely.'); + negatable: false, help: 'Ignore the YAML configuration file entirely.') + ..addFlag(_fixFlag, negatable: false, help: 'Preview fixes for failing lints (dry run).') + ..addFlag(_fixApplyFlag, negatable: false, help: 'Apply fixes for failing lints.'); final ArgResults results; try { @@ -143,13 +148,15 @@ Future runApp(List args) async { final fastFail = results[_fastFailFlag] as bool; final quiet = results[_quietFlag] as bool; final generateBaseline = results[_generateBaselineFlag] as bool; + final fix = results[_fixFlag] as bool; + final fixApply = results[_fixApplyFlag] as bool; String? ignoreFileOverride; if (results.wasParsed(_ignoreFileOption)) { ignoreFileOverride = results[_ignoreFileOption] as String?; } - final bool success = await validateSkills( + final bool success = await validateSkillsInternal( skillDirPaths: skillDirPaths, individualSkillPaths: individualSkillPaths, resolvedRules: resolvedRules, @@ -157,6 +164,8 @@ Future runApp(List args) async { fastFail: fastFail, quiet: quiet, generateBaseline: generateBaseline, + fix: fix, + fixApply: fixApply, ignoreFileOverride: ignoreFileOverride, config: config, ); @@ -166,6 +175,9 @@ Future runApp(List args) async { /// Validates skills based on the provided configuration. /// +/// This is the public API for validating skills. It does not support fixing +/// lints as that feature is currently considered internal to the CLI. +/// /// [skillDirPaths] is a list of directories containing multiple skills. /// [individualSkillPaths] is a list of paths to individual skill directories. /// [resolvedRules] is a map of rule names to their severity overrides. @@ -188,6 +200,38 @@ Future validateSkills({ String? ignoreFileOverride, Configuration? config, List customRules = const [], +}) async { + return validateSkillsInternal( + skillDirPaths: skillDirPaths, + individualSkillPaths: individualSkillPaths, + resolvedRules: resolvedRules, + printWarnings: printWarnings, + fastFail: fastFail, + quiet: quiet, + generateBaseline: generateBaseline, + ignoreFileOverride: ignoreFileOverride, + config: config, + customRules: customRules, + ); +} + +/// Internal implementation of skill validation that supports fixing. +/// +/// Kept internal to avoid exposing experimental fix parameters in the public API. +@visibleForTesting +Future validateSkillsInternal({ + List skillDirPaths = const [], + List individualSkillPaths = const [], + Map resolvedRules = const {}, + bool printWarnings = true, + bool fastFail = false, + bool quiet = false, + bool generateBaseline = false, + bool fix = false, + bool fixApply = false, + String? ignoreFileOverride, + Configuration? config, + List customRules = const [], }) async { config ??= Configuration(); var globalAnyFailed = false; @@ -231,8 +275,18 @@ Future validateSkills({ quiet: quiet, ); + final ValidationResult finalResult = await _applyFixesIfNeeded( + skillDir: skillDir, + result: result, + validator: validator, + skillIgnores: skillIgnores, + fix: fix, + fixApply: fixApply, + quiet: quiet, + ); + if (generateBaseline) { - await _generateBaselineFile(result, localIgnoreFile, skillDir, skillDir); + await _generateBaselineFile(finalResult, localIgnoreFile, skillDir, skillDir); } if (!generateBaseline) { @@ -245,7 +299,7 @@ Future validateSkills({ } } - if (!result.isValid) { + if (!finalResult.isValid) { globalAnyFailed = true; if (fastFail) { break; @@ -304,11 +358,21 @@ Future validateSkills({ quiet: quiet, ); + final ValidationResult finalResult = await _applyFixesIfNeeded( + skillDir: entity, + result: result, + validator: validator, + skillIgnores: ignoresMap[p.basename(entity.path)] ?? [], + fix: fix, + fixApply: fixApply, + quiet: quiet, + ); + if (generateBaseline) { - await _generateBaselineFile(result, localIgnoreFile, rootDir, entity); + await _generateBaselineFile(finalResult, localIgnoreFile, rootDir, entity); } - if (!result.isValid) { + if (!finalResult.isValid) { globalAnyFailed = true; if (fastFail) { break; @@ -339,7 +403,7 @@ Future validateSkills({ var foundSingleSkillPassedToD = false; for (final rootPath in skillDirPaths) { final String expandedRootPath = _expandPath(rootPath); - final skillMdFile = File(p.join(expandedRootPath, 'SKILL.md')); + final skillMdFile = File(p.join(expandedRootPath, SkillContext.skillFileName)); if (skillMdFile.existsSync()) { _log.severe( 'Directory "$expandedRootPath" appears to be an individual skill. Use --skill / -s instead of -d / --skills-directory.'); @@ -483,6 +547,98 @@ Future _validateSingleSkill({ return result; } +Future _applyFixesIfNeeded({ + required Directory skillDir, + required ValidationResult result, + required Validator validator, + required List skillIgnores, + required bool fix, + required bool fixApply, + required bool quiet, +}) async { + if (!fix && !fixApply) { + return result; + } + + final SkillContext? context = result.context; + if (context == null) { + return result; + } + + final String skillName = p.basename(skillDir.path); + final skillMdFile = File(p.join(skillDir.path, SkillContext.skillFileName)); + if (!skillMdFile.existsSync()) { + return result; + } + + String currentContent = context.rawContent; + final originalContent = currentContent; + var modified = false; + + for (final SkillRule rule in validator.rules) { + if (rule is FixableRule) { + final bool hasErrors = + result.validationErrors.any((e) => e.ruleId == rule.name && !e.isIgnored); + if (hasErrors) { + try { + final String newContent = await (rule as FixableRule) + .fix(SkillContext.skillFileName, currentContent, context.directory); + if (newContent != currentContent) { + currentContent = newContent; + modified = true; + } + } catch (e) { + _log.severe(" Failed to apply fix for rule '${rule.name}': $e"); + } + } + } + } + + if (modified) { + if (fixApply) { + await skillMdFile.writeAsString(currentContent); + if (!quiet) { + _log.info(' Applied fixes for $skillName'); + } + final ValidationResult newResult = await validator.validate(skillDir); + _applyIgnores(newResult, skillIgnores, skillDir); + return newResult; + } else if (fix) { + if (!quiet) { + _log.info(' [Dry Run] Proposed changes for $skillName (SKILL.md):'); + _printDiff(originalContent, currentContent); + } + } + } + + return result; +} + +/// Prints a simple line-by-line diff between [original] and [modified]. +/// +/// **Limitation**: This naive diff algorithm does not handle line additions or +/// removals well, as it compares lines at the same index. It is sufficient for +/// current fixers that only modify existing lines, but should be replaced with +/// a more robust diffing solution (e.g., `package:diff`) if future fixers +/// add or remove lines. +void _printDiff(String original, String modified) { + final List origLines = original.split('\n'); + final List modLines = modified.split('\n'); + final int maxLines = origLines.length > modLines.length ? origLines.length : modLines.length; + for (var i = 0; i < maxLines; i++) { + final String orig = i < origLines.length ? origLines[i] : ''; + final String mod = i < modLines.length ? modLines[i] : ''; + if (orig != mod) { + if (orig.isNotEmpty) { + _log.info('- Line ${i + 1}: $orig'); + } + if (mod.isNotEmpty) { + _log.info('+ Line ${i + 1}: $mod'); + } + } + } +} + Future _generateBaselineFile(ValidationResult result, String? ignoreFileOverride, Directory rootDir, Directory skillDir) async { final String ignorePath = ignoreFileOverride != null diff --git a/tool/dart_skills_lint/lib/src/fixable_rule.dart b/tool/dart_skills_lint/lib/src/fixable_rule.dart new file mode 100644 index 0000000..2510673 --- /dev/null +++ b/tool/dart_skills_lint/lib/src/fixable_rule.dart @@ -0,0 +1,19 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:io'; + +/// Interface for rules that support fixes. +/// Kept internal to the package for now. +abstract class FixableRule { + /// Returns the updated content of the file at [filePath]. + /// [currentContent] is the content after previous fixes have been applied. + /// If the rule does not support fixing the file at [filePath], it should return [currentContent]. + /// + /// Rules should rely on [currentContent] for the current state of the file. + /// If a rule needs structured access (like parsed YAML), it should parse + /// [currentContent] itself, as structured data in [SkillContext] may be stale + /// if previous rules applied fixes. + Future fix(String filePath, String currentContent, Directory directory); +} diff --git a/tool/dart_skills_lint/lib/src/models/skill_context.dart b/tool/dart_skills_lint/lib/src/models/skill_context.dart index bf4eaaa..6501728 100644 --- a/tool/dart_skills_lint/lib/src/models/skill_context.dart +++ b/tool/dart_skills_lint/lib/src/models/skill_context.dart @@ -10,6 +10,12 @@ class SkillContext { this.yamlParsingError, }); + /// The required filename for skill documentation. + static const String skillFileName = 'SKILL.md'; + + /// Regex to match the YAML frontmatter in SKILL.md. + static final RegExp skillStartRegex = RegExp(r'^---\s*\n(.*?)\n---\s*\n', dotAll: true); + final Directory directory; /// Guaranteed to be non-null because we only run rules if SKILL.md exists. diff --git a/tool/dart_skills_lint/lib/src/rules/absolute_paths_rule.dart b/tool/dart_skills_lint/lib/src/rules/absolute_paths_rule.dart index 9de9dce..cf2bb46 100644 --- a/tool/dart_skills_lint/lib/src/rules/absolute_paths_rule.dart +++ b/tool/dart_skills_lint/lib/src/rules/absolute_paths_rule.dart @@ -1,11 +1,13 @@ +import 'dart:io'; import 'package:path/path.dart'; +import '../fixable_rule.dart'; import '../models/analysis_severity.dart'; import '../models/skill_context.dart'; import '../models/skill_rule.dart'; import '../models/validation_error.dart'; /// Enforces that links in SKILL.md do not use absolute paths. -class AbsolutePathsRule extends SkillRule { +class AbsolutePathsRule extends SkillRule implements FixableRule { AbsolutePathsRule({this.severity = defaultSeverity}); static const String ruleName = 'check-absolute-paths'; @@ -18,7 +20,7 @@ class AbsolutePathsRule extends SkillRule { final AnalysisSeverity severity; static final _markdownLinkRegex = RegExp(r'\[.*?\]\((.*?)\)'); - static const _skillFileName = 'SKILL.md'; + static const String _skillFileName = SkillContext.skillFileName; @override Future> validate(SkillContext context) async { @@ -44,4 +46,26 @@ class AbsolutePathsRule extends SkillRule { return errors; } + + @override + Future fix(String filePath, String currentContent, Directory directory) async { + if (filePath != SkillContext.skillFileName) { + return currentContent; + } + + return currentContent.replaceAllMapped(_markdownLinkRegex, (match) { + final String path = match.group(1)!; + if (isAbsolute(path) || windows.isAbsolute(path)) { + final file = File(path); + if (file.existsSync()) { + final String relativePath = relative(path, from: directory.path); + final String posixRelativePath = relativePath.replaceAll(r'\', '/'); + final String fullMatch = match.group(0)!; + final int lastParen = fullMatch.lastIndexOf('('); + return '${fullMatch.substring(0, lastParen + 1)}$posixRelativePath)'; + } + } + return match.group(0)!; + }); + } } diff --git a/tool/dart_skills_lint/lib/src/rules/name_format_rule.dart b/tool/dart_skills_lint/lib/src/rules/name_format_rule.dart index 9334d43..e22b0e6 100644 --- a/tool/dart_skills_lint/lib/src/rules/name_format_rule.dart +++ b/tool/dart_skills_lint/lib/src/rules/name_format_rule.dart @@ -1,12 +1,15 @@ +import 'dart:io'; +import 'package:meta/meta.dart'; import 'package:path/path.dart'; import 'package:yaml/yaml.dart'; +import '../fixable_rule.dart'; import '../models/analysis_severity.dart'; import '../models/skill_context.dart'; import '../models/skill_rule.dart'; import '../models/validation_error.dart'; /// Enforces constraints on the skill name field. -class NameFormatRule extends SkillRule { +class NameFormatRule extends SkillRule implements FixableRule { NameFormatRule({this.severity = defaultSeverity}); static const String ruleName = 'invalid-skill-name'; @@ -20,7 +23,7 @@ class NameFormatRule extends SkillRule { static const maxNameLength = 64; static final _validNameRegex = RegExp(r'^[a-z0-9\-]+$'); - static const _skillFileName = 'SKILL.md'; + static const String _skillFileName = SkillContext.skillFileName; static const _nameFieldUrl = 'https://agentskills.io/specification#name-field'; @override @@ -32,7 +35,7 @@ class NameFormatRule extends SkillRule { } final YamlMap yaml = context.parsedYaml!; - final String skillName = yaml['name']?.toString() ?? ''; + final String skillName = getNameNode(yaml)?.value.toString() ?? ''; if (skillName.isEmpty) { return errors; // Handled by required fields check @@ -91,10 +94,62 @@ class NameFormatRule extends SkillRule { severity: severity, file: _skillFileName, message: - 'Skill name ($skillName) must exactly match the name of its parent directory ($dirName) (see $_nameFieldUrl)', + 'Skill name ($skillName) must exactly match the parent directory name ($dirName) (see $_nameFieldUrl)', )); } return errors; } + + @override + Future fix(String filePath, String currentContent, Directory directory) async { + if (filePath != SkillContext.skillFileName) { + return currentContent; + } + + final RegExpMatch? match = SkillContext.skillStartRegex.firstMatch(currentContent); + if (match == null) { + return currentContent; + } + final String yamlStr = match.group(1)!; + + final dynamic yamlObj; + try { + yamlObj = loadYaml(yamlStr); + } catch (e) { + return currentContent; + } + + if (yamlObj is! YamlMap) { + return currentContent; + } + + final YamlMap yaml = yamlObj; + final YamlNode? nameNode = getNameNode(yaml); + if (nameNode == null) { + return currentContent; + } + + final String dirName = basename(directory.path); + + final currentName = nameNode.value.toString(); + if (currentName == dirName) { + return currentContent; + } + + final int yamlOffset = currentContent.indexOf(yamlStr, match.start); + + // ignore: specify_nonobvious_local_variable_types + final span = nameNode.span; + final String before = currentContent.substring(0, yamlOffset + span.start.offset); + final String after = currentContent.substring(yamlOffset + span.end.offset); + + return '$before$dirName$after'; + } + + /// Returns the YAML node for the skill name. + @visibleForTesting + static YamlNode? getNameNode(YamlMap yaml) { + return yaml.nodes['name']; + } } diff --git a/tool/dart_skills_lint/lib/src/rules/trailing_whitespace_rule.dart b/tool/dart_skills_lint/lib/src/rules/trailing_whitespace_rule.dart index d5d2883..2e9b0f5 100644 --- a/tool/dart_skills_lint/lib/src/rules/trailing_whitespace_rule.dart +++ b/tool/dart_skills_lint/lib/src/rules/trailing_whitespace_rule.dart @@ -2,6 +2,10 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'dart:io'; +import 'package:meta/meta.dart'; + +import '../fixable_rule.dart'; import '../models/analysis_severity.dart'; import '../models/skill_context.dart'; import '../models/skill_rule.dart'; @@ -9,7 +13,7 @@ import '../models/validation_error.dart'; /// Enforces that lines in SKILL.md do not have trailing whitespace, /// except for exactly two spaces which indicate a hard line break. -class TrailingWhitespaceRule extends SkillRule { +class TrailingWhitespaceRule extends SkillRule implements FixableRule { TrailingWhitespaceRule({this.severity = defaultSeverity}); static const String ruleName = 'check-trailing-whitespace'; @@ -61,4 +65,32 @@ class TrailingWhitespaceRule extends SkillRule { return errors; } + + @override + Future fix(String filePath, String currentContent, Directory directory) async { + if (filePath != 'SKILL.md') { + return currentContent; + } + + return currentContent.split('\n').map(fixLine).join('\n'); + } + + @visibleForTesting + String fixLine(String line) { + final bool hasCR = line.endsWith('\r'); + final String lineWithoutCR = hasCR ? line.substring(0, line.length - 1) : line; + + final RegExpMatch? match = _whitespaceRegExp.firstMatch(lineWithoutCR); + if (match == null) { + return line; + } + + final String whitespace = match.group(1)!; + if (whitespace == ' ') { + return line; // Keep the 2 space hard line break. + } + + final String fixedLine = lineWithoutCR.replaceAll(_whitespaceRegExp, ''); + return hasCR ? '$fixedLine\r' : fixedLine; + } } diff --git a/tool/dart_skills_lint/lib/src/validator.dart b/tool/dart_skills_lint/lib/src/validator.dart index 57ef199..70bb479 100644 --- a/tool/dart_skills_lint/lib/src/validator.dart +++ b/tool/dart_skills_lint/lib/src/validator.dart @@ -24,8 +24,12 @@ class ValidationResult { ValidationResult({ this.validationErrors = const [], List warnings = const [], + this.context, }) : _manualWarnings = warnings; + /// The context used during validation. + final SkillContext? context; + /// Whether the skill directory is valid according to the specification. bool get isValid => !validationErrors.any((e) => e.severity == AnalysisSeverity.error && !e.isIgnored); @@ -59,7 +63,7 @@ class Validator { _customRules = customRules ?? [] { _rules = _buildRules(); } - static const _skillFileName = 'SKILL.md'; + static const String _skillFileName = SkillContext.skillFileName; /// The name of the special check for missing files or directories. static const String pathDoesNotExist = 'path-does-not-exist'; @@ -74,12 +78,13 @@ class Validator { final List _customRules; late final List _rules; + /// Returns the rules used by this validator. + List get rules => _rules; + AnalysisSeverity _getSeverity(String name, AnalysisSeverity defaultSeverity) { return _customSeverities[name] ?? defaultSeverity; } - static final _skillStartRegex = RegExp(r'^---\s*\n(.*?)\n---\s*\n', dotAll: true); - /// Validates a single skill directory. /// /// Scans the directory for `SKILL.md`, parses its YAML metadata, and validates @@ -115,7 +120,7 @@ class Validator { YamlMap? parsedYaml; String? yamlParsingError; try { - final RegExpMatch? match = _skillStartRegex.firstMatch(content); + final RegExpMatch? match = SkillContext.skillStartRegex.firstMatch(content); if (match != null) { final String yamlStr = match.group(1)!; final Object? doc = loadYaml(yamlStr); @@ -151,6 +156,7 @@ class Validator { return ValidationResult( validationErrors: validationErrors, + context: context, ); } diff --git a/tool/dart_skills_lint/test/absolute_paths_test.dart b/tool/dart_skills_lint/test/absolute_paths_test.dart index ce55f80..53f8307 100644 --- a/tool/dart_skills_lint/test/absolute_paths_test.dart +++ b/tool/dart_skills_lint/test/absolute_paths_test.dart @@ -5,6 +5,7 @@ import 'dart:io'; import 'package:dart_skills_lint/src/models/analysis_severity.dart'; +import 'package:dart_skills_lint/src/models/skill_context.dart'; import 'package:dart_skills_lint/src/rules/absolute_paths_rule.dart'; import 'package:dart_skills_lint/src/rules/relative_paths_rule.dart'; @@ -109,5 +110,38 @@ void main() { expect(result.warnings, contains(contains('Absolute filepath found in link: /absolute/path.md'))); }); + + test('fixes absolute path to relative if file exists', () async { + final Directory skillDir = await Directory('${tempDir.path}/test-skill').create(); + final File targetFile = await File('${tempDir.path}/target.md').create(); + + await File('${skillDir.path}/SKILL.md') + .writeAsString('${buildFrontmatter(name: 'test-skill')}[Link](${targetFile.path})\n'); + + final rule = AbsolutePathsRule(); + final file = File('${skillDir.path}/SKILL.md'); + final String content = await file.readAsString(); + final context = SkillContext(directory: skillDir, rawContent: content); + + final String fixedContent = await rule.fix('SKILL.md', content, context.directory); + + expect(fixedContent, contains('(../target.md)')); + }); + + test('does not fix absolute path if file does not exist', () async { + final Directory skillDir = await Directory('${tempDir.path}/test-skill').create(); + + await File('${skillDir.path}/SKILL.md') + .writeAsString('${buildFrontmatter(name: 'test-skill')}[Link](/non/existent/file.md)\n'); + + final rule = AbsolutePathsRule(); + final file = File('${skillDir.path}/SKILL.md'); + final String content = await file.readAsString(); + final context = SkillContext(directory: skillDir, rawContent: content); + + final String fixedContent = await rule.fix('SKILL.md', content, context.directory); + + expect(fixedContent, contains('(/non/existent/file.md)')); + }); }); } diff --git a/tool/dart_skills_lint/test/cli_integration_test.dart b/tool/dart_skills_lint/test/cli_integration_test.dart index 8ee089d..f666697 100644 --- a/tool/dart_skills_lint/test/cli_integration_test.dart +++ b/tool/dart_skills_lint/test/cli_integration_test.dart @@ -488,5 +488,104 @@ dart_skills_lint: expect(stderrStr, contains('has 1 trailing space(s)')); await process.shouldExit(1); }); + + test('--fix dry-runs and shows diff but does not modify file', () async { + final Directory skillDir = await Directory('${tempDir.path}/test-skill').create(); + await File('${skillDir.path}/SKILL.md') + .writeAsString('${buildFrontmatter(name: 'test-skill')}Line with 1 space \n'); + + final TestProcess process = await TestProcess.start( + 'dart', + ['bin/cli.dart', '-s', skillDir.path, '--fix', '--check-trailing-whitespace'], + ); + + final List stdout = await process.stdout.rest.toList(); + final String stdoutStr = stdout.join('\n'); + expect(stdoutStr, contains('[Dry Run] Proposed changes for test-skill (SKILL.md):')); + + await process.shouldExit(1); + + // Verify file was not modified + final String content = await File('${skillDir.path}/SKILL.md').readAsString(); + expect(content, contains('Line with 1 space \n')); + }); + + test('--fix-apply modifies file', () async { + final Directory skillDir = await Directory('${tempDir.path}/test-skill').create(); + await File('${skillDir.path}/SKILL.md') + .writeAsString('${buildFrontmatter(name: 'test-skill')}Line with 1 space \n'); + + final TestProcess process = await TestProcess.start( + 'dart', + ['bin/cli.dart', '-s', skillDir.path, '--fix-apply', '--check-trailing-whitespace'], + ); + + final List stdout = await process.stdout.rest.toList(); + expect(stdout.join('\n'), contains('Applied fixes for test-skill')); + + await process.shouldExit(0); + + // Verify file was modified + final String content = await File('${skillDir.path}/SKILL.md').readAsString(); + expect(content, isNot(contains('Line with 1 space \n'))); + expect(content, contains('Line with 1 space\n')); + }); + + test('--fix-apply does not modify file if lint is ignored', () async { + final Directory skillDir = await Directory('${tempDir.path}/test-skill').create(); + await File('${skillDir.path}/SKILL.md') + .writeAsString('${buildFrontmatter(name: 'test-skill')}Line with 1 space \n'); + + final ignoreFile = File('${tempDir.path}/$defaultIgnoreFileName'); + await ignoreFile.writeAsString(jsonEncode({ + SkillsIgnores.skillsKey: { + 'test-skill': [ + { + IgnoreEntry.ruleIdKey: 'check-trailing-whitespace', + IgnoreEntry.fileNameKey: 'SKILL.md' + } + ] + } + })); + + final TestProcess process = await TestProcess.start( + 'dart', + ['bin/cli.dart', '-s', skillDir.path, '--fix-apply', '--check-trailing-whitespace'], + ); + + await process.shouldExit(0); + + final String content = await File('${skillDir.path}/SKILL.md').readAsString(); + expect(content, contains('Line with 1 space \n')); + }); + + test('--fix-apply does not modify file if invalid-skill-name is ignored', () async { + final Directory skillDir = await Directory('${tempDir.path}/my_skill').create(); + await File('${skillDir.path}/SKILL.md').writeAsString(''' +--- +name: wrong-name +description: A test skill +--- +Body'''); + + final ignoreFile = File('${tempDir.path}/$defaultIgnoreFileName'); + await ignoreFile.writeAsString(jsonEncode({ + SkillsIgnores.skillsKey: { + 'my_skill': [ + {IgnoreEntry.ruleIdKey: 'invalid-skill-name', IgnoreEntry.fileNameKey: 'SKILL.md'} + ] + } + })); + + final TestProcess process = await TestProcess.start( + 'dart', + ['bin/cli.dart', '-s', skillDir.path, '--fix-apply'], + ); + + await process.shouldExit(0); + + final String content = await File('${skillDir.path}/SKILL.md').readAsString(); + expect(content, contains('name: wrong-name')); + }); }); } diff --git a/tool/dart_skills_lint/test/field_constraints_test.dart b/tool/dart_skills_lint/test/field_constraints_test.dart index 4358c75..541e7f5 100644 --- a/tool/dart_skills_lint/test/field_constraints_test.dart +++ b/tool/dart_skills_lint/test/field_constraints_test.dart @@ -4,11 +4,13 @@ import 'dart:io'; +import 'package:dart_skills_lint/src/models/skill_context.dart'; import 'package:dart_skills_lint/src/rules/description_length_rule.dart'; import 'package:dart_skills_lint/src/rules/name_format_rule.dart'; import 'package:dart_skills_lint/src/rules/valid_yaml_metadata_rule.dart'; import 'package:dart_skills_lint/src/validator.dart'; import 'package:test/test.dart'; +import 'package:yaml/yaml.dart'; import 'test_utils.dart'; @@ -95,8 +97,30 @@ void main() { final validator = Validator(); final ValidationResult result = await validator.validate(skillDir); expect(result.isValid, isFalse); - expect(result.errors, - contains(contains('must exactly match the name of its parent directory'))); + expect(result.errors, contains(contains('must exactly match the parent directory name'))); + }); + + test('fixes name to match directory name (not replacing underscores)', () async { + final Directory skillDir = await Directory('${tempDir.path}/my_skill').create(); + final file = File('${skillDir.path}/SKILL.md'); + await file.writeAsString(''' +--- +name: wrong-name +description: A test skill +--- +Body'''); + + final rule = NameFormatRule(); + final String content = await file.readAsString(); + final RegExpMatch? match = + RegExp(r'^---\s*\n(.*?)\n---\s*\n', dotAll: true).firstMatch(content); + final parsedYaml = loadYaml(match!.group(1)!) as YamlMap?; + final context = + SkillContext(directory: skillDir, rawContent: content, parsedYaml: parsedYaml); + + final String fixedContent = await rule.fix('SKILL.md', content, context.directory); + + expect(fixedContent, contains('name: my_skill')); }); }); diff --git a/tool/dart_skills_lint/test/fixer_test.dart b/tool/dart_skills_lint/test/fixer_test.dart new file mode 100644 index 0000000..bfb36e9 --- /dev/null +++ b/tool/dart_skills_lint/test/fixer_test.dart @@ -0,0 +1,170 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:io'; + +import 'package:dart_skills_lint/src/entry_point.dart'; +import 'package:dart_skills_lint/src/fixable_rule.dart'; +import 'package:dart_skills_lint/src/models/analysis_severity.dart'; +import 'package:dart_skills_lint/src/models/skill_context.dart'; +import 'package:dart_skills_lint/src/models/skill_rule.dart'; +import 'package:dart_skills_lint/src/models/validation_error.dart'; +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; + +class RuleA extends SkillRule implements FixableRule { + @override + String get name => 'rule-a'; + + @override + AnalysisSeverity get severity => AnalysisSeverity.warning; + + @override + Future> validate(SkillContext context) async { + return [ + ValidationError( + ruleId: name, + message: 'Error A', + severity: AnalysisSeverity.warning, + file: 'SKILL.md', + ) + ]; + } + + @override + Future fix(String filePath, String currentContent, Directory directory) async { + return '$currentContent A'; + } +} + +class RuleB extends SkillRule implements FixableRule { + @override + String get name => 'rule-b'; + + @override + AnalysisSeverity get severity => AnalysisSeverity.warning; + + @override + Future> validate(SkillContext context) async { + return [ + ValidationError( + ruleId: name, + message: 'Error B', + severity: AnalysisSeverity.warning, + file: 'SKILL.md', + ) + ]; + } + + @override + Future fix(String filePath, String currentContent, Directory directory) async { + return '$currentContent B'; + } +} + +class RuleThrows extends SkillRule implements FixableRule { + @override + String get name => 'rule-throws'; + + @override + AnalysisSeverity get severity => AnalysisSeverity.warning; + + @override + Future> validate(SkillContext context) async { + return [ + ValidationError( + ruleId: name, + message: 'Error Throws', + severity: AnalysisSeverity.warning, + file: 'SKILL.md', + ) + ]; + } + + @override + Future fix(String filePath, String currentContent, Directory directory) async { + throw Exception('Fix failed'); + } +} + +void main() { + group('Fixer Sequential Execution', () { + late Directory tempDir; + + setUp(() async { + tempDir = await Directory.systemTemp.createTemp('fixer_test.'); + }); + + tearDown(() async { + await tempDir.delete(recursive: true); + }); + + test('applies fixes in order', () async { + final skillDir = Directory(p.join(tempDir.path, 'test-skill')); + await skillDir.create(); + final skillFile = File(p.join(skillDir.path, 'SKILL.md')); + await skillFile.writeAsString('Original'); + + final bool success = await validateSkillsInternal( + individualSkillPaths: [skillDir.path], + fixApply: true, + quiet: true, + customRules: [RuleA(), RuleB()], + ); + + expect(success, isFalse); + + final String content = await skillFile.readAsString(); + expect(content, 'Original A B'); + }); + + test('--fast-fail stops processing subsequent skills but completes current skill fixes', + () async { + final skillDir1 = Directory(p.join(tempDir.path, 'test-skill-1')); + await skillDir1.create(); + final skillFile1 = File(p.join(skillDir1.path, 'SKILL.md')); + await skillFile1.writeAsString('Original1'); + + final skillDir2 = Directory(p.join(tempDir.path, 'test-skill-2')); + await skillDir2.create(); + final skillFile2 = File(p.join(skillDir2.path, 'SKILL.md')); + await skillFile2.writeAsString('Original2'); + + final bool success = await validateSkillsInternal( + individualSkillPaths: [skillDir1.path, skillDir2.path], + fixApply: true, + fastFail: true, + quiet: true, + customRules: [RuleA()], + ); + + expect(success, isFalse); + + final String content1 = await skillFile1.readAsString(); + expect(content1, 'Original1 A'); + + final String content2 = await skillFile2.readAsString(); + expect(content2, 'Original2'); + }); + + test('handles exceptions in fix method gracefully', () async { + final skillDir = Directory(p.join(tempDir.path, 'test-skill')); + await skillDir.create(); + final skillFile = File(p.join(skillDir.path, 'SKILL.md')); + await skillFile.writeAsString('Original'); + + final bool success = await validateSkillsInternal( + individualSkillPaths: [skillDir.path], + fixApply: true, + quiet: true, + customRules: [RuleThrows()], + ); + + expect(success, isFalse); + + final String content = await skillFile.readAsString(); + expect(content, 'Original'); + }); + }); +} diff --git a/tool/dart_skills_lint/test/trailing_whitespace_test.dart b/tool/dart_skills_lint/test/trailing_whitespace_test.dart index 41ece41..fc280c5 100644 --- a/tool/dart_skills_lint/test/trailing_whitespace_test.dart +++ b/tool/dart_skills_lint/test/trailing_whitespace_test.dart @@ -123,5 +123,28 @@ void main() { expect(errors, isEmpty); }); + + group('Trailing Whitespace Fix', () { + test('removes trailing whitespace', () { + final rule = TrailingWhitespaceRule(); + + expect(rule.fixLine('Line with 1 space '), 'Line with 1 space'); + expect(rule.fixLine('Line with 3 spaces '), 'Line with 3 spaces'); + expect(rule.fixLine('Line with tab\t'), 'Line with tab'); + }); + + test('keeps exactly 2 spaces', () { + final rule = TrailingWhitespaceRule(); + + expect(rule.fixLine('Line with 2 spaces '), 'Line with 2 spaces '); + }); + + test('handles Windows line endings', () { + final rule = TrailingWhitespaceRule(); + + expect(rule.fixLine('Line with 1 space \r'), 'Line with 1 space\r'); + expect(rule.fixLine('Line with 3 spaces \r'), 'Line with 3 spaces\r'); + }); + }); }); }