feat(validate): add validate command for import map linting#29
Conversation
Assisted-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughA new Cobra-based CLI subcommand Changes
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant Cmd as Validate Cmd
participant FS as File System / Stdin
participant Parser as importmap.Parse
participant Validator as importmap.Validator
participant Output as Formatter
User->>Cmd: mappa validate [file] --format text|json
Cmd->>FS: read file or stdin
FS-->>Cmd: raw JSON
Cmd->>Parser: parse import map
Parser-->>Cmd: import map object
Cmd->>Validator: im.Validate()
Validator-->>Cmd: []ValidationError
Cmd->>Output: format (text -> stderr | json -> stdout)
Output-->>User: formatted results + exit code
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Build Artifacts
Built from cf31f90 @ feat/validate |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
cmd/validate/validate.go (2)
57-84: Validate--formatbefore doing I/O and parsing.The format flag is checked at line 82 after reading the file/stdin and parsing JSON. A user running
mappa validate huge.json --format yamldoes the full read + parse before being told the flag is invalid. Move the flag check to the top ofrunso invalid usage fails fast.♻️ Proposed reorder
func run(cmd *cobra.Command, args []string) error { + format, _ := cmd.Flags().GetString("format") + if format != "text" && format != "json" { + return fmt.Errorf("invalid format %q: must be 'text' or 'json'", format) + } + var data []byte var err error @@ errs := im.Validate() - - format, _ := cmd.Flags().GetString("format") - if format != "text" && format != "json" { - return fmt.Errorf("invalid format %q: must be 'text' or 'json'", format) - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/validate/validate.go` around lines 57 - 84, Move the format validation to the start of run so invalid --format values are rejected before any I/O or parsing; specifically, call cmd.Flags().GetString("format") and validate it ("text" or "json") at the top of run (before the osfs.ReadFile / io.ReadAll and before importmap.Parse and im.Validate) and return the same error message if invalid. This keeps the rest of the logic (file/stdin read, importmap.Parse, im.Validate) unchanged but gated by the early check of format.
98-100: Returning an error for "violations found" double-reports.In text mode each violation is already printed to stderr at line 94, and in JSON mode the structured array is on stdout. Returning
fmt.Errorf("import map has %d validation error(s)", …)here causes cobra to additionally emitError: import map has N validation error(s)to stderr, which in JSON mode is extra noise alongside the machine-readable stdout. Consider usingos.Exit(1)from a small wrapper (orcmd.SilenceErrors = trueplusSilenceUsage = true) so the non-zero exit is signaled without an extra human-readable line muddying the JSON workflow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/validate/validate.go` around lines 98 - 100, The code currently returns fmt.Errorf(...) when validation fails (the len(errs) > 0 branch in validate.go), which causes cobra to print an extra human-readable Error line and pollutes JSON output; change this to signal failure without returning an error: either (a) call os.Exit(1) directly after printing the violations in the same function (keep the existing stderr/stdout prints and then exit), or (b) silence cobra by setting the Cobra command fields SilenceErrors = true and SilenceUsage = true on the command that runs the validate logic so you can continue returning without cobra emitting the additional Error line; update the branch that references errs and fmt.Errorf to use one of these approaches (referencing the validate function/errs variable and fmt.Errorf occurrence to locate the change).main_test.go (2)
855-896: Move inline stdin JSON into fixture files.
TestValidateStdinandTestValidateStdinInvalidhand-roll import-map JSON as raw string literals. The equivalent content already exists (or can trivially be added) undertestdata/validate/...; reuse those fixtures and pipe them viaos.Open/cmd.Stdin = fso stdin and file paths share the same inputs.As per coding guidelines: "Don't inline source code in tests; use fixture files instead with input files and expected output."
♻️ Example refactor
- cmd := exec.Command(binary, "validate") - cmd.Stdin = strings.NewReader(`{ - "imports": { - "lit": "/node_modules/lit/index.js" - } -}`) + f, err := os.Open(filepath.Join("testdata", "validate", "valid", "input.json")) + if err != nil { + t.Fatalf("open fixture: %v", err) + } + defer f.Close() + cmd := exec.Command(binary, "validate") + cmd.Stdin = f🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main_test.go` around lines 855 - 896, Replace the inline JSON string literals in TestValidateStdin and TestValidateStdinInvalid by opening the corresponding fixture files and piping them into the command via cmd.Stdin (use os.Open to get an *os.File, assign it to cmd.Stdin, and defer file.Close()), so both tests reuse the existing test fixtures instead of raw string literals; update the tests to open the appropriate valid/invalid fixture before running exec.Command(..., "validate") and keep the existing stdout/stderr checks unchanged.
898-941: JSON-mode tests rely on exported Go field names; consider tightening.Line 918 asserts
e["Message"] != nil, which matches the currentValidationErrorstruct tags (none — fields serialize with their Go names). If anyone later addsjson:"message"tags (typical Go convention for public JSON output), these tests silently break. Either add explicit JSON tags toValidationErrornow and switch the tests to lowercase keys, or pin the wire format with a golden file undertestdata/validate/invalid/expected.jsonand reusecompareOrUpdateGoldento guard against accidental schema drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main_test.go` around lines 898 - 941, The tests TestValidateFormatJSON and TestValidateFormatJSONValid rely on Go-exported field names (checking e["Message"]) which will break if ValidationError is later tagged with json:"message"; update the wire contract by either (A) adding explicit JSON tags to the ValidationError struct (e.g., `Message` → `json:"message"`) and change the tests to check the lowercase key "message", or (B) pin the output with a golden file for the JSON-mode case (create testdata/validate/invalid/expected.json) and have TestValidateFormatJSON use compareOrUpdateGolden to compare stdout against that golden file instead of indexing map keys; pick one approach and update the referenced symbols (ValidationError, TestValidateFormatJSON, TestValidateFormatJSONValid, and compareOrUpdateGolden) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/validate/validate.go`:
- Around line 32-51: The Cobra command Cmd currently returns errors from run
which causes Cobra to print both the error and full usage; to avoid the noisy
usage dump on validation failures set Cmd.SilenceUsage = true (and optionally
Cmd.SilenceErrors = true) so only the error output is printed; update the Cmd
declaration (the variable named Cmd in this file) to enable these flags so run
can still return fmt.Errorf("import map has %d validation error(s)", ...)
without Cobra dumping usage to stderr.
In `@testdata/validate/invalid/input.json`:
- Around line 4-5: The fixture includes "bare" and "empty" keys that aren't
validated by importmap.Validate()/validateSpecifierMap, so either update the
validation logic or trim the fixture: add URL and empty-value checks inside
importmap.Validate() (the validateSpecifierMap path) to produce ValidationError
for non-absolute/invalid URLs and zero-length specifier values (validate "bare"
and "empty"), or remove those keys from testdata/validate/invalid/input.json so
the fixture only contains cases validateSpecifierMap currently triggers (e.g.,
trailing-slash cases); target the Validate()/validateSpecifierMap functions as
the location to implement the new checks if you choose to extend validation.
---
Nitpick comments:
In `@cmd/validate/validate.go`:
- Around line 57-84: Move the format validation to the start of run so invalid
--format values are rejected before any I/O or parsing; specifically, call
cmd.Flags().GetString("format") and validate it ("text" or "json") at the top of
run (before the osfs.ReadFile / io.ReadAll and before importmap.Parse and
im.Validate) and return the same error message if invalid. This keeps the rest
of the logic (file/stdin read, importmap.Parse, im.Validate) unchanged but gated
by the early check of format.
- Around line 98-100: The code currently returns fmt.Errorf(...) when validation
fails (the len(errs) > 0 branch in validate.go), which causes cobra to print an
extra human-readable Error line and pollutes JSON output; change this to signal
failure without returning an error: either (a) call os.Exit(1) directly after
printing the violations in the same function (keep the existing stderr/stdout
prints and then exit), or (b) silence cobra by setting the Cobra command fields
SilenceErrors = true and SilenceUsage = true on the command that runs the
validate logic so you can continue returning without cobra emitting the
additional Error line; update the branch that references errs and fmt.Errorf to
use one of these approaches (referencing the validate function/errs variable and
fmt.Errorf occurrence to locate the change).
In `@main_test.go`:
- Around line 855-896: Replace the inline JSON string literals in
TestValidateStdin and TestValidateStdinInvalid by opening the corresponding
fixture files and piping them into the command via cmd.Stdin (use os.Open to get
an *os.File, assign it to cmd.Stdin, and defer file.Close()), so both tests
reuse the existing test fixtures instead of raw string literals; update the
tests to open the appropriate valid/invalid fixture before running
exec.Command(..., "validate") and keep the existing stdout/stderr checks
unchanged.
- Around line 898-941: The tests TestValidateFormatJSON and
TestValidateFormatJSONValid rely on Go-exported field names (checking
e["Message"]) which will break if ValidationError is later tagged with
json:"message"; update the wire contract by either (A) adding explicit JSON tags
to the ValidationError struct (e.g., `Message` → `json:"message"`) and change
the tests to check the lowercase key "message", or (B) pin the output with a
golden file for the JSON-mode case (create
testdata/validate/invalid/expected.json) and have TestValidateFormatJSON use
compareOrUpdateGolden to compare stdout against that golden file instead of
indexing map keys; pick one approach and update the referenced symbols
(ValidationError, TestValidateFormatJSON, TestValidateFormatJSONValid, and
compareOrUpdateGolden) accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ffbf397-d9c0-40ac-ace3-e4d0e12b8215
📒 Files selected for processing (5)
cmd/validate/validate.gomain.gomain_test.gotestdata/validate/invalid/input.jsontestdata/validate/valid/input.json
- SilenceUsage on validate command, SilenceErrors only for validation failures (not operational errors like missing files) - Move format validation before I/O - Add json tags to ValidationError for stable wire format - Use fixture files for stdin tests instead of inline JSON - Update JSON field checks from "Message" to "message" Assisted-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
main_test.go (1)
855-898: Optional: extract a stdin helper to reduce duplication.
TestValidateStdinandTestValidateStdinInvalidshare identical setup (build command, open fixture, wire stdin/stdout/stderr buffers, run). Consider a small helper likerunCLIStdin(t, stdinPath string, args ...string) (stdout, stderr string, exitCode int)to mirrorrunCLIand keep these tests focused on assertions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main_test.go` around lines 855 - 898, Tests TestValidateStdin and TestValidateStdinInvalid duplicate CLI setup; extract a helper runCLIStdin(t, stdinPath string, args ...string) (stdout, stderr string, exitCode int) that encapsulates building the binary, opening the fixture file, wiring cmd.Stdin/stdout/stderr, running the command and returning captured outputs and exit code; update both TestValidateStdin and TestValidateStdinInvalid to call runCLIStdin (preserving assertions about exit code and stderr content) and reference the helper name when modifying the tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@main_test.go`:
- Around line 855-898: Tests TestValidateStdin and TestValidateStdinInvalid
duplicate CLI setup; extract a helper runCLIStdin(t, stdinPath string, args
...string) (stdout, stderr string, exitCode int) that encapsulates building the
binary, opening the fixture file, wiring cmd.Stdin/stdout/stderr, running the
command and returning captured outputs and exit code; update both
TestValidateStdin and TestValidateStdinInvalid to call runCLIStdin (preserving
assertions about exit code and stderr content) and reference the helper name
when modifying the tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a9b92995-bb81-4af3-ae92-050b8122a8c5
📒 Files selected for processing (3)
cmd/validate/validate.goimportmap/importmap.gomain_test.go
✅ Files skipped from review due to trivial changes (1)
- importmap/importmap.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/validate/validate.go
Summary
mappa validatecommand that checks import maps against the WHATWG spec--format text(default) and--format jsonfor CITest plan
make lintpassesmake testpasses (324 tests)Summary by CodeRabbit
New Features
validateCLI command to check import map files (optional file argument or stdin). Supports--format/-fastext(errors to stderr) orjson(structured array to stdout). Exits non‑zero when validation errors are present; includes help text.Tests