mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
ci: validate docs/redirects.json (#30209)
Adds `docsredirectscheck`, a lint for `docs/redirects.json`, the list of
redirects the documentation website applies for pages that have moved or
been removed. It runs as `make lint/docs-redirects`, part of `make lint`
and `make lint-light`, so a bad redirect is caught in the pull request
that adds it. The file does not exist yet, and the check passes while it
is absent, so this can merge before or after the file does.
## What it checks
It fails on:
- a file that is not a JSON array of well-formed rules (`source`,
`destination`, optional boolean `permanent`, no unknown fields);
- syntax outside the supported subset: a source under `/docs/`, written
as an exact path, a trailing `/:name*`, or a trailing `/:name(.*)`, with
no version segment such as `/@main`;
- an internal destination that is not a page in `docs/manifest.json`, or
a pattern destination that points into a section with no pages or uses a
parameter the source does not define;
- a source that matches a live page, which would hide it;
- duplicate sources, chains (a destination that is another rule's
source), and loops.
It warns, and never fails, when a pull request removes routes from
`docs/manifest.json` while `docs/redirects.json` is unchanged and does
not already cover them. Pages are sometimes removed on purpose. The
comparison is by route rather than by file, so a rename that keeps its
URL (for example `admin/foo/index.md` to `admin/foo.md`) does not warn.
In GitHub Actions the warning is an annotation on the manifest. The base
is the first parent of the PR merge commit, the same approach
`scripts/check_emdash.sh` uses, and the warning is skipped with a note
when it is not available.
Routes are derived from `docs/manifest.json` the way the docs website
derives them, including its plain-suffix stripping of `README.md`,
`index.md`, and `.md`, so the check and the site agree about which pages
exist. `scripts/docsredirectscheck/README.md` documents the rule format
and the limits.
## Testing
- 128 Go tests pass under Go 1.26.5: route derivation, every failing and
passing rule shape, the absent-file pass, exit codes, and the warning
logic through a fake git reader.
- I checked that the tests can fail by breaking the code 16 ways (for
example dropping the `index.md` suffix, disabling the shadow check,
reporting loops twice). All 16 were caught. That run found one gap, a
missing test for "redirects file changed", which is now covered.
- Ran against the repository's real `docs/manifest.json`: passes with no
redirects file, and a file with one of each failure class reports every
problem with the rule index, the source, and how to fix it.
- Ran the warning against real git history: it warns for an older base,
stays silent at `HEAD`, and skips with a note for an unknown revision.
- Lint: `golangci-lint` v2.0.2 built with Go 1.26.5 against the
repository config reports 0 issues. `paralleltestctx`, `intxcheck`,
`typos` 1.46.1, `gofumpt` and `gci` (through `make fmt/go`), and `go
vet` pass.
CI: `lint` (which runs this check), `fmt`, `title`, and the Go and
end-to-end test jobs pass. The `gen` job, and so `required`, fails on
this branch for a reason this PR does not touch: `main` is failing it
too. `docs/manifest/generated/cli.json` has not been regenerated for the
`coder sharing` commands that landed in the latest commit on `main`. It
should clear once `main` is fixed and merged into this branch.
Not run locally: the full `make lint` and `make pre-commit`. The local
hooks are opt-in and were off. The annotation output has been exercised
by unit tests and a local run with `GITHUB_ACTIONS=true`, but not yet on
a real pull request merge commit; this PR does not change the manifest,
so it will not exercise it either. No workflow change is needed, because
the `lint` job already runs `make lint` for any change that is not
offlinedocs-only.
---
🤖 Built with AI assistance.
This commit is contained in:
Nick Vigilante
@@ -780,11 +780,11 @@ gen/docs-manifest: fmt/docs-manifest site/node_modules/.installed | _gen
|
||||
# GitHub Actions linters are run in a separate CI job (lint-actions) that only
|
||||
# triggers when workflow files change, so we skip them here when CI=true.
|
||||
LINT_ACTIONS_TARGETS := $(if $(CI),,lint/actions/actionlint)
|
||||
lint: lint/shellcheck lint/go lint/ts lint/examples lint/helm lint/site-icons lint/markdown lint/docs-html lint/docs-manifest lint/style-claims lint/check-scopes lint/check-experiment-keys lint/migrations lint/bootstrap lint/architecture lint/emdash lint/agents lint/mise-versions $(LINT_ACTIONS_TARGETS)
|
||||
lint: lint/shellcheck lint/go lint/ts lint/examples lint/helm lint/site-icons lint/markdown lint/docs-html lint/docs-manifest lint/docs-redirects lint/style-claims lint/check-scopes lint/check-experiment-keys lint/migrations lint/bootstrap lint/architecture lint/emdash lint/agents lint/mise-versions $(LINT_ACTIONS_TARGETS)
|
||||
.PHONY: lint
|
||||
|
||||
# Fast lint subset for lightweight hooks. Some targets use mise-managed tools.
|
||||
lint-light: lint/shellcheck lint/markdown lint/helm lint/bootstrap lint/migrations lint/actions/actionlint lint/typos lint/emdash lint/style-claims lint/mise-versions
|
||||
lint-light: lint/shellcheck lint/markdown lint/helm lint/bootstrap lint/migrations lint/actions/actionlint lint/typos lint/emdash lint/docs-redirects lint/style-claims lint/mise-versions
|
||||
.PHONY: lint-light
|
||||
|
||||
lint/site-icons:
|
||||
@@ -841,6 +841,17 @@ lint/docs-manifest: _gen/bin/docsmanifestgen
|
||||
_gen/bin/docsmanifestgen check
|
||||
.PHONY: lint/docs-manifest
|
||||
|
||||
# Fails when docs/redirects.json, the redirects the docs site applies for pages
|
||||
# that moved or were removed, is malformed, points at a page that is not in
|
||||
# docs/manifest.json, hides a live page, or has duplicate, chained, or looping
|
||||
# rules. Also warns, without failing, when a pull request removes docs routes
|
||||
# and leaves the file alone. Passes when the file does not exist. See
|
||||
# scripts/docsredirectscheck/README.md.
|
||||
lint/docs-redirects:
|
||||
echo "--- check docs/redirects.json"
|
||||
go run ./scripts/docsredirectscheck
|
||||
.PHONY: lint/docs-redirects
|
||||
|
||||
# Fails when the style guide claims a prose rule is enforced by tooling that is
|
||||
# not enabled, when an enabled rule has no style guide section, or when the
|
||||
# coverage tables on the style guide landing page drift from the annotations.
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
# docsredirectscheck
|
||||
|
||||
`docsredirectscheck` validates `docs/redirects.json`, the list of redirects the
|
||||
documentation website applies for pages that have moved or been removed. It
|
||||
runs as `make lint/docs-redirects` (part of `make lint` and `make lint-light`),
|
||||
so a bad redirect is caught in the pull request that introduces it.
|
||||
|
||||
A missing `docs/redirects.json` passes.
|
||||
|
||||
## The rule file
|
||||
|
||||
`docs/redirects.json` is a JSON array of rules:
|
||||
|
||||
```json
|
||||
[
|
||||
{
|
||||
"source": "/docs/about/architecture",
|
||||
"destination": "/docs/about",
|
||||
"permanent": true
|
||||
},
|
||||
{
|
||||
"source": "/docs/tutorials/ai-agents/:path*",
|
||||
"destination": "/docs/ai-coder/:path*"
|
||||
}
|
||||
]
|
||||
```
|
||||
|
||||
- `source` and `destination` are full site paths that start with `/docs/`.
|
||||
Write them without a version segment such as `/@main`; the docs website adds
|
||||
that itself.
|
||||
- `permanent` is optional and defaults to `true`.
|
||||
- A `source` is an exact path, a trailing `/:name*` (matches the prefix and
|
||||
everything beneath it), or a trailing `/:name(.*)` (matches everything
|
||||
beneath the prefix, not the prefix itself).
|
||||
- A `destination` is an exact path, a trailing `/:name` or `/:name*` that reuses
|
||||
the source's parameter, or an `http(s)://` URL.
|
||||
|
||||
## What it catches
|
||||
|
||||
These fail the check:
|
||||
|
||||
- **A malformed file.** Not valid JSON, not an array, an entry that is not an
|
||||
object, a missing or empty `source` or `destination`, a non-boolean
|
||||
`permanent`, or an unknown field.
|
||||
- **Unsupported syntax.** A source outside `/docs/`, a version segment, or any
|
||||
pattern beyond the forms above.
|
||||
- **A destination that is not a page.** An internal destination must be a route
|
||||
in `docs/manifest.json`. A pattern destination must point into a section that
|
||||
has pages, and may only use a parameter the source defines.
|
||||
- **A source that hides a live page.** Redirects apply before pages, so a rule
|
||||
whose source matches a page in the manifest makes that page unreachable.
|
||||
- **Duplicates, chains, and loops.** Two rules with the same source, a rule
|
||||
whose destination is another rule's source (point it at the final page
|
||||
instead), and cycles.
|
||||
|
||||
## Removed-route warning
|
||||
|
||||
On a pull request the check also compares `docs/manifest.json` with the base
|
||||
branch. If routes were removed, `docs/redirects.json` was not changed, and no
|
||||
existing rule already covers the removed routes, it prints a warning. In GitHub
|
||||
Actions the warning is an annotation on the manifest. It never fails the run,
|
||||
because pages are sometimes removed on purpose.
|
||||
|
||||
The comparison is by route, not by file, so renaming `admin/foo/index.md` to
|
||||
`admin/foo.md` (same URL) is not a removal.
|
||||
|
||||
In GitHub Actions the base is the first parent of the pull request's merge
|
||||
commit, which is the tip of the branch the change merges into. Elsewhere pass
|
||||
`-base <rev>`; with neither, or when the revision is unavailable (a shallow
|
||||
checkout), the warning is skipped with a note.
|
||||
|
||||
## How it decides which pages exist
|
||||
|
||||
It derives routes from `docs/manifest.json` exactly as the docs website does:
|
||||
strip a leading `./`, then strip the suffixes `README.md`, `index.md`, and
|
||||
`.md` one after another, trim slashes, and prefix `/docs`. That includes the
|
||||
website's quirk of matching the suffixes as plain text rather than per path
|
||||
segment, so a file named `my-index.md` becomes the route `/docs/.../my-`. Every
|
||||
manifest entry with a path counts as a page, whether or not it appears in the
|
||||
sidebar. The landing page `/docs` always exists, and `/docs/about` exists when
|
||||
the first page is titled "About".
|
||||
|
||||
## Usage
|
||||
|
||||
```sh
|
||||
go run ./scripts/docsredirectscheck [-manifest path] [-redirects path] [-base rev]
|
||||
```
|
||||
|
||||
Exit status is 0 when the file is valid or absent, 1 when it has problems, and 2
|
||||
when the check itself could not run (for example the manifest is unreadable).
|
||||
|
||||
## Limitations
|
||||
|
||||
- Chains through a pattern destination are not followed, because where such a
|
||||
destination lands depends on the URL being redirected.
|
||||
- Only the three source forms above are understood. Regular-expression groups,
|
||||
version ranges, and mid-path parameters are rejected rather than guessed at.
|
||||
- The removed-route warning compares routes only; it does not know whether an
|
||||
external site links to a removed page.
|
||||
@@ -0,0 +1,142 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"slices"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// Repository-relative paths the advisory check compares across revisions. They
|
||||
// are fixed, independent of the -manifest and -redirects flags, because git
|
||||
// addresses files by their place in the repository.
|
||||
const (
|
||||
manifestRepoPath = "docs/manifest.json"
|
||||
redirectsRepoPath = "docs/redirects.json"
|
||||
)
|
||||
|
||||
// maxListedRoutes bounds how many removed routes one warning spells out.
|
||||
const maxListedRoutes = 10
|
||||
|
||||
// removedRoutes returns the routes live in base that are no longer live in
|
||||
// head, sorted. It compares routes, not files, so a rename that keeps the
|
||||
// public URL (for example admin/foo/index.md to admin/foo.md) is not a
|
||||
// removal.
|
||||
func removedRoutes(base, head map[string]bool) []string {
|
||||
var removed []string
|
||||
for route := range base {
|
||||
if !head[route] {
|
||||
removed = append(removed, route)
|
||||
}
|
||||
}
|
||||
slices.Sort(removed)
|
||||
return removed
|
||||
}
|
||||
|
||||
// uncoveredRoutes returns the removed routes that no rule's source matches,
|
||||
// preserving order.
|
||||
func uncoveredRoutes(removed []string, rules []rule) []string {
|
||||
var uncovered []string
|
||||
for _, route := range removed {
|
||||
covered := false
|
||||
for _, r := range rules {
|
||||
if r.matches(route) {
|
||||
covered = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !covered {
|
||||
uncovered = append(uncovered, route)
|
||||
}
|
||||
}
|
||||
return uncovered
|
||||
}
|
||||
|
||||
func advisoryMessage(uncovered []string) string {
|
||||
listed := uncovered
|
||||
more := ""
|
||||
if len(listed) > maxListedRoutes {
|
||||
more = fmt.Sprintf(" and %d more", len(listed)-maxListedRoutes)
|
||||
listed = listed[:maxListedRoutes]
|
||||
}
|
||||
noun := "routes"
|
||||
if len(uncovered) == 1 {
|
||||
noun = "route"
|
||||
}
|
||||
return fmt.Sprintf(
|
||||
"This change removes %d docs %s from %s without a matching redirect in %s: %s%s. "+
|
||||
"If readers or other sites may still link to a removed page, add a redirect. "+
|
||||
"If the removal is intentional, this warning can be ignored.",
|
||||
len(uncovered), noun, manifestRepoPath, redirectsRepoPath, strings.Join(listed, ", "), more,
|
||||
)
|
||||
}
|
||||
|
||||
// annotation formats a GitHub Actions workflow command. The message must stay
|
||||
// on one line, so the characters the runner treats as delimiters are escaped.
|
||||
func annotation(level, file, title, message string) string {
|
||||
escapeData := strings.NewReplacer("%", "%25", "\r", "%0D", "\n", "%0A")
|
||||
escapeProperty := strings.NewReplacer("%", "%25", "\r", "%0D", "\n", "%0A", ":", "%3A", ",", "%2C")
|
||||
return fmt.Sprintf("::%s file=%s,title=%s::%s", level, escapeProperty.Replace(file), escapeProperty.Replace(title), escapeData.Replace(message))
|
||||
}
|
||||
|
||||
// runAdvisory warns, without ever failing the run, when the change under
|
||||
// review removes docs routes and leaves docs/redirects.json alone. Pages are
|
||||
// sometimes removed on purpose, which is why this is advice and not a rule.
|
||||
// Every step that cannot be completed, for example in a shallow checkout,
|
||||
// skips the check with a note rather than guessing.
|
||||
func runAdvisory(o options, headLive map[string]bool, rules []rule) {
|
||||
skip := func(why string) {
|
||||
_, _ = fmt.Fprintf(o.stdout, "removed-route advisory skipped: %s\n", why)
|
||||
}
|
||||
|
||||
base := o.base
|
||||
if base == "" && o.getenv("GITHUB_BASE_REF") != "" {
|
||||
// On a pull request, actions/checkout checks out the merge commit, whose
|
||||
// first parent is exactly the base the change merges into.
|
||||
base = "HEAD^1"
|
||||
}
|
||||
if base == "" {
|
||||
skip("not a pull request run (pass -base <rev> to enable)")
|
||||
return
|
||||
}
|
||||
if !o.git.HasRev(base) {
|
||||
skip(fmt.Sprintf("base revision %q is not available (shallow checkout?)", base))
|
||||
return
|
||||
}
|
||||
|
||||
baseData, err := o.git.Show(base, manifestRepoPath)
|
||||
if err != nil {
|
||||
skip(fmt.Sprintf("cannot read %s at %s: %v", manifestRepoPath, base, err))
|
||||
return
|
||||
}
|
||||
baseManifest, err := parseManifest(baseData)
|
||||
if err != nil {
|
||||
skip(fmt.Sprintf("%s at %s is not a valid manifest: %v", manifestRepoPath, base, err))
|
||||
return
|
||||
}
|
||||
|
||||
removed := removedRoutes(liveRoutes(baseManifest), headLive)
|
||||
if len(removed) == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
changed, err := o.git.Changed(base, redirectsRepoPath)
|
||||
if err != nil {
|
||||
skip(fmt.Sprintf("cannot compare %s with %s: %v", redirectsRepoPath, base, err))
|
||||
return
|
||||
}
|
||||
if changed {
|
||||
return
|
||||
}
|
||||
|
||||
uncovered := uncoveredRoutes(removed, rules)
|
||||
if len(uncovered) == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
msg := advisoryMessage(uncovered)
|
||||
if o.getenv("GITHUB_ACTIONS") == "true" {
|
||||
_, _ = fmt.Fprintln(o.stdout, annotation("warning", manifestRepoPath, "Docs routes removed without a redirect", msg))
|
||||
return
|
||||
}
|
||||
_, _ = fmt.Fprintf(o.stdout, "warning: %s\n", msg)
|
||||
}
|
||||
@@ -0,0 +1,169 @@
|
||||
// Command docsredirectscheck validates docs/redirects.json, the list of
|
||||
// redirects the documentation website applies for pages that have moved or
|
||||
// been removed.
|
||||
//
|
||||
// Each rule is {"source", "destination", "permanent"?}. Sources and internal
|
||||
// destinations are full site paths starting with /docs/, written in the small
|
||||
// subset of Next.js path syntax the website accepts: an exact path, a trailing
|
||||
// "/:name*", or a trailing "/:name(.*)". A destination may also be an
|
||||
// http(s) URL.
|
||||
//
|
||||
// It fails when:
|
||||
//
|
||||
// - the file is not a JSON array of well-formed rules;
|
||||
// - a source or destination is outside the supported syntax;
|
||||
// - an internal destination is not a page in docs/manifest.json;
|
||||
// - a source is a live docs page, which would hide a page readers can reach;
|
||||
// - two rules share a source, or rules form a chain or a loop.
|
||||
//
|
||||
// It also warns, without failing, when a pull request removes routes from
|
||||
// docs/manifest.json and leaves docs/redirects.json alone, since pages are
|
||||
// sometimes removed on purpose.
|
||||
//
|
||||
// A missing docs/redirects.json passes. Everything it checks about routes is
|
||||
// derived from docs/manifest.json the same way the website derives it.
|
||||
//
|
||||
// Usage:
|
||||
//
|
||||
// docsredirectscheck [-manifest path] [-redirects path] [-base rev]
|
||||
//
|
||||
// Exit status is 0 when the file is valid or absent, 1 when it has problems,
|
||||
// and 2 when the check itself could not run.
|
||||
package main
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"flag"
|
||||
"fmt"
|
||||
"io"
|
||||
"io/fs"
|
||||
"os"
|
||||
"os/exec"
|
||||
"strings"
|
||||
|
||||
"golang.org/x/xerrors"
|
||||
)
|
||||
|
||||
type options struct {
|
||||
manifestPath string
|
||||
redirectsPath string
|
||||
// base is the git revision the change under review started from. Empty
|
||||
// means derive it from the environment.
|
||||
base string
|
||||
git gitReader
|
||||
getenv func(string) string
|
||||
stdout io.Writer
|
||||
stderr io.Writer
|
||||
}
|
||||
|
||||
// gitReader is the slice of git the advisory check needs, so tests can supply
|
||||
// a fake repository.
|
||||
type gitReader interface {
|
||||
// HasRev reports whether rev resolves to a commit.
|
||||
HasRev(rev string) bool
|
||||
// Show returns the contents of path at rev.
|
||||
Show(rev, path string) ([]byte, error)
|
||||
// Changed reports whether path differs between rev and the working tree.
|
||||
Changed(rev, path string) (bool, error)
|
||||
}
|
||||
|
||||
type execGit struct{}
|
||||
|
||||
// validRev rejects a revision git could read as an option, so a value passed
|
||||
// with -base cannot add flags to the git commands below. The commands run
|
||||
// without a shell and path is always one of the fixed repository paths.
|
||||
func validRev(rev string) bool {
|
||||
return rev != "" && !strings.HasPrefix(rev, "-")
|
||||
}
|
||||
|
||||
func (execGit) HasRev(rev string) bool {
|
||||
if !validRev(rev) {
|
||||
return false
|
||||
}
|
||||
return exec.Command("git", "rev-parse", "--verify", "--quiet", rev+"^{commit}").Run() == nil //nolint:gosec // rev is checked by validRev and no shell is involved.
|
||||
}
|
||||
|
||||
func (execGit) Show(rev, path string) ([]byte, error) {
|
||||
if !validRev(rev) {
|
||||
return nil, xerrors.Errorf("invalid revision %q", rev)
|
||||
}
|
||||
return exec.Command("git", "show", rev+":"+path).Output() //nolint:gosec // rev is checked by validRev and no shell is involved.
|
||||
}
|
||||
|
||||
func (execGit) Changed(rev, path string) (bool, error) {
|
||||
if !validRev(rev) {
|
||||
return false, xerrors.Errorf("invalid revision %q", rev)
|
||||
}
|
||||
err := exec.Command("git", "diff", "--quiet", rev, "--", path).Run() //nolint:gosec // rev is checked by validRev and no shell is involved.
|
||||
if err == nil {
|
||||
return false, nil
|
||||
}
|
||||
var exitErr *exec.ExitError
|
||||
if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 {
|
||||
return true, nil
|
||||
}
|
||||
return false, err
|
||||
}
|
||||
|
||||
func main() {
|
||||
o := options{
|
||||
git: execGit{},
|
||||
getenv: os.Getenv,
|
||||
stdout: os.Stdout,
|
||||
stderr: os.Stderr,
|
||||
}
|
||||
flag.StringVar(&o.manifestPath, "manifest", manifestRepoPath, "path to the docs manifest")
|
||||
flag.StringVar(&o.redirectsPath, "redirects", redirectsRepoPath, "path to the redirects file")
|
||||
flag.StringVar(&o.base, "base", "", "git revision the change started from, for the removed-route warning (default: the pull request base in GitHub Actions)")
|
||||
flag.Parse()
|
||||
os.Exit(run(o))
|
||||
}
|
||||
|
||||
func run(o options) int {
|
||||
manifestData, err := os.ReadFile(o.manifestPath)
|
||||
if err != nil {
|
||||
_, _ = fmt.Fprintf(o.stderr, "cannot read manifest %s: %v\n", o.manifestPath, err)
|
||||
return 2
|
||||
}
|
||||
m, err := parseManifest(manifestData)
|
||||
if err != nil {
|
||||
_, _ = fmt.Fprintf(o.stderr, "invalid manifest %s: %v\n", o.manifestPath, err)
|
||||
return 2
|
||||
}
|
||||
live := liveRoutes(m)
|
||||
|
||||
var (
|
||||
rules []rule
|
||||
code int
|
||||
)
|
||||
redirectsData, err := os.ReadFile(o.redirectsPath)
|
||||
switch {
|
||||
case errors.Is(err, fs.ErrNotExist):
|
||||
_, _ = fmt.Fprintf(o.stdout, "%s not found; nothing to validate\n", o.redirectsPath)
|
||||
case err != nil:
|
||||
_, _ = fmt.Fprintf(o.stderr, "cannot read %s: %v\n", o.redirectsPath, err)
|
||||
return 2
|
||||
default:
|
||||
var problems []problem
|
||||
rules, problems = checkRedirects(redirectsData, live)
|
||||
if len(problems) > 0 {
|
||||
for _, p := range problems {
|
||||
_, _ = fmt.Fprintln(o.stderr, formatProblem(o.redirectsPath, p))
|
||||
}
|
||||
_, _ = fmt.Fprintf(o.stderr, "%s: %d %s found\n", o.redirectsPath, len(problems), plural(len(problems), "problem", "problems"))
|
||||
code = 1
|
||||
} else {
|
||||
_, _ = fmt.Fprintf(o.stdout, "%s: %d %s OK\n", o.redirectsPath, len(rules), plural(len(rules), "rule", "rules"))
|
||||
}
|
||||
}
|
||||
|
||||
runAdvisory(o, live, rules)
|
||||
return code
|
||||
}
|
||||
|
||||
func plural(n int, one, many string) string {
|
||||
if n == 1 {
|
||||
return one
|
||||
}
|
||||
return many
|
||||
}
|
||||
@@ -0,0 +1,989 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"errors"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
"golang.org/x/xerrors"
|
||||
)
|
||||
|
||||
// liveOf builds a live-route set the way liveRoutes would for a manifest
|
||||
// whose pages are the given routes. "/docs" (the landing page) is always live.
|
||||
func liveOf(routes ...string) map[string]bool {
|
||||
m := map[string]bool{"/docs": true}
|
||||
for _, r := range routes {
|
||||
m[r] = true
|
||||
}
|
||||
return m
|
||||
}
|
||||
|
||||
type wantProblem struct {
|
||||
index int
|
||||
contains string
|
||||
}
|
||||
|
||||
// requireProblems asserts that got holds exactly the wanted problems, in any
|
||||
// order, matching each wanted problem to a distinct reported one by rule index
|
||||
// and message substring.
|
||||
func requireProblems(t *testing.T, got []problem, want ...wantProblem) {
|
||||
t.Helper()
|
||||
|
||||
var msgs []string
|
||||
for _, p := range got {
|
||||
msgs = append(msgs, formatProblem("docs/redirects.json", p))
|
||||
}
|
||||
require.Len(t, got, len(want), "unexpected problems:\n%s", strings.Join(msgs, "\n"))
|
||||
|
||||
used := make([]bool, len(got))
|
||||
for _, w := range want {
|
||||
found := false
|
||||
for i, p := range got {
|
||||
if used[i] || p.index != w.index || !strings.Contains(p.message, w.contains) {
|
||||
continue
|
||||
}
|
||||
used[i] = true
|
||||
found = true
|
||||
break
|
||||
}
|
||||
require.True(t, found, "no problem for rule %d containing %q in:\n%s", w.index, w.contains, strings.Join(msgs, "\n"))
|
||||
}
|
||||
}
|
||||
|
||||
func TestConvertPathToRoute(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// These mirror convertPathToRoute in the docs website, including its
|
||||
// quirks: the README.md, index.md and .md suffixes are stripped one after
|
||||
// another with a plain suffix match, not per path segment.
|
||||
cases := []struct {
|
||||
name string
|
||||
in string
|
||||
want string
|
||||
}{
|
||||
{"plain page", "./admin/external-auth.md", "admin/external-auth"},
|
||||
{"no leading dot slash", "admin/external-auth.md", "admin/external-auth"},
|
||||
{"index page", "./admin/templates/index.md", "admin/templates"},
|
||||
{"README page", "./ai-coder/README.md", "ai-coder"},
|
||||
{"root README", "./README.md", ""},
|
||||
{"deeply nested index", "./a/b/c/d/index.md", "a/b/c/d"},
|
||||
{"leading and trailing slashes trimmed", "/admin/users/", "admin/users"},
|
||||
{"non-markdown path is left alone", "./images/logo.svg", "images/logo.svg"},
|
||||
{"file whose name merely ends in index.md", "./install/my-index.md", "install/my-"},
|
||||
{"file whose name merely ends in README.md", "./install/OLD-README.md", "install/OLD-"},
|
||||
{"empty", "", ""},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
require.Equal(t, tc.want, convertPathToRoute(tc.in))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestDocsPagePath(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
in string
|
||||
want string
|
||||
wantOK bool
|
||||
}{
|
||||
{"empty path has no page", "", "", false},
|
||||
{"root README is the landing page", "./README.md", "/docs", true},
|
||||
{"plain page", "./about.md", "/docs/about", true},
|
||||
{"nested index page", "./admin/templates/index.md", "/docs/admin/templates", true},
|
||||
{"no leading dot slash", "admin/users.md", "/docs/admin/users", true},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
got, ok := docsPagePath(tc.in)
|
||||
require.Equal(t, tc.wantOK, ok)
|
||||
require.Equal(t, tc.want, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestLiveRoutes(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m, err := parseManifest([]byte(`{
|
||||
"versions": ["main"],
|
||||
"routes": [
|
||||
{"title": "About", "path": "./README.md", "children": [
|
||||
{"title": "Architecture", "path": "./about/architecture.md"}
|
||||
]},
|
||||
{"title": "Admin", "path": "./admin/index.md", "children": [
|
||||
{"title": "Users", "path": "admin/users.md"},
|
||||
{"title": "Section heading without a page"},
|
||||
{"title": "Deep", "path": "./admin/a/b.md", "children": [
|
||||
{"title": "Deeper", "path": "./admin/a/b/c/index.md"}
|
||||
]}
|
||||
]}
|
||||
]
|
||||
}`))
|
||||
require.NoError(t, err)
|
||||
|
||||
got := liveRoutes(m)
|
||||
for _, want := range []string{
|
||||
"/docs",
|
||||
// The root About page is also served at /docs/about.
|
||||
"/docs/about",
|
||||
"/docs/about/architecture",
|
||||
"/docs/admin",
|
||||
"/docs/admin/users",
|
||||
"/docs/admin/a/b",
|
||||
"/docs/admin/a/b/c",
|
||||
} {
|
||||
require.True(t, got[want], "expected %s to be live; got %v", want, got)
|
||||
}
|
||||
require.False(t, got["/docs/section-heading-without-a-page"])
|
||||
require.Len(t, got, 7)
|
||||
}
|
||||
|
||||
func TestLiveRoutesAboutSpecialCaseOnlyForRootAbout(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m, err := parseManifest([]byte(`{"routes": [{"title": "Overview", "path": "./README.md"}]}`))
|
||||
require.NoError(t, err)
|
||||
got := liveRoutes(m)
|
||||
require.True(t, got["/docs"])
|
||||
require.False(t, got["/docs/about"], "only a root page titled About is also served at /docs/about")
|
||||
}
|
||||
|
||||
func TestParseManifestErrors(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for name, in := range map[string]string{
|
||||
"not json": "nope",
|
||||
"no routes key": `{"versions": []}`,
|
||||
"routes object": `{"routes": {}}`,
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
_, err := parseManifest([]byte(in))
|
||||
require.Error(t, err)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckRedirectsStructure(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
live := liveOf("/docs/admin/users", "/docs/admin/groups")
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
in string
|
||||
want []wantProblem
|
||||
}{
|
||||
{"empty array is valid", `[]`, nil},
|
||||
{"whitespace and newline only array", "[\n]\n", nil},
|
||||
{
|
||||
"valid rule",
|
||||
`[{"source": "/docs/old", "destination": "/docs/admin/users", "permanent": true}]`,
|
||||
nil,
|
||||
},
|
||||
{
|
||||
"permanent is optional",
|
||||
`[{"source": "/docs/old", "destination": "/docs/admin/users"}]`,
|
||||
nil,
|
||||
},
|
||||
{"not json", `{oops`, []wantProblem{{-1, "not valid JSON"}}},
|
||||
{"empty file", ``, []wantProblem{{-1, "not valid JSON"}}},
|
||||
{"object instead of array", `{"source": "/docs/old"}`, []wantProblem{{-1, "must be a JSON array"}}},
|
||||
{"null", `null`, []wantProblem{{-1, "must be a JSON array"}}},
|
||||
{"entry is not an object", `[1]`, []wantProblem{{0, "must be an object"}}},
|
||||
{"entry is a string", `["/docs/old"]`, []wantProblem{{0, "must be an object"}}},
|
||||
{
|
||||
"missing source",
|
||||
`[{"destination": "/docs/admin/users"}]`,
|
||||
[]wantProblem{{0, `"source" is required`}},
|
||||
},
|
||||
{
|
||||
"empty destination",
|
||||
`[{"source": "/docs/old", "destination": ""}]`,
|
||||
[]wantProblem{{0, `"destination" is required`}},
|
||||
},
|
||||
{
|
||||
"non-string source",
|
||||
`[{"source": 7, "destination": "/docs/admin/users"}]`,
|
||||
[]wantProblem{{0, `"source" is required`}},
|
||||
},
|
||||
{
|
||||
"permanent must be boolean",
|
||||
`[{"source": "/docs/old", "destination": "/docs/admin/users", "permanent": "yes"}]`,
|
||||
[]wantProblem{{0, `"permanent" must be a boolean`}},
|
||||
},
|
||||
{
|
||||
"unknown field",
|
||||
`[{"source": "/docs/old", "destination": "/docs/admin/users", "statusCode": 301}]`,
|
||||
[]wantProblem{{0, `unknown field "statusCode"`}},
|
||||
},
|
||||
{
|
||||
"problems are reported for every bad entry",
|
||||
`[
|
||||
{"source": "/docs/old", "destination": "/docs/admin/users"},
|
||||
5,
|
||||
{"source": "/docs/older", "destination": "/docs/admin/groups", "permanent": 1}
|
||||
]`,
|
||||
[]wantProblem{{1, "must be an object"}, {2, `"permanent" must be a boolean`}},
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
_, got := checkRedirects([]byte(tc.in), live)
|
||||
requireProblems(t, got, tc.want...)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckRedirectsSourceForms(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
live := liveOf("/docs/target")
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
source string
|
||||
want []wantProblem
|
||||
}{
|
||||
{"exact", "/docs/some/old-page", nil},
|
||||
{"exact with underscore and digits", "/docs/security/0001_user_apikeys_invalidation", nil},
|
||||
{"exact with uppercase and dot", "/docs/Some/Old.Page", nil},
|
||||
{"trailing wildcard", "/docs/tutorials/ai-agents/:path*", nil},
|
||||
{"trailing regex wildcard", "/docs/coder-oss/latest/:slug(.*)", nil},
|
||||
{"trailing slash is normalized", "/docs/some/old-page/", nil},
|
||||
{"not under /docs", "/agents", []wantProblem{{0, "source must start with /docs/"}}},
|
||||
{"bare /docs", "/docs", []wantProblem{{0, "source must start with /docs/"}}},
|
||||
{"only /docs/", "/docs/", []wantProblem{{0, "source must start with /docs/"}}},
|
||||
{"relative", "docs/old", []wantProblem{{0, "source must start with /docs/"}}},
|
||||
{"bare parameter without modifier", "/docs/old/:slug", []wantProblem{{0, "unsupported"}}},
|
||||
{"parameter in the middle", "/docs/:slug*/old", []wantProblem{{0, "unsupported"}}},
|
||||
{"version regex", `/docs/@:version(v2\.33\.[0-9]+)/old`, []wantProblem{{0, "unsupported"}}},
|
||||
{"star on its own", "/docs/old/*", []wantProblem{{0, "unsupported"}}},
|
||||
{"query string", "/docs/old?x=1", []wantProblem{{0, "unsupported"}}},
|
||||
{"fragment", "/docs/old#section", []wantProblem{{0, "unsupported"}}},
|
||||
{"group", "/docs/(old|older)", []wantProblem{{0, "unsupported"}}},
|
||||
{"version segment", "/docs/@main/old", []wantProblem{{0, "version segment"}}},
|
||||
{"empty segment", "/docs//old", []wantProblem{{0, "unsupported"}}},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
data := `[{"source": ` + quote(tc.source) + `, "destination": "/docs/target"}]`
|
||||
_, got := checkRedirects([]byte(data), live)
|
||||
requireProblems(t, got, tc.want...)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckRedirectsDestinations(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
live := liveOf("/docs/admin/users", "/docs/ai-coder/agents", "/docs/ai-coder/agents/tools")
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
source string
|
||||
destination string
|
||||
want []wantProblem
|
||||
}{
|
||||
{"live page", "/docs/old", "/docs/admin/users", nil},
|
||||
{"landing page", "/docs/old", "/docs", nil},
|
||||
{"landing page with trailing slash", "/docs/old", "/docs/", nil},
|
||||
{"fragment is ignored", "/docs/old", "/docs/admin/users#sso", nil},
|
||||
{"query is ignored", "/docs/old", "/docs/admin/users?tab=1", nil},
|
||||
{"trailing slash is ignored", "/docs/old", "/docs/admin/users/", nil},
|
||||
{"external https", "/docs/old", "https://example.com/some/page", nil},
|
||||
{"external http", "/docs/old", "http://example.com/", nil},
|
||||
{
|
||||
"missing page",
|
||||
"/docs/old", "/docs/admin/nope",
|
||||
[]wantProblem{{0, `destination "/docs/admin/nope" is not a page`}},
|
||||
},
|
||||
{
|
||||
"destination is a section prefix but not a page",
|
||||
"/docs/old", "/docs/ai-coder",
|
||||
[]wantProblem{{0, "is not a page"}},
|
||||
},
|
||||
{
|
||||
"relative destination",
|
||||
"/docs/old", "admin/users",
|
||||
[]wantProblem{{0, "destination must be a /docs/ path or an http(s) URL"}},
|
||||
},
|
||||
{
|
||||
"protocol-relative destination",
|
||||
"/docs/old", "//evil.example.com/x",
|
||||
[]wantProblem{{0, "destination must be a /docs/ path or an http(s) URL"}},
|
||||
},
|
||||
{
|
||||
"non-http scheme",
|
||||
"/docs/old", "javascript:alert(1)",
|
||||
[]wantProblem{{0, "destination must be a /docs/ path or an http(s) URL"}},
|
||||
},
|
||||
{
|
||||
"external URL without a host",
|
||||
"/docs/old", "https://",
|
||||
[]wantProblem{{0, "invalid URL"}},
|
||||
},
|
||||
{
|
||||
"destination with a version segment",
|
||||
"/docs/old", "/docs/@main/admin/users",
|
||||
[]wantProblem{{0, "version segment"}},
|
||||
},
|
||||
{
|
||||
"pattern destination under an existing section",
|
||||
"/docs/tutorials/ai-agents/:path*", "/docs/ai-coder/:path*",
|
||||
nil,
|
||||
},
|
||||
{
|
||||
"pattern destination at the docs root",
|
||||
"/docs/coder-oss/latest/:slug(.*)", "/docs/:slug",
|
||||
nil,
|
||||
},
|
||||
{
|
||||
"pattern destination whose section does not exist",
|
||||
"/docs/old/:path*", "/docs/nonexistent/:path*",
|
||||
[]wantProblem{{0, `no docs page exists under "/docs/nonexistent"`}},
|
||||
},
|
||||
{
|
||||
"destination parameter the source does not define",
|
||||
"/docs/old/:path*", "/docs/ai-coder/:rest*",
|
||||
[]wantProblem{{0, "does not define"}},
|
||||
},
|
||||
{
|
||||
"exact source with parameter destination",
|
||||
"/docs/old", "/docs/ai-coder/:path*",
|
||||
[]wantProblem{{0, "does not define"}},
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
data := `[{"source": ` + quote(tc.source) + `, "destination": ` + quote(tc.destination) + `}]`
|
||||
_, got := checkRedirects([]byte(data), live)
|
||||
requireProblems(t, got, tc.want...)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckRedirectsShadowing(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
live := liveOf(
|
||||
"/docs/admin/users",
|
||||
"/docs/admin/users/sso",
|
||||
"/docs/ai-coder/agents",
|
||||
"/docs/target",
|
||||
)
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
source string
|
||||
want []wantProblem
|
||||
}{
|
||||
{"exact source is a live page", "/docs/admin/users", []wantProblem{{0, "hides a live docs page"}}},
|
||||
{"exact source with trailing slash is still live", "/docs/admin/users/", []wantProblem{{0, "hides a live docs page"}}},
|
||||
{"exact source that is not live", "/docs/admin/old-users", nil},
|
||||
{"wildcard whose prefix is a live page", "/docs/admin/users/:path*", []wantProblem{{0, "hides a live docs page"}}},
|
||||
{"wildcard above live pages", "/docs/admin/:path*", []wantProblem{{0, "hides a live docs page"}}},
|
||||
{"regex wildcard above live pages", "/docs/admin/users/:slug(.*)", []wantProblem{{0, "hides a live docs page"}}},
|
||||
{"wildcard over a prefix with no live pages", "/docs/retired/:path*", nil},
|
||||
{"regex wildcard does not match its bare prefix", "/docs/ai-coder/agents/:slug(.*)", nil},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
data := `[{"source": ` + quote(tc.source) + `, "destination": "/docs/target"}]`
|
||||
_, got := checkRedirects([]byte(data), live)
|
||||
requireProblems(t, got, tc.want...)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckRedirectsDuplicatesChainsAndLoops(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
live := liveOf("/docs/a", "/docs/b", "/docs/c", "/docs/final")
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
in string
|
||||
want []wantProblem
|
||||
}{
|
||||
{
|
||||
"independent rules are fine",
|
||||
`[
|
||||
{"source": "/docs/old1", "destination": "/docs/a"},
|
||||
{"source": "/docs/old2", "destination": "/docs/b"}
|
||||
]`,
|
||||
nil,
|
||||
},
|
||||
{
|
||||
"two rules may share a destination",
|
||||
`[
|
||||
{"source": "/docs/old1", "destination": "/docs/final"},
|
||||
{"source": "/docs/old2", "destination": "/docs/final"}
|
||||
]`,
|
||||
nil,
|
||||
},
|
||||
{
|
||||
"duplicate exact source",
|
||||
`[
|
||||
{"source": "/docs/old", "destination": "/docs/a"},
|
||||
{"source": "/docs/old", "destination": "/docs/b"}
|
||||
]`,
|
||||
[]wantProblem{{1, "duplicate of rule [0]"}},
|
||||
},
|
||||
{
|
||||
"duplicate source differing only by trailing slash",
|
||||
`[
|
||||
{"source": "/docs/old", "destination": "/docs/a"},
|
||||
{"source": "/docs/old/", "destination": "/docs/b"}
|
||||
]`,
|
||||
[]wantProblem{{1, "duplicate of rule [0]"}},
|
||||
},
|
||||
{
|
||||
"duplicate pattern source",
|
||||
`[
|
||||
{"source": "/docs/old/:path*", "destination": "/docs/a/:path*"},
|
||||
{"source": "/docs/old/:path*", "destination": "/docs/b/:path*"}
|
||||
]`,
|
||||
[]wantProblem{{1, "duplicate of rule [0]"}},
|
||||
},
|
||||
{
|
||||
"exact and pattern with the same prefix are not duplicates",
|
||||
`[
|
||||
{"source": "/docs/old", "destination": "/docs/a"},
|
||||
{"source": "/docs/old/:slug(.*)", "destination": "/docs/:slug"}
|
||||
]`,
|
||||
nil,
|
||||
},
|
||||
{
|
||||
"redirect to itself",
|
||||
`[{"source": "/docs/old", "destination": "/docs/old"}]`,
|
||||
[]wantProblem{{0, "redirects to itself"}, {0, "is not a page"}},
|
||||
},
|
||||
{
|
||||
"chain through an exact source",
|
||||
`[
|
||||
{"source": "/docs/old", "destination": "/docs/older"},
|
||||
{"source": "/docs/older", "destination": "/docs/final"}
|
||||
]`,
|
||||
[]wantProblem{{0, "redirect chain"}, {0, "is not a page"}},
|
||||
},
|
||||
{
|
||||
"chain through a wildcard source",
|
||||
`[
|
||||
{"source": "/docs/old", "destination": "/docs/retired/page"},
|
||||
{"source": "/docs/retired/:path*", "destination": "/docs/final"}
|
||||
]`,
|
||||
[]wantProblem{{0, "redirect chain"}, {0, "is not a page"}},
|
||||
},
|
||||
{
|
||||
"every rule that redirects into another rule is reported",
|
||||
`[
|
||||
{"source": "/docs/r1", "destination": "/docs/r2"},
|
||||
{"source": "/docs/r2", "destination": "/docs/r3"},
|
||||
{"source": "/docs/r3", "destination": "/docs/final"}
|
||||
]`,
|
||||
[]wantProblem{
|
||||
{0, "redirect chain"},
|
||||
{0, "is not a page"},
|
||||
{1, "redirect chain"},
|
||||
{1, "is not a page"},
|
||||
},
|
||||
},
|
||||
{
|
||||
"two-rule loop",
|
||||
`[
|
||||
{"source": "/docs/x", "destination": "/docs/y"},
|
||||
{"source": "/docs/y", "destination": "/docs/x"}
|
||||
]`,
|
||||
[]wantProblem{
|
||||
{0, "redirect loop"},
|
||||
{0, "is not a page"},
|
||||
{1, "is not a page"},
|
||||
},
|
||||
},
|
||||
{
|
||||
"three-rule loop",
|
||||
`[
|
||||
{"source": "/docs/x", "destination": "/docs/y"},
|
||||
{"source": "/docs/y", "destination": "/docs/z"},
|
||||
{"source": "/docs/z", "destination": "/docs/x"}
|
||||
]`,
|
||||
[]wantProblem{
|
||||
{0, "redirect loop"},
|
||||
{0, "is not a page"},
|
||||
{1, "is not a page"},
|
||||
{2, "is not a page"},
|
||||
},
|
||||
},
|
||||
{
|
||||
"external destinations never chain",
|
||||
`[
|
||||
{"source": "/docs/old", "destination": "https://example.com/docs/older"},
|
||||
{"source": "/docs/older", "destination": "/docs/final"}
|
||||
]`,
|
||||
nil,
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
_, got := checkRedirects([]byte(tc.in), live)
|
||||
requireProblems(t, got, tc.want...)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestChainMessageNamesTheFinalPage(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
live := liveOf("/docs/final")
|
||||
_, got := checkRedirects([]byte(`[
|
||||
{"source": "/docs/old", "destination": "/docs/older"},
|
||||
{"source": "/docs/older", "destination": "/docs/final"}
|
||||
]`), live)
|
||||
|
||||
var chain *problem
|
||||
for i := range got {
|
||||
if strings.Contains(got[i].message, "redirect chain") {
|
||||
chain = &got[i]
|
||||
}
|
||||
}
|
||||
require.NotNil(t, chain, "expected a chain problem, got %v", got)
|
||||
require.Equal(t, 0, chain.index)
|
||||
require.Contains(t, chain.message, "/docs/older")
|
||||
require.Contains(t, chain.message, "/docs/final", "the message should tell the author where to point the rule instead")
|
||||
}
|
||||
|
||||
func TestFormatProblem(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
require.Equal(t,
|
||||
`docs/redirects.json: must be a JSON array of rules`,
|
||||
formatProblem("docs/redirects.json", problem{index: -1, message: "must be a JSON array of rules"}),
|
||||
)
|
||||
require.Equal(t,
|
||||
`docs/redirects.json[3] (source "/docs/x"): bad`,
|
||||
formatProblem("docs/redirects.json", problem{index: 3, source: "/docs/x", message: "bad"}),
|
||||
)
|
||||
require.Equal(t,
|
||||
`docs/redirects.json[3]: bad`,
|
||||
formatProblem("docs/redirects.json", problem{index: 3, message: "bad"}),
|
||||
)
|
||||
}
|
||||
|
||||
func TestRuleMatches(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
live := liveOf("/docs/target")
|
||||
rules, problems := checkRedirects([]byte(`[
|
||||
{"source": "/docs/exact", "destination": "/docs/target"},
|
||||
{"source": "/docs/tail/:path*", "destination": "/docs/target"},
|
||||
{"source": "/docs/rx/:slug(.*)", "destination": "/docs/target"}
|
||||
]`), live)
|
||||
require.Empty(t, problems)
|
||||
require.Len(t, rules, 3)
|
||||
|
||||
cases := []struct {
|
||||
rule int
|
||||
path string
|
||||
want bool
|
||||
}{
|
||||
{0, "/docs/exact", true},
|
||||
{0, "/docs/exact/", true},
|
||||
{0, "/docs/exact/child", false},
|
||||
{1, "/docs/tail", true},
|
||||
{1, "/docs/tail/a/b", true},
|
||||
{1, "/docs/tailored", false},
|
||||
{2, "/docs/rx", false},
|
||||
{2, "/docs/rx/a", true},
|
||||
{2, "/docs/rx/a/b", true},
|
||||
{2, "/docs/rxa", false},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
require.Equal(t, tc.want, rules[tc.rule].matches(tc.path), "rule %d vs %s", tc.rule, tc.path)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRemovedRoutes(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
base := liveOf("/docs/a", "/docs/b", "/docs/c")
|
||||
head := liveOf("/docs/a", "/docs/c", "/docs/d")
|
||||
require.Equal(t, []string{"/docs/b"}, removedRoutes(base, head))
|
||||
|
||||
// A file rename that keeps the public URL (index.md to a flat file) does
|
||||
// not change the route set, so it is not a removal.
|
||||
require.Empty(t, removedRoutes(base, liveOf("/docs/a", "/docs/b", "/docs/c")))
|
||||
require.Empty(t, removedRoutes(nil, head))
|
||||
}
|
||||
|
||||
func TestUncoveredRoutes(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
live := liveOf("/docs/target")
|
||||
rules, problems := checkRedirects([]byte(`[
|
||||
{"source": "/docs/moved", "destination": "/docs/target"},
|
||||
{"source": "/docs/tutorials/:path*", "destination": "/docs/target"}
|
||||
]`), live)
|
||||
require.Empty(t, problems)
|
||||
|
||||
removed := []string{"/docs/moved", "/docs/tutorials/intro", "/docs/gone"}
|
||||
require.Equal(t, []string{"/docs/gone"}, uncoveredRoutes(removed, rules))
|
||||
require.Equal(t, removed, uncoveredRoutes(removed, nil))
|
||||
}
|
||||
|
||||
// ---- run (CLI behavior) ----
|
||||
|
||||
type fakeGit struct {
|
||||
revs map[string]bool
|
||||
files map[string]string // "rev:path" -> contents
|
||||
changed map[string]bool // "rev:path" -> changed since rev
|
||||
showErr error
|
||||
}
|
||||
|
||||
func (f *fakeGit) HasRev(rev string) bool { return f.revs[rev] }
|
||||
|
||||
func (f *fakeGit) Show(rev, path string) ([]byte, error) {
|
||||
if f.showErr != nil {
|
||||
return nil, f.showErr
|
||||
}
|
||||
s, ok := f.files[rev+":"+path]
|
||||
if !ok {
|
||||
return nil, xerrors.New("not in revision")
|
||||
}
|
||||
return []byte(s), nil
|
||||
}
|
||||
|
||||
func (f *fakeGit) Changed(rev, path string) (bool, error) {
|
||||
return f.changed[rev+":"+path], nil
|
||||
}
|
||||
|
||||
const manifestTwoPages = `{"routes": [
|
||||
{"title": "About", "path": "./README.md"},
|
||||
{"title": "Users", "path": "./admin/users.md"},
|
||||
{"title": "Groups", "path": "./admin/groups.md"}
|
||||
]}`
|
||||
|
||||
const manifestOnePage = `{"routes": [
|
||||
{"title": "About", "path": "./README.md"},
|
||||
{"title": "Users", "path": "./admin/users.md"}
|
||||
]}`
|
||||
|
||||
type runEnv struct {
|
||||
dir string
|
||||
manifestPath string
|
||||
redirectsPath string
|
||||
stdout bytes.Buffer
|
||||
stderr bytes.Buffer
|
||||
}
|
||||
|
||||
func newRunEnv(t *testing.T, manifest, redirects string) *runEnv {
|
||||
t.Helper()
|
||||
dir := t.TempDir()
|
||||
e := &runEnv{
|
||||
dir: dir,
|
||||
manifestPath: filepath.Join(dir, "manifest.json"),
|
||||
redirectsPath: filepath.Join(dir, "redirects.json"),
|
||||
}
|
||||
if manifest != "" {
|
||||
require.NoError(t, os.WriteFile(e.manifestPath, []byte(manifest), 0o600))
|
||||
}
|
||||
if redirects != "" {
|
||||
require.NoError(t, os.WriteFile(e.redirectsPath, []byte(redirects), 0o600))
|
||||
}
|
||||
return e
|
||||
}
|
||||
|
||||
func (e *runEnv) run(base string, git gitReader, env map[string]string) int {
|
||||
return run(options{
|
||||
manifestPath: e.manifestPath,
|
||||
redirectsPath: e.redirectsPath,
|
||||
base: base,
|
||||
git: git,
|
||||
getenv: func(k string) string { return env[k] },
|
||||
stdout: &e.stdout,
|
||||
stderr: &e.stderr,
|
||||
})
|
||||
}
|
||||
|
||||
func TestRunAbsentRedirectsFilePasses(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
e := newRunEnv(t, manifestTwoPages, "")
|
||||
require.Equal(t, 0, e.run("", &fakeGit{}, nil))
|
||||
require.Contains(t, e.stdout.String(), "not found")
|
||||
require.Empty(t, e.stderr.String())
|
||||
}
|
||||
|
||||
func TestRunValidRedirectsPasses(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
e := newRunEnv(t, manifestTwoPages, `[{"source": "/docs/old", "destination": "/docs/admin/users"}]`)
|
||||
require.Equal(t, 0, e.run("", &fakeGit{}, nil))
|
||||
require.Empty(t, e.stderr.String())
|
||||
require.Contains(t, e.stdout.String(), "1 rule")
|
||||
}
|
||||
|
||||
func TestRunInvalidRedirectsFailsWithActionableMessages(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
e := newRunEnv(t, manifestTwoPages, `[
|
||||
{"source": "/docs/old", "destination": "/docs/admin/nope"},
|
||||
{"source": "/docs/admin/users", "destination": "/docs/admin/groups"}
|
||||
]`)
|
||||
require.Equal(t, 1, e.run("", &fakeGit{}, nil))
|
||||
out := e.stderr.String()
|
||||
require.Contains(t, out, `[0] (source "/docs/old")`)
|
||||
require.Contains(t, out, `"/docs/admin/nope" is not a page`)
|
||||
require.Contains(t, out, `[1] (source "/docs/admin/users")`)
|
||||
require.Contains(t, out, "hides a live docs page")
|
||||
}
|
||||
|
||||
func TestRunMissingManifestIsAnError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
e := newRunEnv(t, "", `[]`)
|
||||
require.Equal(t, 2, e.run("", &fakeGit{}, nil))
|
||||
require.Contains(t, e.stderr.String(), "manifest")
|
||||
}
|
||||
|
||||
func TestRunInvalidManifestIsAnError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
e := newRunEnv(t, `{"routes": "nope"}`, `[]`)
|
||||
require.Equal(t, 2, e.run("", &fakeGit{}, nil))
|
||||
require.Contains(t, e.stderr.String(), "manifest")
|
||||
}
|
||||
|
||||
func TestRunUnreadableRedirectsFileIsAnError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
e := newRunEnv(t, manifestTwoPages, "")
|
||||
// A directory where the file should be: present but unreadable as a file.
|
||||
require.NoError(t, os.Mkdir(e.redirectsPath, 0o700))
|
||||
require.Equal(t, 2, e.run("", &fakeGit{}, nil))
|
||||
}
|
||||
|
||||
func advisoryGit(redirectsChanged bool) *fakeGit {
|
||||
return &fakeGit{
|
||||
revs: map[string]bool{"HEAD^1": true, "abc123": true},
|
||||
files: map[string]string{"HEAD^1:docs/manifest.json": manifestTwoPages},
|
||||
changed: map[string]bool{"HEAD^1:docs/redirects.json": redirectsChanged},
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunAdvisoryWarnsWhenRoutesRemovedWithoutRedirects(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
e := newRunEnv(t, manifestOnePage, "")
|
||||
code := e.run("", advisoryGit(false), map[string]string{"GITHUB_BASE_REF": "main"})
|
||||
require.Equal(t, 0, code, "the advisory check must never fail the run")
|
||||
out := e.stdout.String() + e.stderr.String()
|
||||
require.Contains(t, out, "/docs/admin/groups")
|
||||
require.Contains(t, out, "docs/redirects.json")
|
||||
}
|
||||
|
||||
func TestRunAdvisoryUsesGitHubAnnotationsInActions(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
e := newRunEnv(t, manifestOnePage, "")
|
||||
code := e.run("", advisoryGit(false), map[string]string{
|
||||
"GITHUB_BASE_REF": "main",
|
||||
"GITHUB_ACTIONS": "true",
|
||||
})
|
||||
require.Equal(t, 0, code)
|
||||
var annotation string
|
||||
for _, line := range strings.Split(e.stdout.String()+e.stderr.String(), "\n") {
|
||||
if strings.HasPrefix(line, "::warning ") {
|
||||
annotation = line
|
||||
}
|
||||
}
|
||||
require.NotEmpty(t, annotation, "expected a ::warning annotation")
|
||||
// A workflow command ends at the newline, so the whole message has to be on
|
||||
// the same line.
|
||||
require.Contains(t, annotation, "file=docs/manifest.json")
|
||||
require.Contains(t, annotation, "/docs/admin/groups")
|
||||
require.Contains(t, annotation, "docs/redirects.json")
|
||||
}
|
||||
|
||||
func TestRunAdvisoryQuietWhenRedirectsFileChanged(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
e := newRunEnv(t, manifestOnePage, `[{"source": "/docs/admin/groups", "destination": "/docs/admin/users"}]`)
|
||||
code := e.run("", advisoryGit(true), map[string]string{"GITHUB_BASE_REF": "main"})
|
||||
require.Equal(t, 0, code)
|
||||
require.NotContains(t, e.stdout.String()+e.stderr.String(), "warning")
|
||||
}
|
||||
|
||||
func TestRunAdvisoryQuietWhenRedirectsFileChangedForAnotherRule(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// The change edits docs/redirects.json but no rule covers the removed
|
||||
// route. The author is already working with redirects in this change, so
|
||||
// the advisory stays quiet; it only nudges changes that never touch the
|
||||
// file. Without the "file changed" check this would warn.
|
||||
e := newRunEnv(t, manifestOnePage, `[{"source": "/docs/unrelated", "destination": "/docs/admin/users"}]`)
|
||||
code := e.run("", advisoryGit(true), map[string]string{"GITHUB_BASE_REF": "main"})
|
||||
require.Equal(t, 0, code)
|
||||
require.NotContains(t, e.stdout.String()+e.stderr.String(), "warning")
|
||||
}
|
||||
|
||||
func TestRunAdvisoryQuietWhenRemovedRouteAlreadyRedirected(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// The redirects file did not change in this PR, but it already covers the
|
||||
// removed route.
|
||||
e := newRunEnv(t, manifestOnePage, `[{"source": "/docs/admin/groups", "destination": "/docs/admin/users"}]`)
|
||||
code := e.run("", advisoryGit(false), map[string]string{"GITHUB_BASE_REF": "main"})
|
||||
require.Equal(t, 0, code)
|
||||
require.NotContains(t, e.stdout.String()+e.stderr.String(), "warning")
|
||||
}
|
||||
|
||||
func TestRunAdvisoryQuietWhenNoRouteWasRemoved(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Same routes before and after, for example a file rename that keeps its
|
||||
// URL.
|
||||
e := newRunEnv(t, manifestTwoPages, "")
|
||||
code := e.run("", advisoryGit(false), map[string]string{"GITHUB_BASE_REF": "main"})
|
||||
require.Equal(t, 0, code)
|
||||
require.NotContains(t, e.stdout.String()+e.stderr.String(), "warning")
|
||||
}
|
||||
|
||||
func TestRunAdvisorySkippedWithoutABase(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
e := newRunEnv(t, manifestOnePage, "")
|
||||
code := e.run("", advisoryGit(false), nil)
|
||||
require.Equal(t, 0, code)
|
||||
out := e.stdout.String() + e.stderr.String()
|
||||
require.NotContains(t, out, "::warning")
|
||||
require.Contains(t, out, "skipped")
|
||||
}
|
||||
|
||||
func TestRunAdvisorySkippedWhenBaseRevisionIsMissing(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// A shallow checkout has no HEAD^1.
|
||||
g := advisoryGit(false)
|
||||
g.revs = map[string]bool{}
|
||||
e := newRunEnv(t, manifestOnePage, "")
|
||||
code := e.run("", g, map[string]string{"GITHUB_BASE_REF": "main"})
|
||||
require.Equal(t, 0, code)
|
||||
out := e.stdout.String() + e.stderr.String()
|
||||
require.Contains(t, out, "skipped")
|
||||
require.NotContains(t, out, "::warning")
|
||||
}
|
||||
|
||||
func TestRunAdvisorySkippedWhenBaseHasNoManifest(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
g := advisoryGit(false)
|
||||
g.files = map[string]string{}
|
||||
e := newRunEnv(t, manifestOnePage, "")
|
||||
code := e.run("", g, map[string]string{"GITHUB_BASE_REF": "main"})
|
||||
require.Equal(t, 0, code)
|
||||
require.Contains(t, e.stdout.String()+e.stderr.String(), "skipped")
|
||||
}
|
||||
|
||||
func TestRunAdvisoryHonorsExplicitBase(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
g := advisoryGit(false)
|
||||
g.files = map[string]string{"abc123:docs/manifest.json": manifestTwoPages}
|
||||
g.changed = map[string]bool{}
|
||||
e := newRunEnv(t, manifestOnePage, "")
|
||||
code := e.run("abc123", g, nil)
|
||||
require.Equal(t, 0, code)
|
||||
require.Contains(t, e.stdout.String()+e.stderr.String(), "/docs/admin/groups")
|
||||
}
|
||||
|
||||
func TestRunAdvisoryStillRunsWhenRedirectsFileIsInvalid(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
e := newRunEnv(t, manifestOnePage, `{oops`)
|
||||
code := e.run("", advisoryGit(false), map[string]string{"GITHUB_BASE_REF": "main"})
|
||||
require.Equal(t, 1, code)
|
||||
require.Contains(t, e.stderr.String(), "not valid JSON")
|
||||
}
|
||||
|
||||
func TestExecGitRejectsOptionLikeRevisions(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// A value passed with -base must never be read by git as an option. These
|
||||
// return before any git process starts.
|
||||
g := execGit{}
|
||||
require.False(t, g.HasRev(""))
|
||||
require.False(t, g.HasRev("--output=somefile"))
|
||||
_, err := g.Show("--help", manifestRepoPath)
|
||||
require.Error(t, err)
|
||||
_, err = g.Changed("-p", redirectsRepoPath)
|
||||
require.Error(t, err)
|
||||
}
|
||||
|
||||
// TestRepositoryManifest guards the route logic against the real manifest:
|
||||
// it must parse, and well-known pages must come out as live routes.
|
||||
func TestRepositoryManifest(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
data, err := os.ReadFile(filepath.Join("..", "..", "docs", "manifest.json"))
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
t.Skip("docs/manifest.json not found relative to the test")
|
||||
}
|
||||
require.NoError(t, err)
|
||||
|
||||
m, err := parseManifest(data)
|
||||
require.NoError(t, err)
|
||||
live := liveRoutes(m)
|
||||
require.Greater(t, len(live), 300)
|
||||
require.True(t, live["/docs"])
|
||||
require.True(t, live["/docs/about"])
|
||||
require.True(t, live["/docs/admin/external-auth"])
|
||||
}
|
||||
|
||||
// TestRepositoryRedirectsFile runs the real check against the repository's
|
||||
// own files, so a bad edit to docs/redirects.json fails `go test` as well as
|
||||
// `make lint`. It passes when the file does not exist.
|
||||
func TestRepositoryRedirectsFile(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
manifest, err := os.ReadFile(filepath.Join("..", "..", "docs", "manifest.json"))
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
t.Skip("docs/manifest.json not found relative to the test")
|
||||
}
|
||||
require.NoError(t, err)
|
||||
redirects, err := os.ReadFile(filepath.Join("..", "..", "docs", "redirects.json"))
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
t.Skip("docs/redirects.json does not exist yet")
|
||||
}
|
||||
require.NoError(t, err)
|
||||
|
||||
m, err := parseManifest(manifest)
|
||||
require.NoError(t, err)
|
||||
_, problems := checkRedirects(redirects, liveRoutes(m))
|
||||
var msgs []string
|
||||
for _, p := range problems {
|
||||
msgs = append(msgs, formatProblem("docs/redirects.json", p))
|
||||
}
|
||||
require.Empty(t, problems, strings.Join(msgs, "\n"))
|
||||
}
|
||||
|
||||
// quote renders a string as a JSON string literal for a test fixture. The
|
||||
// fixtures are ASCII, where Go and JSON string escaping agree.
|
||||
func quote(s string) string {
|
||||
return strconv.Quote(s)
|
||||
}
|
||||
@@ -0,0 +1,88 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"strings"
|
||||
|
||||
"golang.org/x/xerrors"
|
||||
)
|
||||
|
||||
// docsRoot is the public path every docs page lives under.
|
||||
const docsRoot = "/docs"
|
||||
|
||||
// convertPathToRoute turns a manifest page path into the URL slug the docs
|
||||
// website serves it at, for example "./admin/external-auth.md" becomes
|
||||
// "admin/external-auth" and "./ai-coder/index.md" becomes "ai-coder".
|
||||
//
|
||||
// It mirrors the docs website's convertPathToRoute exactly, including its
|
||||
// quirk: the README.md, index.md and .md suffixes are stripped one after
|
||||
// another with a plain suffix match, not per path segment, so a file named
|
||||
// "my-index.md" becomes the slug "my-". The check has to agree with the
|
||||
// website about which URLs exist, so the quirk is kept on purpose.
|
||||
func convertPathToRoute(pathStr string) string {
|
||||
route := strings.TrimPrefix(pathStr, "./")
|
||||
for _, suffix := range []string{"README.md", "index.md", ".md"} {
|
||||
route = strings.TrimSuffix(route, suffix)
|
||||
}
|
||||
route = strings.TrimLeft(route, "/")
|
||||
return strings.TrimRight(route, "/")
|
||||
}
|
||||
|
||||
// docsPagePath returns the public path of a manifest page, or false when the
|
||||
// manifest entry has no page of its own (a section heading).
|
||||
func docsPagePath(routePath string) (string, bool) {
|
||||
if routePath == "" {
|
||||
return "", false
|
||||
}
|
||||
slug := convertPathToRoute(routePath)
|
||||
if slug == "" {
|
||||
return docsRoot, true
|
||||
}
|
||||
return docsRoot + "/" + slug, true
|
||||
}
|
||||
|
||||
type manifestRoute struct {
|
||||
Title string `json:"title"`
|
||||
Path string `json:"path"`
|
||||
Children []manifestRoute `json:"children"`
|
||||
}
|
||||
|
||||
type manifest struct {
|
||||
Routes []manifestRoute `json:"routes"`
|
||||
}
|
||||
|
||||
func parseManifest(data []byte) (manifest, error) {
|
||||
var m manifest
|
||||
if err := json.Unmarshal(data, &m); err != nil {
|
||||
return manifest{}, xerrors.Errorf("invalid JSON: %w", err)
|
||||
}
|
||||
if m.Routes == nil {
|
||||
return manifest{}, xerrors.New(`no "routes" array`)
|
||||
}
|
||||
return m, nil
|
||||
}
|
||||
|
||||
// liveRoutes returns every public path the docs website serves for the
|
||||
// manifest. The website prerenders a page for every manifest entry that has a
|
||||
// path, whether or not it is shown in the sidebar, so all of them count.
|
||||
//
|
||||
// Two routes exist without a matching manifest entry of their own: the landing
|
||||
// page at /docs, and /docs/about, which the website also serves for the root
|
||||
// page when it is titled "About".
|
||||
func liveRoutes(m manifest) map[string]bool {
|
||||
live := map[string]bool{docsRoot: true}
|
||||
var walk func(routes []manifestRoute)
|
||||
walk = func(routes []manifestRoute) {
|
||||
for _, r := range routes {
|
||||
if p, ok := docsPagePath(r.Path); ok {
|
||||
live[p] = true
|
||||
if p == docsRoot && r.Title == "About" {
|
||||
live[docsRoot+"/about"] = true
|
||||
}
|
||||
}
|
||||
walk(r.Children)
|
||||
}
|
||||
}
|
||||
walk(m.Routes)
|
||||
return live
|
||||
}
|
||||
@@ -0,0 +1,516 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"cmp"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/url"
|
||||
"regexp"
|
||||
"slices"
|
||||
"strings"
|
||||
)
|
||||
|
||||
type patternKind int
|
||||
|
||||
const (
|
||||
// kindExact matches one path.
|
||||
kindExact patternKind = iota
|
||||
// kindWildcard is "/prefix/:name*". It matches the prefix itself and
|
||||
// everything beneath it.
|
||||
kindWildcard
|
||||
// kindRegexWildcard is "/prefix/:name(.*)". It matches everything beneath
|
||||
// the prefix but not the bare prefix.
|
||||
kindRegexWildcard
|
||||
)
|
||||
|
||||
// pattern is a source or destination path in the small subset of Next.js
|
||||
// path syntax the docs website accepts.
|
||||
type pattern struct {
|
||||
kind patternKind
|
||||
// prefix is the static part of the path with no trailing slash. For an
|
||||
// exact pattern it is the whole path.
|
||||
prefix string
|
||||
// param is the parameter name of a wildcard pattern, empty for exact.
|
||||
param string
|
||||
}
|
||||
|
||||
func (p pattern) matches(path string) bool {
|
||||
path = normalizePath(path)
|
||||
switch p.kind {
|
||||
case kindWildcard:
|
||||
return path == p.prefix || strings.HasPrefix(path, p.prefix+"/")
|
||||
case kindRegexWildcard:
|
||||
return strings.HasPrefix(path, p.prefix+"/")
|
||||
default:
|
||||
return path == p.prefix
|
||||
}
|
||||
}
|
||||
|
||||
// key identifies a source for duplicate detection. The parameter name is left
|
||||
// out because two patterns that differ only by it match the same paths.
|
||||
func (p pattern) key() string {
|
||||
return fmt.Sprintf("%d %s", p.kind, p.prefix)
|
||||
}
|
||||
|
||||
type destination struct {
|
||||
external bool
|
||||
// raw is the destination as written, for messages.
|
||||
raw string
|
||||
// pattern is the path part of an internal destination. The query string
|
||||
// and fragment are dropped because they do not change which page it is.
|
||||
pattern pattern
|
||||
// valid is false when the destination could not be parsed, in which case
|
||||
// no cross-rule check uses it.
|
||||
valid bool
|
||||
}
|
||||
|
||||
// rule is one validated entry of docs/redirects.json.
|
||||
type rule struct {
|
||||
index int
|
||||
source string
|
||||
destination string
|
||||
src pattern
|
||||
dst destination
|
||||
}
|
||||
|
||||
func (r rule) matches(path string) bool {
|
||||
return r.src.matches(path)
|
||||
}
|
||||
|
||||
// problem is one thing wrong with docs/redirects.json. index is the position
|
||||
// of the offending rule in the array, or -1 for a problem with the whole file.
|
||||
type problem struct {
|
||||
index int
|
||||
source string
|
||||
message string
|
||||
}
|
||||
|
||||
func formatProblem(file string, p problem) string {
|
||||
switch {
|
||||
case p.index < 0:
|
||||
return fmt.Sprintf("%s: %s", file, p.message)
|
||||
case p.source != "":
|
||||
return fmt.Sprintf("%s[%d] (source %q): %s", file, p.index, p.source, p.message)
|
||||
default:
|
||||
return fmt.Sprintf("%s[%d]: %s", file, p.index, p.message)
|
||||
}
|
||||
}
|
||||
|
||||
var (
|
||||
// staticSegment is one literal path segment.
|
||||
staticSegment = regexp.MustCompile(`^[A-Za-z0-9._~%+-]+$`)
|
||||
// versionSegment is a literal "@ref" segment. The docs website adds the
|
||||
// version to a URL itself, so rules are written without it.
|
||||
versionSegment = regexp.MustCompile(`^@[A-Za-z0-9._~%+-]+$`)
|
||||
// sourceWildcard matches ":name*".
|
||||
sourceWildcard = regexp.MustCompile(`^:([A-Za-z_][A-Za-z0-9_]*)\*$`)
|
||||
// sourceRegexWildcard matches ":name(.*)".
|
||||
sourceRegexWildcard = regexp.MustCompile(`^:([A-Za-z_][A-Za-z0-9_]*)\(\.\*\)$`)
|
||||
// destinationParam matches ":name" and ":name*".
|
||||
destinationParam = regexp.MustCompile(`^:([A-Za-z_][A-Za-z0-9_]*)\*?$`)
|
||||
)
|
||||
|
||||
// normalizePath drops trailing slashes so "/docs/a/" and "/docs/a" compare
|
||||
// equal, the way the website routes them.
|
||||
func normalizePath(p string) string {
|
||||
return strings.TrimRight(p, "/")
|
||||
}
|
||||
|
||||
// pathRole says whether a path is being parsed as a rule's source or its
|
||||
// destination, because the two accept different pattern syntax.
|
||||
type pathRole int
|
||||
|
||||
const (
|
||||
roleSource pathRole = iota
|
||||
roleDestination
|
||||
)
|
||||
|
||||
// parsePath parses a /docs/... path. Sources allow an exact path, a trailing
|
||||
// "/:name*" and a trailing "/:name(.*)". Destinations allow an exact path and
|
||||
// a trailing "/:name" or "/:name*". It returns a message describing the
|
||||
// problem when the path is outside that subset.
|
||||
func parsePath(path string, role pathRole) (pattern, string) {
|
||||
segments := strings.Split(strings.TrimPrefix(path, "/"), "/")
|
||||
last := len(segments) - 1
|
||||
|
||||
unsupported := func(why string) string {
|
||||
if role == roleDestination {
|
||||
return fmt.Sprintf("unsupported destination %q (%s): use an exact path or a trailing /:name or /:name*", path, why)
|
||||
}
|
||||
return fmt.Sprintf("unsupported source %q (%s): use an exact path, a trailing /:name*, or a trailing /:name(.*)", path, why)
|
||||
}
|
||||
|
||||
for _, seg := range segments[:last] {
|
||||
if err := checkStaticSegment(seg, path, unsupported); err != "" {
|
||||
return pattern{}, err
|
||||
}
|
||||
}
|
||||
|
||||
lastSeg := segments[last]
|
||||
prefix := "/" + strings.Join(segments[:last], "/")
|
||||
if last == 0 {
|
||||
prefix = ""
|
||||
}
|
||||
|
||||
if role == roleDestination {
|
||||
if m := destinationParam.FindStringSubmatch(lastSeg); m != nil && last > 0 {
|
||||
return pattern{kind: kindWildcard, prefix: prefix, param: m[1]}, ""
|
||||
}
|
||||
} else {
|
||||
if m := sourceWildcard.FindStringSubmatch(lastSeg); m != nil && last > 0 {
|
||||
return pattern{kind: kindWildcard, prefix: prefix, param: m[1]}, ""
|
||||
}
|
||||
if m := sourceRegexWildcard.FindStringSubmatch(lastSeg); m != nil && last > 0 {
|
||||
return pattern{kind: kindRegexWildcard, prefix: prefix, param: m[1]}, ""
|
||||
}
|
||||
}
|
||||
|
||||
if err := checkStaticSegment(lastSeg, path, unsupported); err != "" {
|
||||
return pattern{}, err
|
||||
}
|
||||
return pattern{kind: kindExact, prefix: "/" + strings.Join(segments, "/")}, ""
|
||||
}
|
||||
|
||||
func checkStaticSegment(seg, path string, unsupported func(string) string) string {
|
||||
switch {
|
||||
case seg == "":
|
||||
return unsupported("empty path segment")
|
||||
case versionSegment.MatchString(seg):
|
||||
return fmt.Sprintf("%q contains a version segment (%s): the docs website adds /@<ref> itself, so write the path without it", path, seg)
|
||||
case !staticSegment.MatchString(seg):
|
||||
return unsupported(fmt.Sprintf("segment %q", seg))
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
func parseSource(raw string) (pattern, string) {
|
||||
s := normalizePath(raw)
|
||||
if !strings.HasPrefix(s, docsRoot+"/") {
|
||||
return pattern{}, "source must start with /docs/ (write the full site path, for example /docs/admin/old-page)"
|
||||
}
|
||||
return parsePath(s, roleSource)
|
||||
}
|
||||
|
||||
func parseDestination(raw string) (destination, string) {
|
||||
d := destination{raw: raw}
|
||||
if strings.HasPrefix(raw, "http://") || strings.HasPrefix(raw, "https://") {
|
||||
u, err := url.Parse(raw)
|
||||
if err != nil || u.Host == "" {
|
||||
return d, fmt.Sprintf("destination %q is an invalid URL", raw)
|
||||
}
|
||||
d.external = true
|
||||
d.valid = true
|
||||
return d, ""
|
||||
}
|
||||
|
||||
p := raw
|
||||
if i := strings.IndexAny(p, "?#"); i >= 0 {
|
||||
p = p[:i]
|
||||
}
|
||||
p = normalizePath(p)
|
||||
if p != docsRoot && !strings.HasPrefix(p, docsRoot+"/") {
|
||||
return d, fmt.Sprintf("destination %q is not valid: destination must be a /docs/ path or an http(s) URL", raw)
|
||||
}
|
||||
pat, msg := parsePath(p, roleDestination)
|
||||
if msg != "" {
|
||||
return d, msg
|
||||
}
|
||||
d.pattern = pat
|
||||
d.valid = true
|
||||
return d, ""
|
||||
}
|
||||
|
||||
// checkRedirects validates the contents of docs/redirects.json against the set
|
||||
// of live docs routes. It returns the rules whose source could be parsed, which
|
||||
// the advisory check uses to see whether a removed route is already covered,
|
||||
// and every problem it found, ordered by rule.
|
||||
func checkRedirects(data []byte, live map[string]bool) ([]rule, []problem) {
|
||||
var entries []json.RawMessage
|
||||
if err := json.Unmarshal(data, &entries); err != nil {
|
||||
var typeErr *json.UnmarshalTypeError
|
||||
if errors.As(err, &typeErr) {
|
||||
// Valid JSON, but an object or scalar rather than an array.
|
||||
return nil, []problem{{index: -1, message: "must be a JSON array of rules"}}
|
||||
}
|
||||
return nil, []problem{{index: -1, message: fmt.Sprintf("is not valid JSON: %v", err)}}
|
||||
}
|
||||
if entries == nil {
|
||||
// A JSON null decodes to a nil slice without an error.
|
||||
return nil, []problem{{index: -1, message: "must be a JSON array of rules"}}
|
||||
}
|
||||
|
||||
var (
|
||||
rules []rule
|
||||
problems []problem
|
||||
)
|
||||
for i, entry := range entries {
|
||||
r, ps := checkEntry(i, entry, live)
|
||||
problems = append(problems, ps...)
|
||||
if r != nil {
|
||||
rules = append(rules, *r)
|
||||
}
|
||||
}
|
||||
|
||||
problems = append(problems, checkAcrossRules(rules)...)
|
||||
slices.SortStableFunc(problems, func(a, b problem) int { return cmp.Compare(a.index, b.index) })
|
||||
return rules, problems
|
||||
}
|
||||
|
||||
const entryShape = `must be an object with "source", "destination" and optional "permanent"`
|
||||
|
||||
// checkEntry validates one array element on its own. It returns the rule when
|
||||
// the source parsed, so that cross-rule checks can still use it.
|
||||
func checkEntry(i int, entry json.RawMessage, live map[string]bool) (*rule, []problem) {
|
||||
if bytes.Equal(bytes.TrimSpace(entry), []byte("null")) {
|
||||
return nil, []problem{{index: i, message: entryShape}}
|
||||
}
|
||||
var fields map[string]json.RawMessage
|
||||
if err := json.Unmarshal(entry, &fields); err != nil {
|
||||
return nil, []problem{{index: i, message: entryShape}}
|
||||
}
|
||||
|
||||
var problems []problem
|
||||
add := func(source, msg string) {
|
||||
problems = append(problems, problem{index: i, source: source, message: msg})
|
||||
}
|
||||
|
||||
keys := make([]string, 0, len(fields))
|
||||
for k := range fields {
|
||||
keys = append(keys, k)
|
||||
}
|
||||
slices.Sort(keys)
|
||||
for _, k := range keys {
|
||||
switch k {
|
||||
case "source", "destination", "permanent":
|
||||
default:
|
||||
add("", fmt.Sprintf("unknown field %q (allowed: source, destination, permanent)", k))
|
||||
}
|
||||
}
|
||||
|
||||
source, sourceOK := stringField(fields, "source")
|
||||
if !sourceOK {
|
||||
add("", `"source" is required and must be a non-empty string`)
|
||||
}
|
||||
dest, destOK := stringField(fields, "destination")
|
||||
if !destOK {
|
||||
add(source, `"destination" is required and must be a non-empty string`)
|
||||
}
|
||||
if raw, ok := fields["permanent"]; ok {
|
||||
var b bool
|
||||
if bytes.Equal(bytes.TrimSpace(raw), []byte("null")) || json.Unmarshal(raw, &b) != nil {
|
||||
add(source, `"permanent" must be a boolean`)
|
||||
}
|
||||
}
|
||||
|
||||
if !sourceOK {
|
||||
return nil, problems
|
||||
}
|
||||
src, msg := parseSource(source)
|
||||
if msg != "" {
|
||||
add(source, msg)
|
||||
return nil, problems
|
||||
}
|
||||
|
||||
r := &rule{index: i, source: source, destination: dest, src: src}
|
||||
if destOK {
|
||||
d, msg := parseDestination(dest)
|
||||
r.dst = d
|
||||
if msg != "" {
|
||||
add(source, msg)
|
||||
} else {
|
||||
for _, m := range checkDestination(d, src, live) {
|
||||
add(source, m)
|
||||
}
|
||||
}
|
||||
}
|
||||
if m := checkShadowing(src, live); m != "" {
|
||||
add(source, m)
|
||||
}
|
||||
return r, problems
|
||||
}
|
||||
|
||||
func stringField(fields map[string]json.RawMessage, name string) (string, bool) {
|
||||
raw, ok := fields[name]
|
||||
if !ok {
|
||||
return "", false
|
||||
}
|
||||
var s string
|
||||
if err := json.Unmarshal(raw, &s); err != nil || strings.TrimSpace(s) == "" {
|
||||
return "", false
|
||||
}
|
||||
return s, true
|
||||
}
|
||||
|
||||
// checkDestination verifies that an internal destination lands on a page that
|
||||
// exists and that any parameter it uses is one the source defines.
|
||||
func checkDestination(d destination, src pattern, live map[string]bool) []string {
|
||||
if d.external {
|
||||
return nil
|
||||
}
|
||||
var msgs []string
|
||||
pat := d.pattern
|
||||
if pat.kind == kindExact {
|
||||
if !live[pat.prefix] {
|
||||
msgs = append(msgs, fmt.Sprintf("destination %q is not a page in docs/manifest.json; point it at an existing page", d.raw))
|
||||
}
|
||||
return msgs
|
||||
}
|
||||
|
||||
if src.param != pat.param {
|
||||
msgs = append(msgs, fmt.Sprintf("destination uses :%s, which the source does not define; the source's parameter is %s", pat.param, describeParam(src)))
|
||||
}
|
||||
if !prefixHasPages(pat.prefix, live) {
|
||||
msgs = append(msgs, fmt.Sprintf("no docs page exists under %q; point the destination at an existing section", pat.prefix))
|
||||
}
|
||||
return msgs
|
||||
}
|
||||
|
||||
func describeParam(p pattern) string {
|
||||
if p.param == "" {
|
||||
return "absent (the source is an exact path)"
|
||||
}
|
||||
return ":" + p.param
|
||||
}
|
||||
|
||||
// prefixHasPages reports whether a pattern destination with this static
|
||||
// prefix can land on a page: the prefix is a page or a section of pages.
|
||||
func prefixHasPages(prefix string, live map[string]bool) bool {
|
||||
if prefix == docsRoot || live[prefix] {
|
||||
return true
|
||||
}
|
||||
for route := range live {
|
||||
if strings.HasPrefix(route, prefix+"/") {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// checkShadowing reports a source that matches live docs pages. Redirects are
|
||||
// applied before pages, so such a rule would hide a page readers can reach.
|
||||
func checkShadowing(src pattern, live map[string]bool) string {
|
||||
var hidden []string
|
||||
for route := range live {
|
||||
if src.matches(route) {
|
||||
hidden = append(hidden, route)
|
||||
}
|
||||
}
|
||||
if len(hidden) == 0 {
|
||||
return ""
|
||||
}
|
||||
slices.Sort(hidden)
|
||||
more := ""
|
||||
if len(hidden) > 1 {
|
||||
more = fmt.Sprintf(" (and %d more)", len(hidden)-1)
|
||||
}
|
||||
return fmt.Sprintf("source hides a live docs page: %s%s; remove the redirect, or remove the page from docs/manifest.json first", hidden[0], more)
|
||||
}
|
||||
|
||||
// checkAcrossRules finds duplicate sources, chains, and loops.
|
||||
func checkAcrossRules(rules []rule) []problem {
|
||||
var problems []problem
|
||||
|
||||
seen := make(map[string]int, len(rules))
|
||||
for _, r := range rules {
|
||||
k := r.src.key()
|
||||
if first, dup := seen[k]; dup {
|
||||
problems = append(problems, problem{
|
||||
index: r.index,
|
||||
source: r.source,
|
||||
message: fmt.Sprintf("duplicate of rule [%d]: two rules with the same source; remove one", first),
|
||||
})
|
||||
continue
|
||||
}
|
||||
seen[k] = r.index
|
||||
}
|
||||
|
||||
// next[i] is the rule whose source the destination of rule i lands on.
|
||||
// Only exact internal destinations are followed. Where a pattern
|
||||
// destination ends up depends on the URL being redirected, so chains
|
||||
// through one are not checked.
|
||||
next := make(map[int]int, len(rules))
|
||||
byIndex := make(map[int]rule, len(rules))
|
||||
for _, r := range rules {
|
||||
byIndex[r.index] = r
|
||||
}
|
||||
for _, r := range rules {
|
||||
if !r.dst.valid || r.dst.external || r.dst.pattern.kind != kindExact {
|
||||
continue
|
||||
}
|
||||
for _, other := range rules {
|
||||
if other.matches(r.dst.pattern.prefix) {
|
||||
next[r.index] = other.index
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for _, r := range rules {
|
||||
if _, ok := next[r.index]; !ok {
|
||||
continue
|
||||
}
|
||||
path, cycleStart, cyclic := followRedirects(r.index, next)
|
||||
switch {
|
||||
case cyclic && cycleStart == 0:
|
||||
cycle := path
|
||||
// Report a loop once, on its lowest-numbered rule.
|
||||
if r.index != slices.Min(cycle) {
|
||||
continue
|
||||
}
|
||||
problems = append(problems, problem{index: r.index, source: r.source, message: describeLoop(cycle, byIndex)})
|
||||
case cyclic:
|
||||
// Leads into a loop it is not part of.
|
||||
problems = append(problems, problem{
|
||||
index: r.index,
|
||||
source: r.source,
|
||||
message: fmt.Sprintf("redirect chain into a loop: destination %q is the source of rule [%d], which never reaches a page", r.dst.raw, path[1]),
|
||||
})
|
||||
default:
|
||||
last := byIndex[path[len(path)-1]]
|
||||
problems = append(problems, problem{
|
||||
index: r.index,
|
||||
source: r.source,
|
||||
message: fmt.Sprintf(
|
||||
"redirect chain: destination %q is the source of rule [%d], which redirects to %q; point this rule at %q directly",
|
||||
r.dst.raw, path[1], last.dst.raw, last.dst.raw,
|
||||
),
|
||||
})
|
||||
}
|
||||
}
|
||||
return problems
|
||||
}
|
||||
|
||||
// followRedirects walks next from start. It returns the rules visited in
|
||||
// order, and when the walk revisits a rule, cyclic is true and cycleStart is
|
||||
// the position in path where that rule first appeared.
|
||||
func followRedirects(start int, next map[int]int) (path []int, cycleStart int, cyclic bool) {
|
||||
pos := map[int]int{start: 0}
|
||||
path = []int{start}
|
||||
cur := start
|
||||
for {
|
||||
n, ok := next[cur]
|
||||
if !ok {
|
||||
return path, 0, false
|
||||
}
|
||||
if at, seen := pos[n]; seen {
|
||||
return path, at, true
|
||||
}
|
||||
pos[n] = len(path)
|
||||
path = append(path, n)
|
||||
cur = n
|
||||
}
|
||||
}
|
||||
|
||||
func describeLoop(cycle []int, byIndex map[int]rule) string {
|
||||
if len(cycle) == 1 {
|
||||
return "redirects to itself; remove the rule or point it at a different page"
|
||||
}
|
||||
parts := make([]string, 0, len(cycle)+1)
|
||||
for _, idx := range cycle {
|
||||
parts = append(parts, fmt.Sprintf("rule [%d] %q", idx, byIndex[idx].source))
|
||||
}
|
||||
parts = append(parts, fmt.Sprintf("rule [%d]", cycle[0]))
|
||||
return "redirect loop: " + strings.Join(parts, " -> ") + "; break the loop by pointing one rule at a real page"
|
||||
}
|
||||
Reference in New Issue
Block a user