Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion internal/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,13 @@ func validateBody(r *Report, c *concept.Concept) {
// validateLinks checks that all cross-links within the bundle resolve to an
// existing concept. Both absolute (/path/to/concept.md) and relative links
// are checked.
//
// Relative links (without a leading /) resolve from the concept's own
// directory: a link [X](organizations/cloaked) in pages/about.md targets
// pages/organizations/cloaked, not organizations/cloaked. When such a
// relative link is broken but the same target would resolve as an absolute
// path, the error message suggests the absolute form (e.g.
// /organizations/cloaked) so authors can fix the link.
func validateLinks(r *Report, b *bundle.Bundle) {
for _, c := range b.Concepts {
links := extractLinks(c.Body)
Expand All @@ -104,19 +111,57 @@ func validateLinks(r *Report, b *bundle.Bundle) {
continue // external URL or non-concept link, skip
}
if !b.HasConcept(target) {
r.add(c.ID, SeverityError, fmt.Sprintf("broken link: [%s] -> %s (concept %s not found)", link.Text, link.Target, target))
// The link didn't resolve as written. Check whether the same
// target would resolve as an absolute (bundle-root-relative)
// path; if so, the author likely intended an absolute link.
// resolveLink with an empty fromConceptID yields the bundle-root
// interpretation. This only differs from the as-written result
// for relative links (a leading-/ target resolves identically
// regardless of fromConceptID), so an already-absolute broken
// link correctly falls through to the plain message below.
if absTarget := resolveLink("", link); absTarget != "" && absTarget != target && b.HasConcept(absTarget) {
r.add(c.ID, SeverityError, fmt.Sprintf(
"broken link: [%s] -> %s (relative links resolve from the current concept's directory; use /%s%s for an absolute path)",
link.Text, link.Target, absTarget, fragmentOf(link.Target)))
} else {
r.add(c.ID, SeverityError, fmt.Sprintf(
"broken link: [%s] -> %s (concept %s not found)",
link.Text, link.Target, target))
}
}
}
}
}

// fragmentOf returns the #fragment portion of a raw link target (including the
// leading #), or "" if there is none. Fragments are ignored for concept
// resolution, but preserving them in a suggested path keeps the hint faithful
// to what the author wrote (e.g. organizations/cloaked#section suggests
// /organizations/cloaked#section).
func fragmentOf(raw string) string {
if idx := strings.IndexByte(raw, '#'); idx != -1 {
return raw[idx:]
}
return ""
}

// Link represents a markdown link found in a concept body.
type Link struct {
Text string
Target string
}

// ExtractLinks parses all markdown links [text](target) from a body.
//
// Link targets may be absolute or relative. Absolute targets begin with /
// and resolve against the bundle root (e.g. /tables/users.md -> the
// concept tables/users regardless of where the linking concept lives).
// Relative targets lack a leading / and resolve against the linking
// concept's own directory: a target organizations/cloaked in a concept at
// pages/about resolves to pages/organizations/cloaked, not
// organizations/cloaked. External URLs (http://, https://, mailto:, and
// pure-fragment links like #section) are returned as links but are ignored
// by concept resolution.
func ExtractLinks(body string) []Link {
return extractLinks(body)
}
Expand Down
137 changes: 137 additions & 0 deletions internal/validate/validate_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
package validate

import (
"os"
"path/filepath"
"strings"
"testing"

"github.com/okfcli/okf/internal/bundle"
)

func TestExtractLinks(t *testing.T) {
Expand Down Expand Up @@ -84,3 +89,135 @@ func TestNormalizePath(t *testing.T) {
}
}
}

// --- validateLinks integration tests ---

func TestValidateLinks_RelativeLinkSuggestsAbsolutePath(t *testing.T) {
// A concept at pages/about has [Cloaked](organizations/cloaked). The
// relative target resolves to pages/organizations/cloaked (broken), but
// the absolute target organizations/cloaked exists. The error should
// suggest the absolute path.
b := testBundle(t, map[string]string{
"pages/about.md": "---\ntype: Page\ntitle: About\n---\n\nSee [Cloaked](organizations/cloaked).",
"organizations/cloaked.md": "---\ntype: Org\ntitle: Cloaked\n---\n\nbody",
})

r := &Report{}
validateLinks(r, b)

var msg string
for _, f := range r.Findings {
if f.ConceptID == "pages/about" && strings.Contains(f.Message, "broken link") {
msg = f.Message
break
}
}
if msg == "" {
t.Fatalf("no broken-link finding for pages/about: %+v", r.Findings)
}
if !strings.Contains(msg, "use /organizations/cloaked for an absolute path") {
t.Errorf("error %q does not suggest absolute path", msg)
}
}

func TestValidateLinks_NonexistentConceptNoAbsolutePathSuggestion(t *testing.T) {
// A link to a genuinely nonexistent concept should report "broken link:"
// but must NOT mention "absolute path".
b := testBundle(t, map[string]string{
"pages/about.md": "---\ntype: Page\ntitle: About\n---\n\nSee [Ghost](/nonexistent/ghost).",
})

r := &Report{}
validateLinks(r, b)

var msg string
for _, f := range r.Findings {
if f.ConceptID == "pages/about" && strings.Contains(f.Message, "broken link") {
msg = f.Message
break
}
}
if msg == "" {
t.Fatalf("no broken-link finding for pages/about: %+v", r.Findings)
}
if !strings.Contains(msg, "broken link:") {
t.Errorf("error %q does not contain 'broken link:'", msg)
}
if strings.Contains(msg, "absolute path") {
t.Errorf("error %q should not mention 'absolute path' for a genuinely missing concept", msg)
}
}

func TestValidateLinks_AlreadyAbsoluteBrokenNoSuggestion(t *testing.T) {
// A link that is already written as an absolute path (/...) but does not
// resolve must NOT produce an "absolute path" suggestion — there is
// nothing to suggest, so it falls through to the plain broken-link message.
b := testBundle(t, map[string]string{
"pages/about.md": "---\ntype: Page\ntitle: About\n---\n\nSee [Cloaked](/organizations/cloaked).",
})

r := &Report{}
validateLinks(r, b)

var msg string
for _, f := range r.Findings {
if f.ConceptID == "pages/about" && strings.Contains(f.Message, "broken link") {
msg = f.Message
break
}
}
if msg == "" {
t.Fatalf("no broken-link finding for pages/about: %+v", r.Findings)
}
if strings.Contains(msg, "absolute path") {
t.Errorf("error %q should not suggest an absolute path for an already-absolute broken link", msg)
}
}

func TestValidateLinks_SuggestionPreservesFragment(t *testing.T) {
// A broken relative link that carries a #fragment should retain the
// fragment in the suggested absolute path (fragments are ignored for
// resolution but kept in the displayed hint).
b := testBundle(t, map[string]string{
"pages/about.md": "---\ntype: Page\ntitle: About\n---\n\nSee [Cloaked](organizations/cloaked#section).",
"organizations/cloaked.md": "---\ntype: Org\ntitle: Cloaked\n---\n\nbody",
})

r := &Report{}
validateLinks(r, b)

var msg string
for _, f := range r.Findings {
if f.ConceptID == "pages/about" && strings.Contains(f.Message, "broken link") {
msg = f.Message
break
}
}
if msg == "" {
t.Fatalf("no broken-link finding for pages/about: %+v", r.Findings)
}
if !strings.Contains(msg, "use /organizations/cloaked#section for an absolute path") {
t.Errorf("error %q does not preserve the #fragment in the suggested path", msg)
}
}

// --- helpers ---

func testBundle(t *testing.T, files map[string]string) *bundle.Bundle {
t.Helper()
dir := t.TempDir()
for path, content := range files {
full := filepath.Join(dir, path)
if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil {
t.Fatalf("mkdir: %v", err)
}
if err := os.WriteFile(full, []byte(content), 0644); err != nil {
t.Fatalf("write: %v", err)
}
}
b, err := bundle.Load(dir)
if err != nil {
t.Fatalf("Load: %v", err)
}
return b
}
Loading