From dbcdd053640e75a108bea9f1c4b2fa6c31f606f5 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Tue, 21 Apr 2026 09:12:06 +0200 Subject: [PATCH 1/3] add multi-page documentation validation for plugins --- README.md | 1 + pkg/analysis/passes/analysis.go | 2 + pkg/analysis/passes/plugindocs/plugindocs.go | 225 ++++++++++++++++++ .../passes/plugindocs/plugindocs_test.go | 164 +++++++++++++ .../testdata/empty-docspath/src/plugin.json | 6 + .../testdata/no-docspath/src/plugin.json | 5 + .../testdata/with-docspath/src/plugin.json | 6 + 7 files changed, 409 insertions(+) create mode 100644 pkg/analysis/passes/plugindocs/plugindocs.go create mode 100644 pkg/analysis/passes/plugindocs/plugindocs_test.go create mode 100644 pkg/analysis/passes/plugindocs/testdata/empty-docspath/src/plugin.json create mode 100644 pkg/analysis/passes/plugindocs/testdata/no-docspath/src/plugin.json create mode 100644 pkg/analysis/passes/plugindocs/testdata/with-docspath/src/plugin.json diff --git a/README.md b/README.md index daed6ded..4f0322d9 100644 --- a/README.md +++ b/README.md @@ -308,6 +308,7 @@ Run "mage gen:readme" to regenerate this section. | No Tracking Scripts / `trackingscripts` | Detects if there are any known tracking scripts, which are not allowed. | None | | Organization (exists) / `org` | Verifies the org specified in the plugin ID exists. | None | | package.json / `packagejson` | Ensures that package.json exists and the version matches the plugin.json | None | +| Plugin Docs / `plugindocs` | Runs the `@grafana/plugin-docs-cli validate` command to check multi-page documentation (only for plugins that set `docsPath` in `plugin.json`). | `node`, `npx` | | Plugin Name formatting / `pluginname` | Validates the plugin ID used conforms to our naming convention. | None | | Provenance attestation validation / `provenance` | Validates the provenance attestation if the plugin was built with a pipeline supporting provenance attestation (e.g Github Actions). | None | | Published / `published-plugin` | Detects whether any version of this plugin exists in the Grafana plugin catalog currently. | None | diff --git a/pkg/analysis/passes/analysis.go b/pkg/analysis/passes/analysis.go index 874354cb..bc0f3514 100644 --- a/pkg/analysis/passes/analysis.go +++ b/pkg/analysis/passes/analysis.go @@ -35,6 +35,7 @@ import ( "github.com/grafana/plugin-validator/pkg/analysis/passes/org" "github.com/grafana/plugin-validator/pkg/analysis/passes/osvscanner" "github.com/grafana/plugin-validator/pkg/analysis/passes/packagejson" + "github.com/grafana/plugin-validator/pkg/analysis/passes/plugindocs" "github.com/grafana/plugin-validator/pkg/analysis/passes/pluginname" "github.com/grafana/plugin-validator/pkg/analysis/passes/provenance" "github.com/grafana/plugin-validator/pkg/analysis/passes/published" @@ -87,6 +88,7 @@ var Analyzers = []*analysis.Analyzer{ org.Analyzer, osvscanner.Analyzer, packagejson.Analyzer, + plugindocs.Analyzer, pluginname.Analyzer, provenance.Analyzer, published.Analyzer, diff --git a/pkg/analysis/passes/plugindocs/plugindocs.go b/pkg/analysis/passes/plugindocs/plugindocs.go new file mode 100644 index 00000000..0ce4a300 --- /dev/null +++ b/pkg/analysis/passes/plugindocs/plugindocs.go @@ -0,0 +1,225 @@ +package plugindocs + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + "github.com/grafana/plugin-validator/pkg/analysis" + "github.com/grafana/plugin-validator/pkg/analysis/passes/sourcecode" + "github.com/grafana/plugin-validator/pkg/logme" +) + +const ( + cliPackage = "@grafana/plugin-docs-cli" + cliRunCommand = "validate" + // runTimeout bounds the full `npx --yes ...` invocation, including package download on first run. + runTimeout = 120 * time.Second +) + +var ( + pluginDocsError = &analysis.Rule{ + Name: "plugin-docs-error", + Severity: analysis.Error, + } + pluginDocsWarning = &analysis.Rule{ + Name: "plugin-docs-warning", + Severity: analysis.Warning, + } + pluginDocsInfo = &analysis.Rule{ + Name: "plugin-docs-info", + Severity: analysis.Recommendation, + } + pluginDocsCliFailure = &analysis.Rule{ + Name: "plugin-docs-cli-failure", + Severity: analysis.Warning, + } +) + +var Analyzer = &analysis.Analyzer{ + Name: "plugindocs", + Requires: []*analysis.Analyzer{sourcecode.Analyzer}, + Run: run, + Rules: []*analysis.Rule{ + pluginDocsError, + pluginDocsWarning, + pluginDocsInfo, + pluginDocsCliFailure, + }, + ReadmeInfo: analysis.ReadmeInfo{ + Name: "Plugin Docs", + Description: fmt.Sprintf("Runs the `%s validate` command to check multi-page documentation (only for plugins that set `docsPath` in `plugin.json`).", cliPackage), + Dependencies: "`node`, `npx`", + }, +} + +// cliDiagnostic mirrors the Diagnostic shape produced by `plugin-docs-cli validate --json`. +// See plugin-tools/packages/plugin-docs-cli/src/validation/types.ts. +type cliDiagnostic struct { + Rule string `json:"rule"` + Severity string `json:"severity"` + File string `json:"file,omitempty"` + Line int `json:"line,omitempty"` + Title string `json:"title"` + Detail string `json:"detail"` +} + +type cliResult struct { + Valid bool `json:"valid"` + Diagnostics []cliDiagnostic `json:"diagnostics"` +} + +func run(pass *analysis.Pass) (interface{}, error) { + if os.Getenv("SKIP_PLUGIN_DOCS_CLI") != "" { + logme.Debugln("SKIP_PLUGIN_DOCS_CLI set, skipping plugin docs validation") + return nil, nil + } + + sourceCodeDir, ok := pass.ResultOf[sourcecode.Analyzer].(string) + if !ok || sourceCodeDir == "" { + // no source code available - can't validate docs + return nil, nil + } + + // hard gate: only run if the plugin has opted in via `docsPath` in src/plugin.json. + // this short-circuits before any external process is spawned for the ~99% of plugins + // that haven't opted into multi-page docs. + hasDocs, err := pluginHasDocsPath(sourceCodeDir) + if err != nil { + logme.Debugln("plugindocs: failed to inspect src/plugin.json, skipping:", err) + return nil, nil + } + if !hasDocs { + logme.Debugln("plugindocs: docsPath not set in src/plugin.json, skipping") + return nil, nil + } + + npxBin, err := exec.LookPath("npx") + if err != nil { + logme.Debugln("plugindocs: npx not found on PATH, skipping plugin docs validation") + return nil, nil + } + + ctx, cancel := context.WithTimeout(context.Background(), runTimeout) + defer cancel() + + cmd := exec.CommandContext(ctx, npxBin, "--yes", cliPackage, cliRunCommand, "--json", "--strict") + cmd.Dir = sourceCodeDir + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + runErr := cmd.Run() + + // the CLI exits 1 when any `error` severity diagnostic is present, or when something + // goes wrong before validation (e.g. could not find src/plugin.json). distinguish by + // whether stdout contains parseable JSON. + var result cliResult + if jsonErr := json.Unmarshal(stdout.Bytes(), &result); jsonErr != nil { + reportCliFailure(pass, runErr, stderr.String(), stdout.String()) + return nil, nil + } + + for _, d := range result.Diagnostics { + rule := ruleForSeverity(d.Severity) + pass.ReportResult( + pass.AnalyzerName, + rule, + formatTitle(d), + d.Detail, + ) + } + + return nil, nil +} + +// pluginHasDocsPath reports whether src/plugin.json exists and has a non-empty docsPath field. +// returns (false, nil) when plugin.json is missing - a missing src/plugin.json means the +// validator was invoked without source code, which is a skip condition, not an error. +func pluginHasDocsPath(sourceCodeDir string) (bool, error) { + pluginJSONPath := filepath.Join(sourceCodeDir, "src", "plugin.json") + raw, err := os.ReadFile(pluginJSONPath) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, fmt.Errorf("read %s: %w", pluginJSONPath, err) + } + + var parsed struct { + DocsPath string `json:"docsPath"` + } + if err := json.Unmarshal(raw, &parsed); err != nil { + return false, fmt.Errorf("parse %s: %w", pluginJSONPath, err) + } + + return strings.TrimSpace(parsed.DocsPath) != "", nil +} + +func ruleForSeverity(severity string) *analysis.Rule { + switch severity { + case "error": + return pluginDocsError + case "warning": + return pluginDocsWarning + case "info": + return pluginDocsInfo + default: + // unknown severity from the CLI - surface it as a warning so it's visible + // without blocking publishing. log for debugging. + logme.DebugFln("plugindocs: unknown CLI severity %q, defaulting to warning", severity) + return pluginDocsWarning + } +} + +// formatTitle composes a human-readable title that preserves the CLI rule name and +// file:line origin, since a single generic validator rule wraps many CLI rules. +func formatTitle(d cliDiagnostic) string { + var location string + if d.File != "" { + if d.Line > 0 { + location = fmt.Sprintf(" (%s:%d)", d.File, d.Line) + } else { + location = fmt.Sprintf(" (%s)", d.File) + } + } + return fmt.Sprintf("[%s] %s%s", d.Rule, d.Title, location) +} + +func reportCliFailure(pass *analysis.Pass, runErr error, stderr, stdout string) { + var msg strings.Builder + msg.WriteString("Could not run `") + msg.WriteString(cliPackage) + msg.WriteString(" ") + msg.WriteString(cliRunCommand) + msg.WriteString("`.") + if runErr != nil { + msg.WriteString(" error: ") + msg.WriteString(runErr.Error()) + } + detail := strings.TrimSpace(stderr) + if detail == "" { + detail = strings.TrimSpace(stdout) + } + if detail == "" { + detail = "No output captured from the CLI." + } + // bound the detail so a noisy CLI failure doesn't produce a multi-MB diagnostic. + const maxDetail = 4096 + if len(detail) > maxDetail { + detail = detail[:maxDetail] + "...[truncated]" + } + + pass.ReportResult( + pass.AnalyzerName, + pluginDocsCliFailure, + msg.String(), + detail, + ) +} diff --git a/pkg/analysis/passes/plugindocs/plugindocs_test.go b/pkg/analysis/passes/plugindocs/plugindocs_test.go new file mode 100644 index 00000000..563a42a1 --- /dev/null +++ b/pkg/analysis/passes/plugindocs/plugindocs_test.go @@ -0,0 +1,164 @@ +package plugindocs + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/grafana/plugin-validator/pkg/analysis" + "github.com/grafana/plugin-validator/pkg/analysis/passes/sourcecode" + "github.com/grafana/plugin-validator/pkg/testpassinterceptor" +) + +func TestPluginHasDocsPath(t *testing.T) { + tests := []struct { + name string + sourceDir string + want bool + expectErr bool + }{ + { + name: "docsPath set", + sourceDir: filepath.Join("testdata", "with-docspath"), + want: true, + }, + { + name: "docsPath not present", + sourceDir: filepath.Join("testdata", "no-docspath"), + want: false, + }, + { + name: "docsPath is empty string", + sourceDir: filepath.Join("testdata", "empty-docspath"), + want: false, + }, + { + name: "plugin.json missing", + sourceDir: filepath.Join("testdata", "does-not-exist"), + want: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := pluginHasDocsPath(tc.sourceDir) + if tc.expectErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } +} + +func TestRuleForSeverity(t *testing.T) { + require.Equal(t, pluginDocsError, ruleForSeverity("error")) + require.Equal(t, pluginDocsWarning, ruleForSeverity("warning")) + require.Equal(t, pluginDocsInfo, ruleForSeverity("info")) + // unknown severities fall back to warning so unexpected CLI output is still surfaced + require.Equal(t, pluginDocsWarning, ruleForSeverity("debug")) + require.Equal(t, pluginDocsWarning, ruleForSeverity("")) +} + +func TestFormatTitle(t *testing.T) { + tests := []struct { + name string + in cliDiagnostic + want string + }{ + { + name: "file and line", + in: cliDiagnostic{Rule: "no-script-tags", Title: "Script tag found", File: "docs/index.md", Line: 12}, + want: "[no-script-tags] Script tag found (docs/index.md:12)", + }, + { + name: "file only", + in: cliDiagnostic{Rule: "has-markdown-files", Title: "No markdown files found", File: "docs/"}, + want: "[has-markdown-files] No markdown files found (docs/)", + }, + { + name: "no location", + in: cliDiagnostic{Rule: "manifest-valid", Title: "Manifest is invalid"}, + want: "[manifest-valid] Manifest is invalid", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.want, formatTitle(tc.in)) + }) + } +} + +// TestSkipsWhenNoDocsPath is the end-to-end test for the hard gate. A plugin without a +// docsPath must produce zero diagnostics and must NOT invoke the CLI. We don't mock exec +// here: if the gate leaks, a spurious CLI invocation would be visible as diagnostics or +// test latency. The test passes even on machines without node/npx installed. +func TestSkipsWhenNoDocsPath(t *testing.T) { + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: "./", + ResultOf: map[*analysis.Analyzer]interface{}{ + sourcecode.Analyzer: filepath.Join("testdata", "no-docspath"), + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 0) +} + +func TestSkipsWhenDocsPathIsEmpty(t *testing.T) { + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: "./", + ResultOf: map[*analysis.Analyzer]interface{}{ + sourcecode.Analyzer: filepath.Join("testdata", "empty-docspath"), + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 0) +} + +// TestSkipsWhenNoSourceCode covers the case where the validator was invoked against a +// zip archive only (no source code reference). The analyzer should skip silently. +func TestSkipsWhenNoSourceCode(t *testing.T) { + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: "./", + ResultOf: map[*analysis.Analyzer]interface{}{ + // sourcecode.Analyzer intentionally absent - matches how the runner records + // "no source code provided" (sourcecode.run returns (nil, nil) in that case). + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 0) +} + +// TestSkipsWhenEnvVarSet covers the operator escape hatch - SKIP_PLUGIN_DOCS_CLI=1 must +// bypass everything, including plugins that have docsPath set. +func TestSkipsWhenEnvVarSet(t *testing.T) { + t.Setenv("SKIP_PLUGIN_DOCS_CLI", "1") + + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: "./", + ResultOf: map[*analysis.Analyzer]interface{}{ + sourcecode.Analyzer: filepath.Join("testdata", "with-docspath"), + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 0) +} diff --git a/pkg/analysis/passes/plugindocs/testdata/empty-docspath/src/plugin.json b/pkg/analysis/passes/plugindocs/testdata/empty-docspath/src/plugin.json new file mode 100644 index 00000000..0e4367de --- /dev/null +++ b/pkg/analysis/passes/plugindocs/testdata/empty-docspath/src/plugin.json @@ -0,0 +1,6 @@ +{ + "id": "test-plugin-empty-docspath", + "type": "datasource", + "name": "Test Plugin With Empty Docs", + "docsPath": "" +} diff --git a/pkg/analysis/passes/plugindocs/testdata/no-docspath/src/plugin.json b/pkg/analysis/passes/plugindocs/testdata/no-docspath/src/plugin.json new file mode 100644 index 00000000..f4ada894 --- /dev/null +++ b/pkg/analysis/passes/plugindocs/testdata/no-docspath/src/plugin.json @@ -0,0 +1,5 @@ +{ + "id": "test-plugin-no-docspath", + "type": "datasource", + "name": "Test Plugin Without Docs" +} diff --git a/pkg/analysis/passes/plugindocs/testdata/with-docspath/src/plugin.json b/pkg/analysis/passes/plugindocs/testdata/with-docspath/src/plugin.json new file mode 100644 index 00000000..917eefb1 --- /dev/null +++ b/pkg/analysis/passes/plugindocs/testdata/with-docspath/src/plugin.json @@ -0,0 +1,6 @@ +{ + "id": "test-plugin-with-docspath", + "type": "datasource", + "name": "Test Plugin With Docs", + "docsPath": "docs" +} From 936ddb1515eb18a4a3c32840166329ffbe9bd393 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Tue, 21 Apr 2026 09:28:55 +0200 Subject: [PATCH 2/3] cleanup --- pkg/analysis/passes/plugindocs/plugindocs.go | 54 +++++++++++++++---- .../passes/plugindocs/plugindocs_test.go | 19 ------- 2 files changed, 44 insertions(+), 29 deletions(-) diff --git a/pkg/analysis/passes/plugindocs/plugindocs.go b/pkg/analysis/passes/plugindocs/plugindocs.go index 0ce4a300..cff157e5 100644 --- a/pkg/analysis/passes/plugindocs/plugindocs.go +++ b/pkg/analysis/passes/plugindocs/plugindocs.go @@ -16,11 +16,48 @@ import ( "github.com/grafana/plugin-validator/pkg/logme" ) +// boundedBuffer is an io.Writer that accumulates bytes up to `limit` and silently +// discards anything beyond that. It reports writes as fully successful so the +// subprocess never sees backpressure or broken pipes from buffer fullness. +type boundedBuffer struct { + buf bytes.Buffer + limit int + truncated bool +} + +func (b *boundedBuffer) Write(p []byte) (int, error) { + remaining := b.limit - b.buf.Len() + if remaining <= 0 { + b.truncated = true + return len(p), nil + } + if len(p) > remaining { + b.buf.Write(p[:remaining]) + b.truncated = true + return len(p), nil + } + return b.buf.Write(p) +} + +func (b *boundedBuffer) Bytes() []byte { return b.buf.Bytes() } +func (b *boundedBuffer) String() string { return b.buf.String() } + const ( - cliPackage = "@grafana/plugin-docs-cli" + cliPackage = "@grafana/plugin-docs-cli" + // cliVersion pins the CLI so validation is reproducible across runs and can't be + // influenced by whatever version `latest` resolves to on a given day. Bump this + // deliberately when adopting new validation rules from the CLI. + cliVersion = "0.0.10" cliRunCommand = "validate" // runTimeout bounds the full `npx --yes ...` invocation, including package download on first run. runTimeout = 120 * time.Second + // waitDelay caps how long Wait blocks after the context is canceled, so grandchild + // processes spawned by npx (node, the CLI itself) can't keep the stdout/stderr pipes + // open forever and stall the validator run. + waitDelay = 5 * time.Second + // maxBufferBytes bounds stdout and stderr separately. A misbehaving CLI could otherwise + // emit enough output to OOM the validator host. + maxBufferBytes = 4 * 1024 * 1024 ) var ( @@ -76,11 +113,6 @@ type cliResult struct { } func run(pass *analysis.Pass) (interface{}, error) { - if os.Getenv("SKIP_PLUGIN_DOCS_CLI") != "" { - logme.Debugln("SKIP_PLUGIN_DOCS_CLI set, skipping plugin docs validation") - return nil, nil - } - sourceCodeDir, ok := pass.ResultOf[sourcecode.Analyzer].(string) if !ok || sourceCodeDir == "" { // no source code available - can't validate docs @@ -109,11 +141,13 @@ func run(pass *analysis.Pass) (interface{}, error) { ctx, cancel := context.WithTimeout(context.Background(), runTimeout) defer cancel() - cmd := exec.CommandContext(ctx, npxBin, "--yes", cliPackage, cliRunCommand, "--json", "--strict") + cmd := exec.CommandContext(ctx, npxBin, "--yes", cliPackage+"@"+cliVersion, cliRunCommand, "--json", "--strict") cmd.Dir = sourceCodeDir - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr + cmd.WaitDelay = waitDelay + stdout := &boundedBuffer{limit: maxBufferBytes} + stderr := &boundedBuffer{limit: maxBufferBytes} + cmd.Stdout = stdout + cmd.Stderr = stderr runErr := cmd.Run() diff --git a/pkg/analysis/passes/plugindocs/plugindocs_test.go b/pkg/analysis/passes/plugindocs/plugindocs_test.go index 563a42a1..4e4448db 100644 --- a/pkg/analysis/passes/plugindocs/plugindocs_test.go +++ b/pkg/analysis/passes/plugindocs/plugindocs_test.go @@ -143,22 +143,3 @@ func TestSkipsWhenNoSourceCode(t *testing.T) { require.NoError(t, err) require.Len(t, interceptor.Diagnostics, 0) } - -// TestSkipsWhenEnvVarSet covers the operator escape hatch - SKIP_PLUGIN_DOCS_CLI=1 must -// bypass everything, including plugins that have docsPath set. -func TestSkipsWhenEnvVarSet(t *testing.T) { - t.Setenv("SKIP_PLUGIN_DOCS_CLI", "1") - - var interceptor testpassinterceptor.TestPassInterceptor - pass := &analysis.Pass{ - RootDir: "./", - ResultOf: map[*analysis.Analyzer]interface{}{ - sourcecode.Analyzer: filepath.Join("testdata", "with-docspath"), - }, - Report: interceptor.ReportInterceptor(), - } - - _, err := Analyzer.Run(pass) - require.NoError(t, err) - require.Len(t, interceptor.Diagnostics, 0) -} From 74f39b368c9c6489324cafc8617cfe98382a3150 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Tue, 21 Apr 2026 16:37:16 +0200 Subject: [PATCH 3/3] pr feedback --- pkg/analysis/passes/metadata/types.go | 1 + pkg/analysis/passes/plugindocs/plugindocs.go | 39 ++------ .../passes/plugindocs/plugindocs_test.go | 95 ++++++++++--------- 3 files changed, 59 insertions(+), 76 deletions(-) diff --git a/pkg/analysis/passes/metadata/types.go b/pkg/analysis/passes/metadata/types.go index 663a82c8..2ac39ce0 100644 --- a/pkg/analysis/passes/metadata/types.go +++ b/pkg/analysis/passes/metadata/types.go @@ -10,6 +10,7 @@ type Metadata struct { Backend bool `json:"backend"` Alerting bool `json:"alerting"` Dependencies MetadataDependencies `json:"dependencies"` + DocsPath string `json:"docsPath"` } type Info struct { diff --git a/pkg/analysis/passes/plugindocs/plugindocs.go b/pkg/analysis/passes/plugindocs/plugindocs.go index cff157e5..a45c2d2c 100644 --- a/pkg/analysis/passes/plugindocs/plugindocs.go +++ b/pkg/analysis/passes/plugindocs/plugindocs.go @@ -5,13 +5,12 @@ import ( "context" "encoding/json" "fmt" - "os" "os/exec" - "path/filepath" "strings" "time" "github.com/grafana/plugin-validator/pkg/analysis" + "github.com/grafana/plugin-validator/pkg/analysis/passes/nestedmetadata" "github.com/grafana/plugin-validator/pkg/analysis/passes/sourcecode" "github.com/grafana/plugin-validator/pkg/logme" ) @@ -81,7 +80,7 @@ var ( var Analyzer = &analysis.Analyzer{ Name: "plugindocs", - Requires: []*analysis.Analyzer{sourcecode.Analyzer}, + Requires: []*analysis.Analyzer{nestedmetadata.Analyzer, sourcecode.Analyzer}, Run: run, Rules: []*analysis.Rule{ pluginDocsError, @@ -119,16 +118,16 @@ func run(pass *analysis.Pass) (interface{}, error) { return nil, nil } - // hard gate: only run if the plugin has opted in via `docsPath` in src/plugin.json. + // hard gate: only run if the plugin has opted in via `docsPath` in plugin.json. // this short-circuits before any external process is spawned for the ~99% of plugins // that haven't opted into multi-page docs. - hasDocs, err := pluginHasDocsPath(sourceCodeDir) - if err != nil { - logme.Debugln("plugindocs: failed to inspect src/plugin.json, skipping:", err) + metadatamap, ok := pass.ResultOf[nestedmetadata.Analyzer].(nestedmetadata.Metadatamap) + if !ok { return nil, nil } - if !hasDocs { - logme.Debugln("plugindocs: docsPath not set in src/plugin.json, skipping") + meta, ok := metadatamap[nestedmetadata.MainPluginJson] + if !ok || strings.TrimSpace(meta.DocsPath) == "" { + logme.Debugln("plugindocs: docsPath not set in plugin.json, skipping") return nil, nil } @@ -173,28 +172,6 @@ func run(pass *analysis.Pass) (interface{}, error) { return nil, nil } -// pluginHasDocsPath reports whether src/plugin.json exists and has a non-empty docsPath field. -// returns (false, nil) when plugin.json is missing - a missing src/plugin.json means the -// validator was invoked without source code, which is a skip condition, not an error. -func pluginHasDocsPath(sourceCodeDir string) (bool, error) { - pluginJSONPath := filepath.Join(sourceCodeDir, "src", "plugin.json") - raw, err := os.ReadFile(pluginJSONPath) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - return false, fmt.Errorf("read %s: %w", pluginJSONPath, err) - } - - var parsed struct { - DocsPath string `json:"docsPath"` - } - if err := json.Unmarshal(raw, &parsed); err != nil { - return false, fmt.Errorf("parse %s: %w", pluginJSONPath, err) - } - - return strings.TrimSpace(parsed.DocsPath) != "", nil -} func ruleForSeverity(severity string) *analysis.Rule { switch severity { diff --git a/pkg/analysis/passes/plugindocs/plugindocs_test.go b/pkg/analysis/passes/plugindocs/plugindocs_test.go index 4e4448db..7cac33ce 100644 --- a/pkg/analysis/passes/plugindocs/plugindocs_test.go +++ b/pkg/analysis/passes/plugindocs/plugindocs_test.go @@ -7,52 +7,12 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/plugin-validator/pkg/analysis" + "github.com/grafana/plugin-validator/pkg/analysis/passes/metadata" + "github.com/grafana/plugin-validator/pkg/analysis/passes/nestedmetadata" "github.com/grafana/plugin-validator/pkg/analysis/passes/sourcecode" "github.com/grafana/plugin-validator/pkg/testpassinterceptor" ) -func TestPluginHasDocsPath(t *testing.T) { - tests := []struct { - name string - sourceDir string - want bool - expectErr bool - }{ - { - name: "docsPath set", - sourceDir: filepath.Join("testdata", "with-docspath"), - want: true, - }, - { - name: "docsPath not present", - sourceDir: filepath.Join("testdata", "no-docspath"), - want: false, - }, - { - name: "docsPath is empty string", - sourceDir: filepath.Join("testdata", "empty-docspath"), - want: false, - }, - { - name: "plugin.json missing", - sourceDir: filepath.Join("testdata", "does-not-exist"), - want: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - got, err := pluginHasDocsPath(tc.sourceDir) - if tc.expectErr { - require.Error(t, err) - return - } - require.NoError(t, err) - require.Equal(t, tc.want, got) - }) - } -} - func TestRuleForSeverity(t *testing.T) { require.Equal(t, pluginDocsError, ruleForSeverity("error")) require.Equal(t, pluginDocsWarning, ruleForSeverity("warning")) @@ -101,7 +61,10 @@ func TestSkipsWhenNoDocsPath(t *testing.T) { pass := &analysis.Pass{ RootDir: "./", ResultOf: map[*analysis.Analyzer]interface{}{ - sourcecode.Analyzer: filepath.Join("testdata", "no-docspath"), + sourcecode.Analyzer: filepath.Join("testdata", "no-docspath"), + nestedmetadata.Analyzer: nestedmetadata.Metadatamap{ + nestedmetadata.MainPluginJson: metadata.Metadata{}, + }, }, Report: interceptor.ReportInterceptor(), } @@ -116,7 +79,28 @@ func TestSkipsWhenDocsPathIsEmpty(t *testing.T) { pass := &analysis.Pass{ RootDir: "./", ResultOf: map[*analysis.Analyzer]interface{}{ - sourcecode.Analyzer: filepath.Join("testdata", "empty-docspath"), + sourcecode.Analyzer: filepath.Join("testdata", "empty-docspath"), + nestedmetadata.Analyzer: nestedmetadata.Metadatamap{ + nestedmetadata.MainPluginJson: metadata.Metadata{DocsPath: ""}, + }, + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 0) +} + +func TestSkipsWhenDocsPathIsWhitespaceOnly(t *testing.T) { + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: "./", + ResultOf: map[*analysis.Analyzer]interface{}{ + sourcecode.Analyzer: filepath.Join("testdata", "empty-docspath"), + nestedmetadata.Analyzer: nestedmetadata.Metadatamap{ + nestedmetadata.MainPluginJson: metadata.Metadata{DocsPath: " "}, + }, }, Report: interceptor.ReportInterceptor(), } @@ -131,10 +115,31 @@ func TestSkipsWhenDocsPathIsEmpty(t *testing.T) { func TestSkipsWhenNoSourceCode(t *testing.T) { var interceptor testpassinterceptor.TestPassInterceptor pass := &analysis.Pass{ - RootDir: "./", + RootDir: "./", ResultOf: map[*analysis.Analyzer]interface{}{ // sourcecode.Analyzer intentionally absent - matches how the runner records // "no source code provided" (sourcecode.run returns (nil, nil) in that case). + nestedmetadata.Analyzer: nestedmetadata.Metadatamap{ + nestedmetadata.MainPluginJson: metadata.Metadata{DocsPath: "docs"}, + }, + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 0) +} + +// TestSkipsWhenNoMetadata covers the case where nestedmetadata did not produce a result +// (e.g. archive was empty or invalid). +func TestSkipsWhenNoMetadata(t *testing.T) { + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: "./", + ResultOf: map[*analysis.Analyzer]interface{}{ + sourcecode.Analyzer: filepath.Join("testdata", "with-docspath"), + // nestedmetadata.Analyzer intentionally absent }, Report: interceptor.ReportInterceptor(), }