diff --git a/.github/workflows/dart_skills_lint_workflow.yaml b/.github/workflows/dart_skills_lint_workflow.yaml index 7c322c9..4cf45f9 100644 --- a/.github/workflows/dart_skills_lint_workflow.yaml +++ b/.github/workflows/dart_skills_lint_workflow.yaml @@ -91,3 +91,22 @@ jobs: - run: dart pub get - run: dart format --output=none --set-exit-if-changed . + + pana_score: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: dart-lang/setup-dart@v1 + with: + sdk: stable + + - run: dart pub get + + - name: Install pana + run: dart pub global activate pana + + # pana --exit-code-threshold N fails the step when + # (max - granted) > N. Threshold 0 means any point drop fails; + # current package score is 160/160 and we want to keep it there. + - name: Pana score gate (160/160 required) + run: dart pub global run pana --no-warning --exit-code-threshold 0 . diff --git a/tool/dart_skills_lint/CHANGELOG.md b/tool/dart_skills_lint/CHANGELOG.md index 0221fe5..27337b3 100644 --- a/tool/dart_skills_lint/CHANGELOG.md +++ b/tool/dart_skills_lint/CHANGELOG.md @@ -1,3 +1,32 @@ +## 0.3.1 + +- `--fix` now writes fixes to disk; pair with `--dry-run` + (`--fix --dry-run`) to preview the proposed diff without writing. + The legacy `--fix-apply` flag still works but is deprecated and + emits a notice on stderr. +- Running the CLI with no arguments and no `.claude/skills` or + `.agents/skills` directory present now prints a short onboarding + guide explaining how to point the linter at a skill or a skills + root. +- `description-too-long` errors now report the actual character + count and show an excerpt with a `|HERE|` marker at the cutoff + so authors can see exactly where the text went over. The same + diagnostic shape is now used for the `compatibility` field's + 500-character limit. +- `invalid-skill-name` errors now disambiguate the frontmatter + `name:` field from the parent directory name, quote the offending + value, and suggest a normalized form. The directory-mismatch + error offers both directions of the fix (edit the field or + rename the directory). +- `check-relative-paths` errors now include the resolved absolute + path and, when a near-miss filename exists in the same + directory, surface a `Did you mean "..."?` suggestion that + preserves the link's directory prefix. +- New `example/` directory with reference `valid` and `invalid` + skill fixtures and a walkthrough. +- New "Recipes" section in `README.md` with copy-pasteable GitHub + Actions and pre-commit hook integrations. + ## 0.3.0 - Exposed `ConfigParser.loadConfig()` API to load configuration files programmatically. diff --git a/tool/dart_skills_lint/CONTRIBUTING.md b/tool/dart_skills_lint/CONTRIBUTING.md index b5f6c9b..15e8864 100644 --- a/tool/dart_skills_lint/CONTRIBUTING.md +++ b/tool/dart_skills_lint/CONTRIBUTING.md @@ -42,6 +42,33 @@ you edit an existing file, you shouldn't update the year. // 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. +## Embedding the linter in tests + +If your project already uses `dart_skills_lint`, you can also call it +from your own test suite — handy when you want skill validation to fail +the same Dart-test pipeline that already gates the rest of your code: + +```dart +import 'package:dart_skills_lint/dart_skills_lint.dart'; +import 'package:test/test.dart'; + +void main() { + test('Run skills linter', () async { + // Load whatever's in dart_skills_lint.yaml so the CLI and tests + // share configuration. Pass `customRules: [...]` to inject any + // custom SkillRule implementations. + final config = await ConfigParser.loadConfig(); + await validateSkills(config: config); + }); +} +``` + +`Validator` and `ValidationResult` are also exposed for tests that +need to inspect errors programmatically. Custom rule authoring lives +in the +[`dart-skills-lint-validation`](skills/dart-skills-lint-validation/SKILL.md) +skill. + ## Testing and coverage Run the test suite from the package root (`tool/dart_skills_lint`): @@ -78,3 +105,43 @@ This project follows We pledge to maintain an open and welcoming environment. For details, see our [code of conduct](https://dart.dev/code-of-conduct). + +## Rule-stability policy (SemVer) + +Lint rules are part of `dart_skills_lint`'s public API. Adopters wire +the linter into pre-commit hooks and CI gates, so a rule that silently +flips from "warning" to "error" can break a downstream build with no +code change of their own. We version rule changes the same way we +version code changes: + +- **Patch release (`0.3.X` → `0.3.X+1`, `1.0.X` → `1.0.X+1`)** — + bug fixes to existing rules, including diagnostic message + rewording, internal refactors, and fixes that *narrow* what a rule + matches (fewer false positives). The set of error states a passing + skill needs to clear does not grow. + +- **Minor release (`0.3.X` → `0.4.0`, `1.0.X` → `1.1.0`)** — new + rules, **shipping with `defaultSeverity: AnalysisSeverity.disabled`** + so existing skills keep passing. Adopters opt in by enabling the + rule via flag or YAML config. Performance improvements that don't + change diagnostics also land here. A rule's diagnostic message may + expand to include additional context. + +- **Major release (`0.X` → `1.0`, `1.X` → `2.0`)** — any change that + can fail a previously-passing skill: removing a rule (so configs + referencing it stop working), upgrading a rule's default severity + (`disabled → warning`, `warning → error`), broadening what a rule + matches (more true positives = more failures), or renaming a rule. + Releases bump the major version and the CHANGELOG calls out the + exact rules affected. + +Rationale: adopters should be able to set `dart_skills_lint: ^1.0.0` +in `pubspec.yaml` and trust that a `dart pub upgrade` never turns +green CI red without their consent. Surprises belong in major +releases, and only there. + +If you're proposing a change that doesn't fit cleanly into one of the +buckets above, say so on the PR and the maintainers will decide where +it lands. New built-in rules **must** include a `## ` +entry in `RULES.md` describing default severity and behavior — see +the existing entries for the expected shape. diff --git a/tool/dart_skills_lint/README.md b/tool/dart_skills_lint/README.md index e46103f..834816a 100644 --- a/tool/dart_skills_lint/README.md +++ b/tool/dart_skills_lint/README.md @@ -6,11 +6,10 @@ A static analysis linter for Agent Skills to ensure they meet the specification - [Overview](#overview) - [Installation](#installation) - [Usage](#usage) - - [CLI vs Dart Test](#cli-vs-dart-test) - [Rule Precedence](#rule-precedence) -- [Configuration](#configuration) - [Specification Validation](#specification-validation) -- [Best Practices](#best-practices) +- [Recipes](#recipes) +- [Contributing](#contributing) ## Overview @@ -47,23 +46,11 @@ dart pub global activate dart_skills_lint ## Usage -There are three ways to interact with `dart_skills_lint`. - -### CLI vs Dart Test - -Depending on your workflow, you should choose the appropriate interaction mode: - -* **Use the CLI when:** - * **Ad-hoc Validation**: You want to quickly check a specific skill you are working on without running the entire test suite. - * **Baseline Generation**: You are integrating the tool into a legacy repo and need to generate an ignore file (`--generate-baseline`). - * **Automated Fixes**: You want to preview or apply fixes (`--fix`, `--fix-apply`) directly to the files. - * **Pre-commit Hooks**: You want a fast, isolated check in a Git pre-commit hook. -* **Use Dart Test when:** - * **CI/CD Integration**: You want to guarantee that no invalid skills are merged by failing the build alongside unit tests. - * **Programmatic Configuration**: You need to inject custom rules or dynamic configurations that are hard to express in static YAML. - * **Ecosystem Consistency**: You want developers to rely on the familiar `dart test` command rather than learning a new tool invocation. - ---- +`dart_skills_lint` runs as a command-line tool, configured by flags or by +a `dart_skills_lint.yaml` file. The CLI is the user-facing surface; it +also has a programmatic API for contributors who need to embed the +linter in their own test suite — see +[`CONTRIBUTING.md`](CONTRIBUTING.md#embedding-the-linter-in-tests). ### 1. As a Command Line Tool with Arguments Run the linter against your skills or root skills directories by passing arguments. @@ -92,8 +79,9 @@ 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. +- `--fix`: Write fixes for failing lints to disk. +- `--dry-run`: When combined with `--fix`, prints the proposed diff without writing. +- `--fix-apply`: *Deprecated* alias for `--fix`. Prints a deprecation notice on use. ### 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). @@ -129,104 +117,107 @@ This ensures that you can always override configuration file settings for a spec --- -### 3. As Dart Test Code -You can integrate the linter into your automated tests by importing the package and calling `validateSkills`. This allows you to enforce skill validity as part of your standard test suite. - -Example `test/lint_skills_test.dart`: -```dart -import 'package:dart_skills_lint/dart_skills_lint.dart'; -import 'package:test/test.dart'; - -void main() { - test('Run skills linter', () async { - final config = Configuration( - directoryConfigs: [ - DirectoryConfig( - path: '../../skills', - rules: {}, - ignoreFile: '.agents/skills/flutter_skills_ignore.json', - ), - ], - ); - - await validateSkills( - skillDirPaths: ['../../skills'], - resolvedRules: { - 'check-relative-paths': AnalysisSeverity.error, - 'check-absolute-paths': AnalysisSeverity.error, - }, - config: config, - ); - }); -} -``` +### 3. Custom Rules -You can also use `Validator` and `ValidationResult` directly if you need to inspect the errors programmatically. +Custom rule authoring lives in the +[`dart-skills-lint-validation`](skills/dart-skills-lint-validation/SKILL.md) +skill — that skill walks through extending `SkillRule` and passing the +rule into the linter. -### Custom Rules +## Specification Validation -You can author custom rules by extending the `SkillRule` class and passing them to `validateSkills` or the `Validator` constructor. +The linter checks each skill against the spec at +[`documentation/knowledge/SPECIFICATION.md`](documentation/knowledge/SPECIFICATION.md). +For the full list of built-in rules — default severities, exact +diagnostic shapes, auto-fix behavior, and how to disable each — see +[`RULES.md`](RULES.md). -Example custom rule: -```dart -import 'package:dart_skills_lint/dart_skills_lint.dart'; +## Recipes -class MyCustomRule extends SkillRule { - @override - final String name = 'my-custom-rule'; +Drop-in snippets for the two most common ways to wire `dart_skills_lint` +into a project's quality gates. Each recipe is exercised by +[`test/recipe_drift_test.dart`](test/recipe_drift_test.dart), so if a +flag here goes stale, CI fails. - @override - final AnalysisSeverity severity = AnalysisSeverity.warning; +### Recipe: GitHub Actions - @override - Future> validate(SkillContext context) async { - final errors = []; - final yaml = context.parsedYaml; - if (yaml == null) return errors; +Save the following as `.github/workflows/lint-skills.yml`. It runs on +every push and PR, installs `dart_skills_lint` globally on the runner, +and validates every skill under `.claude/skills/`. Adjust the path to +match where your skills live. - if (yaml['metadata']?['deprecated'] == true) { - errors.add(ValidationError( - ruleId: name, - severity: severity, - file: 'SKILL.md', - message: 'This skill is marked as deprecated.', - )); - } - return errors; - } -} +```yaml +# .github/workflows/lint-skills.yml +name: Lint Agent Skills +on: + push: + branches: [main] + pull_request: + +permissions: read-all + +jobs: + lint-skills: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: dart-lang/setup-dart@v1 + - run: dart pub global activate dart_skills_lint + - run: dart pub global run dart_skills_lint --skills-directory ./.claude/skills ``` -Then use it in your test: -```dart - await validateSkills( - skillDirPaths: ['../../skills'], - customRules: [MyCustomRule()], - ); +To validate a single skill directory instead, swap the last step: + +```yaml + - run: dart pub global run dart_skills_lint --skill ./.claude/skills/my-skill ``` -## Specification Validation +### Recipe: Dart-native pre-commit hook -The linter checks against the criteria defined in `documentation/knowledge/SPECIFICATION.md` (Section 5.1). Key checks include: +A pre-commit hook that calls into the linter directly — no Husky, no +Python `pre-commit` framework, just Dart and the existing +`dart pub global` tooling. -### 1. Directory and File Structure -- Path existence and directory verification. -- Mandatory `SKILL.md` file at the root. -- Directories starting with a dot `.` (e.g., `.dart_tool`) are ignored when scanning for skills. +Activate the linter once per machine: + +```bash +dart pub global activate dart_skills_lint +``` -### 2. Metadata (YAML Frontmatter) -- Valid YAML syntax. -- Allowed fields: `name`, `description`, `license`, `allowed-tools`, `metadata`, `compatibility`, `category`, `tags`, `version`, `eval_task`. -- Required fields: `name` and `description`. +Then install the hook into the repository (run from the repo root): -### 3. Field Specific Constraints -- **Skill Name (`name`)**: Max 64 characters, lowercase alphanumeric and hyphens only, no leading/trailing/consecutive hyphens. **Must match the parent directory name.** -- **Description (`description`)**: Max 1024 characters. -- **Compatibility (`compatibility`)**: Max 500 characters. +```bash +cat > .git/hooks/pre-commit <<'HOOK' +#!/bin/sh +set -e +# Lint every skill under .claude/skills before each commit. +# Add --skill arguments for other locations as needed. +exec dart pub global run dart_skills_lint --skills-directory ./.claude/skills --quiet +HOOK +chmod +x .git/hooks/pre-commit +``` -### 4. Content Constraints -- **Trailing Whitespace**: Lines in `SKILL.md` should not have trailing whitespace. Exactly 2 spaces at the end of a line are allowed to support Markdown hard line breaks, per the [CommonMark Spec](https://spec.commonmark.org/0.31.2/#hard-line-breaks). -- **Path Constraints**: Checks that **inline** Markdown links do not use absolute paths to enforce portability. Can optionally be configured to check that relative paths point to valid, existing files (disabled by default). *Note: This rule only supports inline Markdown links and does not detect HTML or reference-style links.* +The hook exits non-zero on lint failure, blocking the commit. To +auto-apply fixable lints inside the hook, append `--fix` to the linter +invocation. + +### Recipe: have an agent set it up for you + +If you're using Claude Code, Gemini, or another agent that can read +repository-local skills, paste the following prompt to have the agent +install and validate `dart_skills_lint` for you. The agent will +follow the +[`dart-skills-lint-setup`](skills/dart-skills-lint-setup/SKILL.md) +skill for first-time wiring, then the +[`dart-skills-lint-validation`](skills/dart-skills-lint-validation/SKILL.md) +skill to run the linter and resolve any failures. + +> Set up dart_skills_lint in this project. Use the skill at +> `tool/dart_skills_lint/skills/dart-skills-lint-setup/SKILL.md` +> to add it as a dev_dependency, create the configuration file, +> and wire it into CI. Then use the skill at +> `tool/dart_skills_lint/skills/dart-skills-lint-validation/SKILL.md` +> to run the linter and resolve any failures. ## Contributing diff --git a/tool/dart_skills_lint/RULES.md b/tool/dart_skills_lint/RULES.md new file mode 100644 index 0000000..ef024e9 --- /dev/null +++ b/tool/dart_skills_lint/RULES.md @@ -0,0 +1,162 @@ +# Rules + +The full rule contract for `dart_skills_lint`. Every built-in rule listed +here is registered in +[`lib/src/rule_registry.dart`](lib/src/rule_registry.dart) and pinned to +this document by +[`test/rules_md_consistency_test.dart`](test/rules_md_consistency_test.dart). +If a rule is added, removed, renamed, or has its default severity / +fixability changed, both the registry **and** this file must be updated +in the same commit — the consistency test fails otherwise. + +Severity vocabulary: + +- `error` — failure exits 1 and blocks CI. +- `warning` — printed but does not change exit code. +- `disabled` — not run unless explicitly enabled via CLI flag or + `dart_skills_lint.yaml` `rules:` config. + +All rules are enabled / disabled / escalated the same three ways: + +- CLI: `--` (escalates to `error`), + `--no-` (disables). +- YAML config: `dart_skills_lint.rules.: error|warning|disabled`. +- Per-directory YAML: `dart_skills_lint.directories[].rules.: ...`. + +The "Disable" line under each rule below names the negated CLI flag for +quick reference. + +See [`CONTRIBUTING.md`](CONTRIBUTING.md) for the SemVer policy that +governs how changes to these rules ship. + +--- + +## check-absolute-paths + +- **Default severity:** warning +- **Fixable:** yes +- **What it checks:** inline Markdown links in `SKILL.md` do not use + absolute filesystem paths (POSIX `/foo/bar` or Windows `C:\foo`). + Absolute paths break portability across machines. +- **Diagnostic shape:** + `Absolute filepath found in link: . Skills must use paths relative to SKILL.md so they remain portable across machines.` +- **Auto-fix behavior:** if the absolute path resolves to a file that + exists on disk, the fixer rewrites it to the equivalent POSIX-style + relative path from `SKILL.md`. If the target does not exist the + fixer leaves the link untouched. +- **Disable:** `--no-check-absolute-paths`. + +## check-relative-paths + +- **Default severity:** disabled +- **Fixable:** no +- **What it checks:** inline Markdown links in `SKILL.md` with + relative targets resolve to files that actually exist on disk. + Web URLs, anchors, `mailto:`, `javascript:`, and `data:` links are + skipped. +- **Diagnostic shape:** + `Linked file does not exist: (resolved to ). Did you mean ""?` + The `Did you mean` clause is only included when a near-miss file + is found in the same directory; it's scored by string similarity + against the missing basename. The suggestion preserves the link's + original directory prefix, normalized to forward slashes. +- **Auto-fix behavior:** none. The author is expected to pick the + intended target by hand. +- **Disable:** `--no-check-relative-paths` (also the default state). + +## check-trailing-whitespace + +- **Default severity:** disabled +- **Fixable:** yes +- **What it checks:** lines in `SKILL.md` do not have trailing + whitespace. Exactly two spaces are allowed as a CommonMark hard + line break; one space or three-or-more spaces, or any trailing tab, + is reported. +- **Diagnostic shape:** + `Line has trailing space(s). Only exactly 2 spaces are + allowed for line breaks.` + Trailing tabs report `Line has trailing whitespace containing + tabs.` instead. +- **Auto-fix behavior:** trims violating trailing whitespace from + each offending line. Lines with exactly two trailing spaces are + left alone. +- **Disable:** `--no-check-trailing-whitespace` (also the default + state). + +## description-too-long + +- **Default severity:** error +- **Fixable:** no +- **What it checks:** the YAML frontmatter `description:` field is + at most 1024 characters. +- **Diagnostic shape:** + `Description field is characters; maximum is 1024. Cutoff at + character 1024: ...<40 chars before>|HERE|<40 chars after>... (see + https://agentskills.io/specification#description-field)` + The `|HERE|` marker pins the exact cutoff point so the author can + see what slipped past the limit without having to count characters. +- **Auto-fix behavior:** none. The fix is editorial; the linter + refuses to silently truncate the author's prose. +- **Disable:** `--no-description-too-long`. + +## disallowed-field + +- **Default severity:** disabled +- **Fixable:** no +- **What it checks:** every key in the YAML frontmatter is one of the + spec-allowed fields: `name`, `description`, `license`, + `allowed-tools`, `metadata`, `compatibility`, `category`, `tags`, + `version`, `eval_task`. +- **Diagnostic shape:** + `Disallowed field: (see + https://agentskills.io/specification#frontmatter)` +- **Auto-fix behavior:** none. The fix is destructive (removing a + field) so it requires a human decision. +- **Disable:** `--no-disallowed-field` (also the default state). + +## invalid-skill-name + +- **Default severity:** error +- **Fixable:** yes +- **What it checks:** the frontmatter `name:` field is: + - lowercase + - 1–64 characters + - only lowercase letters, digits, and hyphens + - has no leading, trailing, or consecutive hyphens + - exactly equal to the parent directory's name +- **Diagnostic shape:** each violation produces a separate error + message naming the frontmatter `name:` field explicitly, + quoting the offending value, and suggesting a normalized form + (e.g. `Frontmatter `name` "My_Skill" contains invalid characters. + Only lowercase letters, digits, and hyphens are allowed. + Suggested: "my-skill" (see + https://agentskills.io/specification#name-field)`). + The directory-mismatch error offers both fix directions (edit the + field or rename the directory). +- **Auto-fix behavior:** when the only violation is a directory + mismatch, the fixer rewrites the frontmatter `name:` value to + match the parent directory name. Other violations (invalid + characters, length, etc.) are not auto-fixed because the + normalization is a suggestion and the author may want a different + name entirely. +- **Disable:** `--no-invalid-skill-name`. + +## valid-yaml-metadata + +- **Default severity:** error +- **Fixable:** no +- **What it checks:** + - `SKILL.md` contains a YAML frontmatter block delimited by `---` + that parses without errors. + - Required fields `name` and `description` are both present. + - If `compatibility:` is present, it is at most 500 characters. +- **Diagnostic shape:** + - `Invalid YAML metadata: (see + https://agentskills.io/specification#frontmatter)` + - `Missing required field: (see ...)` + - `Compatibility field is characters; maximum is 500. Cutoff at character 500: ...|HERE|... (see https://agentskills.io/specification#compatibility-field)` + — same shape as `description-too-long`, produced by the shared + `buildLengthDiagnostic` helper. +- **Auto-fix behavior:** none. A broken frontmatter block isn't + safely mechanically repairable. +- **Disable:** `--no-valid-yaml-metadata`. diff --git a/tool/dart_skills_lint/example/README.md b/tool/dart_skills_lint/example/README.md new file mode 100644 index 0000000..9e45ce5 --- /dev/null +++ b/tool/dart_skills_lint/example/README.md @@ -0,0 +1,70 @@ +# dart_skills_lint examples + +Two reference fixtures live in this directory: + +| Fixture | Expected outcome | +| --- | --- | +| [`valid/`](valid/SKILL.md) | All rules pass; the CLI exits 0. | +| [`invalid/`](invalid/SKILL.md) | Multiple rules fail; the CLI exits 1. | + +Use them to take the linter for a spin without writing your own skill +first, and to see exactly what real diagnostic output looks like. + +## Run the valid fixture + +```bash +dart run dart_skills_lint --skill ./example/valid +``` + +You should see: + +``` +Evaluating directory: example/valid +--- Validating skill: valid --- + Skill is valid. +``` + +Exit code: `0`. + +## Run the invalid fixture + +With default rule severities, only `invalid-skill-name` fires (the other +two violations are below their default threshold): + +```bash +dart run dart_skills_lint --skill ./example/invalid +``` + +Exit code: `1`. To see every violation surface as an error, escalate the +other two rules with explicit flags: + +```bash +dart run dart_skills_lint --skill ./example/invalid \ + --disallowed-field --check-absolute-paths +``` + +Three rules now report failures: + +- `invalid-skill-name` — names the offending frontmatter value, calls + out the directory mismatch, and suggests a corrected form. +- `disallowed-field` — names the unknown field (`secret_field`) and + links to the spec's allowed-field list. +- `check-absolute-paths` — flags the `/tmp/...` link as non-portable + and links to the spec section on relative paths. + +The exact wording is asserted by +[`test/example_fixtures_test.dart`](../test/example_fixtures_test.dart), +so the fixtures and their expected diagnostics cannot drift apart. + +## Trying out --fix + +The invalid fixture's `check-absolute-paths` violation is auto-fixable +when the target file exists. To experiment, point it at a real local +file: + +```bash +dart run dart_skills_lint --skill ./example/invalid --fix --dry-run +``` + +`--dry-run` shows the proposed diff without writing; drop it to apply +the change. diff --git a/tool/dart_skills_lint/example/invalid/SKILL.md b/tool/dart_skills_lint/example/invalid/SKILL.md new file mode 100644 index 0000000..4a171dc --- /dev/null +++ b/tool/dart_skills_lint/example/invalid/SKILL.md @@ -0,0 +1,38 @@ +--- +name: NotInvalid +description: A deliberately broken fixture used by example/README.md to show what each rule's error output looks like. +secret_field: not allowed by the spec +--- + +# Invalid example skill + +This skill deliberately trips three rules so the CLI's diagnostic output +can be inspected end-to-end. One fires under defaults; two need to be +enabled to surface as errors (the spec ships them at lower severities). + +1. `invalid-skill-name` *(error by default)* — the frontmatter `name:` + is `NotInvalid`, which is not lowercase **and** does not match the + parent directory `invalid`. +2. `disallowed-field` *(disabled by default; enable via + `--disallowed-field` or YAML config)* — `secret_field:` is not in + the spec's allowed field list. +3. `check-absolute-paths` *(warning by default; escalate to error via + `--check-absolute-paths` or YAML config)* — the link below uses an + absolute filesystem path, which is not portable across machines. + +The broken link: [absolute link](/tmp/this/does/not/exist.md) + +Run it with default rules: + +```bash +dart run dart_skills_lint --skill ./example/invalid +``` + +…and again with every rule turned up to error: + +```bash +dart run dart_skills_lint --skill ./example/invalid \ + --disallowed-field --check-absolute-paths +``` + +Expected: non-zero exit, error messages naming each rule that is enabled. diff --git a/tool/dart_skills_lint/example/valid/SKILL.md b/tool/dart_skills_lint/example/valid/SKILL.md new file mode 100644 index 0000000..d9b3e69 --- /dev/null +++ b/tool/dart_skills_lint/example/valid/SKILL.md @@ -0,0 +1,21 @@ +--- +name: valid +description: >- + Reference fixture for dart_skills_lint. Demonstrates a SKILL.md that + passes every default rule: hyphen-lowercase name matching the parent + directory, a properly sized description, and no other frontmatter fields + that would trigger the disallowed-field check. +--- + +# Valid example skill + +This skill exists so the linter has a known-good fixture to validate +against. It deliberately does nothing useful — it's documentation. + +Run it with: + +```bash +dart run dart_skills_lint --skill ./example/valid +``` + +Expected output: `Skill is valid.` and exit code 0. diff --git a/tool/dart_skills_lint/lib/src/config_parser.dart b/tool/dart_skills_lint/lib/src/config_parser.dart index acf9335..dbf1565 100644 --- a/tool/dart_skills_lint/lib/src/config_parser.dart +++ b/tool/dart_skills_lint/lib/src/config_parser.dart @@ -106,33 +106,67 @@ class ConfigParser { /// Parses the `directories` list from the configuration. /// Validates keys for each directory entry and resolves path-specific rule overrides. /// Appends any parsing errors to `parsingErrors`. + /// + /// Each entry is parsed defensively: a bad `path:` / `ignore_file:` / + /// `rules:` type emits a parsingErrors entry naming the offending field + /// and the entry is skipped, but later entries in the same `directories:` + /// list still parse normally. static List _parseDirectories(YamlMap toolConfig, List parsingErrors) { final directoryConfigs = []; if (toolConfig.containsKey(_directoriesKey)) { final dirs = toolConfig[_directoriesKey]; if (dirs is YamlList) { for (final dir in dirs) { - if (dir is YamlMap && dir.containsKey(_pathKey)) { - final path = dir[_pathKey] as String; + if (dir is! YamlMap || !dir.containsKey(_pathKey)) { + continue; + } - for (final key in dir.keys) { - if (!_allowedDirectoryKeys.contains(key.toString())) { - parsingErrors.add('Unrecognized key "$key" in directory entry for "$path".'); - } + final pathValue = dir[_pathKey]; + if (pathValue is! String) { + parsingErrors.add( + 'Directory entry "$_pathKey" must be a string; got "$pathValue" ' + '(${pathValue.runtimeType}). Skipping entry.', + ); + continue; + } + final String path = pathValue; + + for (final key in dir.keys) { + if (!_allowedDirectoryKeys.contains(key.toString())) { + parsingErrors.add('Unrecognized key "$key" in directory entry for "$path".'); } + } - final rules = {}; - if (dir.containsKey(_rulesKey)) { - final localRules = dir[_rulesKey]; - if (localRules is YamlMap) { - for (final key in localRules.keys) { - rules[key.toString()] = _parseSeverity(localRules[key]?.toString() ?? ''); - } + final rules = {}; + if (dir.containsKey(_rulesKey)) { + final localRules = dir[_rulesKey]; + if (localRules is YamlMap) { + for (final key in localRules.keys) { + rules[key.toString()] = _parseSeverity(localRules[key]?.toString() ?? ''); } + } else { + parsingErrors.add( + 'Directory entry "$_rulesKey" for "$path" must be a map; ' + 'got "$localRules" (${localRules.runtimeType}). Ignoring local rules.', + ); } - final ignoreFile = dir[_ignoreFileKey] as String?; - directoryConfigs.add(DirectoryConfig(path: path, rules: rules, ignoreFile: ignoreFile)); } + + String? ignoreFile; + if (dir.containsKey(_ignoreFileKey)) { + final ignoreFileValue = dir[_ignoreFileKey]; + if (ignoreFileValue is String) { + ignoreFile = ignoreFileValue; + } else if (ignoreFileValue != null) { + parsingErrors.add( + 'Directory entry "$_ignoreFileKey" for "$path" must be a string; ' + 'got "$ignoreFileValue" (${ignoreFileValue.runtimeType}). ' + 'Falling back to the default ignore file.', + ); + } + } + + directoryConfigs.add(DirectoryConfig(path: path, rules: rules, ignoreFile: ignoreFile)); } } } diff --git a/tool/dart_skills_lint/lib/src/cutoff_excerpt.dart b/tool/dart_skills_lint/lib/src/cutoff_excerpt.dart new file mode 100644 index 0000000..62a4f13 --- /dev/null +++ b/tool/dart_skills_lint/lib/src/cutoff_excerpt.dart @@ -0,0 +1,52 @@ +// Shared helper for "field is N characters; max is M" diagnostics that +// also show a |HERE| cutoff excerpt so the author can see exactly +// where the value went over. +// +// Used by both DescriptionLengthRule and the compatibility-length +// check in ValidYamlMetadataRule. Keep the message shape consistent +// across rules so downstream tooling that parses lint output doesn't +// have to learn two formats. + +/// Number of characters of context to show on either side of the cutoff. +const int _excerptContextChars = 40; + +/// Builds a length-overflow diagnostic for a frontmatter field whose +/// value is longer than [maxLength]. +/// +/// Output shape (placeholders shown in backticks): +/// +/// `fieldName` field is `N` characters; maximum is `maxLength`. +/// Cutoff at character `maxLength`: ...`context`|HERE|`context`... +/// (see `docUrl`) +/// +/// The `(see ...)` clause is omitted when [docUrl] is null. Newlines in +/// the excerpt are escaped to `\n` so the message stays on one line. +String buildLengthDiagnostic({ + required String fieldName, + required String value, + required int maxLength, + String? docUrl, +}) { + final String excerpt = _buildCutoffExcerpt(value, maxLength); + final docsClause = docUrl != null ? ' (see $docUrl)' : ''; + return '$fieldName field is ${value.length} characters; ' + 'maximum is $maxLength. ' + 'Cutoff at character $maxLength: $excerpt' + '$docsClause'; +} + +String _buildCutoffExcerpt(String value, int maxLength) { + final int start = (maxLength - _excerptContextChars).clamp(0, value.length); + final int end = (maxLength + _excerptContextChars).clamp(0, value.length); + final String before = value.substring(start, maxLength); + final String after = value.substring(maxLength, end); + final leadingEllipsis = start > 0 ? '...' : ''; + final trailingEllipsis = end < value.length ? '...' : ''; + final String escapedBefore = _escapeForOneLine(before); + final String escapedAfter = _escapeForOneLine(after); + return '$leadingEllipsis$escapedBefore|HERE|$escapedAfter$trailingEllipsis'; +} + +String _escapeForOneLine(String s) { + return s.replaceAll('\n', r'\n').replaceAll('\r', r'\r'); +} diff --git a/tool/dart_skills_lint/lib/src/entry_point.dart b/tool/dart_skills_lint/lib/src/entry_point.dart index 9676383..7404cab 100644 --- a/tool/dart_skills_lint/lib/src/entry_point.dart +++ b/tool/dart_skills_lint/lib/src/entry_point.dart @@ -29,9 +29,43 @@ const _ignoreFileOption = 'ignore-file'; const _ignoreConfigFlag = 'ignore-config'; const _generateBaselineFlag = 'generate-baseline'; const _fixFlag = 'fix'; +const _dryRunFlag = 'dry-run'; const _fixApplyFlag = 'fix-apply'; const _allowMisconfiguredKeysFlag = 'allow-misconfigured-keys'; +/// User-visible deprecation notice for the legacy `--fix-apply` alias. +/// +/// Exposed (not `_`-prefixed) so integration tests can assert it appears on +/// stderr when the alias is used. +const fixApplyDeprecationMsg = + '--fix-apply is deprecated; use --fix instead. ' + 'Pass --fix --dry-run to preview changes without writing.'; + +/// Welcoming first-run guide shown when no args are passed and no default +/// skills directory exists. Exposed so integration tests can assert the +/// exact greeting (drift here changes the new-user experience). +const firstRunGuideMsg = ''' +dart_skills_lint: a linter for Agent Skills (SKILL.md). + +No skills were found to validate. Get started in one of three ways: + + 1. Lint a single skill directory: + dart run dart_skills_lint --skill ./path/to/my-skill + + 2. Lint every skill under a root directory: + dart run dart_skills_lint --skills-directory ./path/to/skills-root + + 3. Drop a skill into one of the auto-discovered default paths + (relative to the current directory) and re-run with no flags: + .claude/skills//SKILL.md + .agents/skills//SKILL.md + +For repo-wide config, create dart_skills_lint.yaml with a +`dart_skills_lint.directories` entry. + +Spec: https://agentskills.io/specification +Run with --help to see every flag.'''; + /// Main entrypoint execution logic for the CLI tool. /// /// Parses arguments and runs validation on the specified directory. @@ -78,8 +112,19 @@ 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; + final fixFlag = results[_fixFlag] as bool; + final dryRun = results[_dryRunFlag] as bool; + final fixApplyAlias = results[_fixApplyFlag] as bool; + + if (fixApplyAlias) { + stderr.writeln(fixApplyDeprecationMsg); + } + + // --fix writes fixes to disk; pair with --dry-run to preview without + // writing. --fix-apply is a deprecated alias for --fix that still + // writes (with a deprecation notice on stderr above). + final bool fix = fixFlag && dryRun; + final bool fixApply = (fixFlag && !dryRun) || fixApplyAlias; String? ignoreFileOverride; if (results.wasParsed(_ignoreFileOption)) { @@ -108,8 +153,8 @@ Future runApp(List args) async { } else { exitCode = 1; } - } on MissingDefaultsException catch (e) { - _printUsage(parser, 'Missing skills directory. Checked defaults: ${e.defaults.join(', ')}'); + } on MissingDefaultsException catch (_) { + stdout.writeln(firstRunGuideMsg); exitCode = 64; } } @@ -164,8 +209,20 @@ ArgParser _createArgParser(String helpFlag) { 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.') + ..addFlag( + _fixFlag, + negatable: false, + help: 'Write fixes for failing lints to disk. Combine with --dry-run to preview.', + ) + ..addFlag( + _dryRunFlag, + negatable: false, + help: 'When passed with --fix, preview proposed changes without writing.', + ) + // help: omitted — flag is hide: true so --help skips it anyway. + // Adopters who hit it still get the runtime deprecation notice + // on stderr (see fixApplyDeprecationMsg above). + ..addFlag(_fixApplyFlag, negatable: false, hide: true) ..addFlag( _allowMisconfiguredKeysFlag, negatable: false, diff --git a/tool/dart_skills_lint/lib/src/levenshtein.dart b/tool/dart_skills_lint/lib/src/levenshtein.dart new file mode 100644 index 0000000..d74b738 --- /dev/null +++ b/tool/dart_skills_lint/lib/src/levenshtein.dart @@ -0,0 +1,39 @@ +import 'dart:math' as math; + +/// Plain Levenshtein edit distance over runes. O(n*m) time, O(m) space. +/// +/// Used by sibling-suggestion logic to score how close an existing filename +/// is to a missing one. Lifted into its own file so the rule that consumes +/// it stays focused on the rule contract and so the function is easy to +/// unit-test in isolation. +int levenshtein(String a, String b) { + if (a == b) { + return 0; + } + if (a.isEmpty) { + return b.length; + } + if (b.isEmpty) { + return a.length; + } + + final List aCodes = a.runes.toList(); + final List bCodes = b.runes.toList(); + + var previous = List.generate(bCodes.length + 1, (j) => j); + var current = List.filled(bCodes.length + 1, 0); + for (var i = 1; i <= aCodes.length; i++) { + current[0] = i; + for (var j = 1; j <= bCodes.length; j++) { + final cost = aCodes[i - 1] == bCodes[j - 1] ? 0 : 1; + final int del = previous[j] + 1; + final int ins = current[j - 1] + 1; + final int sub = previous[j - 1] + cost; + current[j] = math.min(math.min(del, ins), sub); + } + final swap = previous; + previous = current; + current = swap; + } + return previous[bCodes.length]; +} 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 6501728..83ba9ba 100644 --- a/tool/dart_skills_lint/lib/src/models/skill_context.dart +++ b/tool/dart_skills_lint/lib/src/models/skill_context.dart @@ -16,6 +16,11 @@ class SkillContext { /// Regex to match the YAML frontmatter in SKILL.md. static final RegExp skillStartRegex = RegExp(r'^---\s*\n(.*?)\n---\s*\n', dotAll: true); + /// Regex to match inline Markdown links (`[text](target)`). The capture + /// group is the link target. Rules that inspect SKILL.md link targets + /// import this rather than re-defining the pattern. + static final RegExp markdownLinkRegex = RegExp(r'\[.*?\]\((.*?)\)'); + 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 ea6d277..de2dd8e 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 @@ -19,7 +19,6 @@ class AbsolutePathsRule extends SkillRule implements FixableRule { @override final AnalysisSeverity severity; - static final _markdownLinkRegex = RegExp(r'\[.*?\]\((.*?)\)'); static const String _skillFileName = SkillContext.skillFileName; @override @@ -27,13 +26,14 @@ class AbsolutePathsRule extends SkillRule implements FixableRule { final errors = []; // Extract content after YAML frontmatter - final skillStartRegex = RegExp(r'^---\s*\n(.*?)\n---\s*\n', dotAll: true); - final RegExpMatch? match = skillStartRegex.firstMatch(context.rawContent); + final RegExpMatch? match = SkillContext.skillStartRegex.firstMatch(context.rawContent); final String markdownContent = match != null ? context.rawContent.substring(match.end) : context.rawContent; - for (final RegExpMatch linkMatch in _markdownLinkRegex.allMatches(markdownContent)) { + for (final RegExpMatch linkMatch in SkillContext.markdownLinkRegex.allMatches( + markdownContent, + )) { final String path = linkMatch.group(1)!; if (isAbsolute(path) || windows.isAbsolute(path)) { errors.add( @@ -41,7 +41,10 @@ class AbsolutePathsRule extends SkillRule implements FixableRule { ruleId: name, severity: severity, file: _skillFileName, - message: 'Absolute filepath found in link: $path', + message: + 'Absolute filepath found in link: $path. ' + 'Skills must use paths relative to SKILL.md so they remain ' + 'portable across machines.', ), ); } @@ -56,7 +59,7 @@ class AbsolutePathsRule extends SkillRule implements FixableRule { return currentContent; } - return currentContent.replaceAllMapped(_markdownLinkRegex, (match) { + return currentContent.replaceAllMapped(SkillContext.markdownLinkRegex, (match) { final String path = match.group(1)!; if (isAbsolute(path) || windows.isAbsolute(path)) { final file = File(path); diff --git a/tool/dart_skills_lint/lib/src/rules/description_length_rule.dart b/tool/dart_skills_lint/lib/src/rules/description_length_rule.dart index a9e1dc3..9ef96b9 100644 --- a/tool/dart_skills_lint/lib/src/rules/description_length_rule.dart +++ b/tool/dart_skills_lint/lib/src/rules/description_length_rule.dart @@ -1,4 +1,5 @@ import 'package:yaml/yaml.dart'; +import '../cutoff_excerpt.dart'; import '../models/analysis_severity.dart'; import '../models/skill_context.dart'; import '../models/skill_rule.dart'; @@ -38,8 +39,12 @@ class DescriptionLengthRule extends SkillRule { ruleId: name, severity: severity, file: _skillFileName, - message: - 'Description field is too long. Maximum $maxDescriptionLength characters (see $_descriptionFieldUrl)', + message: buildLengthDiagnostic( + fieldName: 'Description', + value: description, + maxLength: maxDescriptionLength, + docUrl: _descriptionFieldUrl, + ), ), ); } 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 4d7407f..b2dd915 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 @@ -41,58 +41,51 @@ class NameFormatRule extends SkillRule implements FixableRule { return errors; // Handled by required fields check } + final String suggestion = suggestNormalizedName(skillName); + if (skillName != skillName.toLowerCase()) { errors.add( - ValidationError( - ruleId: name, - severity: severity, - file: _skillFileName, - message: 'Skill name must be lowercase: $skillName (see $_nameFieldUrl)', + _buildNameFormatError( + 'Frontmatter `name` "$skillName" must be lowercase. ' + 'Suggested: "$suggestion"', ), ); } if (skillName.length > maxNameLength) { errors.add( - ValidationError( - ruleId: name, - severity: severity, - file: _skillFileName, - message: 'Skill name too long. Maximum $maxNameLength characters (see $_nameFieldUrl)', + _buildNameFormatError( + 'Frontmatter `name` is ${skillName.length} characters; ' + 'maximum is $maxNameLength. ' + 'Shorten the `name:` field in SKILL.md.', ), ); } if (!_validNameRegex.hasMatch(skillName)) { errors.add( - ValidationError( - ruleId: name, - severity: severity, - file: _skillFileName, - message: - 'Skill name contains invalid characters. Only lowercase letters, digits, and hyphens allowed (see $_nameFieldUrl)', + _buildNameFormatError( + 'Frontmatter `name` "$skillName" contains invalid characters. ' + 'Only lowercase letters, digits, and hyphens are allowed. ' + 'Suggested: "$suggestion"', ), ); } if (skillName.startsWith('-') || skillName.endsWith('-')) { errors.add( - ValidationError( - ruleId: name, - severity: severity, - file: _skillFileName, - message: 'Skill name cannot have leading or trailing hyphens (see $_nameFieldUrl)', + _buildNameFormatError( + 'Frontmatter `name` "$skillName" has leading or trailing hyphens. ' + 'Suggested: "$suggestion"', ), ); } if (skillName.contains('--')) { errors.add( - ValidationError( - ruleId: name, - severity: severity, - file: _skillFileName, - message: 'Skill name cannot have consecutive hyphens (see $_nameFieldUrl)', + _buildNameFormatError( + 'Frontmatter `name` "$skillName" has consecutive hyphens. ' + 'Suggested: "$suggestion"', ), ); } @@ -100,12 +93,11 @@ class NameFormatRule extends SkillRule implements FixableRule { final String dirName = basename(context.directory.path); if (skillName != dirName) { errors.add( - ValidationError( - ruleId: name, - severity: severity, - file: _skillFileName, - message: - 'Skill name ($skillName) must exactly match the parent directory name ($dirName) (see $_nameFieldUrl)', + _buildNameFormatError( + 'Frontmatter `name` "$skillName" does not match the parent ' + 'directory name "$dirName". ' + 'Fix by either setting `name: $dirName` in SKILL.md ' + 'or renaming the directory from "$dirName" to "$skillName".', ), ); } @@ -113,6 +105,32 @@ class NameFormatRule extends SkillRule implements FixableRule { return errors; } + ValidationError _buildNameFormatError(String message) => ValidationError( + ruleId: name, + severity: severity, + file: _skillFileName, + message: '$message (see $_nameFieldUrl)', + ); + + /// Returns a best-effort normalization of [input] that conforms to the + /// skill name format: lowercase, hyphens only, no consecutive/leading/ + /// trailing hyphens, truncated to [maxNameLength]. + /// + /// This is intentionally a *suggestion* — the author still picks the final + /// name. The output is not guaranteed to match a directory name. + @visibleForTesting + static String suggestNormalizedName(String input) { + String s = input.toLowerCase(); + s = s.replaceAll(RegExp(r'[^a-z0-9\-]+'), '-'); + s = s.replaceAll(RegExp(r'-+'), '-'); + s = s.replaceAll(RegExp(r'^-+|-+$'), ''); + if (s.length > maxNameLength) { + s = s.substring(0, maxNameLength); + s = s.replaceAll(RegExp(r'-+$'), ''); + } + return s; + } + @override Future fix(String filePath, String currentContent, Directory directory) async { if (filePath != SkillContext.skillFileName) { diff --git a/tool/dart_skills_lint/lib/src/rules/relative_paths_rule.dart b/tool/dart_skills_lint/lib/src/rules/relative_paths_rule.dart index 620868b..8383c27 100644 --- a/tool/dart_skills_lint/lib/src/rules/relative_paths_rule.dart +++ b/tool/dart_skills_lint/lib/src/rules/relative_paths_rule.dart @@ -1,5 +1,7 @@ import 'dart:io'; +import 'package:meta/meta.dart'; import 'package:path/path.dart'; +import '../levenshtein.dart'; import '../models/analysis_severity.dart'; import '../models/skill_context.dart'; import '../models/skill_rule.dart'; @@ -18,7 +20,6 @@ class RelativePathsRule extends SkillRule { @override final AnalysisSeverity severity; - static final _markdownLinkRegex = RegExp(r'\[.*?\]\((.*?)\)'); static const _skillFileName = 'SKILL.md'; @override @@ -26,13 +27,14 @@ class RelativePathsRule extends SkillRule { final errors = []; // Extract content after YAML frontmatter - final skillStartRegex = RegExp(r'^---\s*\n(.*?)\n---\s*\n', dotAll: true); - final RegExpMatch? match = skillStartRegex.firstMatch(context.rawContent); + final RegExpMatch? match = SkillContext.skillStartRegex.firstMatch(context.rawContent); final String markdownContent = match != null ? context.rawContent.substring(match.end) : context.rawContent; - for (final RegExpMatch linkMatch in _markdownLinkRegex.allMatches(markdownContent)) { + for (final RegExpMatch linkMatch in SkillContext.markdownLinkRegex.allMatches( + markdownContent, + )) { final String fullPath = linkMatch.group(1)!; // Markdown links can have a title after the URL, separated by spaces. // e.g. [text](url "title") @@ -54,14 +56,22 @@ class RelativePathsRule extends SkillRule { // If Uri parsing fails, treat it as a potential filepath. } - final linkedFile = File(join(context.directory.path, effectivePath)); + final String resolvedPath = absolute(normalize(join(context.directory.path, effectivePath))); + final linkedFile = File(resolvedPath); if (!linkedFile.existsSync()) { + final String? suggestion = findSiblingSuggestion( + originalLink: path, + resolvedPath: resolvedPath, + ); + final suggestionClause = suggestion != null ? ' Did you mean "$suggestion"?' : ''; errors.add( ValidationError( ruleId: name, severity: severity, file: _skillFileName, - message: 'Linked file does not exist: $path', + message: + 'Linked file does not exist: $path (resolved to $resolvedPath).' + '$suggestionClause', ), ); } @@ -70,3 +80,72 @@ class RelativePathsRule extends SkillRule { return errors; } } + +/// Looks for a near-miss sibling **file** next to the missing +/// [resolvedPath] and, if one exists, returns the full suggested link as +/// it should appear in the SKILL.md author's markdown — the original +/// link's directory prefix joined to the matched basename, normalized to +/// forward slashes so the suggestion is portable across platforms. +/// +/// Returns `null` when: +/// - the original link has no parent dir on disk, +/// - the parent dir can't be listed (e.g. permission error), +/// - or no candidate is close enough to the missing basename. +/// +/// [originalLink] is the link text as written in the SKILL.md +/// (`docs/DEATILS.md`); [resolvedPath] is the same link resolved +/// against the skill directory (`/abs/path/skill/docs/DEATILS.md`). +/// +/// Subdirectories of the parent are intentionally excluded from the +/// candidate set — links almost always point at files, and suggesting +/// a directory would be misleading. +@visibleForTesting +String? findSiblingSuggestion({required String originalLink, required String resolvedPath}) { + final String parentPath = dirname(resolvedPath); + final parentDir = Directory(parentPath); + if (!parentDir.existsSync()) { + return null; + } + + final String missingBase = basename(resolvedPath).toLowerCase(); + if (missingBase.isEmpty) { + return null; + } + + // Tunable; chosen to balance typo recall against false positives. + final int threshold = (missingBase.length ~/ 3).clamp(1, missingBase.length); + + final List entries; + try { + entries = parentDir.listSync(); + } on FileSystemException { + return null; + } + + String? best; + int bestDistance = threshold + 1; + for (final entity in entries) { + if (entity is Directory) { + continue; + } + final String candidate = basename(entity.path); + if (candidate == basename(resolvedPath)) { + continue; + } + final int distance = levenshtein(missingBase, candidate.toLowerCase()); + if (distance < bestDistance) { + bestDistance = distance; + best = candidate; + } + } + + if (best == null || bestDistance > threshold) { + return null; + } + + final String dir = dirname(originalLink); + if (dir == '.' || dir.isEmpty) { + return best; + } + return join(dir, best).replaceAll(r'\', '/'); +} diff --git a/tool/dart_skills_lint/lib/src/rules/valid_yaml_metadata_rule.dart b/tool/dart_skills_lint/lib/src/rules/valid_yaml_metadata_rule.dart index b59f85d..a9cf9db 100644 --- a/tool/dart_skills_lint/lib/src/rules/valid_yaml_metadata_rule.dart +++ b/tool/dart_skills_lint/lib/src/rules/valid_yaml_metadata_rule.dart @@ -1,4 +1,5 @@ import 'package:yaml/yaml.dart'; +import '../cutoff_excerpt.dart'; import '../models/analysis_severity.dart'; import '../models/skill_context.dart'; import '../models/skill_rule.dart'; @@ -62,8 +63,12 @@ class ValidYamlMetadataRule extends SkillRule { ruleId: name, severity: severity, file: _skillFileName, - message: - 'Compatibility field is too long. Maximum $maxCompatibilityLength characters (see $_compatibilityFieldUrl)', + message: buildLengthDiagnostic( + fieldName: 'Compatibility', + value: compatibility, + maxLength: maxCompatibilityLength, + docUrl: _compatibilityFieldUrl, + ), ), ); } diff --git a/tool/dart_skills_lint/pubspec.yaml b/tool/dart_skills_lint/pubspec.yaml index 961ee31..e528b20 100644 --- a/tool/dart_skills_lint/pubspec.yaml +++ b/tool/dart_skills_lint/pubspec.yaml @@ -1,8 +1,19 @@ name: dart_skills_lint -description: A static analysis linter for agent skills. -version: 0.3.0 +description: >- + A static analysis linter for Agent Skills (SKILL.md) written in Dart. + Validates frontmatter, naming, paths, and structure for use in CI and + pre-commit hooks. +version: 0.3.1 resolution: workspace repository: https://github.com/flutter/skills +issue_tracker: https://github.com/flutter/skills/issues + +topics: + - agent-skills + - linter + - static-analysis + - cli + - validation environment: sdk: ^3.10.8 diff --git a/tool/dart_skills_lint/skills/dart-skills-lint-setup/SKILL.md b/tool/dart_skills_lint/skills/dart-skills-lint-setup/SKILL.md index 35048cc..07c08d3 100644 --- a/tool/dart_skills_lint/skills/dart-skills-lint-setup/SKILL.md +++ b/tool/dart_skills_lint/skills/dart-skills-lint-setup/SKILL.md @@ -2,25 +2,23 @@ name: dart-skills-lint-setup description: |- Use this skill when you need to set up validation for AI agent skills in a Dart project for the first time. - This includes adding dependencies, configuring the linter, setting up tests, and creating a CI workflow. + Adds the linter as a dev_dependency, creates a configuration file, and generates a baseline for legacy repos. --- # Setting up Skill Validation with dart_skills_lint -## Contents -- [Setup for Dart Developers](#setup-for-dart-developers) -- [Initial Integration in a Repository](#initial-integration-in-a-repository) -- [GitHub Workflow Setup](#github-workflow-setup) +This skill covers **first-time wiring** of `dart_skills_lint` into a +repository. For ongoing use — running the linter, interpreting +output, and writing custom rules — see the +[`dart-skills-lint-validation`](../dart-skills-lint-validation/SKILL.md) +skill. For copy-pasteable CI workflow and pre-commit hook recipes, +see the [`Recipes` section of the README](../../README.md#recipes). -## Setup for Dart Developers -Setup validation in your Dart project: +## Steps + +1. **Add `dart_skills_lint` as a `dev_dependency`.** Prefer a git + dependency (the package isn't on pub.dev yet): -1. Add `dart_skills_lint` to your `pubspec.yaml` as a `dev_dependency`. If it is published to pub.dev: - ```yaml - dev_dependencies: - dart_skills_lint: ^0.2.0 - ``` - If it is a local package or hosted on Git, use a path or git dependency: ```yaml dev_dependencies: dart_skills_lint: @@ -28,42 +26,17 @@ Setup validation in your Dart project: url: https://github.com/flutter/skills.git path: tool/dart_skills_lint ``` - **Note:** The test example below also requires `package:logging` and `package:test` to be added to your `dev_dependencies` if they are not already present. - -2. Integrate the linter into your automated tests by importing the package and calling `validateSkills`. This ensures your skills are automatically validated whenever you run `dart test`. - - Example `test/lint_skills_test.dart`: - ```dart - import 'dart:async'; - import 'package:dart_skills_lint/dart_skills_lint.dart'; - import 'package:logging/logging.dart'; - import 'package:test/test.dart'; - void main() { - test('Run skills linter', () async { - final Level oldLevel = Logger.root.level; - Logger.root.level = Level.ALL; - final StreamSubscription subscription = - Logger.root.onRecord.listen((record) => print(record.message)); + **Isolate the dependency** in a `tool/` package when you can, + instead of putting it on the root `pubspec.yaml` — keeps the + linter's deps out of your runtime closure. If you must add it + to multiple `pubspec.yaml` files, ensure the `ref:` (commit + hash) is identical across all of them so resolution doesn't + diverge. - try { - // Load configuration from the default file (dart_skills_lint.yaml) - final config = await ConfigParser.loadConfig(); +2. **Create `dart_skills_lint.yaml`** at the repository root so both + the CLI and any embedded test invocation share the same config: - final isValid = await validateSkills( - config: config, - ); - expect(isValid, isTrue, reason: 'Skills validation failed. See above for details.'); - } finally { - Logger.root.level = oldLevel; - await subscription.cancel(); - } - }); - } - ``` - -3. **Recommended**: Create a configuration file `dart_skills_lint.yaml` in the root of your project to centralize your rules and directory settings. This ensures both the CLI and your automated tests use the same configuration. - **Note:** If you use `validateSkills` directly in tests, you can load the `dart_skills_lint.yaml` file using `ConfigParser.loadConfig()` and pass it to `validateSkills` to share the same configuration as the CLI. ```yaml dart_skills_lint: rules: @@ -72,45 +45,31 @@ Setup validation in your Dart project: directories: - path: ".agents/skills" ``` - **Note:** The following rules are enabled by default and do not need to be listed unless you want to change their severity or disable them: `check-absolute-paths`, `valid-yaml-metadata`, `invalid-skill-name`, `description-too-long`. -## Initial Integration in a Repository -When adding `dart_skills_lint` to a repository for the first time, follow these best practices: -- **Isolate the dependency**: Add it to a specific package that handles tooling or tests (e.g., `tool/pubspec.yaml`) rather than the root. -- **Keep hashes in sync**: If you must add it to multiple `pubspec.yaml` files (e.g., root and a tool package), ensure the `ref` (commit hash) is identical to avoid resolution conflicts. -- **Generating a Baseline**: If integrating into a repository with existing skills that have legacy errors, use the baseline feature: - ```bash - dart run dart_skills_lint:cli --skills-directory=.agents/skills --generate-baseline - ``` + Rules enabled by default — `check-absolute-paths`, + `valid-yaml-metadata`, `invalid-skill-name`, + `description-too-long` — only need to be listed if you want to + change their severity. See [`RULES.md`](../../RULES.md) for the + full list. + +3. **Generate a baseline** if you're integrating into a repository + with pre-existing skills that have legacy violations you don't + want to fix immediately: + + ```bash + dart run dart_skills_lint:cli --skills-directory=.agents/skills --generate-baseline + ``` -## GitHub Workflow Setup -To enforce skill validation in CI, add a GitHub workflow file (e.g., `.github/workflows/dart_skills_validation.yaml`): + This writes the current set of failures into an ignore file so + the next run exits clean. New violations introduced after the + baseline still surface as errors. -```yaml -name: dart_skills_validation -permissions: read-all +4. **Wire it into CI.** Use the + [GitHub Actions recipe](../../README.md#recipes) from the README + verbatim, or follow the + [pre-commit hook recipe](../../README.md#recipes) below it. -on: - pull_request: - paths: - - '.agents/skills/**' - - 'tool/**' # Adjust to your tool package path - push: - branches: [ main ] - paths: - - '.agents/skills/**' - - 'tool/**' +## When you're done -jobs: - validate: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: dart-lang/setup-dart@v1 - - name: Install dependencies - run: dart pub get - working-directory: tool # Adjust to your tool package path - - name: Run skills validation - run: dart test - working-directory: tool # Adjust to your tool package path -``` +The dart-skills-lint-validation skill takes over from here for +day-to-day use. diff --git a/tool/dart_skills_lint/skills/dart-skills-lint-validation/SKILL.md b/tool/dart_skills_lint/skills/dart-skills-lint-validation/SKILL.md index 28f8723..69a6621 100644 --- a/tool/dart_skills_lint/skills/dart-skills-lint-validation/SKILL.md +++ b/tool/dart_skills_lint/skills/dart-skills-lint-validation/SKILL.md @@ -1,59 +1,64 @@ --- name: dart-skills-lint-validation description: |- - Use this skill when you need to validate that AI agent skills meet the specification. - This includes running the linter via CLI, authoring custom rules, and following the validation workflow. + Use this skill when you need to validate AI agent skills with dart_skills_lint — running the linter, interpreting failures, fixing violations, and authoring custom rules. --- # Validating Skills with dart_skills_lint -## Contents -- [Usage for Agents (CLI)](#usage-for-agents-cli) -- [Authoring Custom Rules](#authoring-custom-rules) -- [Workflow: Validating Skills](#workflow-validating-skills) -- [Specification Reference](#specification-reference) +This skill covers **day-to-day use**: running the linter, walking +through a failing run, and writing a custom rule when defaults +aren't enough. For first-time wiring (adding the dep, creating the +config file, generating a baseline) see +[`dart-skills-lint-setup`](../dart-skills-lint-setup/SKILL.md). The +full rule reference (default severities, diagnostic shapes, +fixability) lives in [`RULES.md`](../../RULES.md). -## Usage for Agents (CLI) -Use the `dart_skills_lint` CLI to validate skills. Choose the appropriate workflow based on your environment: +## Running the linter -**Note on choosing the right method:** -- **If you are a Dart developer**: The method where you add a test to your project (see the `dart-skills-lint-setup` skill) is preferred as it integrates with your existing testing workflow. -- **If you are working on a non-Dart project**: The CLI and global install (Scenario B below) is the best way to use the linter without adding a dependency to your project. +If `dart_skills_lint` is in `pubspec.yaml`: -### Scenario A: The package is in your project dependencies -Use this method if you are working within a project that has `dart_skills_lint` listed in `pubspec.yaml`. -Run: ```bash dart run dart_skills_lint:cli -d .agents/skills ``` -### Scenario B: The package is activated globally -Use this method if you want to validate skills across multiple projects without adding a dependency to each one. -Run: +If it's installed globally with `dart pub global activate`: + ```bash dart pub global run dart_skills_lint:cli -d .agents/skills ``` -### Common Flags -- `-d`, `--skills-directory`: Specifies a root directory containing sub-folders of skills to validate. Can be passed multiple times. -- `-s`, `--skill`: Specifies an individual skill directory to validate directly. Can be passed multiple times. -- `-q`, `--quiet`: Hide non-error validation output. -- `-w`, `--print-warnings`: Enable printing of warning messages. -- `--fast-fail`: Halt execution immediately on the error. -- `--ignore-config`: Ignore the YAML configuration file entirely. -- `--fix`: Preview fixes for failing lints (dry run). -- `--fix-apply`: Apply fixes for failing lints. +Run `dart run dart_skills_lint:cli --help` for the full flag list +(skip the inline duplicate so it never goes stale). + +## Workflow for a failing run + +1. **Run the validator.** +2. **Read the errors.** Each diagnostic names the rule that fired, + the offending value, and a suggested fix when one applies. +3. **Fix the violations.** For fixable rules + (`check-absolute-paths`, `check-trailing-whitespace`, + `invalid-skill-name`), pass `--fix` to write the corrections + to disk; add `--dry-run` to preview the diff first. +4. **Re-run** to confirm the run is clean. -## Authoring Custom Rules -To author custom rules, extend the `SkillRule` class and pass them to `validateSkills`. +### Task progress + +- [ ] Run validator +- [ ] Read errors +- [ ] Fix violations (manual or `--fix` / `--fix --dry-run`) +- [ ] Verify clean run + +## Authoring a custom rule + +Extend `SkillRule` and pass the rule into `validateSkills`: -Example: ```dart import 'package:dart_skills_lint/dart_skills_lint.dart'; -class MyCustomRule extends SkillRule { +class DeprecatedSkillRule extends SkillRule { @override - final String name = 'my-custom-rule'; + final String name = 'deprecated-skill'; @override final AnalysisSeverity severity = AnalysisSeverity.warning; @@ -77,45 +82,27 @@ class MyCustomRule extends SkillRule { } ``` -Use it in your test: +Wire it up in a Dart test: + ```dart -final config = await ConfigParser.loadConfig(); -await validateSkills( - config: config, - customRules: [MyCustomRule()], -); +import 'package:dart_skills_lint/dart_skills_lint.dart'; +import 'package:test/test.dart'; + +void main() { + test('skills pass with deprecated-skill custom rule', () async { + final config = await ConfigParser.loadConfig(); + await validateSkills( + config: config, + customRules: [DeprecatedSkillRule()], + ); + }); +} ``` -## Workflow: Validating Skills -Follow this workflow to validate skills: - -1. **Run the validator**: Execute the linter on your skills directory. - ```bash - dart run dart_skills_lint:cli -d .agents/skills - ``` -2. **Review errors**: Check the output for any errors or warnings. -3. **Fix violations**: Use `--fix-apply` or edit files manually to resolve issues. -4. **Verify**: Re-run the validator to ensure all checks pass. - -### Task Progress -- [ ] Run validator -- [ ] Review errors -- [ ] Fix violations -- [ ] Verify clean run - -## Specification Reference -
-View Skill Specification Constraints - -### Directory and File Structure -- Mandatory `SKILL.md` file at the root of the skill folder. -- Directories starting with a dot `.` (e.g., `.dart_tool`) are ignored. - -### Metadata (YAML Frontmatter) -- Required fields: `name` and `description`. +## Related -### Field Constraints -- **Name**: Max 64 characters, lowercase alphanumeric and hyphens only. Must match the parent directory name. -- **Description**: Max 1024 characters. -- **Compatibility**: Max 500 characters. -
+- [`dart-skills-lint-setup`](../dart-skills-lint-setup/SKILL.md) — + first-time wiring. +- [`RULES.md`](../../RULES.md) — canonical rule reference. +- [`README.md`](../../README.md) — installation, configuration, + integration recipes. diff --git a/tool/dart_skills_lint/test/cli_integration_test.dart b/tool/dart_skills_lint/test/cli_integration_test.dart index 895cd28..1b8ce3f 100644 --- a/tool/dart_skills_lint/test/cli_integration_test.dart +++ b/tool/dart_skills_lint/test/cli_integration_test.dart @@ -308,13 +308,20 @@ dart_skills_lint: final List rest = await process.stdout.rest.toList(); expect(rest, isEmpty); }); - test('fails with 64 when no flags passed and both defaults are missing', () async { + test('prints a first-run guide to stdout and exits 64 when no defaults exist', () async { final TestProcess process = await TestProcess.start('dart', [ p.normalize(p.absolute('bin/cli.dart')), ], workingDirectory: tempDir.path); - final List stderr = await process.stderr.rest.toList(); - expect(stderr.join('\n'), contains('Missing skills directory. Checked defaults:')); + final List stdout = await process.stdout.rest.toList(); + final String stdoutStr = stdout.join('\n'); + expect(stdoutStr, contains('dart_skills_lint: a linter for Agent Skills')); + expect(stdoutStr, contains('--skill ./path/to/my-skill')); + expect(stdoutStr, contains('--skills-directory ./path/to/skills-root')); + expect(stdoutStr, contains('.claude/skills//SKILL.md')); + expect(stdoutStr, contains('.agents/skills//SKILL.md')); + expect(stdoutStr, contains('agentskills.io/specification')); + expect(stdoutStr, contains('--help')); await process.shouldExit(64); }); @@ -563,7 +570,7 @@ dart_skills_lint: await process.shouldExit(1); }); - test('--fix dry-runs and shows diff but does not modify file', () async { + test('--fix --dry-run shows diff but does not modify file', () async { final Directory skillDir = await Directory('${tempDir.path}/test-skill').create(); await File( '${skillDir.path}/SKILL.md', @@ -574,6 +581,7 @@ dart_skills_lint: '-s', skillDir.path, '--fix', + '--dry-run', '--check-trailing-whitespace', ]); @@ -588,7 +596,7 @@ dart_skills_lint: expect(content, contains('Line with 1 space \n')); }); - test('--fix-apply modifies file', () async { + test('--fix without --dry-run writes fixes to disk', () async { final Directory skillDir = await Directory('${tempDir.path}/test-skill').create(); await File( '${skillDir.path}/SKILL.md', @@ -598,7 +606,7 @@ dart_skills_lint: 'bin/cli.dart', '-s', skillDir.path, - '--fix-apply', + '--fix', '--check-trailing-whitespace', ]); @@ -613,7 +621,33 @@ dart_skills_lint: expect(content, contains('Line with 1 space\n')); }); - test('--fix-apply does not modify file if lint is ignored', () async { + test('--fix-apply alias still works but prints a deprecation notice', () 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(); + final List stderr = await process.stderr.rest.toList(); + expect(stderr.join('\n'), contains(fixApplyDeprecationMsg)); + expect(stdout.join('\n'), contains('Applied fixes for test-skill')); + + await process.shouldExit(0); + + // File still modified — alias preserves behavior. + final String content = await File('${skillDir.path}/SKILL.md').readAsString(); + expect(content, contains('Line with 1 space\n')); + }); + + test('--fix 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', @@ -637,7 +671,7 @@ dart_skills_lint: 'bin/cli.dart', '-s', skillDir.path, - '--fix-apply', + '--fix', '--check-trailing-whitespace', ]); @@ -647,7 +681,7 @@ dart_skills_lint: expect(content, contains('Line with 1 space \n')); }); - test('--fix-apply does not modify file if invalid-skill-name is ignored', () async { + test('--fix 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(''' --- @@ -671,7 +705,7 @@ Body'''); 'bin/cli.dart', '-s', skillDir.path, - '--fix-apply', + '--fix', ]); await process.shouldExit(0); diff --git a/tool/dart_skills_lint/test/config_file_test.dart b/tool/dart_skills_lint/test/config_file_test.dart index dfd1d5c..d29b2b3 100644 --- a/tool/dart_skills_lint/test/config_file_test.dart +++ b/tool/dart_skills_lint/test/config_file_test.dart @@ -288,6 +288,37 @@ dart_skills_lint: await process.shouldExit(1); }); + test('bad path: type emits parsing error and lets later entries through', () async { + // First entry has path: 123 (not a string). Second entry is well-formed. + // The bad-type entry should produce a parsingErrors line but must not + // prevent the second entry from being parsed. + await Directory('${tempDir.path}/good-skill').create(); + await File('${tempDir.path}/good-skill/SKILL.md').writeAsString(''' +--- +name: good-skill +description: A valid skill +--- +Body'''); + + await File('${tempDir.path}/dart_skills_lint.yaml').writeAsString(''' +dart_skills_lint: + directories: + - path: 123 + - path: "good-skill" +'''); + + final TestProcess process = await TestProcess.start('dart', [ + p.normalize(p.absolute('bin/cli.dart')), + ], workingDirectory: tempDir.path); + + final List stderr = await process.stderr.rest.toList(); + final String stderrStr = stderr.join('\n'); + expect(stderrStr, contains('Configuration error: Directory entry "path" must be a string')); + // Without the fix, the unchecked cast would throw inside the + // top-level try/catch and 'good-skill' would never run. + await process.shouldExit(1); // exits 1 due to parsing error + }); + test('fails on invalid directory key in config by default', () async { await Directory('${tempDir.path}/test-skill').create(); await File('${tempDir.path}/test-skill/SKILL.md').writeAsString(''' diff --git a/tool/dart_skills_lint/test/example_fixtures_test.dart b/tool/dart_skills_lint/test/example_fixtures_test.dart new file mode 100644 index 0000000..43a4af6 --- /dev/null +++ b/tool/dart_skills_lint/test/example_fixtures_test.dart @@ -0,0 +1,84 @@ +// 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 'package:path/path.dart' as p; +import 'package:test/test.dart'; +import 'package:test_process/test_process.dart'; + +/// Drift guard for the `example/valid` and `example/invalid` fixtures. +/// +/// The fixtures and `example/README.md` make precise claims about which +/// rules fire and what their diagnostics look like. This test pins both: +/// +/// - `example/valid` must exit 0 with no error output under default rules. +/// - `example/invalid` must exit 1 with `invalid-skill-name` under default +/// rules, and must surface all three intended rules when the other two +/// are escalated. +/// +/// Failures here mean either the fixtures have drifted from the README, +/// or a rule's diagnostic wording has changed without the README catching +/// up. Fix one or the other — do not silence the test. +void main() { + group('example fixtures', () { + final String cliPath = p.normalize(p.absolute('bin/cli.dart')); + final String validPath = p.normalize(p.absolute('example/valid')); + final String invalidPath = p.normalize(p.absolute('example/invalid')); + + test('example/valid passes with default rules', () async { + final TestProcess process = await TestProcess.start('dart', [cliPath, '--skill', validPath]); + + final List stdout = await process.stdout.rest.toList(); + final String stdoutStr = stdout.join('\n'); + expect(stdoutStr, contains('--- Validating skill: valid ---')); + expect(stdoutStr, contains('Skill is valid.')); + await process.shouldExit(0); + }); + + test('example/invalid fails on invalid-skill-name with default rules', () async { + final TestProcess process = await TestProcess.start('dart', [ + cliPath, + '--skill', + invalidPath, + ]); + + final List stderr = await process.stderr.rest.toList(); + final String stderrStr = stderr.join('\n'); + + // Disambiguated frontmatter-vs-dir wording plus a normalized + // suggestion — exercises the diagnostic shape from name_format_rule. + expect(stderrStr, contains('Frontmatter `name` "NotInvalid" must be lowercase')); + expect(stderrStr, contains('does not match the parent directory name "invalid"')); + expect(stderrStr, contains('Suggested: "notinvalid"')); + + await process.shouldExit(1); + }); + + test( + 'example/invalid surfaces disallowed-field and check-absolute-paths when escalated', + () async { + final TestProcess process = await TestProcess.start('dart', [ + cliPath, + '--skill', + invalidPath, + '--disallowed-field', + '--check-absolute-paths', + ]); + + final List stderr = await process.stderr.rest.toList(); + final String stderrStr = stderr.join('\n'); + + // disallowed-field + expect(stderrStr, contains('Disallowed field: secret_field')); + // check-absolute-paths now spells out the portability rationale + // in the error message itself. + expect(stderrStr, contains('Absolute filepath found in link: /tmp/this/does/not/exist.md')); + expect(stderrStr, contains('portable')); + // invalid-skill-name still fires. + expect(stderrStr, contains('Frontmatter `name`')); + + await process.shouldExit(1); + }, + ); + }); +} diff --git a/tool/dart_skills_lint/test/field_constraints_test.dart b/tool/dart_skills_lint/test/field_constraints_test.dart index 31c37f8..1321cda 100644 --- a/tool/dart_skills_lint/test/field_constraints_test.dart +++ b/tool/dart_skills_lint/test/field_constraints_test.dart @@ -29,16 +29,20 @@ void main() { }); group('Skill Name', () { - test('fails if not lowercase', () async { + test('fails if not lowercase, error names the frontmatter field', () async { final Directory skillDir = await Directory('${tempDir.path}/Skill-Name').create(); await File('${skillDir.path}/SKILL.md').writeAsString('${buildFrontmatter()}Body'); final validator = Validator(); final ValidationResult result = await validator.validate(skillDir); expect(result.isValid, isFalse); - expect(result.errors, contains(contains('lowercase'))); + expect( + result.errors, + contains(contains('Frontmatter `name` "Skill-Name" must be lowercase')), + ); + expect(result.errors, contains(contains('Suggested: "skill-name"'))); }); - test('fails if too long (> ${NameFormatRule.maxNameLength} chars)', () async { + test('fails if too long, error reports both lengths and names the field', () async { final String longName = 'a' * (NameFormatRule.maxNameLength + 1); final Directory skillDir = await Directory('${tempDir.path}/$longName').create(); await File( @@ -49,11 +53,12 @@ void main() { expect(result.isValid, isFalse); expect( result.errors, - contains(contains('Maximum ${NameFormatRule.maxNameLength} characters')), + contains(contains('Frontmatter `name` is ${longName.length} characters')), ); + expect(result.errors, contains(contains('maximum is ${NameFormatRule.maxNameLength}'))); }); - test('fails if contains invalid characters', () async { + test('fails if contains invalid characters; suggests hyphen-normalized form', () async { final Directory skillDir = await Directory('${tempDir.path}/skill_name').create(); await File( '${skillDir.path}/SKILL.md', @@ -61,10 +66,14 @@ void main() { final validator = Validator(); final ValidationResult result = await validator.validate(skillDir); expect(result.isValid, isFalse); - expect(result.errors, contains(contains('lowercase letters, digits, and hyphens'))); + expect( + result.errors, + contains(contains('Frontmatter `name` "skill_name" contains invalid characters')), + ); + expect(result.errors, contains(contains('Suggested: "skill-name"'))); }); - test('fails if has leading hyphen', () async { + test('fails if has leading hyphen; suggests stripped form', () async { final Directory skillDir = await Directory('${tempDir.path}/-skill-name').create(); await File( '${skillDir.path}/SKILL.md', @@ -72,10 +81,11 @@ void main() { final validator = Validator(); final ValidationResult result = await validator.validate(skillDir); expect(result.isValid, isFalse); - expect(result.errors, contains(contains('leading or trailing hyphens'))); + expect(result.errors, contains(contains('"-skill-name" has leading or trailing hyphens'))); + expect(result.errors, contains(contains('Suggested: "skill-name"'))); }); - test('fails if has trailing hyphen', () async { + test('fails if has trailing hyphen; suggests stripped form', () async { final Directory skillDir = await Directory('${tempDir.path}/skill-name-').create(); await File( '${skillDir.path}/SKILL.md', @@ -83,10 +93,11 @@ void main() { final validator = Validator(); final ValidationResult result = await validator.validate(skillDir); expect(result.isValid, isFalse); - expect(result.errors, contains(contains('leading or trailing hyphens'))); + expect(result.errors, contains(contains('"skill-name-" has leading or trailing hyphens'))); + expect(result.errors, contains(contains('Suggested: "skill-name"'))); }); - test('fails if has consecutive hyphens', () async { + test('fails if has consecutive hyphens; suggests collapsed form', () async { final Directory skillDir = await Directory('${tempDir.path}/skill--name').create(); await File( '${skillDir.path}/SKILL.md', @@ -94,10 +105,11 @@ void main() { final validator = Validator(); final ValidationResult result = await validator.validate(skillDir); expect(result.isValid, isFalse); - expect(result.errors, contains(contains('consecutive hyphens'))); + expect(result.errors, contains(contains('"skill--name" has consecutive hyphens'))); + expect(result.errors, contains(contains('Suggested: "skill-name"'))); }); - test('fails if name does not match directory name', () async { + test('mismatched name vs dir: error offers both directions to fix', () async { final Directory skillDir = await Directory('${tempDir.path}/wrong-name').create(); await File( '${skillDir.path}/SKILL.md', @@ -105,7 +117,29 @@ 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 parent directory name'))); + expect( + result.errors, + contains( + contains( + 'Frontmatter `name` "right-name" does not match the parent ' + 'directory name "wrong-name"', + ), + ), + ); + expect(result.errors, contains(contains('setting `name: wrong-name` in SKILL.md'))); + expect( + result.errors, + contains(contains('renaming the directory from "wrong-name" to "right-name"')), + ); + }); + + test('suggestNormalizedName normalizes case, separators, edges, length', () { + expect(NameFormatRule.suggestNormalizedName('My_Cool Skill!'), 'my-cool-skill'); + expect(NameFormatRule.suggestNormalizedName('--leading--double--'), 'leading-double'); + expect( + NameFormatRule.suggestNormalizedName('a' * (NameFormatRule.maxNameLength + 10)), + 'a' * NameFormatRule.maxNameLength, + ); }); test('fixes name to match directory name (not replacing underscores)', () async { @@ -149,14 +183,47 @@ Body'''); expect(result.isValid, isFalse); expect( result.errors, - contains(contains('Maximum ${DescriptionLengthRule.maxDescriptionLength} characters')), + contains(contains('maximum is ${DescriptionLengthRule.maxDescriptionLength}')), ); }); + + test('error message includes char count and |HERE| cutoff excerpt', () async { + // 50 chars before, 50 chars after the cutoff for a distinctive excerpt. + final String before = 'B' * 50; + final String after = 'A' * 50; + final String longDesc = + 'P' * (DescriptionLengthRule.maxDescriptionLength - 50) + before + after; + expect(longDesc.length, DescriptionLengthRule.maxDescriptionLength + 50); + + final Directory skillDir = await Directory('${tempDir.path}/skill-name').create(); + await File( + '${skillDir.path}/SKILL.md', + ).writeAsString('${buildFrontmatter(name: 'skill-name', description: longDesc)}Body'); + final validator = Validator(); + final ValidationResult result = await validator.validate(skillDir); + expect(result.isValid, isFalse); + + final String error = result.errors.firstWhere((e) => e.contains('Description field is')); + expect(error, contains('Description field is ${longDesc.length} characters')); + expect(error, contains('maximum is ${DescriptionLengthRule.maxDescriptionLength}')); + expect( + error, + contains('Cutoff at character ${DescriptionLengthRule.maxDescriptionLength}'), + ); + expect(error, contains('|HERE|')); + // The chars right before/after the cutoff should appear in the excerpt. + expect(error, contains('BBBBB|HERE|AAAAA')); + }); }); group('Compatibility', () { - test('fails if too long (> ${ValidYamlMetadataRule.maxCompatibilityLength} chars)', () async { - final String longComp = 'a' * (ValidYamlMetadataRule.maxCompatibilityLength + 1); + test('fails if too long with shared char-count + |HERE| excerpt shape', () async { + // Put a distinctive run of characters straddling the cutoff so the + // excerpt is visible in the assertion. + final String before = 'B' * 50; + final String after = 'A' * 50; + final String longComp = + 'P' * (ValidYamlMetadataRule.maxCompatibilityLength - 50) + before + after; final Directory skillDir = await Directory('${tempDir.path}/skill-name').create(); await File('${skillDir.path}/SKILL.md').writeAsString(''' --- @@ -168,10 +235,16 @@ Body'''); final validator = Validator(); final ValidationResult result = await validator.validate(skillDir); expect(result.isValid, isFalse); + final String error = result.errors.firstWhere((e) => e.contains('Compatibility field')); + // Same diagnostic shape as description-too-long, generated by the + // shared buildLengthDiagnostic helper. + expect(error, contains('Compatibility field is ${longComp.length} characters')); + expect(error, contains('maximum is ${ValidYamlMetadataRule.maxCompatibilityLength}')); expect( - result.errors, - contains(contains('Maximum ${ValidYamlMetadataRule.maxCompatibilityLength} characters')), + error, + contains('Cutoff at character ${ValidYamlMetadataRule.maxCompatibilityLength}'), ); + expect(error, contains('BBBBB|HERE|AAAAA')); }); }); }); diff --git a/tool/dart_skills_lint/test/recipe_drift_test.dart b/tool/dart_skills_lint/test/recipe_drift_test.dart new file mode 100644 index 0000000..81f4a9c --- /dev/null +++ b/tool/dart_skills_lint/test/recipe_drift_test.dart @@ -0,0 +1,243 @@ +// 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:path/path.dart' as p; +import 'package:test/test.dart'; +import 'package:test_process/test_process.dart'; +import 'package:yaml/yaml.dart'; + +/// Drift guard for the `## Recipes` section of README.md. +/// +/// The README ships copy-pasteable integration recipes. When a flag or +/// command in them goes stale, downstream adopters silently run a +/// broken pipeline. This test reads the README at test time and +/// asserts each recipe is still well-formed. +/// +/// Three checks, deliberately small: +/// 1. The README still has recipe code blocks with non-empty bodies. +/// 2. The GitHub Actions YAML still parses and still wires up the +/// expected setup-dart + install + invocation steps. +/// 3. The pre-commit hook body actually runs end-to-end against the +/// valid and invalid example fixtures and exits with the right code. +/// +/// Everything that used to translate `dart pub global run` lines into +/// `dart bin/cli.dart` lines and replay them is gone — it was fragile +/// and didn't catch anything the structural assertion above doesn't. +void main() { + group('README Recipes drift', () { + late _RecipeReader reader; + final String cliPath = p.normalize(p.absolute('bin/cli.dart')); + final String validFixture = p.normalize(p.absolute('example/valid')); + final String invalidFixture = p.normalize(p.absolute('example/invalid')); + + setUpAll(() { + reader = _RecipeReader.fromFile(p.normalize(p.absolute('README.md'))); + }); + + test('README has all expected recipes with non-empty bodies', () { + expect(reader.yamlBlocks, isNotEmpty, reason: 'GitHub Actions YAML recipe missing'); + expect(reader.shellBlocks, isNotEmpty, reason: 'pre-commit hook shell recipe missing'); + for (final _RecipeBlock block in reader.allBlocks) { + expect(block.body.trim(), isNotEmpty); + } + }); + + test('agent recipe references both setup and validation skills by path', () { + // The "have an agent set it up for you" recipe is plain prose + // inside a blockquote, not a fenced code block, so check the raw + // README text for the skill paths it should point at. + final String readme = File(p.normalize(p.absolute('README.md'))).readAsStringSync(); + final int recipesIdx = readme.indexOf('## Recipes'); + expect(recipesIdx, isNonNegative, reason: 'README has no Recipes section'); + final String recipesSection = readme.substring(recipesIdx); + expect( + recipesSection, + contains('skills/dart-skills-lint-setup/SKILL.md'), + reason: 'agent recipe lost its pointer to the setup skill', + ); + expect( + recipesSection, + contains('skills/dart-skills-lint-validation/SKILL.md'), + reason: 'agent recipe lost its pointer to the validation skill', + ); + }); + + test('GitHub Actions recipe parses and wires up setup-dart + install + invocation', () { + final YamlMap doc = reader.workflowYaml; + expect(doc['name'], 'Lint Agent Skills'); + + final jobs = doc['jobs'] as YamlMap; + expect(jobs.keys, contains('lint-skills')); + final lintJob = jobs['lint-skills'] as YamlMap; + final steps = lintJob['steps'] as YamlList; + + expect(reader.stepsUsing(steps), contains('dart-lang/setup-dart@v1')); + + final List runs = reader.stepsRunning(steps); + expect( + runs.any((r) => r.contains('dart pub global activate dart_skills_lint')), + isTrue, + reason: 'workflow no longer installs dart_skills_lint', + ); + expect( + runs.any( + (r) => + r.contains('dart pub global run dart_skills_lint') && + r.contains('--skills-directory'), + ), + isTrue, + reason: 'workflow no longer runs the linter against a skills directory', + ); + }); + + test('pre-commit hook body exits 0 on a valid fixture, non-zero on an invalid one', () async { + // Run the actual hook (rewritten to call bin/cli.dart instead of a + // globally-activated linter) against both example fixtures. This + // catches drift in the hook's exec line, exit-code propagation, and + // the linter's response to a known-good vs known-bad skill — all in + // one place. + final String hookBody = reader.preCommitHookBody.replaceAll( + 'dart pub global run dart_skills_lint', + 'dart "$cliPath"', + ); + + await _runHookAgainst(hookBody, validFixture, expectZeroExit: true); + await _runHookAgainst(hookBody, invalidFixture, expectZeroExit: false); + }); + }, skip: Platform.isWindows ? 'recipe drift uses POSIX shell' : null); +} + +Future _runHookAgainst( + String hookBody, + String fixturePath, { + required bool expectZeroExit, +}) async { + // The recipe targets a roots-directory (--skills-directory); fixtures + // are individual skills, so swap the flag to --skill and substitute + // the fixture path in for the placeholder ./.claude/skills. + final String runnable = hookBody + .replaceAll('--skills-directory', '--skill') + .replaceAll('./.claude/skills', fixturePath); + + final Directory tmp = await Directory.systemTemp.createTemp('recipe_hook.'); + try { + final hookFile = File(p.join(tmp.path, 'pre-commit')); + await hookFile.writeAsString(runnable); + final ProcessResult chmod = await Process.run('chmod', ['+x', hookFile.path]); + expect(chmod.exitCode, 0); + + final TestProcess process = await TestProcess.start(hookFile.path, const []); + final int exit = await process.exitCode; + if (expectZeroExit) { + expect(exit, 0, reason: 'hook should exit 0 against fixture $fixturePath'); + } else { + expect(exit, isNonZero, reason: 'hook should exit non-zero against fixture $fixturePath'); + } + } finally { + if (tmp.existsSync()) { + await tmp.delete(recursive: true); + } + } +} + +/// Small parser-and-accessor for the recipe section of README.md. The +/// tests above read like a list of assertions; the parsing lives here. +class _RecipeReader { + _RecipeReader._(this.allBlocks); + + factory _RecipeReader.fromFile(String readmePath) { + final String content = File(readmePath).readAsStringSync(); + return _RecipeReader._(_extractBlocks(content)); + } + + final List<_RecipeBlock> allBlocks; + + List<_RecipeBlock> get yamlBlocks => + allBlocks.where((b) => b.language == 'yaml').toList(growable: false); + + List<_RecipeBlock> get shellBlocks => + allBlocks.where((b) => b.language == 'bash').toList(growable: false); + + /// The first YAML block that contains a `jobs:` key — the actual + /// workflow file the recipe documents (vs. small snippet variants). + YamlMap get workflowYaml { + final _RecipeBlock block = yamlBlocks.firstWhere( + (b) => b.body.contains('jobs:'), + orElse: () => fail('no full workflow YAML block found under Recipes'), + ); + final Object? doc = loadYaml(block.body); + expect(doc, isA(), reason: 'workflow YAML failed to parse as a map'); + return doc! as YamlMap; + } + + /// The body between `<<'HOOK'` and `HOOK` markers in the pre-commit + /// shell recipe — the executable hook itself, sans wrapping `cat >` / + /// `chmod +x` plumbing. + String get preCommitHookBody { + final _RecipeBlock block = shellBlocks.firstWhere( + (b) => b.body.contains('.git/hooks/pre-commit') && b.body.contains('HOOK'), + orElse: () => fail('pre-commit HEREDOC recipe missing'), + ); + // Matches a shell HEREDOC of the form + // <<'HOOK' + // ...body lines... + // HOOK + // capturing the body (everything between the opening `<<'HOOK'` + // newline and the closing `HOOK` line, exclusive). dotAll lets `.` + // span newlines so the body matches across lines; the inner `.*?` + // is non-greedy so we stop at the first closing `HOOK`. + final heredoc = RegExp(r"<<'HOOK'\n(.*?)\nHOOK", dotAll: true); + final RegExpMatch? match = heredoc.firstMatch(block.body); + expect(match, isNotNull, reason: 'HEREDOC body could not be parsed'); + return match!.group(1)!; + } + + List stepsUsing(YamlList steps) => steps + .whereType() + .where((s) => s.containsKey('uses')) + .map((s) => s['uses'] as String) + .toList(growable: false); + + List stepsRunning(YamlList steps) => steps + .whereType() + .where((s) => s.containsKey('run')) + .map((s) => s['run'] as String) + .toList(growable: false); + + static List<_RecipeBlock> _extractBlocks(String readme) { + // Matches the README's `## Recipes` heading and captures everything + // from the line after the heading up to (but not including) the + // next `## ` heading. multiLine makes `^` anchor at line starts so + // the lookahead picks up sibling H2 headings; dotAll lets the + // non-greedy body span line breaks. + final section = RegExp(r'^## Recipes\s*\n(.*?)(?=^## )', multiLine: true, dotAll: true); + final RegExpMatch? match = section.firstMatch(readme); + if (match == null) { + return const []; + } + final String body = match.group(1)!; + // Matches a fenced code block of the form + // ``` + // ...body... + // ``` + // capturing the language tag (group 1, may be empty) and the body + // (group 2). The language tag is [a-zA-Z0-9_-]* so we accept + // ```yaml, ```bash, ```dart, etc. multiLine + dotAll let the + // opening/closing backticks anchor to line starts and the inner + // body span newlines. + final fence = RegExp(r'^```([a-zA-Z0-9_-]*)\s*\n(.*?)^```', multiLine: true, dotAll: true); + return [ + for (final RegExpMatch m in fence.allMatches(body)) + _RecipeBlock((m.group(1) ?? '').trim(), m.group(2)!), + ]; + } +} + +class _RecipeBlock { + _RecipeBlock(this.language, this.body); + final String language; + final String body; +} diff --git a/tool/dart_skills_lint/test/relative_paths_test.dart b/tool/dart_skills_lint/test/relative_paths_test.dart index 8b74b45..3225ad8 100644 --- a/tool/dart_skills_lint/test/relative_paths_test.dart +++ b/tool/dart_skills_lint/test/relative_paths_test.dart @@ -8,6 +8,7 @@ import 'package:dart_skills_lint/src/models/analysis_severity.dart'; import 'package:dart_skills_lint/src/rules/absolute_paths_rule.dart'; import 'package:dart_skills_lint/src/rules/relative_paths_rule.dart'; import 'package:dart_skills_lint/src/validator.dart'; +import 'package:path/path.dart' as p; import 'package:test/test.dart'; import 'test_utils.dart'; @@ -45,7 +46,7 @@ void main() { expect(result.warnings, isEmpty); }); - test('warns with missing relative file path', () async { + test('warns with missing relative file path and reports resolved path', () async { final Directory skillDir = await Directory('${tempDir.path}/test-skill').create(); await File('${skillDir.path}/SKILL.md').writeAsString( '${buildFrontmatter(name: 'test-skill')}[Link to a references file missing](references/MISSING.md)\n', @@ -58,6 +59,49 @@ void main() { expect(result.isValid, isTrue); expect(result.warnings, contains(contains('Linked file does not exist'))); + expect(result.warnings, contains(contains('references/MISSING.md'))); + // The diagnostic includes the resolved absolute path. The exact + // shape differs by platform (POSIX `/...` vs Windows `C:\...`), + // so just assert the prefix and that what follows is absolute. + final String warning = result.warnings.firstWhere((w) => w.contains('resolved to ')); + final int prefixIdx = warning.indexOf('resolved to '); + final String resolved = warning.substring(prefixIdx + 'resolved to '.length); + expect(p.isAbsolute(resolved), isTrue, reason: 'resolved path "$resolved" is not absolute'); + }); + + test('did-you-mean: suggests near-miss sibling file when one exists', () async { + final Directory skillDir = await Directory('${tempDir.path}/test-skill').create(); + await File( + '${skillDir.path}/SKILL.md', + ).writeAsString('${buildFrontmatter(name: 'test-skill')}[Link](references/DEATILS.md)\n'); + final Directory refs = await Directory('${skillDir.path}/references').create(); + await File('${refs.path}/DETAILS.md').writeAsString('Details'); + + final validator = Validator( + ruleOverrides: {RelativePathsRule.ruleName: AnalysisSeverity.warning}, + ); + final ValidationResult result = await validator.validate(skillDir); + expect(result.isValid, isTrue); + // Suggestion preserves the link's directory prefix so the user + // gets back a copy-pasteable replacement, not just a basename. + expect(result.warnings, contains(contains('Did you mean "references/DETAILS.md"?'))); + }); + + test('did-you-mean: stays silent when nothing in the sibling dir is close', () async { + final Directory skillDir = await Directory('${tempDir.path}/test-skill').create(); + await File( + '${skillDir.path}/SKILL.md', + ).writeAsString('${buildFrontmatter(name: 'test-skill')}[Link](references/MISSING.md)\n'); + final Directory refs = await Directory('${skillDir.path}/references').create(); + await File('${refs.path}/UNRELATED.txt').writeAsString('Nope'); + + final validator = Validator( + ruleOverrides: {RelativePathsRule.ruleName: AnalysisSeverity.warning}, + ); + final ValidationResult result = await validator.validate(skillDir); + expect(result.isValid, isTrue); + expect(result.warnings, contains(contains('Linked file does not exist'))); + expect(result.warnings.any((w) => w.contains('Did you mean')), isFalse); }); test('fails with absolute file path', () async { diff --git a/tool/dart_skills_lint/test/rules_md_consistency_test.dart b/tool/dart_skills_lint/test/rules_md_consistency_test.dart new file mode 100644 index 0000000..51e0bc0 --- /dev/null +++ b/tool/dart_skills_lint/test/rules_md_consistency_test.dart @@ -0,0 +1,201 @@ +// 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/fixable_rule.dart'; +import 'package:dart_skills_lint/src/models/analysis_severity.dart'; +import 'package:dart_skills_lint/src/models/check_type.dart'; +import 'package:dart_skills_lint/src/models/skill_rule.dart'; +import 'package:dart_skills_lint/src/rule_registry.dart'; +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; + +/// Pins [RULES.md](../RULES.md) to [RuleRegistry] so a rule cannot be +/// added, removed, renamed, or have its default severity / fixability +/// changed without the docs catching up in the same commit. +/// +/// Asserts four invariants between the doc and the registry: +/// 1. Every registered rule has a RULES.md entry (catches missing docs). +/// 2. Every RULES.md entry maps to a registered rule (catches stale +/// docs after a rule is removed or renamed). +/// 3. The documented `Default severity:` value equals the rule's +/// `CheckType.defaultSeverity` (catches silent severity changes +/// that should have been a major version bump per +/// `CONTRIBUTING.md`). +/// 4. The documented `Fixable:` value matches whether the rule's class +/// actually implements `FixableRule`. +/// +/// Each failure prints which rule and which field diverged so the fix +/// is obvious. +void main() { + group('RULES.md consistency', () { + late Map docRules; + late Map registryByName; + + setUpAll(() { + final String rulesPath = p.normalize(p.absolute('RULES.md')); + final String content = File(rulesPath).readAsStringSync(); + docRules = _parseRulesDoc(content); + registryByName = {for (final c in RuleRegistry.allChecks) c.name: c}; + }); + + test('every registered rule has a RULES.md entry', () { + final Set missing = registryByName.keys.toSet()..removeAll(docRules.keys); + expect( + missing, + isEmpty, + reason: + 'RuleRegistry contains rules with no RULES.md entry: $missing. ' + 'Add a `## ` section to RULES.md.', + ); + }); + + test('every RULES.md entry maps to a registered rule', () { + final Set orphans = docRules.keys.toSet()..removeAll(registryByName.keys); + expect( + orphans, + isEmpty, + reason: + 'RULES.md documents rules that are not in RuleRegistry: $orphans. ' + 'Either re-register them or remove the section.', + ); + }); + + test('RULES.md "Default severity:" matches CheckType.defaultSeverity', () { + final List mismatches = []; + for (final MapEntry entry in docRules.entries) { + final String name = entry.key; + final CheckType? check = registryByName[name]; + if (check == null) { + continue; + } + if (entry.value.defaultSeverity != check.defaultSeverity) { + mismatches.add( + '$name: RULES.md says ${entry.value.defaultSeverity.name}, ' + 'registry says ${check.defaultSeverity.name}', + ); + } + } + expect( + mismatches, + isEmpty, + reason: + 'Default severity drifted between RULES.md and RuleRegistry:\n' + ' ${mismatches.join('\n ')}', + ); + }); + + test('RULES.md "Fixable:" matches whether the rule implements FixableRule', () { + final List mismatches = []; + for (final MapEntry entry in docRules.entries) { + final String name = entry.key; + final CheckType? check = registryByName[name]; + if (check == null) { + continue; + } + final SkillRule? rule = RuleRegistry.createRule(name, check.defaultSeverity); + if (rule == null) { + mismatches.add('$name: RuleRegistry.createRule returned null'); + continue; + } + final actuallyFixable = rule is FixableRule; + if (entry.value.fixable != actuallyFixable) { + mismatches.add( + '$name: RULES.md says fixable=${entry.value.fixable}, ' + 'class is FixableRule=$actuallyFixable', + ); + } + } + expect( + mismatches, + isEmpty, + reason: + 'Fixable claim drifted between RULES.md and the rule class:\n' + ' ${mismatches.join('\n ')}', + ); + }); + }); +} + +class _DocRule { + _DocRule({required this.defaultSeverity, required this.fixable}); + + final AnalysisSeverity defaultSeverity; + final bool fixable; +} + +/// Parses every `## ` section in RULES.md and extracts the +/// `Default severity:` and `Fixable:` lines. The format the test +/// enforces: +/// +/// ## +/// +/// - **Default severity:** +/// - **Fixable:** +/// ... +/// +/// Sections whose heading does not look like a kebab-case rule name +/// (e.g. the introductory "Rules" `#` heading) are ignored. +Map _parseRulesDoc(String content) { + // Append a sentinel `## ` heading so the last real section terminates + // cleanly. Dart's RegExp doesn't support `\Z`, and a multiline `$` + // matches every newline, so we avoid both by feeding the parser a + // synthetic trailing heading. + final padded = '$content\n## __end__\n'; + final section = RegExp( + r'^## ([a-z][a-z0-9-_]*)\s*\n(.*?)(?=^## )', + multiLine: true, + dotAll: true, + ); + final Map out = {}; + for (final Match m in section.allMatches(padded)) { + final String name = m.group(1)!; + if (name == '__end__') { + continue; + } + final String body = m.group(2)!; + final AnalysisSeverity? severity = _parseSeverity(body); + final bool? fixable = _parseFixable(body); + if (severity == null || fixable == null) { + throw StateError( + 'RULES.md section "$name" is missing a "**Default severity:**" or ' + '"**Fixable:**" line. Found:\n$body', + ); + } + out[name] = _DocRule(defaultSeverity: severity, fixable: fixable); + } + return out; +} + +AnalysisSeverity? _parseSeverity(String body) { + final r = RegExp(r'\*\*Default severity:\*\*\s+(\w+)'); + final RegExpMatch? m = r.firstMatch(body); + if (m == null) { + return null; + } + final String raw = m.group(1)!.toLowerCase(); + for (final AnalysisSeverity s in AnalysisSeverity.values) { + if (s.name == raw) { + return s; + } + } + return null; +} + +bool? _parseFixable(String body) { + final r = RegExp(r'\*\*Fixable:\*\*\s+(\w+)'); + final RegExpMatch? m = r.firstMatch(body); + if (m == null) { + return null; + } + switch (m.group(1)!.toLowerCase()) { + case 'yes': + return true; + case 'no': + return false; + default: + return null; + } +} diff --git a/tool/dart_skills_lint/test/sibling_suggestion_test.dart b/tool/dart_skills_lint/test/sibling_suggestion_test.dart new file mode 100644 index 0000000..431c49d --- /dev/null +++ b/tool/dart_skills_lint/test/sibling_suggestion_test.dart @@ -0,0 +1,81 @@ +// 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/rules/relative_paths_rule.dart'; +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; + +/// Unit tests for findSiblingSuggestion. The full path-rule integration is +/// covered in relative_paths_test.dart; these tests exercise the +/// suggestion logic directly so failure messages point at the algorithm +/// rather than at the rule plumbing. +void main() { + group('findSiblingSuggestion', () { + late Directory tempDir; + + setUp(() async { + tempDir = await Directory.systemTemp.createTemp('sibling_suggestion_test.'); + }); + + tearDown(() async { + if (tempDir.existsSync()) { + await tempDir.delete(recursive: true); + } + }); + + test('returns just the basename when the link had no directory prefix', () async { + // Missing target: DETAILS.md; actual file on disk: DETAILS.md (typo). + await File(p.join(tempDir.path, 'DETAILS.md')).writeAsString('details'); + // Original link was just `DEATILS.md` — no parent dir to preserve. + final String? result = findSiblingSuggestion( + originalLink: 'DEATILS.md', + resolvedPath: p.join(tempDir.path, 'DEATILS.md'), + ); + expect(result, 'DETAILS.md'); + }); + + test('preserves the directory prefix when the link had one', () async { + final Directory refs = await Directory(p.join(tempDir.path, 'references')).create(); + await File(p.join(refs.path, 'DETAILS.md')).writeAsString('details'); + + // Original link was `references/DEATILS.md` — the suggestion should + // include the same prefix so the user can paste it back verbatim. + final String? result = findSiblingSuggestion( + originalLink: 'references/DEATILS.md', + resolvedPath: p.join(refs.path, 'DEATILS.md'), + ); + expect(result, 'references/DETAILS.md'); + }); + + test('returns null when no candidate is close to the missing basename', () async { + await File(p.join(tempDir.path, 'COMPLETELY_UNRELATED.txt')).writeAsString('nope'); + final String? result = findSiblingSuggestion( + originalLink: 'MISSING.md', + resolvedPath: p.join(tempDir.path, 'MISSING.md'), + ); + expect(result, isNull); + }); + + test('returns null when the parent directory does not exist', () { + final String? result = findSiblingSuggestion( + originalLink: 'nonexistent/X.md', + resolvedPath: p.join(tempDir.path, 'nonexistent', 'X.md'), + ); + expect(result, isNull); + }); + + test('ignores directories — only files are candidates', () async { + // Create a *directory* whose name is close to the missing file's + // basename. It should not be offered as a suggestion. + await Directory(p.join(tempDir.path, 'DETAILS.md')).create(); + final String? result = findSiblingSuggestion( + originalLink: 'DEATILS.md', + resolvedPath: p.join(tempDir.path, 'DEATILS.md'), + ); + expect(result, isNull); + }); + }); +}