From 5354ea68488ee46274687fb156dbc17b740d0ec4 Mon Sep 17 00:00:00 2001 From: Jon Olson Date: Sun, 13 Sep 2026 12:54:14 -0700 Subject: [PATCH] Start pull-request descriptions without an opening heading. The template and policy checker require a heading before the change summary. Let descriptions open directly with the result, and update the contributor and review guidance to match. Continue to require an opening description, accepting the old heading only as the first section. Render GitHub-flavored Markdown with Goldmark and inspect the HTML with x/net/html so headings, separators, and empty blocks do not count as descriptions. Headings inside code examples are not treated as PR sections. Both dependencies are used by the internal policy checker. --- .github/pull_request_template.md | 2 - AGENTS.md | 4 +- CONTRIBUTING.md | 13 +- go.mod | 5 + go.sum | 4 + internal/ci/checkpr/metadata.go | 172 +++++++++-- internal/ci/checkpr/metadata_test.go | 420 ++++++++++++++++++++++++++- 7 files changed, 584 insertions(+), 36 deletions(-) create mode 100644 go.sum diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index be8b309..ea2e325 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,5 +1,3 @@ -## What this does - `) +var disallowedHTML = regexp.MustCompile(`(?i)<(/?)(title|textarea|style|xmp|iframe|noembed|noframes|script|plaintext)([\t\n\f\r />])`) var requiredPullRequestSections = []string{ - "What this does", "Why", "Documentation", } @@ -44,9 +52,15 @@ func readPullRequestEvent(name string) (pullRequestMetadata, error) { func checkPullRequest(metadata pullRequestMetadata) []finding { findings := checkSubject("pull-request", strings.TrimSpace(metadata.Title)) - sections := pullRequestSections(htmlComment.ReplaceAllString(metadata.Body, "")) + sections := pullRequestSections(metadata.Body) + if !sections[""] { + findings = append(findings, newFinding( + errorLevel, "pr-body", "pull-request", + "pull-request opening description is missing or empty", + )) + } for _, required := range requiredPullRequestSections { - if strings.TrimSpace(sections[required]) != "" { + if sections["#"+required] { continue } findings = append(findings, newFinding( @@ -57,19 +71,143 @@ func checkPullRequest(metadata pullRequestMetadata) []finding { return findings } -func pullRequestSections(body string) map[string]string { - sections := make(map[string]string) - current := "" - scanner := bufio.NewScanner(strings.NewReader(body)) - for scanner.Scan() { - line := scanner.Text() - if strings.HasPrefix(line, "## ") { - current = strings.TrimSpace(strings.TrimPrefix(line, "## ")) - continue +type pullRequestBody struct { + sections map[string]bool + current string + first bool +} + +func pullRequestSections(body string) map[string]bool { + result := pullRequestBody{sections: make(map[string]bool), first: true} + markdown := goldmark.New( + goldmark.WithExtensions(extension.GFM, extension.Footnote), + goldmark.WithRendererOptions(markdownhtml.WithUnsafe()), + ) + source := []byte(body) + document := markdown.Parser().Parse(text.NewReader(source)) + removeDecorations(document, source) + var rendered bytes.Buffer + if err := markdown.Renderer().Render(&rendered, source, document); err != nil { + return result.sections + } + filtered := disallowedHTML.ReplaceAll(rendered.Bytes(), []byte("<$1$2$3")) + htmlDocument, err := xhtml.Parse(bytes.NewReader(filtered)) + if err != nil { + return result.sections + } + result.addHTMLNode(htmlDocument, true) + return result.sections +} + +func (body *pullRequestBody) heading(level int, title string, sections bool) { + if sections { + body.addHeading(level, title) + } else if body.current == "" && !body.sections[""] { + body.current = "#" + body.first = false + } +} + +func (body *pullRequestBody) addHeading(level int, title string) { + title = strings.TrimSpace(title) + if body.first && level == 2 && title == "What this does" { + body.current = "" + } else if body.current == "" && !body.sections[""] || level <= 2 { + body.current = "#" + title + } + body.first = false +} + +func (body *pullRequestBody) addHTMLNode(node *xhtml.Node, sections bool) { + if node.Type == xhtml.ElementNode { + if len(node.Data) == 2 && node.Data[0] == 'h' && node.Data[1] >= '1' && node.Data[1] <= '6' { + body.heading(int(node.Data[1]-'0'), htmlText(node), sections) + return + } + switch node.Data { + case "html", "body", "div", "section", "article", "main": + default: + sections = false + } + } + if node.Type == xhtml.TextNode && visibleText(node.Data) { + body.sections[body.current] = true + body.first = false + } + for _, child := range visibleChildren(node) { + body.addHTMLNode(child, sections) + } +} + +func htmlText(node *xhtml.Node) string { + if node.Type == xhtml.TextNode { + return node.Data + } + var result strings.Builder + for _, child := range visibleChildren(node) { + result.WriteString(htmlText(child)) + } + return result.String() +} + +func visibleText(value string) bool { + return strings.ContainsFunc(value, func(r rune) bool { + return r != '\u2800' && !unicode.IsSpace(r) && !unicode.IsControl(r) && !unicode.IsMark(r) && + !unicode.Is(unicode.Cf, r) && !unicode.Is(unicode.Other_Default_Ignorable_Code_Point, r) + }) +} + +func removeDecorations(node ast.Node, source []byte) { + if node.Kind() == ast.KindBlockquote { + removeAlertMarker(node.FirstChild(), source) + } + for child := node.FirstChild(); child != nil; { + next := child.NextSibling() + switch child.Kind() { + case extast.KindFootnoteList, extast.KindFootnoteLink: + node.RemoveChild(node, child) + default: + removeDecorations(child, source) } - if current != "" { - sections[current] += line + "\n" + child = next + } +} + +func removeAlertMarker(node ast.Node, source []byte) { + paragraph, ok := node.(*ast.Paragraph) + if !ok || paragraph.Lines().Len() == 0 || paragraph.FirstChild() == nil { + return + } + line := paragraph.Lines().At(0) + switch strings.TrimSpace(string(line.Value(source))) { + case "[!NOTE]", "[!TIP]", "[!IMPORTANT]", "[!WARNING]", "[!CAUTION]": + for child := paragraph.FirstChild(); child != nil && child.Pos() < line.Stop; child = paragraph.FirstChild() { + paragraph.RemoveChild(paragraph, child) + } + } +} + +func collapsedDetails(node *xhtml.Node) bool { + if node.Type != xhtml.ElementNode || node.Data != "details" { + return false + } + for _, attr := range node.Attr { + if attr.Key == "open" { + return false + } + } + return true +} + +func visibleChildren(node *xhtml.Node) []*xhtml.Node { + var children []*xhtml.Node + collapsed := collapsedDetails(node) + for child := node.FirstChild; child != nil; child = child.NextSibling { + if !collapsed { + children = append(children, child) + } else if child.Type == xhtml.ElementNode && child.Data == "summary" { + return []*xhtml.Node{child} } } - return sections + return children } diff --git a/internal/ci/checkpr/metadata_test.go b/internal/ci/checkpr/metadata_test.go index 8f92889..e41041c 100644 --- a/internal/ci/checkpr/metadata_test.go +++ b/internal/ci/checkpr/metadata_test.go @@ -1,13 +1,14 @@ package main -import "testing" +import ( + "strings" + "testing" +) func TestCheckPullRequestAcceptsCompletedTemplate(t *testing.T) { metadata := pullRequestMetadata{ Title: "Enforce pull-request policy.", - Body: `## What this does - -Adds deterministic policy checks. + Body: `Adds deterministic policy checks. ## Why @@ -27,9 +28,7 @@ Updated the contribution guide. func TestCheckPullRequestAcceptsOptionalHardwareEvidence(t *testing.T) { metadata := pullRequestMetadata{ Title: "Enforce pull-request policy.", - Body: `## What this does - -Adds deterministic policy checks. + Body: `Adds deterministic policy checks. ## Why @@ -82,9 +81,7 @@ A concrete rationale. } func completedPullRequestBody() string { - return `## What this does - -A concrete result. + return `A concrete result. ## Why @@ -95,3 +92,406 @@ A concrete rationale. Documentation remains accurate. ` } + +func TestCheckPullRequestAcceptsLegacyOpeningHeading(t *testing.T) { + metadata := pullRequestMetadata{ + Title: "Enforce pull-request policy.", + Body: "## What this does\n\n" + completedPullRequestBody(), + } + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Fatalf("checkPullRequest() = %#v, want no findings", findings) + } +} + +func TestCheckPullRequestRequiresOpeningDescription(t *testing.T) { + for _, opening := range []string{"", "\n\n", "## What this does\n\n\n\n"} { + metadata := pullRequestMetadata{ + Title: "Enforce pull-request policy.", + Body: opening + "## Why\n\nA concrete rationale.\n\n## Documentation\n\nDocumentation remains accurate.\n", + } + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Fatalf("opening %q: findings = %#v, want pr-body error", opening, findings) + } + } +} + +func TestCheckPullRequestRejectsHeadingOnlyOpening(t *testing.T) { + for _, heading := range []string{"# Summary", "### Summary", "#### Summary", "##### Summary", "###### Summary", " # Summary", "#", "###\tSummary", "Summary\n=======", "Summary\n-------", "Two-line\nsummary\n======="} { + for _, prefix := range []string{"", "## What this does\n\n"} { + metadata := pullRequestMetadata{ + Title: "Enforce pull-request policy.", + Body: strings.Replace(completedPullRequestBody(), "A concrete result.", prefix+heading, 1), + } + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("heading %q with prefix %q: findings = %#v, want pr-body error", heading, prefix, findings) + } + } + } +} + +func TestCheckPullRequestAcceptsNonHeadingHashText(t *testing.T) { + for _, opening := range []string{"#123 fixes the reported bug.", "####### This is ordinary text.", "A concrete result.\n\n### Details", "A concrete result.\n\nDetails\n-------"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", opening, 1)} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("opening %q: findings = %#v", opening, findings) + } + } +} + +func TestCheckPullRequestPreservesBlocksBeforeThematicBreak(t *testing.T) { + for _, opening := range []string{ + "- Adds deterministic policy checks.\n---", + "A concrete result.\n- Adds policy checks.\n---", + "1. Adds deterministic policy checks.\n---", + "> A concrete result.\n---", + " A concrete result.\n---", + "```text\nA concrete result.\n```\n---", + } { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", opening, 1)} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("opening %q: findings = %#v", opening, findings) + } + } +} + +func TestCheckPullRequestDoesNotCountThematicBreaks(t *testing.T) { + for _, marker := range []string{"- - -", "* * *", "_ _ _", "***", "___", " * * * ", "-\t-\t-", "_______"} { + for _, prefix := range []string{"", "## What this does\n\n"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", prefix+marker, 1)} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("marker %q: findings = %#v, want pr-body error", marker, findings) + } + metadata.Body = strings.Replace(completedPullRequestBody(), "A concrete result.", prefix+"A concrete result.\n"+marker, 1) + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("prose before marker %q: findings = %#v", marker, findings) + } + } + } +} + +func TestCheckPullRequestRejectsMisplacedLegacyDescription(t *testing.T) { + for _, body := range []string{ + "## Why\n\nA concrete rationale.\n\n## What this does\n\nA concrete result.\n\n## Documentation\n\nDocumentation remains accurate.", + "## Documentation\n\nDocumentation remains accurate.\n\n## What this does\n\nA concrete result.\n\n## Why\n\nA concrete rationale.", + "# Summary\n\n## What this does\n\n" + completedPullRequestBody(), + "## Why\n\nA concrete rationale.\n\n##\n\nA concrete result.\n\n## Documentation\n\nDocumentation remains accurate.", + } { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: body} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("body %q: findings = %#v, want pr-body error", body, findings) + } + } +} + +func TestCheckPullRequestRejectsMarkupOnlyOpening(t *testing.T) { + for _, opening := range []string{">", "> # Summary", "> > ### Summary", "```\n```", "```go\n\n```", "~~~text\n~~~", "-", "- # Summary", "1. ### Summary", ""} { + for _, prefix := range []string{"", "## What this does\n\n"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", prefix+opening, 1)} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("opening %q: findings = %#v, want pr-body error", opening, findings) + } + } + } +} + +func TestCheckPullRequestKeepsSectionMarkersInsideCode(t *testing.T) { + metadata := pullRequestMetadata{ + Title: "Enforce pull-request policy.", + Body: "```markdown\n## Why\nExample, not a section.\n## Documentation\nExample, not a section.\n```", + } + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("findings = %#v, want missing sections", findings) + } +} + +func TestCheckPullRequestAcceptsCommentBeforeLegacyOpening(t *testing.T) { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: "\n\n## What this does\n\n" + completedPullRequestBody()} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("findings = %#v", findings) + } +} + +func TestCheckPullRequestAcceptsVisibleMarkdownContent(t *testing.T) { + for _, opening := range []string{"**A concrete result.**", "[Description](https://example.com)", "", "> > A concrete result.", "- **A concrete result.**", "```html\n\n```"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", opening, 1)} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("opening %q: findings = %#v", opening, findings) + } + } +} + +func TestCheckPullRequestRejectsWhitespaceEntities(t *testing.T) { + for _, entity := range []string{" ", " ", " ", "** **", "[ ](https://example.com)"} { + for _, section := range []string{"A concrete result.", "A concrete rationale.", "Documentation remains accurate."} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), section, entity, 1)} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("entity %q in %q: findings = %#v, want pr-body error", entity, section, findings) + } + } + } +} + +func TestCheckPullRequestAcceptsLiteralEntities(t *testing.T) { + for _, opening := range []string{"` `", "` `", "\\ ", "&nbsp;", "©", "```text\n \n```"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", opening, 1)} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("opening %q: findings = %#v", opening, findings) + } + } +} + +func TestCheckPullRequestRejectsHTMLOpeningHeading(t *testing.T) { + for _, heading := range []string{"

Summary

", "

Summary

", "\n

Summary

", "
\n

Summary

\n
"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: heading + "\n\n" + completedPullRequestBody()} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("heading %q: findings = %#v, want pr-body error", heading, findings) + } + } +} + +func TestCheckPullRequestAcceptsHTMLContentBeforeHeading(t *testing.T) { + for _, opening := range []string{"

A concrete result.

", "

A concrete result.

Details

", "\n\nA concrete result.", "
<h1>literal</h1>
", "
literal attribute\">A concrete result.
"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", opening, 1)} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("opening %q: findings = %#v", opening, findings) + } + } +} + +func TestCheckPullRequestRejectsEmptyHTMLContent(t *testing.T) { + for _, opening := range []string{"
", "

 

", "

Summary

A concrete result.

", "

Summary

A concrete result.

"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", opening, 1)} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("opening %q: findings = %#v, want pr-body error", opening, findings) + } + } +} + +func TestCheckPullRequestAcceptsHTMLSections(t *testing.T) { + for _, opening := range []string{"

A concrete result.

", "

What this does

A concrete result.

"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: opening + "

Why

A concrete rationale.

Documentation

Documentation remains accurate.

"} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("opening %q: findings = %#v", opening, findings) + } + } +} + +func TestCheckPullRequestRejectsLeadingNestedHeading(t *testing.T) { + for _, opening := range []string{"

Summary

A concrete result.

", "> # Summary\n> A concrete result.", "- # Summary\n\n A concrete result.", "> > # Summary\n> > A concrete result.", ">

Summary

\n>\n> A concrete result."} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", opening, 1)} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("opening %q: findings = %#v, want pr-body error", opening, findings) + } + } +} + +func TestCheckPullRequestRejectsFormatOnlyText(t *testing.T) { + for _, content := range []string{"​", "­", "⁠", "\u200b\u00ad\u2060", "

​

", "`\u200b`"} { + for _, section := range []string{"A concrete result.", "A concrete rationale.", "Documentation remains accurate."} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), section, content, 1)} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("content %q in %q: findings = %#v, want pr-body error", content, section, findings) + } + } + } +} + +func TestCheckPullRequestPreservesProseBeforeNestedHeading(t *testing.T) { + for _, opening := range []string{"> A concrete result.\n>\n> # Details", "- A concrete result.\n\n # Details", "A concrete result.\n\n> # Details\n> More information.", "A\u200b concrete result.", "`​`"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", opening, 1)} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("opening %q: findings = %#v", opening, findings) + } + } +} + +func TestCheckPullRequestDoesNotPromoteNestedSections(t *testing.T) { + for _, body := range []string{"A concrete result.\n\n

Why

A concrete rationale.

Documentation

Documentation remains accurate.

", "A concrete result.\n\n

Why

A concrete rationale.

Documentation

Documentation remains accurate.

", "A concrete result.\n\n", "A concrete result.\n\n> ## Why\n> A concrete rationale.\n>\n> ## Documentation\n> Documentation remains accurate.", "A concrete result.\n\n>

Why

A concrete rationale.

Documentation

Documentation remains accurate.

"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: body} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("body %q: findings = %#v, want pr-body error", body, findings) + } + } +} + +func TestCheckPullRequestRejectsEmptyGFMTables(t *testing.T) { + for _, content := range []string{"| |\n| --- |", "| | |\n| --- | --- |\n| | |", "|   |\n| --- |\n| ​ |"} { + for _, section := range []string{"A concrete result.", "A concrete rationale.", "Documentation remains accurate."} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), section, content, 1)} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("table %q in %q: findings = %#v, want pr-body error", content, section, findings) + } + } + } +} + +func TestCheckPullRequestAcceptsGFMContent(t *testing.T) { + for _, opening := range []string{"| Result |\n| --- |\n| A concrete result. |", "| |\n| --- |\n| A concrete result. |", "- [x] A concrete result.", "~~Old behavior~~ A concrete result."} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", opening, 1)} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("opening %q: findings = %#v", opening, findings) + } + } +} + +func TestCheckPullRequestAcceptsTagFilteredAndUnwrappedHTML(t *testing.T) { + for _, content := range []string{"", "", "", "hidden"} { + for _, section := range []string{"A concrete result.", "A concrete rationale.", "Documentation remains accurate."} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), section, content, 1)} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("content %q in %q: findings = %#v, want no findings", content, section, findings) + } + } + } +} + +func TestCheckPullRequestRejectsLegacyLeadingNestedHeading(t *testing.T) { + for _, opening := range []string{"

Summary

A concrete result.

", "> # Summary\n> A concrete result.", "- # Summary\n\n A concrete result.", "

Summary

A concrete result.

", "### Summary\n\nA concrete result."} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: "## What this does\n\n" + strings.Replace(completedPullRequestBody(), "A concrete result.", opening, 1)} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("opening %q: findings = %#v, want pr-body error", opening, findings) + } + } +} + +func TestCheckPullRequestRejectsTagFilteredSectionNames(t *testing.T) { + for _, title := range []string{"Why", "Documentation"} { + for _, tag := range []string{"script", "style", "title"} { + heading := "

<" + tag + ">" + title + "

" + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "## "+title, heading, 1)} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("heading %q: findings = %#v, want pr-body error", heading, findings) + } + } + } +} + +func TestCheckPullRequestPreservesVisibleSectionTitles(t *testing.T) { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "## Why", "

Why

", 1)} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("findings = %#v", findings) + } +} + +func TestCheckPullRequestRejectsFootnoteOnlyContent(t *testing.T) { + for _, content := range []string{"[^note]: A concrete result with scope.", "[^note]\n\n[^note]: A concrete result with scope."} { + for _, section := range []string{"A concrete result.", "A concrete rationale.", "Documentation remains accurate."} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), section, content, 1)} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("footnote %q in %q: findings = %#v, want pr-body error", content, section, findings) + } + } + } +} + +func TestCheckPullRequestPreservesDescriptionWithFootnote(t *testing.T) { + body := strings.Replace(completedPullRequestBody(), "A concrete result.", "A concrete result.[^note]", 1) + "\n[^note]: Supporting detail.\n" + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: body} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("findings = %#v", findings) + } + metadata.Body = strings.Replace(body, "Documentation remains accurate.", "", 1) + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("findings = %#v, want missing Documentation", findings) + } +} + +func TestCheckPullRequestRejectsIgnorableOnlyText(t *testing.T) { + for _, content := range []string{"️", "͏", "ᅟ", "ㅤ", "⠀", "\u0301", "\u0007"} { + for _, section := range []string{"A concrete result.", "A concrete rationale.", "Documentation remains accurate."} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), section, content, 1)} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("content %q in %q: findings = %#v, want pr-body error", content, section, findings) + } + } + } +} + +func TestCheckPullRequestPreservesVisibleTextWithMarks(t *testing.T) { + for _, opening := range []string{"\u2801\u2803", "Cafe\u0301 support.", "日本語の説明。", "✈️ A concrete result."} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", opening, 1)} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("opening %q: findings = %#v", opening, findings) + } + } +} + +func TestCheckPullRequestRejectsEmptyAlerts(t *testing.T) { + for _, marker := range []string{"NOTE", "TIP", "IMPORTANT", "WARNING", "CAUTION"} { + for _, section := range []string{"A concrete result.", "A concrete rationale.", "Documentation remains accurate."} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), section, "> [!"+marker+"]", 1)} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("marker %q in %q: findings = %#v, want pr-body error", marker, section, findings) + } + } + } +} + +func TestCheckPullRequestPreservesAlertContentAndLiteralMarkers(t *testing.T) { + for _, opening := range []string{"> [!NOTE]\n> A concrete result.", "> [!TIP]\n>\n> A concrete result.", "> ` [!NOTE] `", "> \\[!NOTE]", "[!NOTE]", "> [!NOTE] is a literal marker in this sentence."} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", opening, 1)} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("opening %q: findings = %#v", opening, findings) + } + } +} + +func TestCheckPullRequestAcceptsDisallowedRawHTMLExamples(t *testing.T) { + for _, tag := range []string{"title", "textarea", "style", "xmp", "iframe", "noembed", "noframes", "script", "plaintext", "SCRIPT"} { + for _, section := range []string{"A concrete result.", "A concrete rationale.", "Documentation remains accurate."} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), section, "<"+tag+">Example.", 1)} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("tag %q in %q: findings = %#v", tag, section, findings) + } + } + } +} + +func TestCheckPullRequestPreservesRawHTMLRoles(t *testing.T) { + for _, role := range []string{"doc-endnotes", "doc-noteref"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", "

A concrete result.

", 1)} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("role %q: findings = %#v", role, findings) + } + } +} + +func TestCheckPullRequestRejectsCollapsedDetailsContent(t *testing.T) { + for _, section := range []string{"A concrete result.", "A concrete rationale.", "Documentation remains accurate."} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), section, "

Hidden detail.

", 1)} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("section %q: findings = %#v, want pr-body error", section, findings) + } + } + for _, attribute := range []string{"", " open"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: "A concrete result.\n\n

Why

Rationale.

Documentation

Updated.

"} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("attribute %q: findings = %#v, want pr-body error", attribute, findings) + } + } +} + +func TestCheckPullRequestPreservesVisibleDetailsContent(t *testing.T) { + for _, opening := range []string{"
A concrete result.

Supporting detail.

", "

A concrete result.

"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: strings.Replace(completedPullRequestBody(), "A concrete result.", opening, 1)} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("opening %q: findings = %#v", opening, findings) + } + } +} + +func TestCheckPullRequestKeepsSectionsInDocumentFlow(t *testing.T) { + sections := "

Why

A concrete rationale.

Documentation

Documentation remains accurate.

" + for _, container := range []string{"figure", "aside", "nav", "form", "dl"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: "A concrete result.\n\n<" + container + ">" + sections + ""} + if findings := checkPullRequest(metadata); !hasFinding(findings, errorLevel, "pr-body") { + t.Errorf("container %q: findings = %#v, want pr-body error", container, findings) + } + } + for _, container := range []string{"div", "section", "article", "main"} { + metadata := pullRequestMetadata{Title: "Enforce pull-request policy.", Body: "A concrete result.\n\n<" + container + ">" + sections + ""} + if findings := checkPullRequest(metadata); len(findings) != 0 { + t.Errorf("container %q: findings = %#v", container, findings) + } + } +}