diff --git a/internal/validate/validate.go b/internal/validate/validate.go index 408a3c5..4744910 100644 --- a/internal/validate/validate.go +++ b/internal/validate/validate.go @@ -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) @@ -104,12 +111,40 @@ 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 @@ -117,6 +152,16 @@ type Link struct { } // 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) } diff --git a/internal/validate/validate_test.go b/internal/validate/validate_test.go index 3964853..f8fae12 100644 --- a/internal/validate/validate_test.go +++ b/internal/validate/validate_test.go @@ -1,7 +1,12 @@ package validate import ( + "os" + "path/filepath" + "strings" "testing" + + "github.com/okfcli/okf/internal/bundle" ) func TestExtractLinks(t *testing.T) { @@ -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 +}