Expose custom validators then use one for last_modified

tool/dart_skills_lint/CHANGELOG.md

@@ -1,3 +1,12 @@
+## 0.2.0
+
+- Refactored validator to a pluggable rule-based architecture.
+- Added support for custom rules via `SkillRule`.
+- Added runtime assertion for duplicate rule names.
+- Added warning when a rule emits an error with severity different from its definition.
+- Updated `README.md` with custom rules documentation.
+- **Breaking Change**: Enabling a rule via CLI flag now sets its severity to `error` instead of `warning`.
+
 ## 0.1.0
 
 - Initial version.

tool/dart_skills_lint/README.md

@@ -129,6 +129,48 @@ void main() {
 
 You can also use `Validator` and `ValidationResult` directly if you need to inspect the errors programmatically.
 
+### Custom Rules
+
+You can author custom rules by extending the `SkillRule` class and passing them to `validateSkills` or the `Validator` constructor.
+
+Example custom rule:
+```dart
+import 'package:dart_skills_lint/dart_skills_lint.dart';
+
+class MyCustomRule extends SkillRule {
+  @override
+  final String name = 'my-custom-rule';
+
+  @override
+  final AnalysisSeverity severity = AnalysisSeverity.warning;
+
+  @override
+  Future<List<ValidationError>> validate(SkillContext context) async {
+    final errors = <ValidationError>[];
+    final yaml = context.parsedYaml;
+    if (yaml == null) return errors;
+
+    if (yaml['metadata']?['deprecated'] == true) {
+      errors.add(ValidationError(
+        ruleId: name,
+        severity: severity,
+        file: 'SKILL.md',
+        message: 'This skill is marked as deprecated.',
+      ));
+    }
+    return errors;
+  }
+}
+```
+
+Then use it in your test:
+```dart
+    await validateSkills(
+      skillDirPaths: ['../../skills'],
+      customRules: [MyCustomRule()],
+    );
+```
+
 ## Specification Validation
 
 The linter checks against the criteria defined in `documentation/knowledge/SPECIFICATION.md` (Section 5.1). Key checks include:

tool/dart_skills_lint/lib/dart_skills_lint.dart

@@ -5,5 +5,7 @@
 export 'src/config_parser.dart';
 export 'src/entry_point.dart';
 export 'src/models/analysis_severity.dart';
+export 'src/models/skill_context.dart';
+export 'src/models/skill_rule.dart';
 export 'src/models/validation_error.dart';
 export 'src/validator.dart';

tool/dart_skills_lint/lib/src/entry_point.dart

@@ -12,10 +12,13 @@ import 'package:path/path.dart' as p;
 
 import 'config_parser.dart';
 import 'models/analysis_severity.dart';
+import 'models/check_type.dart';
 import 'models/ignore_entry.dart';
+import 'models/skill_rule.dart';
 import 'models/skills_ignores.dart';
 import 'models/validation_error.dart';
-import 'rules.dart';
+import 'rule_registry.dart';
+
 import 'skills_ignores_storage.dart';
 import 'validator.dart';
 
@@ -64,14 +67,18 @@ Future<void> runApp(List<String> args) async {
 
   final parser = ArgParser()
     ..addFlag(helpFlag, abbr: 'h', negatable: false, help: 'Show usage information.')
-    ..addFlag(_printWarningsFlag, abbr: 'w', defaultsTo: true, help: 'Print validation warnings.')
-    ..addFlag(relativePathsCheck.name, help: 'Check if relative paths exist.')
-    ..addFlag(disallowedFieldCheck.name, help: 'Check for disallowed fields in YAML metadata.')
-    ..addFlag(validYamlMetadataCheck.name,
-        defaultsTo: true, help: 'Check if YAML metadata is valid.')
-    ..addFlag(descriptionTooLongCheck.name,
-        defaultsTo: true, help: 'Check if description is too long.')
-    ..addFlag(invalidSkillNameCheck.name, defaultsTo: true, help: 'Check if skill name is invalid.')
+    ..addFlag(_printWarningsFlag, abbr: 'w', defaultsTo: true, help: 'Print validation warnings.');
+
+  // Dynamically add flags for all registered rules.
+  for (final CheckType check in RuleRegistry.allChecks) {
+    parser.addFlag(
+      check.name,
+      defaultsTo: check.defaultSeverity != AnalysisSeverity.disabled,
+      help: check.help,
+    );
+  }
+
+  parser
     ..addFlag(_fastFailFlag,
         negatable: false, help: 'Fail immediately on the first skill validation error.')
     ..addFlag(_quietFlag,
@@ -180,6 +187,7 @@ Future<bool> validateSkills({
   bool generateBaseline = false,
   String? ignoreFileOverride,
   Configuration? config,
+  List<SkillRule> customRules = const [],
 }) async {
   config ??= Configuration();
   var globalAnyFailed = false;
@@ -199,23 +207,15 @@ Future<bool> validateSkills({
       continue;
     }
 
-    final localRules = Map<String, AnalysisSeverity>.from(resolvedRules);
-    String? localIgnoreFile;
-
-    for (final DirectoryConfig dirConfig in config.directoryConfigs) {
-      final String normalizedConfigPath = p.normalize(dirConfig.path);
-      if (normalizedSkillPath.startsWith(normalizedConfigPath)) {
-        localRules.addAll(dirConfig.rules);
-        localIgnoreFile = dirConfig.ignoreFile;
-        break;
-      }
-    }
+    final Map<String, AnalysisSeverity> localRules =
+        _resolveRulesForPath(normalizedSkillPath, config, resolvedRules);
+    String? localIgnoreFile = _resolveIgnoreFileForPath(normalizedSkillPath, config);
 
     if (ignoreFileOverride != null) {
       localIgnoreFile = ignoreFileOverride;
     }
 
-    final validator = Validator(ruleOverrides: localRules);
+    final validator = Validator(ruleOverrides: localRules, customRules: customRules);
 
     final Map<String, List<IgnoreEntry>> ignoresMap =
         await _loadIgnores(localIgnoreFile, skillDir.parent);
@@ -267,23 +267,15 @@ Future<bool> validateSkills({
       continue;
     }
 
-    final localRules = Map<String, AnalysisSeverity>.from(resolvedRules);
-    String? localIgnoreFile;
-
-    for (final DirectoryConfig dirConfig in config.directoryConfigs) {
-      final String normalizedConfigPath = p.normalize(dirConfig.path);
-      if (normalizedRootPath.startsWith(normalizedConfigPath)) {
-        localRules.addAll(dirConfig.rules);
-        localIgnoreFile = dirConfig.ignoreFile;
-        break;
-      }
-    }
+    final Map<String, AnalysisSeverity> localRules =
+        _resolveRulesForPath(normalizedRootPath, config, resolvedRules);
+    String? localIgnoreFile = _resolveIgnoreFileForPath(normalizedRootPath, config);
 
     if (ignoreFileOverride != null) {
       localIgnoreFile = ignoreFileOverride;
     }
 
-    final validator = Validator(ruleOverrides: localRules);
+    final validator = Validator(ruleOverrides: localRules, customRules: customRules);
 
     List<FileSystemEntity> entities;
     try {
@@ -371,66 +363,90 @@ Future<bool> validateSkills({
 Map<String, AnalysisSeverity> resolveRules(ArgResults results, Configuration config) {
   final resolved = <String, AnalysisSeverity>{};
 
-  resolved[relativePathsCheck.name] = relativePathsCheck.defaultSeverity;
-  resolved[absolutePathsCheck.name] = absolutePathsCheck.defaultSeverity;
-  resolved[disallowedFieldCheck.name] = disallowedFieldCheck.defaultSeverity;
-  resolved[validYamlMetadataCheck.name] = validYamlMetadataCheck.defaultSeverity;
-  resolved[descriptionTooLongCheck.name] = descriptionTooLongCheck.defaultSeverity;
-  resolved[invalidSkillNameCheck.name] = invalidSkillNameCheck.defaultSeverity;
-  resolved[pathDoesNotExistCheck.name] = pathDoesNotExistCheck.defaultSeverity;
+  // 1. Initialize with default severities from the registry.
+  for (final CheckType check in RuleRegistry.allChecks) {
+    resolved[check.name] = check.defaultSeverity;
+  }
 
+  // 2. Override with configurations from the YAML file.
   resolved.addAll(config.configuredRules);
 
-  if (results.wasParsed(relativePathsCheck.name)) {
-    resolved[relativePathsCheck.name] = (results[relativePathsCheck.name] as bool)
-        ? AnalysisSeverity.warning
-        : AnalysisSeverity.disabled;
-  }
-  if (results.wasParsed(disallowedFieldCheck.name)) {
-    resolved[disallowedFieldCheck.name] = (results[disallowedFieldCheck.name] as bool)
-        ? AnalysisSeverity.warning
-        : AnalysisSeverity.disabled;
-  }
-  if (results.wasParsed(validYamlMetadataCheck.name)) {
-    resolved[validYamlMetadataCheck.name] = (results[validYamlMetadataCheck.name] as bool)
-        ? validYamlMetadataCheck.defaultSeverity
-        : AnalysisSeverity.disabled;
-  }
-  if (results.wasParsed(descriptionTooLongCheck.name)) {
-    resolved[descriptionTooLongCheck.name] = (results[descriptionTooLongCheck.name] as bool)
-        ? descriptionTooLongCheck.defaultSeverity
-        : AnalysisSeverity.disabled;
-  }
-  if (results.wasParsed(invalidSkillNameCheck.name)) {
-    resolved[invalidSkillNameCheck.name] = (results[invalidSkillNameCheck.name] as bool)
-        ? invalidSkillNameCheck.defaultSeverity
-        : AnalysisSeverity.disabled;
+  // 3. Override with CLI flags. CLI flags take highest precedence.
+  for (final CheckType check in RuleRegistry.allChecks) {
+    final String name = check.name;
+
+    // Skip if the flag was not passed on the command line.
+    if (!results.wasParsed(name)) {
+      continue;
+    }
+
+    // TODO(reidbaker): Handle options in addition to flags.
+    final Object? value = results[name];
+    if (value is! bool) {
+      continue;
+    }
+
+    if (value) {
+      // If the user explicitly enabled the rule via flag (e.g., --rule), set to error.
+      resolved[name] = AnalysisSeverity.error;
+    } else {
+      // If the user explicitly disabled the rule via flag (e.g., --no-rule).
+      resolved[name] = AnalysisSeverity.disabled;
+    }
   }
 
   return resolved;
 }
 
+Map<String, AnalysisSeverity> _resolveRulesForPath(
+    String normalizedPath, Configuration config, Map<String, AnalysisSeverity> baseRules) {
+  final localRules = Map<String, AnalysisSeverity>.from(baseRules);
+  for (final DirectoryConfig dirConfig in config.directoryConfigs) {
+    final String normalizedConfigPath = p.normalize(dirConfig.path);
+    if (normalizedPath.startsWith(normalizedConfigPath)) {
+      localRules.addAll(dirConfig.rules);
+      break;
+    }
+  }
+  return localRules;
+}
+
+String? _resolveIgnoreFileForPath(String normalizedPath, Configuration config) {
+  for (final DirectoryConfig dirConfig in config.directoryConfigs) {
+    final String normalizedConfigPath = p.normalize(dirConfig.path);
+    if (normalizedPath.startsWith(normalizedConfigPath)) {
+      return dirConfig.ignoreFile;
+    }
+  }
+  return null;
+}
+
 Future<Map<String, List<IgnoreEntry>>> _loadIgnores(
     String? ignoreFileOverride, Directory rootDir) async {
   final String ignorePath = ignoreFileOverride != null
       ? p.normalize(_expandPath(ignoreFileOverride))
       : p.join(rootDir.path, defaultIgnoreFileName);
+
   final file = File(ignorePath);
-  if (!file.existsSync()) {
-    if (ignoreFileOverride != null) {
-      _log.warning('File not found generating-baseline');
-      try {
-        await file.writeAsString(jsonEncode({SkillsIgnores.skillsKey: <String, dynamic>{}}));
-      } catch (_) {
-        // Fallback or ignore write errors
-      }
+
+  if (file.existsSync()) {
+    final storage = SkillsIgnoresStorage();
+    final SkillsIgnores ignores = await storage.load(ignorePath);
+    return ignores.skills;
+  }
+
+  // If a custom ignore file was specified but not found, create an empty one
+  // so the user can start adding ignores to it.
+  if (ignoreFileOverride != null) {
+    _log.warning('File not found generating-baseline');
+    try {
+      await file.writeAsString(jsonEncode({SkillsIgnores.skillsKey: <String, dynamic>{}}));
+    } catch (_) {
+      // Ignore write errors, we will just return empty ignores.
     }
-    return {};
   }
 
-  final storage = SkillsIgnoresStorage();
-  final SkillsIgnores ignores = await storage.load(ignorePath);
-  return ignores.skills;
+  return {};
 }
 
 void _applyIgnores(ValidationResult result, List<IgnoreEntry> ignores, Directory skillDir) {
@@ -440,7 +456,7 @@ void _applyIgnores(ValidationResult result, List<IgnoreEntry> ignores, Directory
     }
     final String fileName = error.file;
     for (final ignore in ignores) {
-      if (ignore.ruleId == error.ruleId && ignore.fileName == fileName) {
+      if (ignore.ruleId == error.ruleId && p.normalize(ignore.fileName) == p.normalize(fileName)) {
         error.isIgnored = true;
         ignore.used = true;
         break;

tool/dart_skills_lint/lib/src/models/check_type.dart

@@ -9,9 +9,13 @@ class CheckType {
   const CheckType({
     required this.name,
     required this.defaultSeverity,
+    required this.help,
   });
   final String name;
 
   /// The default severity if not overridden by config or flags.
   final AnalysisSeverity defaultSeverity;
+
+  /// The help message displayed by the CLI.
+  final String help;
 }

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

@@ -0,0 +1,21 @@
+import 'dart:io';
+import 'package:yaml/yaml.dart';
+
+/// Context provided to [SkillRule]s during validation.
+class SkillContext {
+  SkillContext({
+    required this.directory,
+    required this.rawContent,
+    this.parsedYaml,
+    this.yamlParsingError,
+  });
+
+  final Directory directory;
+
+  /// Guaranteed to be non-null because we only run rules if SKILL.md exists.
+  final String rawContent;
+
+  final YamlMap? parsedYaml;
+
+  final String? yamlParsingError;
+}

tool/dart_skills_lint/lib/src/models/skill_rule.dart

@@ -0,0 +1,27 @@
+import 'analysis_severity.dart';
+import 'skill_context.dart';
+import 'validation_error.dart';
+
+/// Abstract base class for all skill validation rules.
+///
+/// Custom rules should follow these guidelines to play nice with others:
+/// 1. **Unique Name**: The [name] must be unique to allow for overrides in
+///    configuration.
+/// 2. **Statelessness**: Rules should not maintain state between [validate] calls.
+/// 3. **Use Context**: Prefer using data in [SkillContext] (like [context.parsedYaml])
+///    rather than reading files manually to avoid duplicate I/O.
+/// 4. **Handle Parsing Errors**: If [context.parsedYaml] is null, check
+///    [context.yamlParsingError]. Rules that require valid YAML should return
+///    quickly if parsing failed.
+/// 5. **Respect Severity**: The rule should use its [severity] when creating
+///    [ValidationError]s unless there is a good reason not to.
+abstract class SkillRule {
+  /// The unique name of the rule (e.g., 'check-relative-paths').
+  /// Used in configuration and flags.
+  String get name;
+
+  AnalysisSeverity get severity;
+
+  /// Validates the skill provided in [context].
+  Future<List<ValidationError>> validate(SkillContext context);
+}