Skip to content

Commit

Permalink
Formalize the idea of "partial" parse and compilation errors.
Browse files Browse the repository at this point in the history
Previously, the highlighter (in pkg/edit/highlight) uses the criteria
err.From == len(src) to judge whether an error may be caused by incomplete
source (henceforth "partial error"). The criteria works for parse errors, but
fails to identify many partial compilation errors. Store this information in an
extra field of diag.Error and use that instead.

This field is now populated with the same criteria of err.From == len(src), so
no change in behavior has been introduced yet. Future commits will identify more
instances of partial compilation errors and populate the field accordingly.

Also add a fuzz test to verify that the criteria is correct for parse errors.
  • Loading branch information
xiaq committed Aug 5, 2024
1 parent 525ee9a commit 51e5d70
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 31 deletions.
4 changes: 4 additions & 0 deletions pkg/diag/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import (
type Error[T ErrorTag] struct {
Message string
Context Context
// Indicates whether the error may be caused by partial input. More
// formally, this field should be true iff there exists a string x such that
// appending it to the input eliminates the error.
Partial bool
}

// ErrorTag is used to parameterize [Error] into different concrete types. The
Expand Down
12 changes: 2 additions & 10 deletions pkg/edit/highlight.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"strings"

"src.elv.sh/pkg/cli"
"src.elv.sh/pkg/diag"
"src.elv.sh/pkg/edit/highlight"
"src.elv.sh/pkg/eval"
"src.elv.sh/pkg/fsutil"
Expand All @@ -16,18 +15,11 @@ import (

func initHighlighter(appSpec *cli.AppSpec, ed *Editor, ev *eval.Evaler, nb eval.NsBuilder) {
hl := highlight.NewHighlighter(highlight.Config{
Check: func(t parse.Tree) (string, []diag.RangeError) {
Check: func(t parse.Tree) (string, []*eval.CompilationError) {
autofixes, err := ev.CheckTree(t, nil)
autofix := strings.Join(autofixes, "; ")
ed.autofix.Store(autofix)

compErrors := eval.UnpackCompilationErrors(err)
rangeErrors := make([]diag.RangeError, len(compErrors))
for i, compErr := range compErrors {
rangeErrors[i] = compErr
}

return autofix, rangeErrors
return autofix, eval.UnpackCompilationErrors(err)
},
HasCommand: func(cmd string) bool { return hasCommand(ev, cmd) },
AutofixTip: func(autofix string) ui.Text {
Expand Down
19 changes: 10 additions & 9 deletions pkg/edit/highlight/highlight.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ import (
"time"

"src.elv.sh/pkg/diag"
"src.elv.sh/pkg/eval"
"src.elv.sh/pkg/parse"
"src.elv.sh/pkg/ui"
)

// Config keeps configuration for highlighting code.
type Config struct {
Check func(n parse.Tree) (string, []diag.RangeError)
Check func(n parse.Tree) (string, []*eval.CompilationError)
HasCommand func(name string) bool
AutofixTip func(autofix string) ui.Text
}
Expand All @@ -31,24 +32,24 @@ func highlight(code string, cfg Config, lateCb func(ui.Text)) (ui.Text, []ui.Tex
var tips []ui.Text
var errorRegions []region

addDiagError := func(err diag.RangeError) {
r := err.Range()
if r.From < len(code) {
tips = append(tips, ui.T(err.Error()))
errorRegions = append(errorRegions, region{
r.From, r.To, semanticRegion, errorRegion})
addDiagError := func(err error, r diag.Ranging, partial bool) {
if partial {
return
}
tips = append(tips, ui.T(err.Error()))
errorRegions = append(errorRegions, region{
r.From, r.To, semanticRegion, errorRegion})
}

tree, errParse := parse.Parse(parse.Source{Name: "[interactive]", Code: code}, parse.Config{})
for _, err := range parse.UnpackErrors(errParse) {
addDiagError(err)
addDiagError(err, err.Range(), err.Partial)
}

if cfg.Check != nil {
autofix, diagErrors := cfg.Check(tree)
for _, err := range diagErrors {
addDiagError(err)
addDiagError(err, err.Range(), err.Partial)
}
if autofix != "" && cfg.AutofixTip != nil {
tips = append(tips, cfg.AutofixTip(autofix))
Expand Down
10 changes: 2 additions & 8 deletions pkg/edit/highlight/highlighter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"testing"
"time"

"src.elv.sh/pkg/diag"
"src.elv.sh/pkg/eval"
"src.elv.sh/pkg/parse"
"src.elv.sh/pkg/testutil"
Expand Down Expand Up @@ -84,14 +83,9 @@ func TestHighlighter_AutofixesAndCheckErrors(t *testing.T) {
ev := eval.NewEvaler()
ev.AddModule("mod1", &eval.Ns{})
hl := NewHighlighter(Config{
Check: func(t parse.Tree) (string, []diag.RangeError) {
Check: func(t parse.Tree) (string, []*eval.CompilationError) {
autofixes, err := ev.CheckTree(t, nil)
compErrors := eval.UnpackCompilationErrors(err)
rangeErrors := make([]diag.RangeError, len(compErrors))
for i, compErr := range compErrors {
rangeErrors[i] = compErr
}
return strings.Join(autofixes, "; "), rangeErrors
return strings.Join(autofixes, "; "), eval.UnpackCompilationErrors(err)
},
AutofixTip: func(s string) ui.Text { return ui.T("autofix: " + s) },
})
Expand Down
6 changes: 5 additions & 1 deletion pkg/eval/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@ func (CompilationErrorTag) ErrorTag() string { return "compilation error" }
func (cp *compiler) errorpf(r diag.Ranger, format string, args ...any) {
cp.errors = append(cp.errors, &CompilationError{
Message: fmt.Sprintf(format, args...),
Context: *diag.NewContext(cp.src.Name, cp.src.Code, r)})
Context: *diag.NewContext(cp.src.Name, cp.src.Code, r),
// TODO: This criteria is too strict and only captures a small subset of
// partial compilation errors.
Partial: r.Range().From == len(cp.src.Code),
})
}

// UnpackCompilationErrors returns the constituent compilation errors if the
Expand Down
36 changes: 35 additions & 1 deletion pkg/parse/fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,47 @@ package parse

import (
"testing"
"unicode/utf8"
)

func FuzzParse(f *testing.F) {
func FuzzParse_CrashOrVerySlow(f *testing.F) {
f.Add("echo")
f.Add("put $x")
f.Add("put foo bar | each {|x| echo $x }")
f.Fuzz(func(t *testing.T, code string) {
Parse(Source{Name: "fuzz", Code: code}, Config{})
})
}

func FuzzError_Partial(f *testing.F) {
for _, test := range testCases {
f.Add(test.code)
}
f.Fuzz(func(t *testing.T, code string) {
if !utf8.ValidString(code) {
t.SkipNow()
}
_, err := Parse(Source{Name: "fuzz", Code: code}, Config{})
if err != nil {
t.SkipNow()
}
// If code has no parse error, then every prefix of it (as long as it's
// valid UTF-8) should have either no parse errors or only partial parse
// errors.
for i := range code {
if i == 0 {
continue
}
prefix := code[:i]
_, err := Parse(Source{Name: "fuzz", Code: prefix}, Config{})
if err == nil {
continue
}
for _, err := range UnpackErrors(err) {
if !err.Partial {
t.Errorf("prefix %q of valid %q has non-partial error: %v", prefix, code, err)
}
}
}
})
}
6 changes: 4 additions & 2 deletions pkg/parse/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (

// parser maintains some mutable states of parsing.
//
// NOTE: The str member is assumed to be valid UF-8.
// NOTE: The src member is assumed to be valid UF-8.
type parser struct {
srcName string
src string
Expand Down Expand Up @@ -103,7 +103,9 @@ func (ps *parser) backup() {
func (ps *parser) errorp(r diag.Ranger, e error) {
err := &Error{
Message: e.Error(),
Context: *diag.NewContext(ps.srcName, ps.src, r)}
Context: *diag.NewContext(ps.srcName, ps.src, r),
Partial: r.Range().From == len(ps.src),
}
ps.errors = append(ps.errors, err)
}

Expand Down

0 comments on commit 51e5d70

Please sign in to comment.