Skip to content

Docs: Validate multi-page docs#560

Merged
sunker merged 3 commits intomainfrom
sunker/multi-page-docs-validation
Apr 23, 2026
Merged

Docs: Validate multi-page docs#560
sunker merged 3 commits intomainfrom
sunker/multi-page-docs-validation

Conversation

@sunker
Copy link
Copy Markdown
Contributor

@sunker sunker commented Apr 21, 2026

Adds a plugindocs analyzer that runs @grafana/plugin-docs-cli validate as a subprocess and surfaces its diagnostics in the plugin-validator report. Authors now see the same rules locally (via CLI and serve) that the submission pipeline enforces at review time.

Only runs when a plugin has opted into multi-page docs by setting docsPath in src/plugin.json - every other plugin short-circuits before any external process is spawned. When it does run, CLI severities map to the validator's model (error → error, warning → warning, info → recommendation) and each title carries the CLI rule name and file:line.

Requires node and npx on PATH; skips silently when absent. SKIP_PLUGIN_DOCS_CLI=1 disables it unconditionally.

All explicit validation rules in the docs cli are defined here: grafana/grafana-catalog-team#769.

@cla-assistant
Copy link
Copy Markdown

cla-assistant Bot commented Apr 21, 2026

CLA assistant check
All committers have signed the CLA.

@sunker sunker marked this pull request as ready for review April 21, 2026 07:29
@sunker sunker requested review from a team as code owners April 21, 2026 07:29
@grafana-plugins-platform-bot grafana-plugins-platform-bot Bot moved this from 📬 Triage to 🔬 In review in Grafana Catalog Team Apr 21, 2026
Copy link
Copy Markdown
Member

@xnyo xnyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small suggestion 🙂

Comment on lines +179 to +197
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
}
Copy link
Copy Markdown
Member

@xnyo xnyo Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than reading plugin.json manually, can we depend on nestedmetadata or metadata analyzer instead of parsing plugin.json directly? Example of this pattern in another analyzer:

metadatamap, ok := pass.ResultOf[nestedmetadata.Analyzer].(nestedmetadata.Metadatamap)
if !ok {
return nil, nil
}
reportCount := 0
for key, data := range metadatamap {
if strings.TrimSpace(data.Info.Logos.Small) == "" {
reportCount++
pass.ReportResult(
pass.AnalyzerName,
logos,
fmt.Sprintf("plugin.json: invalid empty small logo path for %s", key),
"Logo path cannot be empty",
)
}
if strings.TrimSpace(data.Info.Logos.Large) == "" {
reportCount++
pass.ReportResult(
pass.AnalyzerName,
logos,
fmt.Sprintf("plugin.json: invalid empty large logo path for %s", key),
"Logo path cannot be empty",
)
}
}

diff --git a/pkg/analysis/passes/metadata/types.go b/pkg/analysis/passes/metadata/types.go
index 663a82c..2ac39ce 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 cff157e..a45c2d2 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 4e4448d..94e049e 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(),
 	}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, great shout @xnyo! Feedback applied

@sunker sunker requested a review from xnyo April 22, 2026 13:03
Copy link
Copy Markdown
Member

@xnyo xnyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@sunker sunker merged commit d49fb5f into main Apr 23, 2026
11 checks passed
@sunker sunker deleted the sunker/multi-page-docs-validation branch April 23, 2026 10:57
@github-project-automation github-project-automation Bot moved this from 🔬 In review to 🚀 Shipped in Grafana Catalog Team Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants