diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 88e621a..c3d5858 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -29,7 +29,7 @@ "vscode": { "extensions": [ // renovate: datasource=github-tags depName=anthropics/claude-code - "anthropic.claude-code@2.1.206", + "anthropic.claude-code@2.1.219", // renovate: datasource=github-tags depName=editorconfig/editorconfig-vscode "EditorConfig.EditorConfig@0.18.2", // renovate: datasource=github-tags depName=golang/vscode-go diff --git a/README.md b/README.md index 5fcbcc7..0d980d3 100644 --- a/README.md +++ b/README.md @@ -188,6 +188,10 @@ Select a different output format to change this: ::warning file=.devcontainer/devcontainer.json,line=4,col=12,title=no-image-latest::image "ubuntu:latest" uses the "latest" tag; pin a specific version ``` +Every format reports paths relative to the directory `decolint` runs in, +whichever way the linted directory was named on the command line. A file +outside that directory is reported with its absolute path. + ### Exit codes - `0` — no `error`-severity findings (there may still be `warn` diff --git a/cmd/decolint/main.go b/cmd/decolint/main.go index 2654438..4b352ea 100644 --- a/cmd/decolint/main.go +++ b/cmd/decolint/main.go @@ -266,8 +266,21 @@ func runLint(ctx context.Context, stdout, stderr io.Writer, opts Options, cfg Co var allIssues []linter.Issue var worstSeverity linter.Severity var lintErr error - for _, path := range opts.Paths { - issues, err := lintPath(ctx, l, subst, merge, path) + // A directory named more than once is linted once, so its findings are not reported twice. + seen := make(map[absPath]struct{}, len(opts.Paths)) + for _, target := range opts.Paths { + abs, err := filepath.Abs(target) + if err != nil { + lintErr = errors.Join(lintErr, fmt.Errorf("resolve directory %s: %w", target, err)) + continue + } + dir := absPath(abs) + if _, ok := seen[dir]; ok { + continue + } + seen[dir] = struct{}{} + + issues, err := lintPath(ctx, l, subst, merge, dir) for _, issue := range issues { if issue.Severity > worstSeverity { worstSeverity = issue.Severity @@ -297,50 +310,61 @@ type mergeFn func(ctx context.Context, f discovery.ConfigFile, doc *linter.Docum // lintFile. type substituteFn func(workspaceFolder string, doc *linter.Document) -// lintPath lints the devcontainer directory at path. The directory is opened as an os.Root, so -// every file the lint reads is confined to it. It is an error if path is not a directory. -func lintPath(ctx context.Context, l *linter.Linter, subst substituteFn, merge mergeFn, path string) ([]linter.Issue, error) { - root, err := os.OpenRoot(path) +// absPath is the location of a file or directory, always absolute so that it means the same thing +// however the lint target was named on the command line. Printing it does not yield that absolute +// form; see [absPath.String]. +type absPath string + +// String renders p the way findings and error messages name it: relative to the working directory +// when it is inside it, absolute otherwise — a path reached by climbing out of the working directory +// reads no better than its own location. +func (p absPath) String() string { + wd, err := os.Getwd() if err != nil { - return nil, fmt.Errorf("resolve directory %s: %w", path, err) + return string(p) } - // The root is only read from, so a close error is inconsequential. - defer func() { _ = root.Close() }() - issues, err := lintDir(ctx, l, subst, merge, root) + rel, err := filepath.Rel(wd, string(p)) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return string(p) + } + return rel +} + +// lintPath lints the devcontainer directory dir. It is opened as an os.Root, so every file the lint +// reads is confined to it; because dir is absolute, so is every path derived from it. It is an error +// if dir is not a directory. +func lintPath(ctx context.Context, l *linter.Linter, subst substituteFn, merge mergeFn, dir absPath) ([]linter.Issue, error) { + root, err := os.OpenRoot(string(dir)) if err != nil { - return nil, fmt.Errorf("lint directory %s: %w", path, err) + // Pointing decolint straight at a devcontainer.json is a natural mistake, and "not a + // directory" alone does not say what to pass instead. + if info, statErr := os.Stat(string(dir)); statErr == nil && !info.IsDir() { + return nil, fmt.Errorf("%s is not a directory; pass the directory that contains the devcontainer configuration", dir) + } + // os.OpenRoot's error already names the path. + return nil, fmt.Errorf("lint directory: %w", err) } - return issues, nil + // The root is only read from, so a close error is inconsequential. + defer func() { _ = root.Close() }() + return lintDir(ctx, l, subst, merge, root) } // lintDir lints every configuration file in the devcontainer directory root is opened on. It is an -// error if the directory contains no configuration. Issue paths are the files' locations joined -// onto root's name. +// error if the directory contains no configuration. func lintDir(ctx context.Context, l *linter.Linter, subst substituteFn, merge mergeFn, root *os.Root) ([]linter.Issue, error) { - dir := root.Name() + dir := absPath(root.Name()) if err := ctx.Err(); err != nil { return nil, fmt.Errorf("aborted %s: %w", dir, err) } - // The linted directory is the workspace folder the real tooling would mount, even for a - // configuration discovered in a sub-root like .devcontainer. It is only needed for variable - // substitution, so it is resolved only when that runs. - var workspaceFolder string - if subst != nil { - abs, err := filepath.Abs(dir) - if err != nil { - return nil, fmt.Errorf("resolve workspace folder %s: %w", dir, err) - } - workspaceFolder = abs - } var issues []linter.Issue var errs []error found := false err := discovery.VisitConfigs(root, func(f discovery.ConfigFile) error { found = true if err := ctx.Err(); err != nil { - return fmt.Errorf("aborted %s: %w", filepath.Join(f.Root.Name(), f.Path), err) + return fmt.Errorf("aborted %s: %w", configPath(f), err) } - fileIssues, err := lintFile(ctx, l, subst, merge, f, workspaceFolder) + fileIssues, err := lintFile(ctx, l, subst, merge, f, dir) if err != nil { // A broken file must not stop the remaining files from being linted, so record the // error and keep visiting. @@ -359,35 +383,45 @@ func lintDir(ctx context.Context, l *linter.Linter, subst substituteFn, merge me return issues, errors.Join(errs...) } -// lintFile reads and lints the single configuration file f, reporting issues under -// filepath.Join(f.Root.Name(), f.Path). The file is read through f.Root, so its resolution cannot -// escape that boundary. subst, if non-nil, resolves variables in dev container configurations -// before merge and rules run, so both see resolved values. merge, if non-nil, runs on dev container -// configurations before rules and is skipped when no rule applies to f's type, so a file with no -// active rules does no Feature fetches. -func lintFile(ctx context.Context, l *linter.Linter, subst substituteFn, merge mergeFn, f discovery.ConfigFile, workspaceFolder string) ([]linter.Issue, error) { - display := filepath.Join(f.Root.Name(), f.Path) +// configPath returns the location of the configuration file f. It is absolute because f.Root is the +// lint root or a sub-root of it, both opened on an absolute path (see lintPath). +func configPath(f discovery.ConfigFile) absPath { + return absPath(filepath.Join(f.Root.Name(), f.Path)) +} + +// lintFile reads and lints the single configuration file f, found in the devcontainer directory dir. +// The file is read through f.Root, so its resolution cannot escape that boundary. subst, if non-nil, +// resolves variables in dev container configurations before merge and rules run, so both see +// resolved values. merge, if non-nil, runs on dev container configurations before rules and is +// skipped when no rule applies to f's type, so a file with no active rules does no Feature fetches. +func lintFile(ctx context.Context, l *linter.Linter, subst substituteFn, merge mergeFn, f discovery.ConfigFile, dir absPath) ([]linter.Issue, error) { + location := configPath(f) src, err := f.Root.ReadFile(f.Path) if err != nil { - return nil, fmt.Errorf("read config %s: %w", display, err) + return nil, fmt.Errorf("read config %s: %w", location, err) } doc, err := linter.ParseDocument(src) if err != nil { - return nil, fmt.Errorf("%s: %w", display, err) + return nil, fmt.Errorf("%s: %w", location, err) } if subst != nil && f.Type == linter.Devcontainer { - subst(workspaceFolder, doc) + // The linted directory is the workspace folder the real tooling would mount, even for a + // configuration discovered in a sub-root like .devcontainer. + subst(string(dir), doc) } if merge != nil && f.Type == linter.Devcontainer && l.HasRules(f.Type) { if err := merge(ctx, f, doc); err != nil { - return nil, fmt.Errorf("merge features %s: %w", display, err) + return nil, fmt.Errorf("merge features %s: %w", location, err) } } // Give rules read access to the config file's directory, confined to the discovery root so - // resolution cannot escape it (see Context.Dir). - dir, err := fs.Sub(f.Root.FS(), path.Dir(filepath.ToSlash(f.Path))) + // resolution cannot escape it (see linter.Dir). + sub, err := fs.Sub(f.Root.FS(), path.Dir(filepath.ToSlash(f.Path))) if err != nil { - return nil, fmt.Errorf("open config dir %s: %w", display, err) + return nil, fmt.Errorf("open config dir %s: %w", location, err) } - return l.LintDocument(display, f.Type, doc, dir), nil + // The reported path can be relative to the containing directory itself, naming no directory at + // all, so the name comes from the absolute location. + name := filepath.Base(filepath.Dir(string(location))) + return l.LintDocument(location.String(), f.Type, doc, linter.Dir{FS: sub, Name: name}), nil } diff --git a/cmd/decolint/main_test.go b/cmd/decolint/main_test.go index 33e438d..706aa1e 100644 --- a/cmd/decolint/main_test.go +++ b/cmd/decolint/main_test.go @@ -267,6 +267,58 @@ func TestRun(t *testing.T) { } } +// TestRun_ReportedPathsAreWorkingDirectoryRelative checks that findings name files the same way +// however the lint target was named, and that a target outside the working directory is named +// absolutely. +func TestRun_ReportedPathsAreWorkingDirectoryRelative(t *testing.T) { + // Uses t.Chdir, which cannot be combined with t.Parallel. + + const fixture = "testdata/e2e/violations" + abs, err := filepath.Abs(fixture) + if err != nil { + t.Fatalf("Abs: %v", err) + } + reportedPaths := func(t *testing.T, target string) []string { + t.Helper() + var stdout, stderr bytes.Buffer + run(t.Context(), []string{"-format=json", "-platform=codespaces", target}, &stdout, &stderr) + var issues []linter.Issue + if err := json.Unmarshal(stdout.Bytes(), &issues); err != nil { + t.Fatalf("output is not a JSON issue array: %v\noutput: %s", err, stdout.String()) + } + if len(issues) == 0 { + t.Fatalf("no findings for %s; the fixture is expected to trip codespaces rules", target) + } + var paths []string + for _, issue := range issues { + paths = append(paths, issue.Path) + } + return paths + } + + t.Run("a target inside the working directory is named relative to it", func(t *testing.T) { + want := reportedPaths(t, fixture) + if diff := cmp.Diff(want, reportedPaths(t, abs)); diff != "" { + t.Errorf("paths from the absolute target differ from the relative one (-relative +absolute):\n%s", diff) + } + for _, p := range want { + if !strings.HasPrefix(p, fixture) { + t.Errorf("path = %q, want it under %q", p, fixture) + } + } + }) + + t.Run("a target outside the working directory is named absolutely", func(t *testing.T) { + // Uses t.Chdir, which cannot be combined with t.Parallel. + t.Chdir(t.TempDir()) + for _, p := range reportedPaths(t, abs) { + if !filepath.IsAbs(p) { + t.Errorf("path = %q, want an absolute path", p) + } + } + }) +} + func TestRun_Flags(t *testing.T) { t.Parallel() @@ -704,6 +756,61 @@ func TestRun_Init(t *testing.T) { }) } +func TestRunLint_DeduplicatesTargets(t *testing.T) { + t.Parallel() + + dir := writeDevcontainer(t, `{"image": "ubuntu:latest"}`) + cfg := Config{ + Format: "json", + Rules: map[string]linter.Severity{"no-image-latest": linter.SeverityError}, + } + + lint := func(t *testing.T, paths []string) int { + t.Helper() + var stdout bytes.Buffer + if _, err := runLint(t.Context(), &stdout, io.Discard, Options{Paths: paths}, cfg); err != nil { + t.Fatalf("runLint: %v", err) + } + var issues []linter.Issue + if err := json.Unmarshal(stdout.Bytes(), &issues); err != nil { + t.Fatalf("output is not a JSON issue array: %v\noutput: %s", err, stdout.String()) + } + return len(issues) + } + + want := lint(t, []string{dir}) + if want == 0 { + t.Fatal("no findings for the fixture; the test cannot tell deduplication from a silent lint") + } + if got := lint(t, []string{dir, dir + string(filepath.Separator)}); got != want { + t.Errorf("findings for the directory named twice = %d, want %d", got, want) + } +} + +func TestRunLint_UnresolvableTarget(t *testing.T) { + // Uses t.Chdir, which cannot be combined with t.Parallel. + + // A target whose location cannot be resolved is recorded like any other per-target failure, so + // the findings collected so far are still written. + dir := t.TempDir() + t.Chdir(dir) + if err := os.Remove(dir); err != nil { + t.Fatal(err) + } + + var stdout bytes.Buffer + hasIssue, err := runLint(t.Context(), &stdout, io.Discard, Options{Paths: []string{"."}}, Config{Format: "json"}) + if err == nil || !strings.Contains(err.Error(), "resolve directory") { + t.Errorf("err = %v, want a directory resolution error", err) + } + if hasIssue { + t.Error("hasIssue = true, want false") + } + if got := strings.TrimSpace(stdout.String()); got != "[]" { + t.Errorf("stdout = %q, want an empty JSON issue array", got) + } +} + func TestRunLint(t *testing.T) { t.Parallel() @@ -1270,7 +1377,7 @@ func TestLintPath(t *testing.T) { t.Fatal(err) } - issues, err := lintPath(t.Context(), newLinter(), noSubst, nil, dir) + issues, err := lintPath(t.Context(), newLinter(), noSubst, nil, absPath(dir)) if err != nil { t.Fatalf("lintPath: %v", err) } @@ -1317,19 +1424,28 @@ func TestLintPath(t *testing.T) { t.Run("directory without config", func(t *testing.T) { t.Parallel() - _, err := lintPath(t.Context(), newLinter(), noSubst, nil, t.TempDir()) + _, err := lintPath(t.Context(), newLinter(), noSubst, nil, absPath(t.TempDir())) if err == nil || !strings.Contains(err.Error(), "no devcontainer configuration found") { t.Errorf("err = %v, want 'no devcontainer configuration found'", err) } }) + t.Run("a configuration file is not a directory", func(t *testing.T) { + t.Parallel() + file := filepath.Join(writeDevcontainer(t, body), ".devcontainer", "devcontainer.json") + _, err := lintPath(t.Context(), newLinter(), noSubst, nil, absPath(file)) + if err == nil || !strings.Contains(err.Error(), "is not a directory; pass the directory") { + t.Errorf("err = %v, want the error to say a directory is expected", err) + } + }) + t.Run("merge error aborts the file", func(t *testing.T) { t.Parallel() dir := writeDevcontainer(t, body) wantErr := errors.New("fetch failed") merge := func(context.Context, discovery.ConfigFile, *linter.Document) error { return wantErr } - if _, err := lintPath(t.Context(), newLinter(), noSubst, merge, dir); !errors.Is(err, wantErr) { + if _, err := lintPath(t.Context(), newLinter(), noSubst, merge, absPath(dir)); !errors.Is(err, wantErr) { t.Errorf("lintPath error = %v, want %v", err, wantErr) } }) @@ -1343,7 +1459,7 @@ func TestLintPath(t *testing.T) { return nil } - if _, err := lintPath(t.Context(), linter.New(), noSubst, merge, dir); err != nil { + if _, err := lintPath(t.Context(), linter.New(), noSubst, merge, absPath(dir)); err != nil { t.Fatalf("lintPath: %v", err) } if called { @@ -1373,7 +1489,7 @@ func TestLintPath(t *testing.T) { } subst := func(string, *linter.Document) {} - if _, err := lintPath(t.Context(), l, subst, merge, dir); err != nil { + if _, err := lintPath(t.Context(), l, subst, merge, absPath(dir)); err != nil { t.Fatalf("lintPath: %v", err) } if called { @@ -1382,26 +1498,120 @@ func TestLintPath(t *testing.T) { }) } -func TestLintDir_WorkspaceFolderError(t *testing.T) { +// dirNameRule is a stub rule that reports the name of the directory the linted file sits in, used to +// observe the [linter.Dir] a lint hands to rules without depending on a rule that reads it. +var dirNameRule = &linter.Rule{ + ID: "test-dir-name", + Description: "reports the containing directory's name", + FileTypes: []linter.FileType{linter.Feature}, + Paths: []string{"/id"}, + Check: func(ctx *linter.Context, node *linter.Node) []linter.Finding { + return []linter.Finding{{Message: ctx.Dir.Name, Offset: node.Value.StartOffset}} + }, +} + +// TestLintPath_DirName checks that rules are handed the name of the directory the configuration file +// sits in wherever the lint runs from, while the path the finding is reported at follows the working +// directory. Linting a directory from inside it leaves that path with no directory component, so the +// name cannot be taken from there. +func TestLintPath_DirName(t *testing.T) { // Uses t.Chdir, which cannot be combined with t.Parallel. - // A relative root name resolves against the working directory; deleting it makes the - // workspace folder unresolvable, which must surface as an error. - dir := t.TempDir() - t.Chdir(dir) - root, err := os.OpenRoot(".") + parent := t.TempDir() + // t.TempDir can hand back a path through a symlink (/var on macOS), which is not the path the + // process reports as its working directory; ask for the one findings are reported against. + t.Chdir(parent) + parent, err := os.Getwd() if err != nil { + t.Fatalf("Getwd: %v", err) + } + target := filepath.Join(parent, "my-feature") + sibling := filepath.Join(parent, "elsewhere") + for _, dir := range []string{target, sibling} { + if err := os.Mkdir(dir, 0o755); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(target, "devcontainer-feature.json"), []byte(`{"id": "my-feature"}`), 0o644); err != nil { t.Fatal(err) } - defer func() { _ = root.Close() }() + + tests := []struct { + name string + wd string + wantPath string + }{ + {"from the directory itself", target, "devcontainer-feature.json"}, + {"from its parent", parent, filepath.Join("my-feature", "devcontainer-feature.json")}, + {"from outside", sibling, filepath.Join(target, "devcontainer-feature.json")}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Chdir(tt.wd) + l := linter.New() + l.RegisterRule(dirNameRule, linter.SeverityWarn) + + issues, err := lintPath(t.Context(), l, nil, nil, absPath(target)) + if err != nil { + t.Fatalf("lintPath: %v", err) + } + if len(issues) != 1 { + t.Fatalf("got %d issues %v, want 1", len(issues), issues) + } + if issues[0].Message != "my-feature" { + t.Errorf("ctx.Dir.Name = %q, want %q", issues[0].Message, "my-feature") + } + if issues[0].Path != tt.wantPath { + t.Errorf("Path = %q, want %q", issues[0].Path, tt.wantPath) + } + }) + } +} + +func TestAbsPathString(t *testing.T) { + // Uses t.Chdir, which cannot be combined with t.Parallel. + + wd := t.TempDir() + // t.TempDir can hand back a path through a symlink (/var on macOS), which is not the path the + // process reports as its working directory; ask for the one absPath renders against. + t.Chdir(wd) + wd, err := os.Getwd() + if err != nil { + t.Fatalf("Getwd: %v", err) + } + + tests := []struct { + name string + path absPath + want string + }{ + {"inside the working directory", absPath(filepath.Join(wd, ".devcontainer", "devcontainer.json")), filepath.Join(".devcontainer", "devcontainer.json")}, + {"the working directory itself", absPath(wd), "."}, + {"the parent of the working directory", absPath(filepath.Dir(wd)), filepath.Dir(wd)}, + {"a sibling of the working directory", absPath(filepath.Join(filepath.Dir(wd), "elsewhere")), filepath.Join(filepath.Dir(wd), "elsewhere")}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.path.String(); got != tt.want { + t.Errorf("absPath(%q).String() = %q, want %q", string(tt.path), got, tt.want) + } + }) + } +} + +func TestAbsPathString_NoWorkingDirectory(t *testing.T) { + // Uses t.Chdir, which cannot be combined with t.Parallel. + + // With the working directory gone there is nothing to render against, so the path stays absolute. + dir := t.TempDir() + t.Chdir(dir) if err := os.Remove(dir); err != nil { t.Fatal(err) } - subst := func(string, *linter.Document) {} - if _, err := lintDir(t.Context(), linter.New(), subst, nil, root); err == nil || - !strings.Contains(err.Error(), "resolve workspace folder") { - t.Errorf("err = %v, want a workspace folder resolution error", err) + p := absPath(filepath.Join(dir, "devcontainer.json")) + if got := p.String(); got != string(p) { + t.Errorf("absPath(%q).String() = %q, want it unchanged", string(p), got) } } diff --git a/cmd/decolint/opts.go b/cmd/decolint/opts.go index 2486333..d6e75d3 100644 --- a/cmd/decolint/opts.go +++ b/cmd/decolint/opts.go @@ -12,7 +12,7 @@ import ( // Options holds the parsed command-line arguments. It is purely the CLI's view of the world; see // Config for the on-disk config file's shape, and mergeConfig for how the two are reconciled. type Options struct { - // Paths are the directories to lint. + // Paths are the directories to lint, as named on the command line; runLint resolves them. Paths []string // DenyWarnings mirrors [Config.DenyWarnings]. When -deny-warnings is explicitly given it takes // precedence over the config file's "denyWarnings" member, in either direction (see diff --git a/cmd/decolint/opts_test.go b/cmd/decolint/opts_test.go index f3f6c53..d9c48d8 100644 --- a/cmd/decolint/opts_test.go +++ b/cmd/decolint/opts_test.go @@ -224,11 +224,26 @@ func TestParseOptions_Config(t *testing.T) { func TestParseOptions_Paths(t *testing.T) { t.Parallel() - opts, err := parseOptions(nil, io.Discard) - if err != nil { - t.Fatalf("parseOptions: %v", err) + // Arguments are taken as given; resolving and deduplicating them is runLint's job (see + // TestRunLint_DeduplicatesTargets). + tests := []struct { + name string + args []string + want []string + }{ + {"no arguments lint the working directory", nil, []string{"."}}, + {"arguments are kept as named", []string{".", "/work/repo", "."}, []string{".", "/work/repo", "."}}, } - if diff := cmp.Diff([]string{"."}, opts.Paths); diff != "" { - t.Errorf("Paths mismatch (-want +got):\n%s", diff) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + opts, err := parseOptions(tt.args, io.Discard) + if err != nil { + t.Fatalf("parseOptions: %v", err) + } + if diff := cmp.Diff(tt.want, opts.Paths); diff != "" { + t.Errorf("Paths mismatch (-want +got):\n%s", diff) + } + }) } } diff --git a/feature/compose.go b/feature/compose.go index 5802f54..6a237a7 100644 --- a/feature/compose.go +++ b/feature/compose.go @@ -43,8 +43,12 @@ func composeContributors(ctx context.Context, f *Fetcher, fsRoot *os.Root, confi return nil, true, nil } // The Compose files resolve relative to the referencing devcontainer.json's directory on the real - // filesystem, as "docker compose" itself would. - baseDir := filepath.Join(fsRoot.Name(), configDir) + // filesystem, as "docker compose" itself would. The directory is made absolute so that compose-go + // resolves the build context to an absolute path too, which composeDisplayRef relies on. + baseDir, err := filepath.Abs(filepath.Join(fsRoot.Name(), configDir)) + if err != nil { + return nil, true, fmt.Errorf("resolve %s: %w", configDir, err) + } svc, err := loadComposeService(ctx, baseDir, paths, service, localEnv) if err != nil { return nil, true, err @@ -225,6 +229,10 @@ func composeBuildMetadata(ctx context.Context, f *Fetcher, baseDir, firstCompose // composeDisplayRef renders a Dockerfile path for display: relative to the configuration directory // when it lies within it, and absolute otherwise (a build context reached through "extends" or a // "../" Compose file may sit outside it). +// +// It is deliberately not relative to the working directory: the reference identifies a contributor +// within the merged configuration (see [contributor.displayID]), so it must not depend on where +// decolint ran. func composeDisplayRef(baseDir, path string) string { if rel, err := filepath.Rel(baseDir, path); err == nil && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) { return rel diff --git a/feature/compose_test.go b/feature/compose_test.go index d742794..eea8654 100644 --- a/feature/compose_test.go +++ b/feature/compose_test.go @@ -334,6 +334,42 @@ func TestMerge_ComposeFileOutsideConfigDir(t *testing.T) { }) } +// TestMerge_ComposeDockerfileRefIsRootIndependent checks that a Dockerfile reached through a compose +// build is named the same way however the lint root was named. The reference appears in the error a +// broken Dockerfile produces, and reaches the merged configuration as a contributor's display id. +func TestMerge_ComposeDockerfileRefIsRootIndependent(t *testing.T) { + // Uses t.Chdir, which cannot be combined with t.Parallel. + + repo := t.TempDir() + writeFiles(t, repo, map[string]string{ + ".devcontainer/devcontainer.json": `{"dockerComposeFile": "docker-compose.yml", "service": "app"}`, + // The build context sits outside the .devcontainer directory, so the reference must name the + // Dockerfile's own location rather than a name relative to the configuration directory. + ".devcontainer/docker-compose.yml": "services:\n app:\n build:\n context: ..\n", + "Dockerfile": "FROM scratch\nRUN --bogus=1 x\n", + }) + configDir := filepath.Join(repo, ".devcontainer") + want := filepath.Join(repo, "Dockerfile") + + // The lint root is named relative to the working directory in one run and absolutely in the other. + t.Chdir(repo) + for _, dir := range []string{".devcontainer", configDir} { + t.Run(dir, func(t *testing.T) { + root, err := hujson.Parse([]byte(`{"dockerComposeFile": "docker-compose.yml", "service": "app"}`)) + if err != nil { + t.Fatalf("parse devcontainer.json: %v", err) + } + err = Merge(t.Context(), NewFetcher(), openRoot(t, dir), ".", nil, &root) + if err == nil { + t.Fatal("Merge: got nil error, want the Dockerfile's parse error") + } + if !strings.Contains(err.Error(), want) { + t.Errorf("Merge error = %v, want it to name the Dockerfile as %q", err, want) + } + }) + } +} + // mergeOutsideDir parses src as the .devcontainer/devcontainer.json of repo and merges it with the // root confined to that .devcontainer directory, the confinement discovery applies. func mergeOutsideDir(t *testing.T, repo, src string) *hujson.Value { diff --git a/format/github.go b/format/github.go index 07ceb78..910b2c7 100644 --- a/format/github.go +++ b/format/github.go @@ -3,6 +3,7 @@ package format import ( "fmt" "io" + "path/filepath" "strings" "github.com/bare-devcontainer/decolint/linter" @@ -11,7 +12,8 @@ import ( // GitHubFormat prints one GitHub Actions workflow command (::error/::warning) per issue. type GitHubFormat struct{} -// WriteIssues writes issues to w as GitHub Actions workflow commands. +// WriteIssues writes issues to w as GitHub Actions workflow commands. Paths are written with "/" +// separators, which is what GitHub matches an annotation against. func (GitHubFormat) WriteIssues(w io.Writer, issues []linter.Issue) error { for _, issue := range issues { command := "error" @@ -22,7 +24,7 @@ func (GitHubFormat) WriteIssues(w io.Writer, issues []linter.Issue) error { w, "::%s file=%s,line=%d,col=%d,title=%s::%s\n", command, - escapeGitHubProperty(issue.Path), + escapeGitHubProperty(filepath.ToSlash(issue.Path)), issue.Line, issue.Col, escapeGitHubProperty(issue.RuleID), diff --git a/format/github_test.go b/format/github_test.go index 8ff67d3..b33586c 100644 --- a/format/github_test.go +++ b/format/github_test.go @@ -2,6 +2,7 @@ package format import ( "errors" + "path/filepath" "strings" "testing" ) @@ -22,6 +23,26 @@ func TestGitHubWriteIssues(t *testing.T) { } } +// TestGitHubWriteIssues_PathSeparators checks that a path built with the host's separators is +// written with "/" ones, which is what GitHub matches an annotation against. +func TestGitHubWriteIssues_PathSeparators(t *testing.T) { + t.Parallel() + + issues := testIssues()[:1] + issues[0].Path = filepath.Join(".devcontainer", "go", "devcontainer.json") + + var sb strings.Builder + if err := (GitHubFormat{}).WriteIssues(&sb, issues); err != nil { + t.Fatalf("WriteIssues: %v", err) + } + + want := `::warning file=.devcontainer/go/devcontainer.json,line=4,col=12,title=no-image-latest::image "ubuntu:latest" uses the "latest" tag; pin a specific version +` + if sb.String() != want { + t.Errorf("WriteIssues github = %q, want %q", sb.String(), want) + } +} + func TestGitHubWriteIssues_WriteError(t *testing.T) { t.Parallel() diff --git a/linter/linter.go b/linter/linter.go index 4040869..0b89e98 100644 --- a/linter/linter.go +++ b/linter/linter.go @@ -6,7 +6,6 @@ package linter import ( "fmt" - "io/fs" "sort" ) @@ -55,11 +54,11 @@ func (l *Linter) HasRules(t FileType) bool { } // LintDocument applies the linter's rules to doc, a configuration file of the given type, and -// returns the findings sorted by position. path is used only for reporting. dir, if non-nil, gives -// rules read access to the directory containing the file (see Context.Dir); it may be nil for an -// in-memory document. It reads the document as given; any mutation of its tree (see Document.Tree) -// must happen before calling it. -func (l *Linter) LintDocument(path string, fileType FileType, doc *Document, dir fs.FS) []Issue { +// returns the findings sorted by position. path is used only for reporting, and dir describes the +// directory containing the file; both are handed to the rules as [Context.Path] and [Context.Dir], +// and dir may be left zero for an in-memory document. It reads the document as given; any mutation +// of its tree (see Document.Tree) must happen before calling it. +func (l *Linter) LintDocument(path string, fileType FileType, doc *Document, dir Dir) []Issue { patterns := l.patterns[fileType] if len(patterns) == 0 { return nil diff --git a/linter/linter_test.go b/linter/linter_test.go index 9d1941b..7647a57 100644 --- a/linter/linter_test.go +++ b/linter/linter_test.go @@ -2,7 +2,6 @@ package linter import ( "fmt" - "io/fs" "slices" "strings" "testing" @@ -46,7 +45,7 @@ func lintSource(t *testing.T, l *Linter, path string, fileType FileType, src str if err != nil { t.Fatalf("ParseDocument: %v", err) } - return l.LintDocument(path, fileType, doc, nil) + return l.LintDocument(path, fileType, doc, Dir{}) } func TestLintDocument_Position(t *testing.T) { @@ -109,7 +108,7 @@ func TestLintDocument_TreeMutation(t *testing.T) { l := New() l.RegisterRule(flagRule, SeverityWarn) - issues := l.LintDocument("devcontainer.json", Devcontainer, doc, nil) + issues := l.LintDocument("devcontainer.json", Devcontainer, doc, Dir{}) if len(issues) != 1 { t.Fatalf("got %d issues %v, want 1", len(issues), issues) } @@ -122,8 +121,8 @@ func TestLintDocument_ContextDir(t *testing.T) { t.Parallel() // A stub rule records the Dir it is handed so we can assert LintDocument passes it through - // unchanged, including the nil case for an in-memory document. - var got fs.FS + // unchanged, including the zero case for an in-memory document. + var got Dir dirRule := &Rule{ ID: "dir-rule", FileTypes: []FileType{Devcontainer}, @@ -134,13 +133,13 @@ func TestLintDocument_ContextDir(t *testing.T) { }, } - want := fstest.MapFS{"install.sh": {Data: []byte("#!/bin/sh\n")}} + want := Dir{FS: fstest.MapFS{"install.sh": {Data: []byte("#!/bin/sh\n")}}, Name: "my-feature"} for _, tt := range []struct { name string - dir fs.FS + dir Dir }{ {"with directory", want}, - {"nil directory", nil}, + {"zero directory", Dir{}}, } { t.Run(tt.name, func(t *testing.T) { doc, err := ParseDocument([]byte(`{"name": "test"}`)) @@ -149,13 +148,13 @@ func TestLintDocument_ContextDir(t *testing.T) { } l := New() l.RegisterRule(dirRule, SeverityWarn) - got = nil + got = Dir{} l.LintDocument("devcontainer.json", Devcontainer, doc, tt.dir) - if got == nil && tt.dir != nil { - t.Fatalf("ctx.Dir = nil, want the passed filesystem") + if got.Name != tt.dir.Name { + t.Errorf("ctx.Dir.Name = %q, want %q", got.Name, tt.dir.Name) } - if tt.dir == nil && got != nil { - t.Fatalf("ctx.Dir = %v, want nil", got) + if (got.FS == nil) != (tt.dir.FS == nil) { + t.Errorf("ctx.Dir.FS = %v, want %v", got.FS, tt.dir.FS) } }) } diff --git a/linter/rule.go b/linter/rule.go index 4747c29..a076d1f 100644 --- a/linter/rule.go +++ b/linter/rule.go @@ -138,17 +138,28 @@ func ParseCategory(name string) (Category, error) { // Context carries everything a rule needs to inspect a single configuration file. type Context struct { - // Path is the path of the file being linted. + // Path is the path of the file being linted, for reporting only: it is spelled however the + // caller named the file, so a rule must not read anything into its shape. Use [Dir] for the + // containing directory. Path string // Type is the kind of configuration file being linted. Type FileType // Root is the HuJSON syntax tree of the file. It preserves comments and byte offsets into the // original source. Root *hujson.Value - // Dir gives read access to the directory containing the file being linted, confined to the lint - // root. It is nil when the caller has no backing filesystem (e.g. an in-memory document); a rule - // that inspects sibling files must return no findings when it is nil. - Dir fs.FS + // Dir describes the directory containing the file being linted. + Dir Dir +} + +// Dir is the directory a configuration file is linted in. Each field is independently optional, and +// a rule that needs one must return no findings when it is unset. +type Dir struct { + // FS gives read access to the directory's contents, confined to the lint root. It is nil when + // the caller has no backing filesystem, e.g. an in-memory document. + FS fs.FS + // Name is the directory's own name, taken from where the directory actually is rather than from + // [Context.Path]. It is empty when the caller has no directory to name. + Name string } // Severity indicates how a finding should be treated: whether it's reported as an error or a diff --git a/rules/category_test.go b/rules/category_test.go index 12c2fbe..18bd5f3 100644 --- a/rules/category_test.go +++ b/rules/category_test.go @@ -121,7 +121,7 @@ func TestRegisterRules_CategoryOverride(t *testing.T) { if err := rules.RegisterRules(l, nil, tt.overrides); err != nil { t.Fatalf("RegisterRules: %v", err) } - issues := lintSource(t, l, "devcontainer.json", linter.Devcontainer, src, nil) + issues := lintSource(t, l, "devcontainer.json", linter.Devcontainer, src, linter.Dir{}) if fired := ruleFired(issues, "no-seccomp-override"); fired != tt.wantFired { t.Errorf("no-seccomp-override fired = %v, want %v (issues: %v)", fired, tt.wantFired, issues) } diff --git a/rules/feature_install_script_not_executable.go b/rules/feature_install_script_not_executable.go index 2055e89..9ba8c4b 100644 --- a/rules/feature_install_script_not_executable.go +++ b/rules/feature_install_script_not_executable.go @@ -23,10 +23,10 @@ var FeatureInstallScriptNotExecutable = &linter.Rule{ func checkFeatureInstallScriptNotExecutable(ctx *linter.Context, node *linter.Node) []linter.Finding { // Windows working trees carry no executable bits (git does not set them there), so the check // would report every install.sh as non-executable. Skip it rather than emit false positives. - if ctx.Dir == nil || runtime.GOOS == "windows" { + if ctx.Dir.FS == nil || runtime.GOOS == "windows" { return nil } - info, err := fs.Stat(ctx.Dir, installScriptName) + info, err := fs.Stat(ctx.Dir.FS, installScriptName) if err != nil || info.IsDir() { return nil } diff --git a/rules/feature_install_script_not_executable_test.go b/rules/feature_install_script_not_executable_test.go index 2aa844a..6f4367c 100644 --- a/rules/feature_install_script_not_executable_test.go +++ b/rules/feature_install_script_not_executable_test.go @@ -35,7 +35,7 @@ func TestFeatureInstallScriptNotExecutable(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - assertIssuesInDir(t, rules.FeatureInstallScriptNotExecutable, linter.SeverityError, path, linter.Feature, src, tt.dir, tt.want) + assertIssuesInDir(t, rules.FeatureInstallScriptNotExecutable, linter.SeverityError, path, linter.Feature, src, linter.Dir{FS: tt.dir}, tt.want) }) } diff --git a/rules/helpers_test.go b/rules/helpers_test.go index abb71bc..3c0f6f8 100644 --- a/rules/helpers_test.go +++ b/rules/helpers_test.go @@ -20,9 +20,8 @@ var errUnreadable = errors.New("unreadable filesystem") // lintSource parses src and applies l's registered rules to it as a file at the given path and of // the given type, failing the test on any error. dir is the directory the file is linted in (see -// [linter.Context.Dir]); it is nil for the in-memory documents most rule tests use in place of a -// real directory. -func lintSource(t *testing.T, l *linter.Linter, path string, fileType linter.FileType, src string, dir fs.FS) []linter.Issue { +// [linter.Dir]). +func lintSource(t *testing.T, l *linter.Linter, path string, fileType linter.FileType, src string, dir linter.Dir) []linter.Issue { t.Helper() doc, err := linter.ParseDocument([]byte(src)) if err != nil { @@ -42,16 +41,16 @@ func assertIssues(t *testing.T, r *linter.Rule, severity linter.Severity, src st // assertIssuesAt is like assertIssues but lets the caller control the path and file type src is // linted as, e.g. to exercise rules that apply only to Features or Templates. The linted file has -// no backing directory; use assertIssuesInDir for rules that inspect sibling files. +// no backing directory; use assertIssuesInDir for rules that inspect the directory they are in. func assertIssuesAt(t *testing.T, r *linter.Rule, severity linter.Severity, path string, fileType linter.FileType, src string, want []linter.Issue) { t.Helper() - assertIssuesInDir(t, r, severity, path, fileType, src, nil, want) + assertIssuesInDir(t, r, severity, path, fileType, src, linter.Dir{}, want) } // assertIssuesInDir is like assertIssuesAt but lets the caller supply the directory the file is -// linted in (see [linter.Context.Dir]), so rules that inspect sibling files can be exercised -// against a fake filesystem (e.g. a [testing/fstest.MapFS]). -func assertIssuesInDir(t *testing.T, r *linter.Rule, severity linter.Severity, path string, fileType linter.FileType, src string, dir fs.FS, want []linter.Issue) { +// linted in (see [linter.Dir]): a fake filesystem (e.g. a [testing/fstest.MapFS]) for rules that +// inspect sibling files, and a directory name for rules that read it. +func assertIssuesInDir(t *testing.T, r *linter.Rule, severity linter.Severity, path string, fileType linter.FileType, src string, dir linter.Dir, want []linter.Issue) { t.Helper() l := linter.New() diff --git a/rules/id_dir_mismatch.go b/rules/id_dir_mismatch.go index bce428a..9256d12 100644 --- a/rules/id_dir_mismatch.go +++ b/rules/id_dir_mismatch.go @@ -2,7 +2,6 @@ package rules import ( "fmt" - "path/filepath" "github.com/bare-devcontainer/decolint/linter" "github.com/tailscale/hujson" @@ -24,13 +23,15 @@ func checkIDDirMismatch(ctx *linter.Context, node *linter.Node) []linter.Finding if !ok || lit.Kind() != '"' { return nil } + if ctx.Dir.Name == "" { + return nil + } id := lit.String() - dir := filepath.Base(filepath.Dir(ctx.Path)) - if id == dir { + if id == ctx.Dir.Name { return nil } return []linter.Finding{{ - Message: fmt.Sprintf("id %q does not match containing directory %q", id, dir), + Message: fmt.Sprintf("id %q does not match containing directory %q", id, ctx.Dir.Name), Offset: node.Value.StartOffset, }} } diff --git a/rules/id_dir_mismatch_test.go b/rules/id_dir_mismatch_test.go index c5fc2ac..b2d53cd 100644 --- a/rules/id_dir_mismatch_test.go +++ b/rules/id_dir_mismatch_test.go @@ -10,38 +10,50 @@ import ( func TestIDDirMismatch(t *testing.T) { t.Parallel() + // The reported path and the directory's name are supplied separately, as the linter supplies + // them: the path is only how the file is named in output (see [linter.Context.Path]), so the + // cases below vary the two independently. + const featurePath = "my-feature/devcontainer-feature.json" + const templatePath = "my-template/devcontainer-template.json" + tests := []struct { name string path string + dir string fileType linter.FileType src string want []linter.Issue }{ - {"no id property", "my-feature/devcontainer-feature.json", linter.Feature, `{"name": "test"}`, nil}, - {"feature matching id", "my-feature/devcontainer-feature.json", linter.Feature, `{"id": "my-feature"}`, nil}, - {"feature mismatched id", "my-feature/devcontainer-feature.json", linter.Feature, `{"id": "other"}`, []linter.Issue{ - {Path: "my-feature/devcontainer-feature.json", Line: 1, Col: 8, RuleID: "id-dir-mismatch", Message: `id "other" does not match containing directory "my-feature"`}, + {"no id property", featurePath, "my-feature", linter.Feature, `{"name": "test"}`, nil}, + {"feature matching id", featurePath, "my-feature", linter.Feature, `{"id": "my-feature"}`, nil}, + {"feature mismatched id", featurePath, "my-feature", linter.Feature, `{"id": "other"}`, []linter.Issue{ + {Path: featurePath, Line: 1, Col: 8, RuleID: "id-dir-mismatch", Message: `id "other" does not match containing directory "my-feature"`}, + }}, + {"feature reported with no directory component", "devcontainer-feature.json", "my-feature", linter.Feature, `{"id": "my-feature"}`, nil}, + {"feature reported with no directory component, mismatched id", "devcontainer-feature.json", "my-feature", linter.Feature, `{"id": "other"}`, []linter.Issue{ + {Path: "devcontainer-feature.json", Line: 1, Col: 8, RuleID: "id-dir-mismatch", Message: `id "other" does not match containing directory "my-feature"`}, }}, - {"feature nested under a parent directory", "src/my-feature/devcontainer-feature.json", linter.Feature, `{"id": "my-feature"}`, nil}, - {"feature nested, id matches parent instead of own directory", "src/my-feature/devcontainer-feature.json", linter.Feature, `{"id": "src"}`, []linter.Issue{ + {"feature nested under a parent directory", "src/my-feature/devcontainer-feature.json", "my-feature", linter.Feature, `{"id": "my-feature"}`, nil}, + {"feature nested, id matches parent instead of own directory", "src/my-feature/devcontainer-feature.json", "my-feature", linter.Feature, `{"id": "src"}`, []linter.Issue{ {Path: "src/my-feature/devcontainer-feature.json", Line: 1, Col: 8, RuleID: "id-dir-mismatch", Message: `id "src" does not match containing directory "my-feature"`}, }}, - {"template matching id", "my-template/devcontainer-template.json", linter.Template, `{"id": "my-template"}`, nil}, - {"template mismatched id", "my-template/devcontainer-template.json", linter.Template, `{"id": "other"}`, []linter.Issue{ - {Path: "my-template/devcontainer-template.json", Line: 1, Col: 8, RuleID: "id-dir-mismatch", Message: `id "other" does not match containing directory "my-template"`}, + {"template matching id", templatePath, "my-template", linter.Template, `{"id": "my-template"}`, nil}, + {"template mismatched id", templatePath, "my-template", linter.Template, `{"id": "other"}`, []linter.Issue{ + {Path: templatePath, Line: 1, Col: 8, RuleID: "id-dir-mismatch", Message: `id "other" does not match containing directory "my-template"`}, }}, - {"non-string id", "my-feature/devcontainer-feature.json", linter.Feature, `{"id": 42}`, nil}, - {"position points at the value, not the key", "my-feature/devcontainer-feature.json", linter.Feature, `{ + {"unnamed directory", featurePath, "", linter.Feature, `{"id": "other"}`, nil}, + {"non-string id", featurePath, "my-feature", linter.Feature, `{"id": 42}`, nil}, + {"position points at the value, not the key", featurePath, "my-feature", linter.Feature, `{ // the feature's id "id": "other" }`, []linter.Issue{ - {Path: "my-feature/devcontainer-feature.json", Line: 3, Col: 9, RuleID: "id-dir-mismatch", Message: `id "other" does not match containing directory "my-feature"`}, + {Path: featurePath, Line: 3, Col: 9, RuleID: "id-dir-mismatch", Message: `id "other" does not match containing directory "my-feature"`}, }}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - assertIssuesAt(t, rules.IDDirMismatch, linter.SeverityError, tt.path, tt.fileType, tt.src, tt.want) + assertIssuesInDir(t, rules.IDDirMismatch, linter.SeverityError, tt.path, tt.fileType, tt.src, linter.Dir{Name: tt.dir}, tt.want) }) } } diff --git a/rules/missing_feature_install_script.go b/rules/missing_feature_install_script.go index a0e10d4..0b16cd3 100644 --- a/rules/missing_feature_install_script.go +++ b/rules/missing_feature_install_script.go @@ -23,10 +23,10 @@ var MissingFeatureInstallScript = &linter.Rule{ } func checkMissingFeatureInstallScript(ctx *linter.Context, node *linter.Node) []linter.Finding { - if ctx.Dir == nil { + if ctx.Dir.FS == nil { return nil } - info, err := fs.Stat(ctx.Dir, installScriptName) + info, err := fs.Stat(ctx.Dir.FS, installScriptName) switch { case err == nil && !info.IsDir(): return nil diff --git a/rules/missing_feature_install_script_test.go b/rules/missing_feature_install_script_test.go index 02dbe42..d4ff677 100644 --- a/rules/missing_feature_install_script_test.go +++ b/rules/missing_feature_install_script_test.go @@ -30,7 +30,7 @@ func TestMissingFeatureInstallScript(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - assertIssuesInDir(t, rules.MissingFeatureInstallScript, linter.SeverityError, path, linter.Feature, src, tt.dir, tt.want) + assertIssuesInDir(t, rules.MissingFeatureInstallScript, linter.SeverityError, path, linter.Feature, src, linter.Dir{FS: tt.dir}, tt.want) }) } @@ -41,6 +41,6 @@ func TestMissingFeatureInstallScript(t *testing.T) { t.Run("unreadable directory reports nothing", func(t *testing.T) { t.Parallel() - assertIssuesInDir(t, rules.MissingFeatureInstallScript, linter.SeverityError, path, linter.Feature, src, errFS{}, nil) + assertIssuesInDir(t, rules.MissingFeatureInstallScript, linter.SeverityError, path, linter.Feature, src, linter.Dir{FS: errFS{}}, nil) }) } diff --git a/rules/rules_test.go b/rules/rules_test.go index 1402f33..c0ad72f 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -83,7 +83,7 @@ func TestRegisterRules_PlatformFilter(t *testing.T) { if err := rules.RegisterRules(l, tt.platforms, rules.Overrides{}); err != nil { t.Fatalf("RegisterRules: %v", err) } - issues := lintSource(t, l, "devcontainer.json", linter.Devcontainer, src, nil) + issues := lintSource(t, l, "devcontainer.json", linter.Devcontainer, src, linter.Dir{}) fired := ruleFired(issues, "no-bind-mount") if fired != tt.wantFired { t.Errorf("no-bind-mount fired = %v, want %v (issues: %v)", fired, tt.wantFired, issues) @@ -103,7 +103,7 @@ func TestRegisterRules_DefaultOffRule(t *testing.T) { if err := rules.RegisterRules(l, nil, rules.Overrides{}); err != nil { t.Fatalf("RegisterRules: %v", err) } - issues := lintSource(t, l, "devcontainer.json", linter.Devcontainer, src, nil) + issues := lintSource(t, l, "devcontainer.json", linter.Devcontainer, src, linter.Dir{}) if ruleFired(issues, "no-seccomp-override") { t.Errorf("no-seccomp-override fired despite being off by default: %v", issues) } @@ -119,7 +119,7 @@ func TestRegisterRules_OverrideEnablesOffDefaultRule(t *testing.T) { if err := rules.RegisterRules(l, nil, overrides); err != nil { t.Fatalf("RegisterRules: %v", err) } - issues := lintSource(t, l, "devcontainer.json", linter.Devcontainer, src, nil) + issues := lintSource(t, l, "devcontainer.json", linter.Devcontainer, src, linter.Dir{}) if !ruleFired(issues, "no-seccomp-override") { t.Errorf("no-seccomp-override did not fire despite being overridden to error: %v", issues) } @@ -173,7 +173,7 @@ func TestRegisterRules_SeverityOverride(t *testing.T) { if err := rules.RegisterRules(l, nil, rules.Overrides{Rules: tt.overrides}); err != nil { t.Fatalf("RegisterRules: %v", err) } - got := lintSource(t, l, "devcontainer.json", linter.Devcontainer, src, nil) + got := lintSource(t, l, "devcontainer.json", linter.Devcontainer, src, linter.Dir{}) if diff := cmp.Diff(tt.want, got); diff != "" { t.Errorf("issues mismatch (-want +got):\n%s", diff) } diff --git a/rules/undefined_template_option.go b/rules/undefined_template_option.go index f96a8a4..2b76fc1 100644 --- a/rules/undefined_template_option.go +++ b/rules/undefined_template_option.go @@ -23,14 +23,14 @@ var UndefinedTemplateOption = &linter.Rule{ } func checkUndefinedTemplateOption(ctx *linter.Context, node *linter.Node) []linter.Finding { - if ctx.Dir == nil { + if ctx.Dir.FS == nil { return nil } obj, ok := node.Value.Value.(*hujson.Object) if !ok { return nil } - refs, err := templateOptionRefs(ctx.Dir) + refs, err := templateOptionRefs(ctx.Dir.FS) if err != nil { return nil } diff --git a/rules/undefined_template_option_test.go b/rules/undefined_template_option_test.go index 9c5dfcd..511eba3 100644 --- a/rules/undefined_template_option_test.go +++ b/rules/undefined_template_option_test.go @@ -76,13 +76,13 @@ func TestUndefinedTemplateOption(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - assertIssuesInDir(t, rules.UndefinedTemplateOption, linter.SeverityError, path, linter.Template, tt.src, tt.dir, tt.want) + assertIssuesInDir(t, rules.UndefinedTemplateOption, linter.SeverityError, path, linter.Template, tt.src, linter.Dir{FS: tt.dir}, tt.want) }) } t.Run("unreadable directory reports nothing", func(t *testing.T) { t.Parallel() - assertIssuesInDir(t, rules.UndefinedTemplateOption, linter.SeverityError, path, linter.Template, withOptions, errFS{}, nil) + assertIssuesInDir(t, rules.UndefinedTemplateOption, linter.SeverityError, path, linter.Template, withOptions, linter.Dir{FS: errFS{}}, nil) }) t.Run("nil directory", func(t *testing.T) { diff --git a/rules/unused_template_option.go b/rules/unused_template_option.go index 8a260cf..73c0418 100644 --- a/rules/unused_template_option.go +++ b/rules/unused_template_option.go @@ -20,14 +20,14 @@ var UnusedTemplateOption = &linter.Rule{ } func checkUnusedTemplateOption(ctx *linter.Context, node *linter.Node) []linter.Finding { - if ctx.Dir == nil { + if ctx.Dir.FS == nil { return nil } obj, ok := node.Value.Value.(*hujson.Object) if !ok { return nil } - refs, err := templateOptionRefs(ctx.Dir) + refs, err := templateOptionRefs(ctx.Dir.FS) if err != nil { return nil } diff --git a/rules/unused_template_option_test.go b/rules/unused_template_option_test.go index 9b58878..c56c2f1 100644 --- a/rules/unused_template_option_test.go +++ b/rules/unused_template_option_test.go @@ -59,25 +59,25 @@ func TestUnusedTemplateOption(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - assertIssuesInDir(t, rules.UnusedTemplateOption, linter.SeverityError, path, linter.Template, src, tt.dir, tt.want) + assertIssuesInDir(t, rules.UnusedTemplateOption, linter.SeverityError, path, linter.Template, src, linter.Dir{FS: tt.dir}, tt.want) }) } t.Run("no options member", func(t *testing.T) { t.Parallel() dir := fstest.MapFS{".devcontainer/devcontainer.json": {Data: []byte("${templateOption:used}")}} - assertIssuesInDir(t, rules.UnusedTemplateOption, linter.SeverityError, path, linter.Template, `{"id": "my-template"}`, dir, nil) + assertIssuesInDir(t, rules.UnusedTemplateOption, linter.SeverityError, path, linter.Template, `{"id": "my-template"}`, linter.Dir{FS: dir}, nil) }) t.Run("options is not an object", func(t *testing.T) { t.Parallel() dir := fstest.MapFS{".devcontainer/devcontainer.json": {Data: []byte("${templateOption:used}")}} - assertIssuesInDir(t, rules.UnusedTemplateOption, linter.SeverityError, path, linter.Template, `{"options": "nope"}`, dir, nil) + assertIssuesInDir(t, rules.UnusedTemplateOption, linter.SeverityError, path, linter.Template, `{"options": "nope"}`, linter.Dir{FS: dir}, nil) }) t.Run("unreadable directory reports nothing", func(t *testing.T) { t.Parallel() - assertIssuesInDir(t, rules.UnusedTemplateOption, linter.SeverityError, path, linter.Template, src, errFS{}, nil) + assertIssuesInDir(t, rules.UnusedTemplateOption, linter.SeverityError, path, linter.Template, src, linter.Dir{FS: errFS{}}, nil) }) t.Run("nil directory", func(t *testing.T) {