Skip to content

forgecomments: ignore code spans and indented code, refuse reference links, more negation forms, Unicode boundaries (#358) #372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DanielNetzer merged 1 commit into from
Nov 4, 2026
Merged

forgecomments: ignore code spans and indented code, refuse reference links, more negation forms, Unicode boundaries (#358) #372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 27 additions & 9 deletions lab/plugins/forgecomments/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,33 @@ To triage the findings, propose a fix round and push it to the same change reque
leading markdown markers, or as the whole content of `**…**`.
- `severity: high`, `medium`, `low` or `info` anywhere in a line.
- A line that starts with `[high]`, `[medium]`, `[low]` or `[info]`, after leading
markdown markers, unless the tag starts a markdown link, as in `[High](…)`.

Within a line, the marker that starts first wins, whatever its form. A marker right after
the word `not` or `no` doesn't count. Lines inside a fenced code block (```` ``` ```` or
`~~~`) are never read. As in CommonMark, only a line holding a run of the opening
character at least as long as the opener, and nothing else, closes the fence, and a fence
left open hides the rest of the five lines. A line is a blockquote line, and never read,
when a `>` appears anywhere in its leading markdown markers, list bullets and spaces, as
in `> x` or `- > x`. A lone CR, U+0085, U+2028 and U+2029 each end a line, as LF does, and
markdown markers, unless the tag starts a markdown link, as in `[High](…)`, a
reference link, as in `[high][ref]` or `[high][]`, or a link reference definition, as
in `[high]: http://x`.

The severity word must end the word: one followed by a Unicode letter, mark or digit, as
in `severity: highé` or `High Severityé`, is not a marker, and neither is `severity`
right after a letter. Within a line, the marker that starts first wins, whatever its
form. A marker doesn't count when the text before it ends in the word `not` or `no`, or
in a word ending in `n't` or `n’t`, as in `isn't`, with only markdown markers,
punctuation and spaces between them, as in `not: severity: high` or `not, severity:
high`.

Text inside an inline code span, as in `` `High Severity` `` or ``` ``severity: high`` ```,
never counts. As in CommonMark, a run of backticks opens a span that the next run of
exactly the same length closes, even on a later line of the same paragraph, and a run
with no such closer is literal. A backslash does not escape a backtick here. Lines inside
a fenced code block (```` ``` ```` or `~~~`) are never read. As in CommonMark, only a line
holding a run of the opening character at least as long as the opener, and nothing
else, closes the fence, and a fence left open hides the rest of the five lines. Outside
a list, a line indented by four or more columns is an indented code line and is never
read; a tab reaches the next multiple of four columns. Such a line never opens or closes
a fence, so a marker on the line after an indented ```` ``` ```` is still read. A list
item starts a list, and an indented line in a list is list content and is read. Any
other line indented by less than four columns ends the list, even a lazy continuation,
so an indented line after it is code. A line is a blockquote line, and never read, when
a `>` appears anywhere in its leading markdown markers, list bullets and spaces, as in
`> x` or `- > x`. A lone CR, U+0085, U+2028 and U+2029 each end a line, as LF does, and
CRLF is one break. Without a marker, `defaultSeverity` applies.
- **Order and bounds.** Findings are ordered highest severity first, and latest comment
first within a severity. At most 500 are kept, so the lowest-severity, oldest ones are
Expand Down
280 changes: 256 additions & 24 deletions lab/plugins/forgecomments/severity.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,36 +50,214 @@ func severityRank(s string) int {
// severityOf reads the first explicit marker in the first five lines of body,
// case-insensitively, and returns fallback when there is none. Format
// characters, such as bidi controls, are dropped first, so they cannot hide a
// marker. Lines inside a fenced code block, fence lines and blockquote lines
// are never read, and a fence left open hides the rest of those lines. Only
// those lines are split off, so a long body costs no more than its first lines.
// marker. Lines inside a fenced code block, fence lines, blockquote lines and
// indented code lines are never read, and a fence left open hides the rest of
// those lines. Text inside an inline code span never counts. Only those lines
// are split off, so a long body costs no more than its first lines.
func severityOf(body, fallback string) string {
for _, l := range readLines(body) {
if s, ok := maskedSeverity(l.text, l.masked); ok {
return s
}
}
return fallback
}

// codeIndent is the indent, in columns, at which a line outside a list is an
// indented code line.
const codeIndent = 4

// readLine is a line that may carry a marker, with format characters dropped,
// and the same line with its inline code spans masked byte for byte, so an
// index into one is an index into the other.
type readLine struct {
text, masked string
}

// readLines is the lines of the first five that are read for a marker, in
// order. It drops fenced code, fence lines, blockquote lines, blank lines and,
// outside a list, lines indented by four or more columns, which CommonMark
// reads as indented code. A paragraph is a run of read lines that no blank or
// dropped line, heading or list item breaks, and its code spans are masked
// across its lines, as a code span may continue onto the next line.
func readLines(body string) []readLine {
var out []readLine
var fenceChar byte
fenceLen := 0
for _, line := range firstLines(body, markerLines) {
line = strings.Map(visible, line)
fenceLen, para := 0, 0
inList := false
flush := func() {
maskParagraph(out[para:])
para = len(out)
}
for _, raw := range firstLines(body, markerLines) {
indent := indentOf(raw)
line := strings.Map(visible, raw)
lead := strings.TrimLeftFunc(line, unicode.IsSpace)
c, n := fenceRun(lead)
if fenceLen > 0 {
// As in CommonMark, only a run of the opener's character, at least as
// long, with nothing after it closes the fence.
if c == fenceChar && n >= fenceLen && strings.TrimSpace(lead[n:]) == "" {
// long, with nothing after it and indented by less than four columns
// outside a list, closes the fence.
if c == fenceChar && n >= fenceLen && strings.TrimSpace(lead[n:]) == "" && (indent < codeIndent || inList) {
fenceLen = 0
}
continue
}
if n > 0 {
blank := strings.TrimSpace(line) == ""
code := indent >= codeIndent && !inList && !blank
item := listItem(lead)
// A list item starts or continues a list. Any other line indented by less
// than four columns ends it, even a lazy continuation, so a later indented
// line is read as code: refusing a marker is the safe side.
if !blank && !code {
if item {
inList = true
} else if indent < codeIndent {
inList = false
}
}
switch {
case code, blank:
// An indented code line, or a run of ` or ~ on one, never opens a fence.
flush()
continue
case n > 0:
flush()
fenceChar, fenceLen = c, n
continue
case quoted(line):
flush()
continue
}
head := heading(lead)
if item || head {
flush()
}
out = append(out, readLine{text: line})
if head {
flush()
}
}
flush()
return out
}

// indentOf is the leading indent of raw in columns, as CommonMark counts it: a
// space is one column and a tab reaches the next multiple of four. Format
// characters are skipped, as they are dropped before a line is read. It stops
// counting at four columns, so its work is bounded.
func indentOf(raw string) int {
col := 0
for _, r := range raw {
switch {
case r == ' ':
col++
case r == '\t':
col += 4 - col%4
case unicode.Is(unicode.Cf, r):
default:
return col
}
if col >= codeIndent {
return col
}
}
return col
}

// listItem reports whether lead, a line without its indent, starts a list
// item: a `-`, `+` or `*` bullet, or one to nine digits and `.` or `)`,
// followed by a space or the end of the line.
func listItem(lead string) bool {
i := 0
for i < len(lead) && i < 9 && lead[i] >= '0' && lead[i] <= '9' {
i++
}
switch {
case i == 0 && lead != "" && strings.IndexByte("-+*", lead[0]) >= 0:
i = 1
case i > 0 && i < len(lead) && (lead[i] == '.' || lead[i] == ')'):
i++
default:
return false
}
return i == len(lead) || lead[i] == ' '
}

// heading reports whether lead, a line without its indent, is an ATX heading:
// one to six `#` followed by a space or the end of the line.
func heading(lead string) bool {
n := 0
for n < len(lead) && n < 7 && lead[n] == '#' {
n++
}
return n >= 1 && n <= 6 && (n == len(lead) || lead[n] == ' ')
}

// maskParagraph masks the code spans of one paragraph, read as its lines
// joined by line breaks.
func maskParagraph(lines []readLine) {
if len(lines) == 0 {
return
}
texts := make([]string, len(lines))
for i, l := range lines {
texts[i] = l.text
}
for i, m := range strings.Split(maskCodeSpans(strings.Join(texts, "\n")), "\n") {
lines[i].masked = m
}
}

// maskCodeSpans replaces every byte of each inline code span in s, backticks
// included, with NUL, keeping line breaks, so no marker can match inside one
// and every index is unchanged. As in CommonMark, a run of backticks opens a
// span that the next run of exactly the same length closes, and a run with no
// such closer is literal. A backslash does not escape a backtick here, so a
// span is masked whenever it could be one. It runs in linear time: each run
// finds its closer through a table built in one pass from the end.
func maskCodeSpans(s string) string {
type run struct{ at, n int }
var runs []run
for i := 0; i < len(s); {
if s[i] != '`' {
i++
continue
}
if quoted(line) {
j := i
for j < len(s) && s[j] == '`' {
j++
}
runs = append(runs, run{i, j - i})
i = j
}
if len(runs) < 2 {
return s
}
closer := make([]int, len(runs))
nextOfLen := map[int]int{}
for k := len(runs) - 1; k >= 0; k-- {
closer[k] = -1
if m, ok := nextOfLen[runs[k].n]; ok {
closer[k] = m
}
nextOfLen[runs[k].n] = k
}
b := []byte(s)
for k := 0; k < len(runs); {
m := closer[k]
if m < 0 {
k++
continue
}
if s, ok := lineSeverity(line); ok {
return s
for i := runs[k].at; i < runs[m].at+runs[m].n; i++ {
if b[i] != '\n' {
b[i] = 0
}
}
k = m + 1
}
return fallback
return string(b)
}

// firstLines splits off at most n leading lines of body. A lone CR, NEL,
Expand Down Expand Up @@ -132,25 +310,39 @@ func quoted(line string) bool {
}

// lineSeverity is the severity of the marker with the smallest index in line,
// whatever its form. A marker that `not` or `no` precedes does not count.
// whatever its form, with the line's own code spans masked. A marker that
// `not`, `no` or a word ending in `n't` precedes does not count.
func lineSeverity(line string) (string, bool) {
return maskedSeverity(line, maskCodeSpans(line))
}

// maskedSeverity is lineSeverity for a line whose code spans are already
// masked in masked. Markers are matched in masked, so none inside a code span
// counts, and negation is read in line, so a masked span never hides a `not`.
// A severity word that runs on into a Unicode letter, mark or digit, as in
// `highé`, is not a marker, and a tag that starts a link, a reference link or
// a link reference definition does not count.
func maskedSeverity(line, masked string) (string, bool) {
best, level := -1, ""
consider := func(at int, l string) {
if (best < 0 || at < best) && !negated(line[:at]) {
best, level = at, l
}
}
rest := stripMarkers(line)
start := len(line) - len(rest)
if m := namedMarker.FindStringSubmatch(rest); m != nil {
rest := stripMarkers(masked)
start := len(masked) - len(rest)
if m := namedMarker.FindStringSubmatch(rest); m != nil && !wordRuneAt(rest, len(m[0])) {
consider(start, m[1])
}
if m := tagMarker.FindStringSubmatch(rest); m != nil && !strings.HasPrefix(rest[len(m[0]):], "(") {
if m := tagMarker.FindStringSubmatch(rest); m != nil && !startsLink(rest[len(m[0]):]) {
consider(start, m[1])
}
for _, re := range []*regexp.Regexp{boldMarker, fieldMarker} {
for _, m := range re.FindAllStringSubmatchIndex(line, -1) {
consider(m[0], line[m[2]:m[3]])
for _, m := range boldMarker.FindAllStringSubmatchIndex(masked, -1) {
consider(m[0], masked[m[2]:m[3]])
}
for _, m := range fieldMarker.FindAllStringSubmatchIndex(masked, -1) {
if !wordRuneBefore(masked, m[0]) && !wordRuneAt(masked, m[3]) {
consider(m[0], masked[m[2]:m[3]])
}
}
if best < 0 {
Expand All @@ -159,12 +351,52 @@ func lineSeverity(line string) (string, bool) {
return strings.ToLower(level), true
}

// startsLink reports whether the text after a `[level]` tag makes it a link,
// `[x](…)`, a reference link, `[x][ref]` or `[x][]`, or a link reference
// definition, `[x]: url`.
func startsLink(after string) bool {
return after != "" && strings.IndexByte("([:", after[0]) >= 0
}

// wordRune reports whether r continues a word: a Unicode letter, mark or
// digit, or an underscore.
func wordRune(r rune) bool {
return unicode.IsLetter(r) || unicode.IsMark(r) || unicode.IsDigit(r) || r == '_'
}

// wordRuneAt reports whether s has a word rune at byte index at.
func wordRuneAt(s string, at int) bool {
if at >= len(s) {
return false
}
r, _ := utf8.DecodeRuneInString(s[at:])
return wordRune(r)
}

// wordRuneBefore reports whether s has a word rune just before byte index at.
func wordRuneBefore(s string, at int) bool {
if at <= 0 {
return false
}
r, _ := utf8.DecodeLastRuneInString(s[:at])
return wordRune(r)
}

// negated reports whether the text before a marker ends in the word `not` or
// `no`, any case, with markdown markers and spaces allowed after it.
// `no`, any case, or in a word ending in `n't` or `n’t`, as in `isn't`, with
// markdown markers, punctuation and spaces allowed after it, as in `not:`.
func negated(before string) bool {
before = strings.TrimRightFunc(before, markerOrSpace)
before = strings.TrimRightFunc(before, func(r rune) bool { return markerOrSpace(r) || unicode.IsPunct(r) })
word := before[strings.LastIndexFunc(before, func(r rune) bool { return !unicode.IsLetter(r) })+1:]
return strings.EqualFold(word, "not") || strings.EqualFold(word, "no")
if strings.EqualFold(word, "not") || strings.EqualFold(word, "no") {
return true
}
for _, suffix := range []string{"n't", "n’t"} {
if len(before) >= len(suffix) && strings.EqualFold(before[len(before)-len(suffix):], suffix) {
return true
}
}
return false
}

// candidate is one finding with the comment's creation time and its position
Expand Down
Loading
Loading