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
13 changes: 7 additions & 6 deletions internal/check/script.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,13 @@ func (s Script) Run(blk nlp.Block, _ *core.File, _ *core.Config) ([]core.Alert,
// don't use our custom regexp2 library, which means the offsets
// (`re2loc`) will be off.
a := core.Alert{
Check: s.Name,
Severity: s.Level,
Span: matchLoc,
Link: s.Link,
Match: matchText,
Action: s.Action}
Check: s.Name,
Severity: s.Level,
Span: matchLoc,
Link: s.Link,
Match: matchText,
Action: s.Action,
HasByteOffsets: true}

if matchMsg, ok := match["message"].(string); ok {
a.Message, a.Description = formatMessages(matchMsg, s.Description, matchText)
Expand Down
7 changes: 4 additions & 3 deletions internal/core/alert.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ type Alert struct {
Message string // the output message
Severity string // 'suggestion', 'warning', or 'error'
Match string // the actual matched text
Line int // the source line
Limit int `json:"-"` // the max times to report
Hide bool `json:"-"` // should we hide this alert?
Line int // the source line
Limit int `json:"-"` // the max times to report
Hide bool `json:"-"` // should we hide this alert?
HasByteOffsets bool `json:"-"` // Span holds byte offsets into the raw document
}

// FormatAlert ensures that all required fields have data.
Expand Down
60 changes: 49 additions & 11 deletions internal/core/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,32 @@ func (f *File) assignLoc(ctx string, blk nlp.Block, pad int, a Alert) (int, []in
return blk.Line + 1, a.Span
}

// locFromByteOffset computes a 1-based line number and a [col, col+len] span
// from absolute byte offsets into the raw document text. This avoids the
// text-search approach used by FindLoc/initialPosition, which can report the
// wrong location when the matched text appears more than once.
func locFromByteOffset(ctx string, begin, end, pad int) (int, []int) {
line := 1
lineStart := 0

for i := 0; i < begin && i < len(ctx); i++ {
if ctx[i] == '\n' {
line++
lineStart = i + 1
}
}

col := nlp.StrLen(ctx[lineStart:begin]) + 1 + pad
matchLen := nlp.StrLen(ctx[begin:end])

span := []int{col, col + matchLen - 1}
if span[1] <= 0 {
span[1] = 1
}

return line, span
}

// SetText updates the file's content, lines, and history.
func (f *File) SetText(s string) {
f.Content = s
Expand All @@ -271,19 +297,31 @@ func (f *File) AddAlert(a Alert, blk nlp.Block, lines, pad int, lookup bool) {
ctx = old
}

// NOTE: If the `ctx` document is large (as could be the case with
// `scope: raw`) this is *slow*. Thus, the cap at 1k.
// When the alert carries byte offsets from a script rule and falls within
// the document, compute line:column directly from those offsets instead of
// performing a text search. This fixes incorrect position reporting for
// script rules with `scope: raw` when the matched text appears more than
// once.
//
// TODO: Actually fix this.
if len(a.Offset) == 0 && strings.Count(ctx, a.Match) > 1 && len(ctx) < 1000 {
a.Offset = append(a.Offset, strings.Fields(ctx[0:a.Span[0]])...)
}
// We use blk.Context (the original document) rather than ctx, which may
// have been modified by ChkToCtx substitutions from earlier alerts.
if a.HasByteOffsets && a.Span[0] >= 0 && a.Span[1] <= len(blk.Context) {
a.Line, a.Span = locFromByteOffset(blk.Context, a.Span[0], a.Span[1], pad)
} else {
// NOTE: If the `ctx` document is large (as could be the case with
// `scope: raw`) this is *slow*. Thus, the cap at 1k.
//
// TODO: Actually fix this.
if len(a.Offset) == 0 && strings.Count(ctx, a.Match) > 1 && len(ctx) < 1000 {
a.Offset = append(a.Offset, strings.Fields(ctx[0:a.Span[0]])...)
}

if !lookup {
a.Line, a.Span = f.assignLoc(ctx, blk, pad, a)
}
if (!lookup && a.Span[0] < 0) || lookup {
a.Line, a.Span = f.FindLoc(ctx, blk.Text, pad, lines, a)
if !lookup {
a.Line, a.Span = f.assignLoc(ctx, blk, pad, a)
}
if (!lookup && a.Span[0] < 0) || lookup {
a.Line, a.Span = f.FindLoc(ctx, blk.Text, pad, lines, a)
}
}

if a.Span[0] > 0 {
Expand Down
2 changes: 1 addition & 1 deletion testdata/features/checks.feature
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Feature: Checks
When I test "checks/Script"
Then the output should contain exactly:
"""
test.md:4:19:Scripts.CustomMsg:Some message
test.md:1:2:Scripts.CustomMsg:Some message
test.md:29:1:Checks.ScriptRE:Consider inserting a new section heading at this point.
test.md:39:1:Checks.ScriptRE:Consider inserting a new section heading at this point.
"""
Expand Down