From 6ff091e5aad3364f425a05098648f8ce266a53a7 Mon Sep 17 00:00:00 2001 From: itsfuad Date: Thu, 8 Oct 2026 23:18:56 +0600 Subject: [PATCH 1/8] Show source fixes beneath diagnostic help lines Replace the code-hint block with fixes attached to help and note lines. A fix is built with Fix.Remove, Fix.Insert or Fix.Replace and drawn as one source line: the line as it will read with added text marked, or the current line with removed text marked. Fixes that cannot be shown faithfully on single lines are not drawn. Attach fixes to unneeded mut, missing separators and closing brackets, redundant separators, trailing and match-arm commas, redundant optional markers and leading zeros. Other missing tokens keep their label. Keep the margin unbroken before help lines and end every diagnostic with one blank line. Known gap: a missing-token fix follows the parser's guess, so it can show a wrong line when more code follows on the same line. --- docs/architecture/infrastructure.md | 8 +- internal/diagnostics/bag.go | 4 +- internal/diagnostics/diagnostic.go | 132 ++--- internal/diagnostics/diagnostic_test.go | 255 ++++++++- internal/diagnostics/emitter.go | 510 +++++++----------- internal/frontend/parser/parse_expr.go | 34 +- internal/frontend/parser/parse_stmt.go | 6 +- internal/frontend/parser/parse_types.go | 4 +- internal/frontend/parser/parser.go | 64 ++- internal/frontend/parser/parser_test.go | 154 +++++- internal/semantics/usage/usage.go | 5 +- internal/semantics/usage/usage_test.go | 22 +- pkg/colors/color.go | 5 + pkg/colors/logger.go | 2 + x_test/negative_missing_comma_fix/peeper.toml | 8 + .../negative_missing_comma_fix/src/main.peep | 9 + .../peeper.toml | 8 + .../src/main.peep | 4 + .../peeper.toml | 8 + .../src/main.peep | 4 + x_test/warning_removal_fixes/peeper.toml | 8 + x_test/warning_removal_fixes/src/main.peep | 12 + .../peeper.toml | 8 + .../src/main.peep | 9 + 24 files changed, 801 insertions(+), 482 deletions(-) create mode 100644 x_test/negative_missing_comma_fix/peeper.toml create mode 100644 x_test/negative_missing_comma_fix/src/main.peep create mode 100644 x_test/negative_missing_semicolon_fix/peeper.toml create mode 100644 x_test/negative_missing_semicolon_fix/src/main.peep create mode 100644 x_test/negative_missing_semicolon_non_ascii_fix/peeper.toml create mode 100644 x_test/negative_missing_semicolon_non_ascii_fix/src/main.peep create mode 100644 x_test/warning_removal_fixes/peeper.toml create mode 100644 x_test/warning_removal_fixes/src/main.peep create mode 100644 x_test/warning_unmodified_mutable_suggestion/peeper.toml create mode 100644 x_test/warning_unmodified_mutable_suggestion/src/main.peep diff --git a/docs/architecture/infrastructure.md b/docs/architecture/infrastructure.md index 00f5c71d..d0c147c2 100644 --- a/docs/architecture/infrastructure.md +++ b/docs/architecture/infrastructure.md @@ -303,8 +303,9 @@ adding a generic cache or transplanting these artifacts by stable ID alone. `diagnostics.Diagnostic` carries severity, message, stable code, file path, ordered labels, and extras. A primary label is the diagnostic origin; secondary labels carry -context such as a previous declaration or recursive generic application. Code hints, -insertions, removals, replacements, notes, and help are extras. +context such as a previous declaration or recursive generic application. Notes and +help lines are extras; each can carry fixes (`Fix.Remove`, `Fix.Insert`, `Fix.Replace`) +whose fixed source line is shown beneath it. `DiagnosticBag` stores `groups[producingPhase][moduleScope]`. `BeginPhase` replaces one phase/module group and returns a scoped writer. `AppendPhase` appends to an @@ -348,7 +349,8 @@ back to bounded file scanning. `diagnostics.SourceCache` stores split source lines under an RW mutex. The bag accepts in-memory source through `AddSourceContent`, allowing diagnostics for source overrides. The emitter supports ANSI/HTML strings and stderr output, source -snippets, labels, code hints, tab expansion, and optional syntax highlighting. +snippets, labels, help and note lines with the fixed source line beneath them, +tab expansion, and optional syntax highlighting. `SyntaxHighlighter` performs lightweight line tokenization; `suggest.go` supplies Levenshtein-based, ambiguity-aware nearest-name suggestions with stable priorities. diff --git a/internal/diagnostics/bag.go b/internal/diagnostics/bag.go index 2d34c16e..68017492 100644 --- a/internal/diagnostics/bag.go +++ b/internal/diagnostics/bag.go @@ -14,9 +14,9 @@ import ( ) const ( - compileFailedMsg = "\nCompilation failed with %d error(s)" + compileFailedMsg = "Compilation failed with %d error(s)" andWarningMsg = " and %d warning(s)" - compileSuccessWithWarning = "\nCompilation succeeded with %d warning(s)\n" + compileSuccessWithWarning = "Compilation succeeded with %d warning(s)\n" ) // DiagnosticBag collects diagnostics during compilation diff --git a/internal/diagnostics/diagnostic.go b/internal/diagnostics/diagnostic.go index 9a44525d..f0e5b660 100644 --- a/internal/diagnostics/diagnostic.go +++ b/internal/diagnostics/diagnostic.go @@ -48,19 +48,53 @@ type DiagnosticExtraKind int const ( ExtraText DiagnosticExtraKind = iota - ExtraCodeHint ) type DiagnosticText struct { Kind string Message string Color colors.COLOR + // Fixes are the source edits this text proposes, shown beneath it. + Fixes []CodeFix +} + +// CodeFix replaces the source at Location with NewText. An empty Location +// inserts; an empty NewText removes. Build one with Fix.Remove, Fix.Insert or +// Fix.Replace. +type CodeFix struct { + Location *source.Location + NewText string +} + +// Fix groups the ways to build a CodeFix, so a call site reads as the edit it +// proposes: Fix.Remove(where), Fix.Insert(where, text), Fix.Replace(where, text). +var Fix fixBuilder + +type fixBuilder struct{} + +// Remove deletes the source at where. +func (fixBuilder) Remove(where *source.Location) CodeFix { + return CodeFix{Location: where} +} + +// Insert adds text at the start of where and keeps everything already there. +func (fixBuilder) Insert(where *source.Location, text string) CodeFix { + if where == nil { + return CodeFix{NewText: text} + } + point := *where + point.End = where.Start + return CodeFix{Location: &point, NewText: text} +} + +// Replace swaps the source at where for newText. +func (fixBuilder) Replace(where *source.Location, newText string) CodeFix { + return CodeFix{Location: where, NewText: newText} } type DiagnosticExtra struct { - Kind DiagnosticExtraKind - Text DiagnosticText - CodeHint CodeHint + Kind DiagnosticExtraKind + Text DiagnosticText } type Diagnostic struct { @@ -74,29 +108,6 @@ type Diagnostic struct { const internalCompilerErrorCode = "ICE0001" -type CodeHintLine struct { - Prefix string - Code string - BaseColor colors.COLOR -} - -type CodeHint struct { - Code string - Lines []CodeHintLine - Labels []CodeHintLabel - Location *source.Location - BaseColor colors.COLOR - GutterColor colors.COLOR -} - -type CodeHintLabel struct { - Line int - Column int - Length int - Message string - Style LabelStyle -} - func NewError(message string) *Diagnostic { return &Diagnostic{Severity: Error, Message: message} } @@ -182,60 +193,9 @@ func (d *Diagnostic) markInternalCompilerError(message string) *Diagnostic { return d } -func (d *Diagnostic) WithCodeHint(loc *source.Location, code string, labels ...CodeHintLabel) *Diagnostic { - if loc == nil { - return d - } - d.WithPrimaryLabel(loc, "") - d.Extras = append(d.Extras, DiagnosticExtra{ - Kind: ExtraCodeHint, - CodeHint: CodeHint{ - Code: code, - Labels: labels, - Location: loc, - GutterColor: colors.GREEN, - }, - }) - return d -} - -func (d *Diagnostic) WithCodeHintLines(loc *source.Location, lines []CodeHintLine, labels ...CodeHintLabel) *Diagnostic { - if loc == nil { - return d - } - d.WithPrimaryLabel(loc, "") - d.Extras = append(d.Extras, DiagnosticExtra{ - Kind: ExtraCodeHint, - CodeHint: CodeHint{ - Lines: append([]CodeHintLine(nil), lines...), - Labels: labels, - Location: loc, - GutterColor: colors.GREEN, - }, - }) - return d -} - -func (d *Diagnostic) WithCodeInsertion(loc *source.Location, code string, labels ...CodeHintLabel) *Diagnostic { - return d.WithCodeHintLines(loc, []CodeHintLine{ - {Prefix: "+", Code: code, BaseColor: colors.GREEN}, - }, labels...) -} - -func (d *Diagnostic) WithCodeRemoval(loc *source.Location, code string, labels ...CodeHintLabel) *Diagnostic { - return d.WithCodeHintLines(loc, []CodeHintLine{ - {Prefix: "-", Code: code, BaseColor: colors.RED}, - }, labels...) -} - -func (d *Diagnostic) WithCodeReplacement(loc *source.Location, oldCode, newCode string, labels ...CodeHintLabel) *Diagnostic { - return d.WithCodeHintLines(loc, []CodeHintLine{ - {Prefix: "-", Code: oldCode, BaseColor: colors.RED}, - {Prefix: "+", Code: newCode, BaseColor: colors.GREEN}, - }, labels...) -} - -func (d *Diagnostic) WithText(kind, message string, color colors.COLOR) *Diagnostic { +// WithText adds a `= kind: message` line. Fixes are the source edits the +// message proposes; each fixed line is shown beneath it. +func (d *Diagnostic) WithText(kind, message string, color colors.COLOR, fixes ...CodeFix) *Diagnostic { if message == "" { return d } @@ -244,17 +204,17 @@ func (d *Diagnostic) WithText(kind, message string, color colors.COLOR) *Diagnos } d.Extras = append(d.Extras, DiagnosticExtra{ Kind: ExtraText, - Text: DiagnosticText{Kind: kind, Message: message, Color: color}, + Text: DiagnosticText{Kind: kind, Message: message, Color: color, Fixes: fixes}, }) return d } -func (d *Diagnostic) WithNote(message string) *Diagnostic { - return d.WithText("note", message, colors.CYAN) +func (d *Diagnostic) WithNote(message string, fixes ...CodeFix) *Diagnostic { + return d.WithText("note", message, colors.CYAN, fixes...) } -func (d *Diagnostic) WithHelp(help string) *Diagnostic { - return d.WithText("help", help, colors.GREEN) +func (d *Diagnostic) WithHelp(help string, fixes ...CodeFix) *Diagnostic { + return d.WithText("help", help, colors.GREEN, fixes...) } const internalCompilerFailureMessage = "internal compiler failure" diff --git a/internal/diagnostics/diagnostic_test.go b/internal/diagnostics/diagnostic_test.go index c635fd3a..d2545cc1 100644 --- a/internal/diagnostics/diagnostic_test.go +++ b/internal/diagnostics/diagnostic_test.go @@ -41,26 +41,23 @@ func TestWithSecondaryLabelRequiresPrimary(t *testing.T) { } } -func TestWithCodeReplacementAddsOrderedExtra(t *testing.T) { +func TestWithHelpCarriesItsFixes(t *testing.T) { d := NewError("immutable") loc := testLoc("main"+peeper.SourceExt, 2, 5) - d.WithCodeReplacement(loc, "maybe", "mut maybe") + d.WithHelp("declare it mutable", Fix.Replace(loc, "mut maybe")) if len(d.Extras) != 1 { t.Fatalf("expected 1 extra entry, got %d", len(d.Extras)) } - if d.Extras[0].Kind != ExtraCodeHint { - t.Fatalf("expected extra kind ExtraCodeHint, got %v", d.Extras[0].Kind) + if d.Extras[0].Kind != ExtraText || d.Extras[0].Text.Kind != "help" { + t.Fatalf("expected a help text extra, got %#v", d.Extras[0]) } - hint := d.Extras[0].CodeHint - if len(hint.Lines) != 2 { - t.Fatalf("expected 2 hint lines, got %d", len(hint.Lines)) + fixes := d.Extras[0].Text.Fixes + if len(fixes) != 1 || fixes[0].Location != loc || fixes[0].NewText != "mut maybe" { + t.Fatalf("unexpected fixes: %#v", fixes) } - if hint.Lines[0].Prefix != "-" || hint.Lines[0].Code != "maybe" { - t.Fatalf("unexpected first replacement line: %#v", hint.Lines[0]) - } - if hint.Lines[1].Prefix != "+" || hint.Lines[1].Code != "mut maybe" { - t.Fatalf("unexpected second replacement line: %#v", hint.Lines[1]) + if len(d.Labels) != 0 { + t.Fatalf("a fix must not add or replace labels, got %#v", d.Labels) } } @@ -121,29 +118,235 @@ func TestEmitterAlignsHeaderAndHelpWithGutter(t *testing.T) { } } -func TestEmitterSuggestionHeaderUsesBlankGutter(t *testing.T) { +func emitFixForTest(t *testing.T, format colors.LogFormat, sourceText string, fixes ...CodeFix) string { + t.Helper() prevFormat := colors.CurrentLogFormat() - colors.SetLogFormat(colors.LogFormatNormal) + colors.SetLogFormat(format) defer colors.SetLogFormat(prevFormat) var out bytes.Buffer emitter := NewEmitter(&out) - samplePath := "sample" + peeper.SourceExt - emitter.cache.AddSource(samplePath, "value\n") + emitter.cache.AddSource("sample"+peeper.SourceExt, sourceText) + emitter.Emit(NewError("broken"). + WithCode("P9999"). + WithPrimaryLabel(fixes[0].Location, ""). + WithHelp("apply the fix", fixes...)) + return out.String() +} + +func fixLocForTest(line, startColumn, endColumn int) *source.Location { + return source.NewLocation("sample"+peeper.SourceExt, + source.Position{Line: line, Column: startColumn}, source.Position{Line: line, Column: endColumn}) +} + +func TestEmitterShowsFixedLineUnderItsText(t *testing.T) { + tests := []struct { + name string + source string + fixes []CodeFix + want string + }{ + {name: "insertion", source: "let total = 5\n", fixes: []CodeFix{Fix.Insert(fixLocForTest(1, 14, 14), ";")}, + want: " = help: apply the fix\n | \n1 | let total = 5;\n"}, + {name: "replacement", source: "return totl;\n", fixes: []CodeFix{Fix.Replace(fixLocForTest(1, 8, 12), "total")}, + want: " = help: apply the fix\n | \n1 | return total;\n"}, + {name: "removal keeps one space", source: "let mut total = 5;\n", fixes: []CodeFix{Fix.Remove(fixLocForTest(1, 5, 8))}, + want: " = help: apply the fix\n | \n1 | let total = 5;\n"}, + {name: "two fixes on one line", source: "if ready {\n", fixes: []CodeFix{Fix.Insert(fixLocForTest(1, 4, 4), "("), Fix.Insert(fixLocForTest(1, 9, 9), ")")}, + want: " = help: apply the fix\n | \n1 | if (ready) {\n"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := emitFixForTest(t, colors.LogFormatNormal, test.source, test.fixes...) + if !strings.Contains(got, test.want) { + t.Fatalf("expected %q in:\n%s", test.want, got) + } + }) + } +} + +func TestEmitterMarksOnlyChangedTextInColour(t *testing.T) { + removal := emitFixForTest(t, colors.LogFormatANSI, "let mut total = 5;\n", Fix.Remove(fixLocForTest(1, 5, 8))) + if !strings.Contains(removal, string(colors.FIX_REMOVED)+"mut"+string(colors.RESET)) { + t.Fatalf("expected removed text in the removal colour, got:\n%q", removal) + } + + replacement := emitFixForTest(t, colors.LogFormatANSI, "return totl;\n", Fix.Replace(fixLocForTest(1, 8, 12), "total")) + if !strings.Contains(replacement, string(colors.FIX_ADDED)+"a"+string(colors.RESET)) { + t.Fatalf("expected only the inserted letter in the addition colour, got:\n%q", replacement) + } + if strings.Contains(replacement, string(colors.FIX_REMOVED)) { + t.Fatalf("a fix that adds text must not show removed text, got:\n%q", replacement) + } +} + +func TestEmitterKeepsMarginUnbrokenBeforeHelp(t *testing.T) { + withHelp := emitFixForTest(t, colors.LogFormatNormal, "let total = 5\n", Fix.Insert(fixLocForTest(1, 14, 14), ";")) + if !strings.Contains(withHelp, "^\n | \n = help: apply the fix\n") { + t.Fatalf("expected a margin line between the snippet and its help, got:\n%s", withHelp) + } + + prevFormat := colors.CurrentLogFormat() + colors.SetLogFormat(colors.LogFormatNormal) + defer colors.SetLogFormat(prevFormat) + var out bytes.Buffer + emitter := NewEmitter(&out) + emitter.cache.AddSource("sample"+peeper.SourceExt, "let total = 5\n") + emitter.Emit(NewError("broken").WithCode("P9999").WithPrimaryLabel(fixLocForTest(1, 14, 14), "")) + if !strings.HasSuffix(out.String(), "^\n\n") { + t.Fatalf("expected an error without help to end with a blank line, got:\n%q", out.String()) + } +} + +func TestFixInsertKeepsExistingSource(t *testing.T) { + got := emitFixForTest(t, colors.LogFormatNormal, "let total = 5\n", Fix.Insert(fixLocForTest(1, 5, 10), "mut ")) + if !strings.Contains(got, "1 | let mut total = 5\n") { + t.Fatalf("expected insertion before `total` without removing it, got:\n%s", got) + } +} + +func TestEmitterEndsEveryDiagnosticWithOneBlankLine(t *testing.T) { + withFix := emitFixForTest(t, colors.LogFormatNormal, "let total = 5\n", Fix.Insert(fixLocForTest(1, 14, 14), ";")) + if !strings.HasSuffix(withFix, "1 | let total = 5;\n\n") || strings.HasSuffix(withFix, "\n\n\n") { + t.Fatalf("expected exactly one blank line after a fixed line, got:\n%q", withFix) + } +} + +func TestEmitterSkipsFixItCannotPlace(t *testing.T) { + got := emitFixForTest(t, colors.LogFormatNormal, "let total = 5\n", Fix.Replace(fixLocForTest(1, 40, 41), ";")) + if !strings.Contains(got, " = help: apply the fix\n") || strings.Contains(got, "1 | let total = 5;") { + t.Fatalf("expected the text without a fixed line, got:\n%s", got) + } +} - loc := source.NewLocation(samplePath, source.Position{Line: 1, Column: 1}, source.Position{Line: 1, Column: 6}) - diag := NewError("replace"). +func TestEmitterPlacesFixesByCharacterNotByte(t *testing.T) { + tests := []struct { + name string + source string + fixes []CodeFix + want string + }{ + {name: "accented letter before the fix", source: "let s = \"café\"\n", fixes: []CodeFix{Fix.Insert(fixLocForTest(1, 15, 15), ";")}, + want: "1 | let s = \"café\";\n"}, + {name: "wide characters before the fix", source: "let s = \"日本語\"\n", fixes: []CodeFix{Fix.Insert(fixLocForTest(1, 14, 14), ";")}, + want: "1 | let s = \"日本語\";\n"}, + {name: "removal after an accented letter", source: "let s = \"héllo\"; let mut total = 5;\n", fixes: []CodeFix{Fix.Remove(fixLocForTest(1, 22, 25))}, + want: "1 | let s = \"héllo\"; let total = 5;\n"}, + {name: "tab before the fix", source: "\tlet total = 5\n", fixes: []CodeFix{Fix.Insert(fixLocForTest(1, 15, 15), ";")}, + want: "let total = 5;\n"}, + {name: "removal directly followed by an insertion", source: "let mut total = 5;\n", fixes: []CodeFix{Fix.Remove(fixLocForTest(1, 5, 8)), Fix.Insert(fixLocForTest(1, 8, 8), "x")}, + want: "1 | let x total = 5;\n"}, + {name: "empty removal keeps both spaces", source: "a b\n", fixes: []CodeFix{Fix.Remove(fixLocForTest(1, 3, 3))}, + want: "1 | a b\n"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := emitFixForTest(t, colors.LogFormatNormal, test.source, test.fixes...) + if !strings.Contains(got, test.want) { + t.Fatalf("expected %q in:\n%s", test.want, got) + } + }) + } +} + +func TestEmitterDrawsNoFixItCannotShowFaithfully(t *testing.T) { + spansLines := source.NewLocation("sample"+peeper.SourceExt, source.Position{Line: 1, Column: 5}, source.Position{Line: 2, Column: 2}) + tests := []struct { + name string + source string + fixes []CodeFix + }{ + {name: "column past the end of a line with a tab", source: "\tlet a = 5\n", fixes: []CodeFix{Fix.Replace(fixLocForTest(1, 40, 41), ";")}}, + {name: "a fix that spans two lines hides the others too", source: "let a = (\n5;\n", fixes: []CodeFix{Fix.Remove(spansLines), Fix.Insert(fixLocForTest(2, 2, 2), ")")}}, + {name: "overlapping fixes", source: "let total = 5;\n", fixes: []CodeFix{Fix.Remove(fixLocForTest(1, 5, 10)), Fix.Remove(fixLocForTest(1, 7, 12))}}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := emitFixForTest(t, colors.LogFormatNormal, test.source, test.fixes...) + if !strings.Contains(got, " = help: apply the fix\n\n") { + t.Fatalf("expected the help text with no fixed line under it, got:\n%s", got) + } + }) + } +} + +func TestEmitterMarksOnlyDroppedLettersWhenReplacementShortens(t *testing.T) { + got := emitFixForTest(t, colors.LogFormatANSI, "return totall;\n", Fix.Replace(fixLocForTest(1, 8, 14), "total")) + if !strings.Contains(got, string(colors.FIX_REMOVED)+"l"+string(colors.RESET)) || strings.Contains(got, string(colors.FIX_ADDED)) { + t.Fatalf("expected only the dropped letter marked as removed, got:\n%q", got) + } + accent := emitFixForTest(t, colors.LogFormatANSI, "let é = 1\n", Fix.Replace(fixLocForTest(1, 5, 6), "è")) + if !strings.Contains(accent, string(colors.FIX_ADDED)+"è"+string(colors.RESET)) { + t.Fatalf("expected the whole replaced character marked, got:\n%q", accent) + } +} + +func TestEmitterSizesMarginForFixLines(t *testing.T) { + prevFormat := colors.CurrentLogFormat() + colors.SetLogFormat(colors.LogFormatNormal) + defer colors.SetLogFormat(prevFormat) + + var out bytes.Buffer + emitter := NewEmitter(&out) + emitter.cache.AddSource("sample"+peeper.SourceExt, strings.Repeat("skip\n", 8)+"nine\nten\n") + emitter.Emit(NewError("broken"). WithCode("P9999"). - WithPrimaryLabel(loc, "replace"). - WithCodeInsertion(loc, "const value = 1;") + WithPrimaryLabel(fixLocForTest(9, 1, 5), ""). + WithNote("continue on the next line", Fix.Insert(fixLocForTest(10, 4, 4), ";"))) + got := out.String() + if !strings.Contains(got, "\n 9 | nine\n") || !strings.Contains(got, " = note: continue on the next line\n | \n10 | ten;\n") { + t.Fatalf("expected a two-digit margin and the fix under its note, got:\n%s", got) + } +} - emitter.Emit(diag) - text := out.String() +func TestEmitterTidiesSpacesAroundARemovedWord(t *testing.T) { + for _, test := range []struct{ name, source, want string }{ + {"after an opening bracket", "fn Add(mut a: i32) {\n", "1 | fn Add(a: i32) {\n"}, + {"at the end of the line", "let value mut\n", "1 | let value\n"}, + {"between two words", "let mut total = 5;\n", "1 | let total = 5;\n"}, + } { + t.Run(test.name, func(t *testing.T) { + start := strings.Index(test.source, "mut") + 1 + got := emitFixForTest(t, colors.LogFormatNormal, test.source, Fix.Remove(fixLocForTest(1, start, start+3))) + if !strings.Contains(got, test.want) { + t.Fatalf("expected %q in:\n%s", test.want, got) + } + }) + } +} + +func TestEmitterFixResultIgnoresTheOrderFixesWereGiven(t *testing.T) { + remove, insert := Fix.Remove(fixLocForTest(1, 5, 8)), Fix.Insert(fixLocForTest(1, 5, 5), "x") + want := " = help: apply the fix\n | \n1 | let x total = 5;\n" + first := emitFixForTest(t, colors.LogFormatNormal, "let mut total = 5;\n", remove, insert) + second := emitFixForTest(t, colors.LogFormatNormal, "let mut total = 5;\n", insert, remove) + if !strings.Contains(first, want) || !strings.Contains(second, want) { + t.Fatalf("expected %q for both orders, got:\n%s\nand:\n%s", want, first, second) + } +} - if !strings.Contains(text, "\n = suggestion:\n") { - t.Fatalf("expected suggestion header aligned with gutter, got:\n%s", text) +func TestEmitterDrawsNoFixWithALineBreakInItsText(t *testing.T) { + got := emitFixForTest(t, colors.LogFormatNormal, "let total = 5\n", Fix.Insert(fixLocForTest(1, 14, 14), ";\nreturn")) + if !strings.Contains(got, " = help: apply the fix\n\n") { + t.Fatalf("expected the help text with no fixed line under it, got:\n%s", got) } - if !strings.Contains(text, "\n | \n") { - t.Fatalf("expected blank gutter spacer lines, got:\n%s", text) +} + +func TestEmitterSeparatesAFixedLineFromTheNextText(t *testing.T) { + prevFormat := colors.CurrentLogFormat() + colors.SetLogFormat(colors.LogFormatNormal) + defer colors.SetLogFormat(prevFormat) + + var out bytes.Buffer + emitter := NewEmitter(&out) + emitter.cache.AddSource("sample"+peeper.SourceExt, "let total = 5\n") + at := fixLocForTest(1, 14, 14) + emitter.Emit(NewError("broken").WithCode("P9999").WithPrimaryLabel(at, ""). + WithHelp("first", Fix.Insert(at, ";")). + WithNote("second"). + WithNote("third")) + want := " = help: first\n | \n1 | let total = 5;\n | \n = note: second\n = note: third\n\n" + if !strings.HasSuffix(out.String(), want) { + t.Fatalf("expected one margin line after the fixed line and none between plain notes, got:\n%s", out.String()) } } diff --git a/internal/diagnostics/emitter.go b/internal/diagnostics/emitter.go index 1d11ed40..46fe4733 100644 --- a/internal/diagnostics/emitter.go +++ b/internal/diagnostics/emitter.go @@ -1,13 +1,14 @@ package diagnostics import ( + "cmp" "fmt" "io" - "os" - pathpkg "path/filepath" + "slices" "sort" "strings" "sync" + "unicode/utf8" "compiler/internal/source" "compiler/pkg/colors" @@ -162,15 +163,6 @@ type Emitter struct { highlighter *SyntaxHighlighter } -type labelContext struct { - filepath string - line int - startCol int - endCol int - codeHint *CodeHint - severity Severity -} - func NewEmitter(w io.Writer) *Emitter { logger := colors.NewLogger(colors.CurrentLogFormat()) return &Emitter{ @@ -215,20 +207,6 @@ func (e *Emitter) printBlankGutter() { e.logger.Fprintf(e.writer, colors.GREY, GUTTER_BLANK, e.currentLineNumWidth, "") } -func (e *Emitter) printAddedGutter(color colors.COLOR) { - if color == "" { - color = colors.GREEN - } - e.logger.Fprintf(e.writer, color, GUTTER_BLANK, e.currentLineNumWidth, "+") -} - -func (e *Emitter) printRemovedGutter(color colors.COLOR) { - if color == "" { - color = colors.RED - } - e.logger.Fprintf(e.writer, color, GUTTER_BLANK, e.currentLineNumWidth, "-") -} - func (e *Emitter) printPipeOnly() { e.printBlankGutter() fmt.Fprintln(e.writer) @@ -287,6 +265,14 @@ func (e *Emitter) calculateLineNumWidthForDiagnostic(diag *Diagnostic) int { } } + for _, extra := range diag.Extras { + for _, fix := range extra.Text.Fixes { + if fix.Location != nil && fix.Location.Start != nil { + lineNumbers[fix.Location.Start.Line] = true + } + } + } + maxLine := 0 for line := range lineNumbers { if line > maxLine { @@ -301,7 +287,6 @@ func (e *Emitter) calculateLineNumWidthForDiagnostic(diag *Diagnostic) int { func (e *Emitter) Emit(diag *Diagnostic) { e.currentLineNumWidth = e.calculateLineNumWidthForDiagnostic(diag) - fallbackHintCtx := e.fallbackCodeHintContext(diag) // Step 1: Print Main Diagnostic Header Block e.printDiagnosticHeader(diag) @@ -370,31 +355,28 @@ func (e *Emitter) Emit(diag *Diagnostic) { lastLine = label.Location.End.Line } } - fmt.Fprintln(e.writer) + // Keep the margin unbroken when help or notes follow the snippet. + if len(diag.Extras) > 0 { + e.printPipeOnly() + } } else if strings.TrimSpace(diag.FilePath) != "" { e.printLocationHeader(diag.FilePath, 1, 1) } - // Step 3: Print Extras and Alternative Code Corrections - suggestionHeaderPrinted := false + // Step 3: Print extra text lines and the fixes they propose + isAfterFixedLine := false for _, extra := range diag.Extras { - switch extra.Kind { - case ExtraText: - e.printText(extra.Text) - case ExtraCodeHint: - hint := extra.CodeHint - if !e.hasRenderableCodeHint(&hint) { - continue - } - if !suggestionHeaderPrinted { - e.printSuggestionHeader() - suggestionHeaderPrinted = true - } - ctx := e.codeHintContext(diag, &hint, fallbackHintCtx) - e.printCodeHint(ctx) + // A fixed line belongs to the text above it; a margin line keeps the + // next text from reading as part of it. + if isAfterFixedLine && extra.Text.Message != "" { e.printPipeOnly() } + isAfterFixedLine = e.printText(extra.Text) } + + // Every diagnostic ends with one blank line, so the next one, or the + // summary, never runs into it. + fmt.Fprintln(e.writer) } func (e *Emitter) printPeeperSnippetBlock(filepath string, label Label, severity Severity) { @@ -455,61 +437,6 @@ func (e *Emitter) printPeeperSnippetBlock(filepath string, label Label, severity } } -func (e *Emitter) fallbackCodeHintContext(diag *Diagnostic) labelContext { - ctx := labelContext{ - filepath: diag.FilePath, - line: 1, - startCol: 1, - endCol: 1, - severity: diag.Severity, - } - for _, label := range diag.Labels { - if label.Location == nil || label.Location.Start == nil { - continue - } - start := label.Location.Start - end := label.Location.End - if end == nil { - end = start - } - if label.Location.Filename != nil && *label.Location.Filename != "" { - ctx.filepath = *label.Location.Filename - } - ctx.line = start.Line - ctx.startCol = start.Column - ctx.endCol = end.Column - if label.Style == Primary { - return ctx - } - } - return ctx -} - -func (e *Emitter) codeHintContext(diag *Diagnostic, hint *CodeHint, fallback labelContext) labelContext { - ctx := fallback - ctx.severity = diag.Severity - ctx.codeHint = hint - - if hint == nil || hint.Location == nil || hint.Location.Start == nil { - return ctx - } - - start := hint.Location.Start - end := hint.Location.End - if end == nil { - end = start - } - - ctx.filepath = diag.FilePath - if hint.Location.Filename != nil && *hint.Location.Filename != "" { - ctx.filepath = *hint.Location.Filename - } - ctx.line = start.Line - ctx.startCol = start.Column - ctx.endCol = end.Column - return ctx -} - func (e *Emitter) printDiagnosticHeader(diag *Diagnostic) { var color colors.COLOR switch diag.Severity { @@ -528,264 +455,193 @@ func (e *Emitter) printDiagnosticHeader(diag *Diagnostic) { e.logger.Fprintln(e.writer, color, diag.Message) } -func (e *Emitter) printCodeHint(ctx labelContext) { - hint := ctx.codeHint - if hint == nil { - return - } - if e.printInlineReplacementHint(ctx, hint) { - return +// printText prints one extra line and reports whether fixed source lines were +// drawn beneath it. +func (e *Emitter) printText(text DiagnosticText) bool { + if text.Message == "" { + return false } - - lines := e.codeHintRenderableLines(hint) - if len(lines) == 0 { - return + color := text.Color + if color == "" { + color = colors.WHITE } - e.printBlankGutter() - fmt.Fprintln(e.writer) - - labelsByLine := make(map[int][]CodeHintLabel) - for _, label := range hint.Labels { - if label.Line <= 0 { - continue - } - labelsByLine[label.Line] = append(labelsByLine[label.Line], label) - } - - for i, line := range lines { - prefix := strings.TrimSpace(line.Prefix) - switch prefix { - case "+": - e.printAddedGutter(hint.GutterColor) - case "-": - e.printRemovedGutter(colors.RED) - default: - e.printBlankGutter() - } - - if line.BaseColor != "" { - e.highlighter.HighlightWithBaseColor(line.Code, e.writer, line.BaseColor) - } else if hint.BaseColor != "" { - e.highlighter.HighlightWithBaseColor(line.Code, e.writer, hint.BaseColor) - } else { - e.highlighter.HighlightWithColor(line.Code, e.writer) - } - fmt.Fprintln(e.writer) - - if labels := labelsByLine[i+1]; len(labels) > 0 { - for _, label := range labels { - e.printCodeHintLabelLine(label, ctx.severity) - } - } + e.printSideNotePrefix() + if text.Kind != "" { + e.logger.Fprintf(e.writer, color, "= %s: ", text.Kind) + } else { + e.logger.Fprintf(e.writer, color, "= ") } + fmt.Fprintln(e.writer, text.Message) + return e.printFixes(text.Fixes) } -func (e *Emitter) printInlineReplacementHint(ctx labelContext, hint *CodeHint) bool { - if hint == nil { - return false - } - lines := e.codeHintRenderableLines(hint) - if len(lines) != 2 { - return false - } - if strings.TrimSpace(lines[0].Prefix) != "-" || strings.TrimSpace(lines[1].Prefix) != "+" { - return false - } - if strings.Contains(lines[0].Code, "\n") || strings.Contains(lines[1].Code, "\n") { - return false - } - if ctx.filepath == "" || ctx.line <= 0 || ctx.startCol <= 0 || ctx.endCol < ctx.startCol { - return false - } - - sourceLine, err := e.cache.GetLine(ctx.filepath, ctx.line) - if err != nil { - return false - } - expandedSourceLine := expandTabs(sourceLine) +// fixSpan is one fix placed on its source line, as byte offsets into the +// tab-expanded line. +type fixSpan struct { + start, end int + newText string +} - oldFrag := lines[0].Code - start := visualColumnToPosition(sourceLine, ctx.startCol) - if start < 0 || start > len(expandedSourceLine) { - return false - } - end := visualColumnToPosition(sourceLine, ctx.endCol) +// fixedLine is a source line together with every fix that changes it. +type fixedLine struct { + file string + number int + source string + spans []fixSpan +} - if oldFrag != "" { - if start+len(oldFrag) <= len(expandedSourceLine) && expandedSourceLine[start:start+len(oldFrag)] == oldFrag { - end = start + len(oldFrag) - } else if idx := strings.Index(expandedSourceLine, oldFrag); idx >= 0 { - start = idx - end = idx + len(oldFrag) - } - } - if end < start || end > len(expandedSourceLine) { +// printFixes shows every source line that fixes change, in the order the +// lines are first mentioned. Fixes that cannot be shown faithfully on single +// lines are not drawn at all; the text that proposes them still stands. +func (e *Emitter) printFixes(fixes []CodeFix) bool { + lines, ok := e.fixedLines(fixes) + if !ok || len(lines) == 0 { return false } - - newFrag := lines[1].Code - replacementLine := expandedSourceLine[:start] + newFrag + expandedSourceLine[end:] - oldAbsStart, oldDiffLen, newAbsStart, newDiffLen := diffHighlightSpans(start, oldFrag, newFrag) - - e.printBlankGutter() - fmt.Fprintln(e.writer) - - if oldAbsStart < 0 { - oldAbsStart = 0 - } - if oldAbsStart > len(expandedSourceLine) { - oldAbsStart = len(expandedSourceLine) + e.printPipeOnly() + for _, line := range lines { + e.printFixedLine(line) } - - relPath := e.diffDisplayPath(ctx.filepath) - e.printBlankGutter() - e.logger.Fprintf(e.writer, colors.RED, " --- a/%s\n", relPath) - e.printBlankGutter() - e.logger.Fprintf(e.writer, colors.GREEN, " +++ b/%s\n", relPath) - e.printBlankGutter() - e.logger.Fprintf(e.writer, colors.GREY, " @@ line %d @@\n", ctx.line) - - e.printBlankGutter() - e.logger.Fprint(e.writer, colors.RED, "- ") - e.printLineWithColoredSpan(expandedSourceLine, oldAbsStart, oldDiffLen, colors.RED) - fmt.Fprintln(e.writer) - - e.printBlankGutter() - e.logger.Fprint(e.writer, colors.GREEN, "+ ") - e.printLineWithColoredSpan(replacementLine, newAbsStart, newDiffLen, colors.GREEN) - fmt.Fprintln(e.writer) - return true } -func diffHighlightSpans(baseStart int, oldFrag, newFrag string) (oldStart, oldLen, newStart, newLen int) { - oldStart, newStart = baseStart, baseStart - oldBytes, newBytes := []byte(oldFrag), []byte(newFrag) - - prefix := 0 - for prefix < len(oldBytes) && prefix < len(newBytes) && oldBytes[prefix] == newBytes[prefix] { - prefix++ - } - - suffix := 0 - for suffix < len(oldBytes)-prefix && suffix < len(newBytes)-prefix && - oldBytes[len(oldBytes)-1-suffix] == newBytes[len(newBytes)-1-suffix] { - suffix++ - } - - oldLen = len(oldBytes) - prefix - suffix - newLen = len(newBytes) - prefix - suffix - oldStart += prefix - newStart += prefix - - return oldStart, oldLen, newStart, newLen -} - -func (e *Emitter) printLineWithColoredSpan(line string, start, length int, spanColor colors.COLOR) { - if spanColor == "" || length <= 0 || start < 0 || start > len(line) { - e.highlighter.HighlightWithColor(line, e.writer) - return - } - end := min(start+length, len(line)) - if start >= end { - e.highlighter.HighlightWithColor(line, e.writer) - return +// fixedLines groups fixes by the line they change and orders each line's fixes +// by position. It reports false when any fix has no usable location, spans +// lines, points outside its line, or overlaps another: drawing only part of a +// set of edits would show a line the fix does not produce. +func (e *Emitter) fixedLines(fixes []CodeFix) ([]fixedLine, bool) { + var lines []fixedLine + for _, fix := range fixes { + loc := fix.Location + if loc == nil || loc.Start == nil || loc.Filename == nil { + return nil, false + } + if loc.End != nil && loc.End.Line != loc.Start.Line || strings.ContainsAny(fix.NewText, "\r\n") { + return nil, false + } + index := slices.IndexFunc(lines, func(line fixedLine) bool { + return line.file == *loc.Filename && line.number == loc.Start.Line + }) + if index < 0 { + sourceLine, err := e.cache.GetLine(*loc.Filename, loc.Start.Line) + if err != nil { + return nil, false + } + index = len(lines) + lines = append(lines, fixedLine{file: *loc.Filename, number: loc.Start.Line, source: sourceLine}) + } + start, ok := columnOffset(lines[index].source, loc.Start.Column) + end := start + if ok && loc.End != nil { + end, ok = columnOffset(lines[index].source, loc.End.Column) + } + if !ok || end < start { + return nil, false + } + lines[index].spans = append(lines[index].spans, fixSpan{start: start, end: end, newText: fix.NewText}) + } + for _, line := range lines { + // An insertion sorts before a wider fix that starts at the same place, + // so the result does not depend on the order the fixes were given in. + slices.SortStableFunc(line.spans, func(a, b fixSpan) int { return cmp.Or(a.start-b.start, a.end-b.end) }) + for i := 1; i < len(line.spans); i++ { + if line.spans[i].start < line.spans[i-1].end { + return nil, false + } + } } - - e.highlighter.HighlightWithColor(line[:start], e.writer) - e.logger.Fprint(e.writer, spanColor, line[start:end]) - e.highlighter.HighlightWithColor(line[end:], e.writer) + return lines, true } -func (e *Emitter) diffDisplayPath(filepath string) string { - if filepath == "" { - return "unknown" - } - rel := filepath - if wd, err := os.Getwd(); err == nil { - if r, err := pathpkg.Rel(wd, filepath); err == nil && r != "" && !strings.HasPrefix(r, "..") { - rel = r +// columnOffset maps a 1-based character column of sourceLine to a byte offset +// in its tab-expanded form. Columns count characters, so a character of +// several bytes or a tab of several spaces is one column. The column just past +// the last character is the end of the line; anything further is not on it. +func columnOffset(sourceLine string, column int) (int, bool) { + offset, visual, current := 0, 0, 1 + for _, ch := range sourceLine { + if current == column { + return offset, true + } + if ch == '\t' { + spaces := TAB_WIDTH - (visual % TAB_WIDTH) + offset += spaces + visual += spaces + } else { + offset += utf8.RuneLen(ch) + visual++ } + current++ } - return pathpkg.ToSlash(rel) -} - -func (e *Emitter) hasRenderableCodeHint(hint *CodeHint) bool { - return len(e.codeHintRenderableLines(hint)) > 0 + return offset, current == column } -func (e *Emitter) codeHintRenderableLines(hint *CodeHint) []CodeHintLine { - if hint == nil { - return nil - } - if len(hint.Lines) > 0 { - return hint.Lines - } - if hint.Code == "" { - return nil - } - rawLines := strings.Split(hint.Code, "\n") - lines := make([]CodeHintLine, 0, len(rawLines)) - for _, line := range rawLines { - lines = append(lines, CodeHintLine{ - Prefix: "+", - Code: line, - BaseColor: hint.BaseColor, +// printFixedLine draws one fixed line. A line that gains text is shown as it +// will read, with only the new characters marked. A line that only loses text +// is shown as it reads now, with the removed part marked; plain output cannot +// show that marking, so it shows the line as it will read. +func (e *Emitter) printFixedLine(line fixedLine) { + text := expandTabs(line.source) + showsResult := e.logger.Format() == colors.LogFormatNormal || + slices.ContainsFunc(line.spans, func(span fixSpan) bool { + _, _, added, _ := diffText(text[span.start:span.end], span.newText) + return added != "" }) - } - return lines -} -func (e *Emitter) printCodeHintLabelLine(label CodeHintLabel, severity Severity) { - if label.Column <= 0 { - return - } - length := label.Length - if length <= 0 { - length = 1 - } - - padding := label.Column - 1 - e.printBlankGutter() - fmt.Fprint(e.writer, strings.Repeat(" ", padding)) - - var color colors.COLOR - if label.Style == Primary { - color = e.getSeverityColor(severity) - } else { - color = colors.BLUE - } - - e.logger.Fprint(e.writer, color, strings.Repeat("~", length)) - if label.Message != "" { - e.logger.Fprintf(e.writer, color, " %s", label.Message) + e.printGutter(line.number) + position := 0 + for i, span := range line.spans { + kept, removed, added, keptAfter := diffText(text[span.start:span.end], span.newText) + // Removing a whole word leaves the space on each side of it next to + // each other, or a stray one beside a bracket or at the line's end; + // drop one so the line reads as a person would type it. + isWordRemoved := showsResult && span.newText == "" && removed != "" + before := text[position:span.start] + if isWordRemoved && span.end == len(text) { + before = strings.TrimSuffix(before, " ") + } + e.highlighter.HighlightWithColor(before, e.writer) + position = span.end + + e.highlighter.HighlightWithColor(kept, e.writer) + if showsResult && added != "" { + e.logger.Fprint(e.writer, colors.FIX_ADDED, added) + } else if !showsResult && removed != "" { + e.logger.Fprint(e.writer, colors.FIX_REMOVED, removed) + } + e.highlighter.HighlightWithColor(keptAfter, e.writer) + + // The space after the word goes when the word followed a space, an + // opening bracket or the line's start, unless the next fix starts on it. + isSpaceNext := strings.HasPrefix(text[position:], " ") + isAfterInsertion := i > 0 && line.spans[i-1].end == span.start && line.spans[i-1].newText != "" + isAfterGap := !isAfterInsertion && (span.start == 0 || strings.ContainsRune(" ([{<", rune(text[span.start-1]))) + isSpaceFree := i+1 == len(line.spans) || line.spans[i+1].start > position + if isWordRemoved && isSpaceNext && isAfterGap && isSpaceFree { + position++ + } } + e.highlighter.HighlightWithColor(text[position:], e.writer) fmt.Fprintln(e.writer) } -func (e *Emitter) printText(text DiagnosticText) { - if text.Message == "" { - return - } - color := text.Color - if color == "" { - color = colors.WHITE +// diffText splits a replacement into what both texts share at the start and +// the end, and what differs in between: the characters that go and the +// characters that come. Only the differing part is marked. +func diffText(oldText, newText string) (kept, removed, added, keptAfter string) { + before, after := []rune(oldText), []rune(newText) + prefix := 0 + for prefix < len(before) && prefix < len(after) && before[prefix] == after[prefix] { + prefix++ } - - e.printSideNotePrefix() - if text.Kind != "" { - e.logger.Fprintf(e.writer, color, "= %s: ", text.Kind) - } else { - e.logger.Fprintf(e.writer, color, "= ") + suffix := 0 + for suffix < len(before)-prefix && suffix < len(after)-prefix && + before[len(before)-1-suffix] == after[len(after)-1-suffix] { + suffix++ } - fmt.Fprintln(e.writer, text.Message) -} - -func (e *Emitter) printSuggestionHeader() { - e.printSideNotePrefix() - e.logger.Fprint(e.writer, colors.GREEN, "= suggestion:") - fmt.Fprintln(e.writer) + return string(before[:prefix]), string(before[prefix : len(before)-suffix]), + string(after[prefix : len(after)-suffix]), string(before[len(before)-suffix:]) } func (e *Emitter) getSeverityColor(severity Severity) colors.COLOR { diff --git a/internal/frontend/parser/parse_expr.go b/internal/frontend/parser/parse_expr.go index e6ebef3f..9cf6576f 100644 --- a/internal/frontend/parser/parse_expr.go +++ b/internal/frontend/parser/parse_expr.go @@ -263,10 +263,17 @@ func (p *Parser) parseNumberLit(sign string) ast.Expr { } if len(literal.Value) > 1 && literal.Value[0] == '0' && numeric.IsDecimal(literal.Value) && (literal.ExplicitType == "" || literal.ExplicitType[0] != 'f') { - p.diag.Add(diagnostics.NewWarning("decimal integer literal has a leading zero"). - WithCode(diagnostics.WarnLeadingZeroDecimal). - WithPrimaryLabel(loc, "remove the leading zero"). - WithHelp("use the `0o` prefix for octal values")) + d := diagnostics.NewWarning("decimal integer literal has a leading zero"). + WithCode(diagnostics.WarnLeadingZeroDecimal) + if zeros := leadingZeroRun(tok.Literal); zeros != "" { + zerosEnd := tok.Start + zerosEnd.Advance(zeros) + d.WithPrimaryLabel(loc, ""). + WithHelp("remove the leading zero", diagnostics.Fix.Remove(source.NewLocation(p.filePath, tok.Start, zerosEnd))) + } else { + d.WithPrimaryLabel(loc, "remove the leading zero") + } + p.diag.Add(d.WithHelp("use the `0o` prefix for octal values")) } if sign == "-" { literal.Value = "-" + literal.Value @@ -274,6 +281,25 @@ func (p *Parser) parseNumberLit(sign string) ast.Expr { return reg(p, &ast.NumberLit{Value: literal.Value, ExplicitType: literal.ExplicitType, Location: loc}) } +// leadingZeroRun returns the zeros, and the digit separators among them, that +// can be dropped from the front of a decimal literal as written. It always +// leaves a digit behind, so `000` keeps one zero, and returns "" when what +// would remain does not start with a digit. +func leadingZeroRun(literal string) string { + digitsEnd := 0 + for digitsEnd < len(literal) && (literal[digitsEnd] == '_' || (literal[digitsEnd] >= '0' && literal[digitsEnd] <= '9')) { + digitsEnd++ + } + run := 0 + for run < digitsEnd-1 && (literal[run] == '0' || literal[run] == '_') { + run++ + } + if run == digitsEnd || literal[run] == '_' { + return "" + } + return literal[:run] +} + func (p *Parser) parseAddressExpr(mode ast.AddressMode, boundary token.Kind) ast.Expr { tok := p.advance() if mode == ast.AddressShared && p.match(token.MUT) { diff --git a/internal/frontend/parser/parse_stmt.go b/internal/frontend/parser/parse_stmt.go index a07a6e65..8071b6a7 100644 --- a/internal/frontend/parser/parse_stmt.go +++ b/internal/frontend/parser/parse_stmt.go @@ -90,7 +90,7 @@ func (p *Parser) parseBlock() *ast.BlockStmt { } var stmts []ast.Stmt for !p.at(token.RBRACE) && !p.at(token.EOF) { - p.consumeRedundant(token.SEMICOLON, diagnostics.InfoUnnecessarySemicolon, "unnecessary semicolons", "remove these semicolons") + p.consumeRedundant(token.SEMICOLON, diagnostics.InfoUnnecessarySemicolon, "semicolon", false) before := p.pos if stmt := p.parseStmt(false); stmt != nil { stmts = append(stmts, stmt) @@ -347,9 +347,11 @@ func (p *Parser) parseMatchStmt() ast.Stmt { })) if p.match(token.COMMA) { comma := p.prev() + loc := source.NewLocation(p.filePath, comma.Start, comma.End) p.diag.Add(diagnostics.NewError("match arms do not use commas"). WithCode(diagnostics.ErrExpectedToken). - WithPrimaryLabel(source.NewLocation(p.filePath, comma.Start, comma.End), "remove this comma")) + WithPrimaryLabel(loc, ""). + WithHelp("remove this comma", diagnostics.Fix.Remove(loc))) } } armListEnd := p.current().End diff --git a/internal/frontend/parser/parse_types.go b/internal/frontend/parser/parse_types.go index eb83df67..8872bd04 100644 --- a/internal/frontend/parser/parse_types.go +++ b/internal/frontend/parser/parse_types.go @@ -170,9 +170,11 @@ func (p *Parser) parseOptionalTypeExpr() ast.TypeExpr { for _, start := range markers[:len(markers)-1] { end := start end.Advance("?") + loc := source.NewLocation(p.filePath, start, end) p.diag.Add(diagnostics.NewInfo("redundant optional marker"). WithCode(diagnostics.InfoRedundantOptional). - WithPrimaryLabel(source.NewLocation(p.filePath, start, end), "remove redundant `?`"). + WithPrimaryLabel(loc, ""). + WithHelp("remove redundant `?`", diagnostics.Fix.Remove(loc)). WithNote("nested optional types are the same as a single optional type")) } return inner diff --git a/internal/frontend/parser/parser.go b/internal/frontend/parser/parser.go index 4841fd2b..ece4fc1d 100644 --- a/internal/frontend/parser/parser.go +++ b/internal/frontend/parser/parser.go @@ -45,7 +45,7 @@ func (p *Parser) ParseModule() *ast.Module { surface := moduleSurface{} for !p.at(token.EOF) { - p.consumeRedundant(token.SEMICOLON, diagnostics.InfoUnnecessarySemicolon, "unnecessary semicolons", "remove these semicolons") + p.consumeRedundant(token.SEMICOLON, diagnostics.InfoUnnecessarySemicolon, "semicolon", false) if p.at(token.IMPORT) { if imp := p.parseImport(); imp != nil { mod.Imports = append(mod.Imports, imp) @@ -599,9 +599,7 @@ func (p *Parser) expectClose(openPos source.Position, kind token.Kind, name stri } prev := p.prev() loc := source.NewLocation(p.filePath, prev.End, prev.End) - p.diag.Add(diagnostics.NewError( - fmt.Sprintf("expected '%s'", string(kind)), - ).WithCode(diagnostics.ErrExpectedToken).WithPrimaryLabel(loc, fmt.Sprintf("add missing `%s` here", string(kind)))) + p.diag.Add(missingTokenError(fmt.Sprintf("expected '%s'", string(kind)), kind, loc)) p.synchronize(kind) if p.current().Kind == kind { return p.advance() @@ -609,7 +607,11 @@ func (p *Parser) expectClose(openPos source.Position, kind token.Kind, name stri return nil } -func (p *Parser) consumeRedundant(kind token.Kind, code string, msg string, label string) { +// consumeRedundant skips a run of kind, named noun in messages, and reports it +// when it holds more than one token. isFirstNeeded says the run starts with a +// token the grammar wants there, such as the comma after a list item, so the +// advice and the fix leave that one alone. +func (p *Parser) consumeRedundant(kind token.Kind, code string, noun string, isFirstNeeded bool) { count := 0 var first, last *token.Token for p.at(kind) { @@ -621,9 +623,24 @@ func (p *Parser) consumeRedundant(kind token.Kind, code string, msg string, labe last = tok } if count > 1 && first != nil && last != nil { - p.diag.Add(diagnostics.NewInfo(msg). - WithCode(code). - WithPrimaryLabel(source.NewLocation(p.filePath, first.Start, last.End), label)) + loc := source.NewLocation(p.filePath, first.Start, last.End) + removed, advice := loc, "remove these "+noun+"s" + if isFirstNeeded { + removed = source.NewLocation(p.filePath, first.End, last.End) + advice = "remove the extra " + noun + if count > 2 { + advice += "s" + } + } + d := diagnostics.NewInfo("unnecessary " + noun + "s").WithCode(code) + // A fixed line can only be shown for a run that sits on one line; a + // longer run keeps the advice beside the marker. + if first.Start.Line == last.End.Line { + d.WithPrimaryLabel(loc, "").WithHelp(advice, diagnostics.Fix.Remove(removed)) + } else { + d.WithPrimaryLabel(loc, advice) + } + p.diag.Add(d) } } @@ -640,11 +657,14 @@ func parseBracedItemList[T any]( } lbraceStart := lbrace.Start var items []T + // The comma that follows an item is consumed with any extra ones at the top + // of the next round, so only a run before the first item is all redundant. + isAfterItem := false for !p.at(token.RBRACE) && !p.at(token.EOF) { for p.at(token.DOC_COMMENT) { p.advance() } - p.consumeRedundant(token.COMMA, diagnostics.InfoRedundantComma, "unnecessary commas", "remove these commas") + p.consumeRedundant(token.COMMA, diagnostics.InfoRedundantComma, "comma", isAfterItem) for p.at(token.DOC_COMMENT) { p.advance() } @@ -652,6 +672,7 @@ func parseBracedItemList[T any]( break } item, ok := parseItem() + isAfterItem = true if ok { items = append(items, item) } else { @@ -659,15 +680,17 @@ func parseBracedItemList[T any]( } if p.at(token.COMMA) { if p.next().Kind == token.RBRACE { + loc := source.NewLocation(p.filePath, p.current().Start, p.current().End) p.diag.Add(diagnostics.NewInfo("trailing comma is unnecessary"). WithCode(diagnostics.InfoTrailingComma). - WithPrimaryLabel(source.NewLocation(p.filePath, p.current().Start, p.current().End), "remove this comma")) + WithPrimaryLabel(loc, ""). + WithHelp("remove this comma", diagnostics.Fix.Remove(loc))) } continue } if !p.at(token.RBRACE) { prev := p.stream[p.pos-1] - p.diag.Add(diagnostics.NewError(itemMsg).WithCode(diagnostics.ErrExpectedToken).WithPrimaryLabel(source.NewLocation(p.filePath, prev.End, prev.End), "add missing `,` here")) + p.diag.Add(missingTokenError(itemMsg, token.COMMA, source.NewLocation(p.filePath, prev.End, prev.End))) // Recovery must always consume or skip the unexpected separator token. // Without this, inputs like `foo();` inside a braced item list keep // reporting the same missing-comma diagnostic forever. @@ -700,12 +723,25 @@ func (p *Parser) consume(kind token.Kind, msg string) *token.Token { } prev := p.stream[p.pos-1] loc := source.NewLocation(p.filePath, prev.End, prev.End) - p.diag.Add(diagnostics.NewError(msg). - WithCode(diagnostics.ErrExpectedToken). - WithPrimaryLabel(loc, fmt.Sprintf("add missing `%s` here", string(kind)))) + p.diag.Add(missingTokenError(msg, kind, loc)) return nil } +// missingTokenError reports a token that should follow loc. Separators and +// closing delimiters are spelled exactly as their kind, so the diagnostic can +// show the line with the token inserted. Other kinds name a category +// ("identifier") or need spacing a blind insertion would get wrong, so they +// only say what is missing. +func missingTokenError(msg string, kind token.Kind, loc *source.Location) *diagnostics.Diagnostic { + d := diagnostics.NewError(msg).WithCode(diagnostics.ErrExpectedToken) + switch kind { + case token.SEMICOLON, token.COMMA, token.RPAREN, token.RBRACK, token.RBRACE, token.GT: + return d.WithPrimaryLabel(loc, ""). + WithHelp(fmt.Sprintf("add missing `%s`", string(kind)), diagnostics.Fix.Insert(loc, string(kind))) + } + return d.WithPrimaryLabel(loc, fmt.Sprintf("add missing `%s` here", string(kind))) +} + // advances token if matched func (p *Parser) match(kind token.Kind) bool { if p.current().Kind != kind { diff --git a/internal/frontend/parser/parser_test.go b/internal/frontend/parser/parser_test.go index 9a2ff025..979d910a 100644 --- a/internal/frontend/parser/parser_test.go +++ b/internal/frontend/parser/parser_test.go @@ -2078,7 +2078,7 @@ func TestParseInterfaceMethodsUnexpectedSemicolonRecovers(t *testing.T) { t.Fatalf("methods: got %d want 2", len(ifaceType.Methods)) } if !strings.Contains(diag.EmitAllToString(), "expected '}' after iface methods") && - !strings.Contains(diag.EmitAllToString(), "add missing `,` here") { + !strings.Contains(diag.EmitAllToString(), "add missing `,`") { t.Fatalf("expected separator recovery diagnostic, got:\n%s", diag.EmitAllToString()) } } @@ -2687,8 +2687,10 @@ func TestParseRedundantOptionalSyntax(t *testing.T) { if item.Severity != diagnostics.Info { t.Fatalf("severity = %v, want info", item.Severity) } - if len(item.Labels) != 1 || !strings.Contains(item.Labels[0].Message, "remove redundant `?`") { - t.Fatalf("missing removal advice: %#v", item) + if len(item.Labels) != 1 || len(item.Extras) == 0 || + item.Extras[0].Text.Message != "remove redundant `?`" || len(item.Extras[0].Text.Fixes) != 1 || + item.Extras[0].Text.Fixes[0] != diagnostics.Fix.Remove(item.Labels[0].Location) { + t.Fatalf("missing removal advice with its fix: %#v", item) } loc := item.Labels[0].Location if loc == nil || loc.Start == nil || loc.End == nil { @@ -2708,3 +2710,149 @@ func TestParseRedundantOptionalSyntax(t *testing.T) { }) } } + +func TestMissingTokenFixOnlyForSeparatorsAndClosers(t *testing.T) { + for _, test := range []struct { + name string + source string + message string + insert string + label string + }{ + {name: "semicolon", source: "fn main() { let total = 5 }", message: "expected ';' after statement", insert: ";"}, + {name: "closing paren", source: "fn main() { let total = (5; }", message: "expected ')'", insert: ")"}, + {name: "identifier", source: "import \"std/io\" as;", label: "add missing `identifier` here"}, + {name: "keyword", source: "fn main() { for a, b v {} }", label: "add missing `in` here"}, + } { + t.Run(test.name, func(t *testing.T) { + _, diag := parseTestModule(test.source) + for _, item := range diag.Diagnostics() { + if item.Code != diagnostics.ErrExpectedToken || len(item.Labels) == 0 { + continue + } + if test.message != "" && item.Message != test.message { + continue + } + if test.insert == "" { + if len(item.Extras) != 0 || item.Labels[0].Message != test.label { + continue + } + return + } + at := item.Labels[0].Location + if len(item.Extras) != 1 || item.Extras[0].Text.Message != "add missing `"+test.insert+"`" || + len(item.Extras[0].Text.Fixes) != 1 || item.Labels[0].Message != "" { + t.Fatalf("expected one insertion fix and an empty label, got: %#v", item) + } + fix := item.Extras[0].Text.Fixes[0] + if fix.NewText != test.insert || *fix.Location.Start != *at.Start || *fix.Location.End != *at.Start { + t.Fatalf("fix must insert %q at the reported point, got: %#v at %v", test.insert, fix, fix.Location) + } + return + } + t.Fatalf("expected diagnostic not found, got:\n%s", diag.EmitAllToString()) + }) + } +} + +func TestTrailingCommaOffersItsRemoval(t *testing.T) { + _, diag := parseTestModule("struct Point { x: i32, y: i32, }") + for _, item := range diag.Diagnostics() { + if item.Code != diagnostics.InfoTrailingComma { + continue + } + if len(item.Labels) != 1 || len(item.Extras) != 1 || item.Extras[0].Text.Message != "remove this comma" || + len(item.Extras[0].Text.Fixes) != 1 || item.Extras[0].Text.Fixes[0] != diagnostics.Fix.Remove(item.Labels[0].Location) { + t.Fatalf("expected the advice with a fix removing the comma, got: %#v", item) + } + return + } + t.Fatalf("expected trailing comma info, got:\n%s", diag.EmitAllToString()) +} + +func TestRedundantSeparatorFixKeepsTheNeededOne(t *testing.T) { + for _, test := range []struct { + name string + source string + code string + removedStart int + removedEnd int + }{ + {name: "commas after an item keep one", source: "struct Point { x: i32,,, y: i32 }", code: diagnostics.InfoRedundantComma, removedStart: 23, removedEnd: 25}, + {name: "commas before the first item all go", source: "struct Point { ,, x: i32 }", code: diagnostics.InfoRedundantComma, removedStart: 16, removedEnd: 18}, + {name: "semicolons all go", source: "fn main() { let total = 5;;; }", code: diagnostics.InfoUnnecessarySemicolon, removedStart: 27, removedEnd: 29}, + } { + t.Run(test.name, func(t *testing.T) { + _, diag := parseTestModule(test.source) + for _, item := range diag.Diagnostics() { + if item.Code != test.code { + continue + } + if len(item.Extras) != 1 || len(item.Extras[0].Text.Fixes) != 1 { + t.Fatalf("expected one removal fix, got: %#v", item) + } + fix := item.Extras[0].Text.Fixes[0] + if fix.NewText != "" || fix.Location.Start.Column != test.removedStart || fix.Location.End.Column != test.removedEnd { + t.Fatalf("fix removes columns %d-%d, want %d-%d", fix.Location.Start.Column, fix.Location.End.Column, test.removedStart, test.removedEnd) + } + return + } + t.Fatalf("expected %s, got:\n%s", test.code, diag.EmitAllToString()) + }) + } +} + +func TestLeadingZeroRun(t *testing.T) { + for literal, want := range map[string]string{ + "007": "00", + "000": "00", + "00_7": "00_", + "0_0_7": "0_0_", + "0_0": "0_", + "007i32": "00", + "00_": "", + "7": "", + "0": "", + } { + if got := leadingZeroRun(literal); got != want { + t.Errorf("leadingZeroRun(%q) = %q, want %q", literal, got, want) + } + } +} + +func TestLeadingZeroFixRemovesSeparatorsWithTheZeros(t *testing.T) { + _, diag := parseTestModule("fn main() -> i32 { return 00_7; }") + for _, item := range diag.Diagnostics() { + if item.Code != diagnostics.WarnLeadingZeroDecimal { + continue + } + if len(item.Extras) == 0 || len(item.Extras[0].Text.Fixes) != 1 { + t.Fatalf("expected a removal fix, got: %#v", item) + } + fix := item.Extras[0].Text.Fixes[0] + if fix.NewText != "" || fix.Location.Start.Column != 27 || fix.Location.End.Column != 30 { + t.Fatalf("fix removes columns %d-%d, want 27-30 (`00_`)", fix.Location.Start.Column, fix.Location.End.Column) + } + return + } + t.Fatalf("expected leading zero warning, got:\n%s", diag.EmitAllToString()) +} + +func TestRedundantCommaAdviceCountsOnlyTheExtraOnes(t *testing.T) { + for source, want := range map[string]string{ + "struct Point { x: i32,, y: i32 }": "remove the extra comma", + "struct Point { x: i32,,, y: i32 }": "remove the extra commas", + "struct Point { ,, x: i32 }": "remove these commas", + } { + _, diag := parseTestModule(source) + found := false + for _, item := range diag.Diagnostics() { + if item.Code == diagnostics.InfoRedundantComma && len(item.Extras) == 1 && item.Extras[0].Text.Message == want { + found = true + } + } + if !found { + t.Errorf("%q: expected advice %q, got:\n%s", source, want, diag.EmitAllToString()) + } + } +} diff --git a/internal/semantics/usage/usage.go b/internal/semantics/usage/usage.go index c5257f84..9061fb57 100644 --- a/internal/semantics/usage/usage.go +++ b/internal/semantics/usage/usage.go @@ -95,9 +95,8 @@ func Analyze(diag *diagnostics.DiagnosticBag, module *module.Module, preludeID m continue } diag.AddWarning(diagnostics.WarnUnmodifiedMutable, - fmt.Sprintf("mutable binding `%s` is never modified", sym.Name), sym.MutableLocation, "remove unnecessary `mut`"). - WithCodeReplacement(sym.MutableLocation, "mut", ""). - WithHelp("remove unnecessary `mut`") + fmt.Sprintf("mutable binding `%s` is never modified", sym.Name), sym.MutableLocation, ""). + WithHelp("remove unnecessary `mut`", diagnostics.Fix.Remove(sym.MutableLocation)) } }) } diff --git a/internal/semantics/usage/usage_test.go b/internal/semantics/usage/usage_test.go index f3fc7df0..89207eb6 100644 --- a/internal/semantics/usage/usage_test.go +++ b/internal/semantics/usage/usage_test.go @@ -365,8 +365,8 @@ func TestUnmodifiedMutableLocalSuggestsRemovingModifier(t *testing.T) { } for _, text := range []string{ "mutable binding `value` is never modified", + "= help: ", "remove unnecessary `mut`", - "= suggestion:", } { if !strings.Contains(out, text) { t.Fatalf("expected %q in removal suggestion, got:\n%s", text, out) @@ -377,12 +377,12 @@ func TestUnmodifiedMutableLocalSuggestsRemovingModifier(t *testing.T) { continue } if len(item.Extras) == 0 { - t.Fatalf("expected W0013 code replacement, got: %#v", item.Extras) + t.Fatalf("expected W0013 help with a fix, got: %#v", item.Extras) } - hint := item.Extras[0].CodeHint - if len(hint.Lines) != 2 || hint.Lines[0].Prefix != "-" || hint.Lines[0].Code != "mut" || - hint.Lines[1].Prefix != "+" || hint.Lines[1].Code != "" { - t.Fatalf("unexpected W0013 code replacement: %#v", hint.Lines) + fixes := item.Extras[0].Text.Fixes + if len(fixes) != 1 || fixes[0].NewText != "" || + fixes[0].Location.End.Column-fixes[0].Location.Start.Column != len("mut") { + t.Fatalf("expected one fix removing exactly `mut`, got: %#v", fixes) } return } @@ -400,12 +400,12 @@ func TestUnmodifiedMutableCommentAdjacentSuggestionUsesModifierSpan(t *testing.T continue } if len(item.Extras) == 0 { - t.Fatalf("expected W0013 code replacement, got: %#v", item.Extras) + t.Fatalf("expected W0013 help with a fix, got: %#v", item.Extras) } - hint := item.Extras[0].CodeHint - if len(hint.Lines) != 2 || hint.Lines[0].Prefix != "-" || hint.Lines[0].Code != "mut" || - hint.Lines[1].Prefix != "+" || hint.Lines[1].Code != "" { - t.Fatalf("unexpected W0013 code replacement: %#v", hint.Lines) + fixes := item.Extras[0].Text.Fixes + if len(fixes) != 1 || fixes[0].NewText != "" || + fixes[0].Location.End.Column-fixes[0].Location.Start.Column != len("mut") { + t.Fatalf("expected one fix removing exactly `mut`, got: %#v", fixes) } return } diff --git a/pkg/colors/color.go b/pkg/colors/color.go index acbc5214..84d3f050 100644 --- a/pkg/colors/color.go +++ b/pkg/colors/color.go @@ -51,4 +51,9 @@ const ( LIGHT_BLUE COLOR = "\033[38;5;81m" LIGHT_GREEN COLOR = "\033[38;5;120m" LIGHT_YELLOW COLOR = "\033[38;5;229m" + + // Text a suggested fix adds or removes: a dark letter on a mid-tone block + // of the same hue, chosen to read on both dark and light terminals. + FIX_ADDED COLOR = "\033[38;5;22;48;5;114m" + FIX_REMOVED COLOR = "\033[38;5;52;48;5;210m" ) diff --git a/pkg/colors/logger.go b/pkg/colors/logger.go index 5aedc961..5b22b499 100644 --- a/pkg/colors/logger.go +++ b/pkg/colors/logger.go @@ -63,6 +63,8 @@ var htmlStyles = map[COLOR]string{ LIGHT_BLUE: "color: #5fd7ff", LIGHT_GREEN: "color: #87ff87", LIGHT_YELLOW: "color: #ffffaf", + FIX_ADDED: "color: #005f00; background-color: #87d787", + FIX_REMOVED: "color: #5f0000; background-color: #ff8787", } func ParseLogFormat(raw string) (LogFormat, error) { diff --git a/x_test/negative_missing_comma_fix/peeper.toml b/x_test/negative_missing_comma_fix/peeper.toml new file mode 100644 index 00000000..2aa1995e --- /dev/null +++ b/x_test/negative_missing_comma_fix/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_missing_comma_fix" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = [" = help: add missing `,`\n | \n2 | x: i32,\n", " = help: add missing `)`\n | \n7 | let total: i32 = (5);\n"] +stderr_excludes = ["\n\n\n"] diff --git a/x_test/negative_missing_comma_fix/src/main.peep b/x_test/negative_missing_comma_fix/src/main.peep new file mode 100644 index 00000000..0a711216 --- /dev/null +++ b/x_test/negative_missing_comma_fix/src/main.peep @@ -0,0 +1,9 @@ +struct Point { + x: i32 + y: i32 +} + +fn main() -> i32 { + let total: i32 = (5; + return total; +} diff --git a/x_test/negative_missing_semicolon_fix/peeper.toml b/x_test/negative_missing_semicolon_fix/peeper.toml new file mode 100644 index 00000000..48aad9b6 --- /dev/null +++ b/x_test/negative_missing_semicolon_fix/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_missing_semicolon_fix" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = ["expected ';' after statement", " = help: add missing `;`\n | \n2 | let total: i32 = 5;\n\nCompilation failed"] +stderr_excludes = ["add missing `;` here", "\n\n\n"] diff --git a/x_test/negative_missing_semicolon_fix/src/main.peep b/x_test/negative_missing_semicolon_fix/src/main.peep new file mode 100644 index 00000000..aa5f8d41 --- /dev/null +++ b/x_test/negative_missing_semicolon_fix/src/main.peep @@ -0,0 +1,4 @@ +fn main() -> i32 { + let total: i32 = 5 + return total; +} diff --git a/x_test/negative_missing_semicolon_non_ascii_fix/peeper.toml b/x_test/negative_missing_semicolon_non_ascii_fix/peeper.toml new file mode 100644 index 00000000..25968c16 --- /dev/null +++ b/x_test/negative_missing_semicolon_non_ascii_fix/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_missing_semicolon_non_ascii_fix" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = [" = help: add missing `;`\n | \n2 | let greeting: str = \"café 日本語\";\n"] +stderr_excludes = ["\n\n\n"] diff --git a/x_test/negative_missing_semicolon_non_ascii_fix/src/main.peep b/x_test/negative_missing_semicolon_non_ascii_fix/src/main.peep new file mode 100644 index 00000000..b7257abf --- /dev/null +++ b/x_test/negative_missing_semicolon_non_ascii_fix/src/main.peep @@ -0,0 +1,4 @@ +fn main() -> i32 { + let greeting: str = "café 日本語" + return 0; +} diff --git a/x_test/warning_removal_fixes/peeper.toml b/x_test/warning_removal_fixes/peeper.toml new file mode 100644 index 00000000..e7568b60 --- /dev/null +++ b/x_test/warning_removal_fixes/peeper.toml @@ -0,0 +1,8 @@ +name = "warning_removal_fixes" +build = "program" + +[test] +mode = "check" +outcome = "success" +stderr_contains = [" = help: remove the extra comma\n | \n2 | x: i32,\n", " = help: remove this comma\n | \n3 | y: i32\n", " = help: remove redundant `?`\n | \n6 | fn Pick(value: ?i32) -> i32 {\n | \n = note: nested optional types", " = help: remove the leading zero\n | \n7 | return 7;;;\n | \n = help: use the `0o` prefix", " = help: remove these semicolons\n | \n7 | return 00_7;\n"] +stderr_excludes = ["\n\n\n"] diff --git a/x_test/warning_removal_fixes/src/main.peep b/x_test/warning_removal_fixes/src/main.peep new file mode 100644 index 00000000..8174cc35 --- /dev/null +++ b/x_test/warning_removal_fixes/src/main.peep @@ -0,0 +1,12 @@ +struct Point { + x: i32,, + y: i32, +} + +fn Pick(value: ??i32) -> i32 { + return 00_7;;; +} + +fn main() -> i32 { + return Pick(1); +} diff --git a/x_test/warning_unmodified_mutable_suggestion/peeper.toml b/x_test/warning_unmodified_mutable_suggestion/peeper.toml new file mode 100644 index 00000000..1477f7a2 --- /dev/null +++ b/x_test/warning_unmodified_mutable_suggestion/peeper.toml @@ -0,0 +1,8 @@ +name = "warning_unmodified_mutable_suggestion" +build = "program" + +[test] +mode = "check" +outcome = "success" +stderr_contains = ["W0013", "mutable binding `total` is never modified", " = help: remove unnecessary `mut`\n | \n2 | let total: i32 = 5;\n\n", " = help: remove unnecessary `mut`\n | \n7 | fn Add(left: i32, right: i32) -> i32 {\n\n"] +stderr_excludes = ["= suggestion:", "let total", "( left", "\n\n\n"] diff --git a/x_test/warning_unmodified_mutable_suggestion/src/main.peep b/x_test/warning_unmodified_mutable_suggestion/src/main.peep new file mode 100644 index 00000000..ecd07c1d --- /dev/null +++ b/x_test/warning_unmodified_mutable_suggestion/src/main.peep @@ -0,0 +1,9 @@ +fn main() -> i32 { + let mut total: i32 = 5; + return Add(total, 1); +} + + +fn Add(mut left: i32, right: i32) -> i32 { + return left + right; +} From 6defb8452d75c7cc091dd137a360f325bfc5b6b0 Mon Sep 17 00:00:00 2001 From: itsfuad Date: Fri, 9 Oct 2026 17:10:51 +0600 Subject: [PATCH 2/8] Report missing and misplaced tokens with reliable fixes Name the token that is missing and show the repaired line where the repair is certain: ':' or '=' in let and const, ':' after a parameter name, '->' before a return type, '=>' in a match arm, ',' in calls, parameters, function types, type parameters, attribute arguments and braced lists, and a closing bracket at the end of a line or before another closer. Several missing commas in one list are one error. Report a token typed for another with a replacement: ';' for ',' in lists, ':' for '=' in a struct literal, '=' for '==' in a condition. Where the repair cannot be told, show both: `let a b;` offers ':' and '='. One word after `mut` is reported without guessing whether it is the name or the type, a repeated `mut` is reported once, and a parameter name without a type keeps the rest of the list. An unclosed call no longer takes the next statement as an argument or drops it. A ';' in a value list closes the list only when its own '}' is missing. A trailing comma before ')' or '>' is the same note as in literals instead of an error. Rename Diagnostic.WithHelp and WithNote to Help and Note, and add HelpWithChoices for repairs that depend on what the author meant. --- internal/diagnostics/diagnostic.go | 22 +- internal/diagnostics/diagnostic_test.go | 38 +- internal/frontend/parser/parse_expr.go | 31 +- internal/frontend/parser/parse_stmt.go | 35 +- internal/frontend/parser/parse_types.go | 111 ++++-- internal/frontend/parser/parser.go | 365 ++++++++++++++++-- internal/frontend/parser/parser_test.go | 340 +++++++++++++++- internal/frontend/parser/recover_literal.go | 2 +- internal/ir/cfg/analyze.go | 4 +- internal/pipeline/loader.go | 2 +- internal/problems/problems.go | 2 +- internal/semantics/analysis/definite_init.go | 2 +- internal/semantics/analysis/flow_thir.go | 4 +- internal/semantics/analysis/ownership.go | 2 +- internal/semantics/analysis/ownership_expr.go | 2 +- internal/semantics/resolver/resolver.go | 6 +- .../semantics/typechecker/assignability.go | 2 +- internal/semantics/typechecker/check_call.go | 8 +- internal/semantics/typechecker/check_expr.go | 8 +- internal/semantics/typechecker/check_fn.go | 11 +- internal/semantics/typechecker/check_stmt.go | 16 +- internal/semantics/typechecker/errors.go | 4 +- .../semantics/typeresolution/instances.go | 2 +- internal/semantics/usage/usage.go | 2 +- .../negative_bang_after_argument/peeper.toml | 8 + .../src/main.peep | 12 + .../peeper.toml | 8 + .../src/main.peep | 12 + x_test/negative_missing_closers/peeper.toml | 8 + x_test/negative_missing_closers/src/main.peep | 19 + .../peeper.toml | 8 + .../src/main.peep | 24 ++ .../peeper.toml | 8 + .../src/main.peep | 9 + .../peeper.toml | 8 + .../src/main.peep | 4 + .../peeper.toml | 8 + .../src/main.peep | 20 + .../peeper.toml | 8 + .../src/main.peep | 18 + .../negative_single_repair_spots/peeper.toml | 8 + .../src/main.peep | 45 +++ .../peeper.toml | 7 + .../src/main.peep | 41 ++ 44 files changed, 1182 insertions(+), 122 deletions(-) create mode 100644 x_test/negative_bang_after_argument/peeper.toml create mode 100644 x_test/negative_bang_after_argument/src/main.peep create mode 100644 x_test/negative_let_missing_separator/peeper.toml create mode 100644 x_test/negative_let_missing_separator/src/main.peep create mode 100644 x_test/negative_missing_closers/peeper.toml create mode 100644 x_test/negative_missing_closers/src/main.peep create mode 100644 x_test/negative_missing_comma_in_lists/peeper.toml create mode 100644 x_test/negative_missing_comma_in_lists/src/main.peep create mode 100644 x_test/negative_missing_token_at_known_spot/peeper.toml create mode 100644 x_test/negative_missing_token_at_known_spot/src/main.peep create mode 100644 x_test/negative_missing_token_unreliable_guess/peeper.toml create mode 100644 x_test/negative_missing_token_unreliable_guess/src/main.peep create mode 100644 x_test/negative_parameter_without_type/peeper.toml create mode 100644 x_test/negative_parameter_without_type/src/main.peep create mode 100644 x_test/negative_semicolon_in_value_list/peeper.toml create mode 100644 x_test/negative_semicolon_in_value_list/src/main.peep create mode 100644 x_test/negative_single_repair_spots/peeper.toml create mode 100644 x_test/negative_single_repair_spots/src/main.peep create mode 100644 x_test/positive_multiline_lists_and_continuations/peeper.toml create mode 100644 x_test/positive_multiline_lists_and_continuations/src/main.peep diff --git a/internal/diagnostics/diagnostic.go b/internal/diagnostics/diagnostic.go index f0e5b660..a48b24f6 100644 --- a/internal/diagnostics/diagnostic.go +++ b/internal/diagnostics/diagnostic.go @@ -209,14 +209,32 @@ func (d *Diagnostic) WithText(kind, message string, color colors.COLOR, fixes .. return d } -func (d *Diagnostic) WithNote(message string, fixes ...CodeFix) *Diagnostic { +func (d *Diagnostic) Note(message string, fixes ...CodeFix) *Diagnostic { return d.WithText("note", message, colors.CYAN, fixes...) } -func (d *Diagnostic) WithHelp(help string, fixes ...CodeFix) *Diagnostic { +func (d *Diagnostic) Help(help string, fixes ...CodeFix) *Diagnostic { return d.WithText("help", help, colors.GREEN, fixes...) } +// Choice is one of several texts that would each repair the code when +// inserted at the same place, and the condition that makes it the right one. +type Choice struct { + If string + Insert string +} + +// HelpWithChoices adds a help line for each choice, with the line as that +// choice would leave it. It is for code that is wrong where the author's +// intent cannot be told, so no single fix may be offered as the answer. +// Insert may carry the spacing the insertion needs; the help names it bare. +func (d *Diagnostic) HelpWithChoices(at *source.Location, choices ...Choice) *Diagnostic { + for _, choice := range choices { + d.Help("if "+choice.If+", add `"+strings.TrimSpace(choice.Insert)+"`", Fix.Insert(at, choice.Insert)) + } + return d +} + const internalCompilerFailureMessage = "internal compiler failure" type PresentationOptions struct { diff --git a/internal/diagnostics/diagnostic_test.go b/internal/diagnostics/diagnostic_test.go index d2545cc1..5be8d4bb 100644 --- a/internal/diagnostics/diagnostic_test.go +++ b/internal/diagnostics/diagnostic_test.go @@ -41,10 +41,10 @@ func TestWithSecondaryLabelRequiresPrimary(t *testing.T) { } } -func TestWithHelpCarriesItsFixes(t *testing.T) { +func TestHelpCarriesItsFixes(t *testing.T) { d := NewError("immutable") loc := testLoc("main"+peeper.SourceExt, 2, 5) - d.WithHelp("declare it mutable", Fix.Replace(loc, "mut maybe")) + d.Help("declare it mutable", Fix.Replace(loc, "mut maybe")) if len(d.Extras) != 1 { t.Fatalf("expected 1 extra entry, got %d", len(d.Extras)) @@ -102,7 +102,7 @@ func TestEmitterAlignsHeaderAndHelpWithGutter(t *testing.T) { diag := NewError("bad"). WithCode("P0005"). WithPrimaryLabel(loc, "bad"). - WithHelp("use const instead") + Help("use const instead") emitter.Emit(diag) text := out.String() @@ -130,7 +130,7 @@ func emitFixForTest(t *testing.T, format colors.LogFormat, sourceText string, fi emitter.Emit(NewError("broken"). WithCode("P9999"). WithPrimaryLabel(fixes[0].Location, ""). - WithHelp("apply the fix", fixes...)) + Help("apply the fix", fixes...)) return out.String() } @@ -292,7 +292,7 @@ func TestEmitterSizesMarginForFixLines(t *testing.T) { emitter.Emit(NewError("broken"). WithCode("P9999"). WithPrimaryLabel(fixLocForTest(9, 1, 5), ""). - WithNote("continue on the next line", Fix.Insert(fixLocForTest(10, 4, 4), ";"))) + Note("continue on the next line", Fix.Insert(fixLocForTest(10, 4, 4), ";"))) got := out.String() if !strings.Contains(got, "\n 9 | nine\n") || !strings.Contains(got, " = note: continue on the next line\n | \n10 | ten;\n") { t.Fatalf("expected a two-digit margin and the fix under its note, got:\n%s", got) @@ -342,11 +342,33 @@ func TestEmitterSeparatesAFixedLineFromTheNextText(t *testing.T) { emitter.cache.AddSource("sample"+peeper.SourceExt, "let total = 5\n") at := fixLocForTest(1, 14, 14) emitter.Emit(NewError("broken").WithCode("P9999").WithPrimaryLabel(at, ""). - WithHelp("first", Fix.Insert(at, ";")). - WithNote("second"). - WithNote("third")) + Help("first", Fix.Insert(at, ";")). + Note("second"). + Note("third")) want := " = help: first\n | \n1 | let total = 5;\n | \n = note: second\n = note: third\n\n" if !strings.HasSuffix(out.String(), want) { t.Fatalf("expected one margin line after the fixed line and none between plain notes, got:\n%s", out.String()) } } + +func TestHelpWithChoicesAddsOneHelpAndOneInsertionPerChoice(t *testing.T) { + loc := testLoc("main"+peeper.SourceExt, 2, 10) + d := NewError("expected ':' or '=' after the name").HelpWithChoices(loc, + Choice{If: "`b` is a type", Insert: ":"}, + Choice{If: "`b` is a value", Insert: " ="}) + + want := []struct{ help, insert string }{ + {"if `b` is a type, add `:`", ":"}, + {"if `b` is a value, add `=`", " ="}, + } + if len(d.Extras) != len(want) { + t.Fatalf("expected %d help lines, got %d", len(want), len(d.Extras)) + } + for i, extra := range d.Extras { + fixes := extra.Text.Fixes + if extra.Text.Kind != "help" || extra.Text.Message != want[i].help || len(fixes) != 1 || + fixes[0].NewText != want[i].insert || *fixes[0].Location.Start != *loc.Start || *fixes[0].Location.End != *loc.Start { + t.Errorf("choice %d: unexpected help %#v", i, extra.Text) + } + } +} diff --git a/internal/frontend/parser/parse_expr.go b/internal/frontend/parser/parse_expr.go index 9cf6576f..8fd4b5fd 100644 --- a/internal/frontend/parser/parse_expr.go +++ b/internal/frontend/parser/parse_expr.go @@ -269,11 +269,11 @@ func (p *Parser) parseNumberLit(sign string) ast.Expr { zerosEnd := tok.Start zerosEnd.Advance(zeros) d.WithPrimaryLabel(loc, ""). - WithHelp("remove the leading zero", diagnostics.Fix.Remove(source.NewLocation(p.filePath, tok.Start, zerosEnd))) + Help("remove the leading zero", diagnostics.Fix.Remove(source.NewLocation(p.filePath, tok.Start, zerosEnd))) } else { d.WithPrimaryLabel(loc, "remove the leading zero") } - p.diag.Add(d.WithHelp("use the `0o` prefix for octal values")) + p.diag.Add(d.Help("use the `0o` prefix for octal values")) } if sign == "-" { literal.Value = "-" + literal.Value @@ -385,15 +385,20 @@ func (p *Parser) parseCall(callee ast.Expr) ast.Expr { } var args []ast.Expr if !p.at(token.RPAREN) { + var missingCommas []diagnostics.CodeFix for { arg := p.parseExpr(precLowest, token.RPAREN) if arg != nil { args = append(args, arg) } - if !p.match(token.COMMA) { + // parseExpr has taken every operator that continues the argument, + // so a token that begins a value here starts the next argument + // without its comma. + if !p.listContinues(arg != nil && p.startsArgument(), &missingCommas) { break } } + p.reportMissingCommas(missingCommas) } end := p.expectClose(start.Start, token.RPAREN, "(") var fallbackEnd source.Position @@ -509,7 +514,7 @@ func (p *Parser) parseArrayLiteral() ast.Expr { if elem == nil { return reg(p, &ast.BadExpr{Location: source.NewLocation(p.filePath, start.Start, start.End)}) } - values, end, ok := parseBracedItemList(p, "expected '{' after array literal type", "expected '}' after array literal", + values, end, ok := parseBracedItemList(p, "expected '{' after array literal type", "expected '}' after array literal", true, func() (ast.Expr, bool) { value := p.parseExpr(precLowest, token.RBRACE) return value, value != nil @@ -583,7 +588,7 @@ func (p *Parser) parseIdentPath(boundary token.Kind) ast.Expr { p.diag.Add(diagnostics.NewError(message). WithCode(diagnostics.ErrInvalidExpression). WithPrimaryLabel(ast.LocOf(typ), "expected a value here"). - WithHelp("if constructing a struct, insert '.' before '{'")) + Help("if constructing a struct, insert '.' before '{'")) return reg(p, &ast.BadExpr{Location: ast.LocOf(typ)}) } return nil @@ -655,13 +660,19 @@ func (p *Parser) parseCompositeLiteral(typ ast.TypeExpr) ast.Expr { if typ != nil { startPos = ast.StartOf(typ) } - fields, end, ok := parseBracedItemList(p, "expected '{' after '.'", "expected '}' after composite literal", + var colons []diagnostics.CodeFix + fields, end, ok := parseBracedItemList(p, "expected '{' after '.'", "expected '}' after composite literal", true, func() (ast.StructLitField, bool) { name := p.parseIdent() if name == nil { return ast.StructLitField{}, false } - if p.consume(token.ASSIGN, "expected '=' after struct literal field name") == nil { + if p.at(token.COLON) { + // `.{ x: 1 }`: the colon of a declaration written where a + // field is set. Reading on keeps the field and its value. + colon := p.advance() + colons = append(colons, diagnostics.Fix.Replace(source.NewLocation(p.filePath, colon.Start, colon.End), " =")) + } else if p.consume(token.ASSIGN, "expected '=' after struct literal field name") == nil { return ast.StructLitField{}, false } value := p.parseExpr(precLowest, token.RBRACE) @@ -674,6 +685,12 @@ func (p *Parser) parseCompositeLiteral(typ ast.TypeExpr) ast.Expr { Location: source.NewLocation(p.filePath, ast.StartOf(name), ast.EndOf(value)), }, true }) + if len(colons) > 0 { + p.diag.Add(diagnostics.NewError("expected '=' after struct literal field name"). + WithCode(diagnostics.ErrExpectedToken). + WithPrimaryLabel(colons[0].Location, ""). + Help("set a field with `=`", colons...)) + } location := source.NewLocation(p.filePath, startPos, end.End) if !ok { return reg(p, &ast.BadExpr{Location: location}) diff --git a/internal/frontend/parser/parse_stmt.go b/internal/frontend/parser/parse_stmt.go index 8071b6a7..74f5fbf0 100644 --- a/internal/frontend/parser/parse_stmt.go +++ b/internal/frontend/parser/parse_stmt.go @@ -127,6 +127,21 @@ func (p *Parser) parseIfStmt() ast.Stmt { if cond == nil { cond = reg(p, &ast.BadExpr{Location: source.NewLocation(p.filePath, start.Start, start.End)}) } + if p.at(token.ASSIGN) { + // `if a = 1 {`: a condition cannot assign, so the `=` stands for a + // comparison and the condition is read as one. + op := p.advance() + loc := source.NewLocation(p.filePath, op.Start, op.End) + p.diag.Add(diagnostics.NewError("expected '==' in condition, found '='"). + WithCode(diagnostics.ErrExpectedToken). + WithPrimaryLabel(loc, ""). + Help("compare with `==`", diagnostics.Fix.Replace(loc, "=="))) + right := p.parseExpr(precLowest, token.LBRACE) + if right == nil { + right = reg(p, &ast.BadExpr{Location: loc}) + } + cond = reg(p, &ast.BinaryExpr{Left: cond, Op: "==", Right: right, Location: source.NewLocation(p.filePath, ast.StartOf(cond), ast.EndOf(right))}) + } var thenBlock *ast.BlockStmt if p.at(token.LBRACE) { thenBlock = p.parseBlock() @@ -322,14 +337,18 @@ func (p *Parser) parseMatchStmt() ast.Stmt { endPos = ast.EndOf(binding) } } - } else if p.at(token.LBRACE) { + } else if p.at(token.LBRACE) && p.isBraceClosedBeforeArrow() { _, end, _ := p.parseMatchPatternFields() endPos = end.End p.diag.Add(diagnostics.NewError("enum variant pattern requires 'with'"). WithCode(diagnostics.ErrInvalidExpression). WithPrimaryLabel(source.NewLocation(p.filePath, ast.StartOf(casePath), end.End), "write `Enum::Variant with { ... }`")) } - if p.consume(token.FATARROW, "expected '=>' after match pattern") == nil { + if p.at(token.LBRACE) { + // `Case { ... }`: the brace opens the arm's body, so only the + // arrow is missing and the body is read as it stands. + p.diag.Add(insertionError("expected '=>' after match pattern", p.prev().End, p.filePath, "=>", " =>")) + } else if p.consume(token.FATARROW, "expected '=>' after match pattern") == nil { p.synchronize(token.LBRACE, token.RBRACE) } body := p.parseBlock() @@ -351,7 +370,7 @@ func (p *Parser) parseMatchStmt() ast.Stmt { p.diag.Add(diagnostics.NewError("match arms do not use commas"). WithCode(diagnostics.ErrExpectedToken). WithPrimaryLabel(loc, ""). - WithHelp("remove this comma", diagnostics.Fix.Remove(loc))) + Help("remove this comma", diagnostics.Fix.Remove(loc))) } } armListEnd := p.current().End @@ -371,8 +390,16 @@ func (p *Parser) parseMatchStmt() ast.Stmt { }) } +// isBraceClosedBeforeArrow reports whether the `{` at the current token closes +// right before a `=>`. Then it holds pattern fields; otherwise it is the body +// of an arm whose arrow was left out. +func (p *Parser) isBraceClosedBeforeArrow() bool { + closer := p.closerAhead(p.pos + 1) + return closer >= 0 && closer+1 < len(p.stream) && p.stream[closer+1].Kind == token.FATARROW +} + func (p *Parser) parseMatchPatternFields() ([]ast.MatchPatternField, *token.Token, bool) { - return parseBracedItemList(p, "expected '{' after enum case", "expected '}' after match pattern fields", + return parseBracedItemList(p, "expected '{' after enum case", "expected '}' after match pattern fields", false, func() (ast.MatchPatternField, bool) { name := p.parseIdent() if name == nil { diff --git a/internal/frontend/parser/parse_types.go b/internal/frontend/parser/parse_types.go index 8872bd04..f34bdf1e 100644 --- a/internal/frontend/parser/parse_types.go +++ b/internal/frontend/parser/parse_types.go @@ -174,8 +174,8 @@ func (p *Parser) parseOptionalTypeExpr() ast.TypeExpr { p.diag.Add(diagnostics.NewInfo("redundant optional marker"). WithCode(diagnostics.InfoRedundantOptional). WithPrimaryLabel(loc, ""). - WithHelp("remove redundant `?`", diagnostics.Fix.Remove(loc)). - WithNote("nested optional types are the same as a single optional type")) + Help("remove redundant `?`", diagnostics.Fix.Remove(loc)). + Note("nested optional types are the same as a single optional type")) } return inner } @@ -276,6 +276,7 @@ func (p *Parser) parseFuncTypeExpr() ast.TypeExpr { lparenPos := p.stream[p.pos-1].Start var params []ast.Param if !p.at(token.RPAREN) { + var missingCommas []diagnostics.CodeFix for { var name *ast.Ident if p.at(token.IDENT) && p.next().Kind == token.COLON { @@ -291,10 +292,11 @@ func (p *Parser) parseFuncTypeExpr() ast.TypeExpr { startPos = ast.StartOf(name) } params = append(params, ast.Param{Name: name, Type: param, Location: source.NewLocation(p.filePath, startPos, ast.EndOf(param))}) - if !p.match(token.COMMA) { + if !p.listContinues(isTypeStart(p.current().Kind), &missingCommas) { break } } + p.reportMissingCommas(missingCommas) } p.expectClose(lparenPos, token.RPAREN, "(") var ret ast.TypeExpr @@ -348,7 +350,7 @@ func (p *Parser) parseEnumTypeExpr() ast.TypeExpr { // --- Shared type body parsers --- func (p *Parser) parseTypeFields(openerMsg, itemMsg string) ([]ast.TypeField, *token.Token, bool) { - return parseBracedItemList(p, openerMsg, itemMsg, + return parseBracedItemList(p, openerMsg, itemMsg, false, func() (ast.TypeField, bool) { name := p.parseIdent() if name == nil { @@ -366,7 +368,7 @@ func (p *Parser) parseTypeFields(openerMsg, itemMsg string) ([]ast.TypeField, *t } func (p *Parser) parseInterfaceMethods() ([]ast.TypeMethod, *token.Token, bool) { - return parseBracedItemList(p, "expected '{' after iface", "expected '}' after iface methods", + return parseBracedItemList(p, "expected '{' after iface", "expected '}' after iface methods", false, func() (ast.TypeMethod, bool) { if p.consume(token.FN, "expected 'fn' before interface method") == nil { return ast.TypeMethod{}, false @@ -397,11 +399,7 @@ func (p *Parser) parseInterfaceMethods() ([]ast.TypeMethod, *token.Token, bool) p.diag.AddError(diagnostics.ErrInvalidDeclaration, "interface method defaults are not supported", ast.LocOf(param.Default), "") } - var ret ast.TypeExpr - if p.match(token.ARROW) { - ret = p.parseTypeExpr() - // nil is OK - type-checker validates return types - } + ret := p.parseReturnType() returnOrigins := p.parseReturnOriginClause() endPos := ast.EndOf(ret) if returnOrigins != nil && returnOrigins.Location != nil { @@ -426,7 +424,7 @@ func (p *Parser) parseInterfaceMethods() ([]ast.TypeMethod, *token.Token, bool) } func (p *Parser) parseEnumVariants() ([]ast.EnumVariant, *token.Token, bool) { - return parseBracedItemList(p, "expected '{' after enum", "expected '}' after enum variants", + return parseBracedItemList(p, "expected '{' after enum", "expected '}' after enum variants", false, func() (ast.EnumVariant, bool) { name := p.parseIdent() if name == nil { @@ -475,16 +473,18 @@ func (p *Parser) parseOptionalTypeParams() []ast.TypeParam { } langlePos := p.stream[p.pos-1].Start var params []ast.TypeParam + var missingCommas []diagnostics.CodeFix for { name := p.parseIdent() if name == nil { break } params = append(params, ast.TypeParam{Name: name, Location: name.Location}) - if !p.match(token.COMMA) { + if !p.listContinues(p.at(token.IDENT), &missingCommas) { break } } + p.reportMissingCommas(missingCommas) p.expectClose(langlePos, token.GT, "<") return params } @@ -494,38 +494,95 @@ func (p *Parser) parseParams() []ast.Param { if p.at(token.RPAREN) { return params } + var missingCommas []diagnostics.CodeFix for { param, ok := p.parseParam() if !ok { break } params = append(params, param) - if !p.match(token.COMMA) { + // `mut` or a name with its type can only begin another parameter. + if !p.listContinues(p.at(token.MUT) || p.startsNamedParam(false), &missingCommas) { break } } + p.reportMissingCommas(missingCommas) return params } -func (p *Parser) parseParam() (ast.Param, bool) { - var ( - mutableLocation *source.Location - modifierStart source.Position - ) - if p.at(token.MUT) { +// startsNamedParam reports whether the current token is a parameter name: +// followed by `:`, or by a token that begins its type when the colon is left +// out. A name followed by anything else is an unnamed parameter's type, unless +// a modifier came before it: only a named parameter takes one. +func (p *Parser) startsNamedParam(hasModifier bool) bool { + if !p.at(token.IDENT) { + return false + } + switch next := p.next(); next.Kind { + case token.COLON: + return true + case token.IDENT, token.AMP, token.ASTERISK, token.QUESTION, token.QQ, token.RAWPTR, token.FN: + if next.Start.Line == p.current().Start.Line { + return true + } + } + return hasModifier +} + +// parseParamModifiers reads the modifier keywords written before a parameter +// and returns where `mut` stood, or nil without one. A modifier written twice +// is reported and counted once. +func (p *Parser) parseParamModifiers() *source.Location { + var mutableLocation *source.Location + for p.at(token.MUT) { tok := p.advance() - mutableLocation = source.NewLocation(p.filePath, tok.Start, tok.End) - modifierStart = tok.Start + if mutableLocation == nil { + mutableLocation = source.NewLocation(p.filePath, tok.Start, tok.End) + continue + } + // The fix takes the space after the keyword with it. + removedEnd := tok.End + if p.isOnLineOfPrev() { + removedEnd = p.current().Start + } + p.diag.Add(diagnostics.NewError("`mut` is repeated"). + WithCode(diagnostics.ErrInvalidDeclaration). + WithPrimaryLabel(source.NewLocation(p.filePath, tok.Start, tok.End), ""). + Help("remove the extra `mut`", diagnostics.Fix.Remove(source.NewLocation(p.filePath, tok.Start, removedEnd)))) } - if p.at(token.IDENT) && p.pos+1 < len(p.stream) && p.stream[p.pos+1].Kind == token.COLON { + return mutableLocation +} + +func (p *Parser) parseParam() (ast.Param, bool) { + mutableLocation := p.parseParamModifiers() + if p.startsNamedParam(mutableLocation != nil) { name := p.parseIdent() if name == nil { return ast.Param{}, false } - if p.consume(token.COLON, "expected ':' after parameter name") == nil { - return ast.Param{}, false + var ty ast.TypeExpr + isNextParam := p.at(token.IDENT) && p.next().Kind == token.COLON + switch { + case p.match(token.COLON): + ty = p.parseTypeExpr() + case isTypeStart(p.current().Kind) && p.isOnLineOfPrev() && !isNextParam: + // `fn f(a i32)`: two names in a row can only be a name and its type. + p.diag.Add(insertionError("expected ':' after parameter name", p.prev().End, p.filePath, ":", ":")) + ty = p.parseTypeExpr() + case isNextParam: + // `fn f(a b: i32)`: the name stands alone. Keeping it as a + // parameter lets the rest of the list and the body be checked. + at := p.prev().End + p.diag.Add(diagnostics.NewError("expected a type after parameter name"). + WithCode(diagnostics.ErrExpectedToken). + WithPrimaryLabel(source.NewLocation(p.filePath, at, at), "add `: Type` here")) + default: + // `fn f(mut a)`, `fn f(mut i32)`: one word after the modifier is + // the name without its type or the type without its name. + p.diag.Add(diagnostics.NewError("expected a name and a type after `mut`"). + WithCode(diagnostics.ErrExpectedToken). + WithPrimaryLabel(source.NewLocation(p.filePath, *mutableLocation.Start, p.prev().End), "write `mut name: Type`")) } - ty := p.parseTypeExpr() // ty may be nil if type parsing failed; continue with name endPos := ast.EndOf(name) if ty != nil { @@ -540,7 +597,7 @@ func (p *Parser) parseParam() (ast.Param, bool) { } startPos := ast.StartOf(name) if mutableLocation != nil { - startPos = modifierStart + startPos = *mutableLocation.Start } return ast.Param{IsMutable: mutableLocation != nil, MutableLocation: mutableLocation, Name: name, Type: ty, Default: defaultValue, Location: source.NewLocation(p.filePath, startPos, endPos)}, true } @@ -551,7 +608,7 @@ func (p *Parser) parseParam() (ast.Param, bool) { if mutableLocation != nil { p.diag.Add(diagnostics.NewError("mutable parameter requires a named binding"). WithCode(diagnostics.ErrInvalidDeclaration). - WithPrimaryLabel(source.NewLocation(p.filePath, modifierStart, p.prev().End), "add a parameter name after the modifier")) + WithPrimaryLabel(source.NewLocation(p.filePath, *mutableLocation.Start, p.prev().End), "add a parameter name after the modifier")) return ast.Param{}, false } return ast.Param{Type: ty, Location: ast.LocOf(ty)}, true diff --git a/internal/frontend/parser/parser.go b/internal/frontend/parser/parser.go index ece4fc1d..b2471172 100644 --- a/internal/frontend/parser/parser.go +++ b/internal/frontend/parser/parser.go @@ -100,7 +100,7 @@ func (p *Parser) reportInvalidModuleStmt(loc *source.Location, msg, help string) WithCode(diagnostics.ErrInvalidDeclaration). WithPrimaryLabel(loc, msg) if help != "" { - diag = diag.WithHelp(help) + diag = diag.Help(help) } p.diag.Add(diag) } @@ -217,14 +217,25 @@ func (p *Parser) parseFnSignature() (name *ast.Ident, typeParams []ast.TypeParam lparenPos := p.stream[p.pos-1].Start params = p.parseParams() p.expectClose(lparenPos, token.RPAREN, "(") - if p.match(token.ARROW) { - returnType = p.parseTypeExpr() - // nil is OK - type-checker validates return types - } + returnType = p.parseReturnType() returnOrigins = p.parseReturnOriginClause() return name, typeParams, params, returnType, returnOrigins, true } +// parseReturnType reads `-> Type` after a parameter list; nil means no return +// value, or a type that failed to parse and was reported. A type written on +// that line without its arrow is reported and read, so the body still parses. +func (p *Parser) parseReturnType() ast.TypeExpr { + if p.match(token.ARROW) { + return p.parseTypeExpr() + } + if isTypeStart(p.current().Kind) && p.isOnLineOfPrev() { + p.diag.Add(insertionError("expected '->' before the return type", p.prev().End, p.filePath, "->", " ->")) + return p.parseTypeExpr() + } + return nil +} + func (p *Parser) parseReturnOriginClause() *ast.ReturnOriginClause { if !p.at(token.FROM) { return nil @@ -233,6 +244,7 @@ func (p *Parser) parseReturnOriginClause() *ast.ReturnOriginClause { sources := make([]*ast.Ident, 0, 1) end := start.End if p.match(token.LPAREN) { + var missingCommas []diagnostics.CodeFix for !p.at(token.RPAREN) && !p.at(token.EOF) { sourceName := p.parseIdent() if sourceName == nil { @@ -240,10 +252,11 @@ func (p *Parser) parseReturnOriginClause() *ast.ReturnOriginClause { } sources = append(sources, sourceName) end = ast.EndOf(sourceName) - if !p.match(token.COMMA) { + if !p.listContinues(p.at(token.IDENT), &missingCommas) { break } } + p.reportMissingCommas(missingCommas) if close := p.consume(token.RPAREN, "expected ')' after reference return origins"); close != nil { end = close.End } @@ -349,22 +362,110 @@ func (p *Parser) parseBindingFields() (name *ast.Ident, ty ast.TypeExpr, value a if p.match(token.COLON) { ty = p.parseTypeExpr() // ty may be nil if type parsing failed; continue with name and value + } else if p.isLoneNameAfterBindingName() { + // `let a b;`: `b` reads as the type or as the value, and only what it + // names can tell. Both repairs are shown, and the binding keeps an + // unknown value so later uses of it stay quiet. + at := source.NewLocation(p.filePath, p.prev().End, p.prev().End) + word := p.advance() + p.diag.Add(diagnostics.NewError("expected ':' or '=' after the name"). + WithCode(diagnostics.ErrExpectedToken). + WithPrimaryLabel(at, ""). + HelpWithChoices(at, + diagnostics.Choice{If: "`" + word.Literal + "` is a type", Insert: ":"}, + diagnostics.Choice{If: "`" + word.Literal + "` is a value", Insert: " ="})) + value = reg(p, &ast.BadExpr{Location: source.NewLocation(p.filePath, word.Start, word.End)}) + } else if p.isTypeWithoutColon() { + // `let a i32 = 1;`: reading the type here keeps the rest of the + // statement from producing errors of its own. + p.diag.Add(insertionError("expected ':' before the type", p.prev().End, p.filePath, ":", ":")) + ty = p.parseTypeExpr() } if p.match(token.ASSIGN) { value = p.parseExpr(precLowest, token.SEMICOLON) + } else if p.startsValue() && p.isOnLineOfPrev() && !p.hasBeforeStatementEnd(token.ASSIGN) { + // `let a 5;`: a value on the same line with no `=` anywhere before the + // statement ends. Reading the value keeps the binding usable. The fix + // is only shown when the statement ends right after that value; + // otherwise more than the `=` is wrong. + at := p.prev().End + value = p.parseExpr(precLowest, token.SEMICOLON) + if p.at(token.SEMICOLON) || !p.isOnLineOfPrev() { + p.diag.Add(insertionError("expected '=' before the value", at, p.filePath, "=", " =")) + } else { + p.diag.Add(diagnostics.NewError("expected '=' before the value"). + WithCode(diagnostics.ErrExpectedToken). + WithPrimaryLabel(source.NewLocation(p.filePath, at, at), "add missing `=` here")) + } } end = p.consume(token.SEMICOLON, "expected ';' after statement") if end == nil { - insertPos := ast.EndOf(value) - if insertPos.IsZero() { - insertPos = ast.EndOf(ty) + // The statement ends where its last token does. + end = &token.Token{Kind: token.SEMICOLON, End: p.prev().End} + } + return name, ty, value, end, true +} + +// isLoneNameAfterBindingName reports whether the binding name is followed on +// its line by one more name that ends the statement or the line. +func (p *Parser) isLoneNameAfterBindingName() bool { + if !p.isOnLineOfPrev() || !p.at(token.IDENT) { + return false + } + next := p.next() + return next.Kind == token.SEMICOLON || next.Kind == token.EOF || next.Start.Line > p.current().End.Line +} + +// isTypeWithoutColon reports whether the tokens after a binding name are its +// type with the colon left out. They must begin a type, and then one of these +// holds: an `=` still follows in the statement (`let a &T = x;`); no value +// can begin that way (`let a ?T;`); or it is an array type with no `{` after +// it, which an array literal would need (`let a [3]T;`). +func (p *Parser) isTypeWithoutColon() bool { + if !p.isOnLineOfPrev() || !isTypeStart(p.current().Kind) { + return false + } + return p.hasBeforeStatementEnd(token.ASSIGN) || !p.startsValue() || + p.at(token.LBRACK) && !p.hasBeforeStatementEnd(token.LBRACE) +} + +// isOnLineOfPrev reports whether the current token sits on the line where the +// previous one ended. +func (p *Parser) isOnLineOfPrev() bool { + return !p.at(token.EOF) && p.current().Start.Line == p.prev().End.Line +} + +// hasBeforeStatementEnd looks ahead on the current line for a token of kind +// outside brackets before the statement's `;`. +func (p *Parser) hasBeforeStatementEnd(kind token.Kind) bool { + depth := 0 + for i := p.pos; i < len(p.stream) && p.stream[i].Start.Line == p.current().Start.Line; i++ { + if depth <= 0 && p.stream[i].Kind == kind { + return true } - if insertPos.IsZero() { - insertPos = ast.EndOf(name) + switch p.stream[i].Kind { + case token.LPAREN, token.LBRACK, token.LBRACE: + depth++ + case token.RPAREN, token.RBRACK, token.RBRACE: + depth-- + case token.SEMICOLON: + if depth <= 0 { + return false + } } - end = &token.Token{Kind: token.SEMICOLON, End: insertPos} } - return name, ty, value, end, true + return false +} + +// insertionError is the diagnostic for a token known to be missing at a point: +// an empty marker there, and a help line whose fix inserts text. shown is the +// token as named in the help; text may carry the spacing the insertion needs. +func insertionError(msg string, at source.Position, filePath, shown, text string) *diagnostics.Diagnostic { + loc := source.NewLocation(filePath, at, at) + return diagnostics.NewError(msg). + WithCode(diagnostics.ErrExpectedToken). + WithPrimaryLabel(loc, ""). + Help("add missing `"+shown+"`", diagnostics.Fix.Insert(loc, text)) } func (p *Parser) parseStructDecl() ast.Decl { @@ -482,15 +583,17 @@ func (p *Parser) parseAttributes() []ast.Attribute { attrEnd = nameTok.End ) if p.match(token.LPAREN) { + var missingCommas []diagnostics.CodeFix for !p.at(token.RPAREN) && !p.at(token.EOF) { arg := p.parseExpr(precLowest, token.RPAREN) if arg != nil { args = append(args, arg) } - if !p.match(token.COMMA) { + if !p.listContinues(arg != nil && p.startsArgument(), &missingCommas) { break } } + p.reportMissingCommas(missingCommas) if end := p.consume(token.RPAREN, "expected ')' after attribute arguments"); end != nil { attrEnd = end.End } else if len(args) > 0 { @@ -599,7 +702,12 @@ func (p *Parser) expectClose(openPos source.Position, kind token.Kind, name stri } prev := p.prev() loc := source.NewLocation(p.filePath, prev.End, prev.End) - p.diag.Add(missingTokenError(fmt.Sprintf("expected '%s'", string(kind)), kind, loc)) + p.diag.Add(p.missingTokenError(fmt.Sprintf("expected '%s'", string(kind)), kind, loc)) + if p.isMissingTokenLikely(kind, loc) { + // The closer belongs right here, so the tokens after it are not part + // of the bracket; skipping ahead to find one would drop them. + return nil + } p.synchronize(kind) if p.current().Kind == kind { return p.advance() @@ -636,7 +744,7 @@ func (p *Parser) consumeRedundant(kind token.Kind, code string, noun string, isF // A fixed line can only be shown for a run that sits on one line; a // longer run keeps the advice beside the marker. if first.Start.Line == last.End.Line { - d.WithPrimaryLabel(loc, "").WithHelp(advice, diagnostics.Fix.Remove(removed)) + d.WithPrimaryLabel(loc, "").Help(advice, diagnostics.Fix.Remove(removed)) } else { d.WithPrimaryLabel(loc, advice) } @@ -648,6 +756,7 @@ func parseBracedItemList[T any]( p *Parser, openerMsg string, itemMsg string, + isExpression bool, parseItem func() (T, bool), ) ([]T, *token.Token, bool) { lbrace := p.consume(token.LBRACE, openerMsg) @@ -660,6 +769,8 @@ func parseBracedItemList[T any]( // The comma that follows an item is consumed with any extra ones at the top // of the next round, so only a run before the first item is all redundant. isAfterItem := false + isClosedEarly := false + var wrongSeparators, missingCommas []diagnostics.CodeFix for !p.at(token.RBRACE) && !p.at(token.EOF) { for p.at(token.DOC_COMMENT) { p.advance() @@ -680,17 +791,49 @@ func parseBracedItemList[T any]( } if p.at(token.COMMA) { if p.next().Kind == token.RBRACE { + p.reportTrailingComma(p.current()) + } + continue + } + if p.at(token.SEMICOLON) { + if !isExpression || p.isSemicolonForComma() { + // `.{ x = 1; y = 2 }`: the `;` stands where a `,` belongs. One + // before the closing brace separates nothing and is removed. loc := source.NewLocation(p.filePath, p.current().Start, p.current().End) - p.diag.Add(diagnostics.NewInfo("trailing comma is unnecessary"). - WithCode(diagnostics.InfoTrailingComma). - WithPrimaryLabel(loc, ""). - WithHelp("remove this comma", diagnostics.Fix.Remove(loc))) + if p.next().Kind == token.RBRACE { + wrongSeparators = append(wrongSeparators, diagnostics.Fix.Remove(loc)) + } else { + wrongSeparators = append(wrongSeparators, diagnostics.Fix.Replace(loc, ",")) + } + p.advance() + continue } + // `[3]i32{1, 2, 3;`: the statement ended where the list should + // have closed. Leaving the `;` for the statement keeps the rest of + // the block from being read as values. + p.diag.Add(insertionError(itemMsg, p.prev().End, p.filePath, "}", "}")) + isClosedEarly = true + break + } + if p.at(token.RPAREN) || p.at(token.RBRACK) { + // `f(.{ x = 1 );`: the closer of a bracket around the list can + // only follow the list's own `}`. + p.diag.Add(insertionError(itemMsg, p.prev().End, p.filePath, "}", "}")) + isClosedEarly = true + break + } + // A token that begins an item right after a good one means only the + // comma is missing. On a later line of a value list it may instead + // begin the statement after a list left unclosed, so there the old + // report stays. After a failed item the skip below must run, or a + // token no item accepts would be offered again without end. + startsItem := ok && (p.at(token.IDENT) || p.at(token.FN) || isExpression && p.startsValue()) + if p.listContinues(startsItem && (p.isOnLineOfPrev() || !isExpression), &missingCommas) { continue } if !p.at(token.RBRACE) { - prev := p.stream[p.pos-1] - p.diag.Add(missingTokenError(itemMsg, token.COMMA, source.NewLocation(p.filePath, prev.End, prev.End))) + prev := p.prev() + p.diag.Add(p.missingTokenError("expected ','", token.COMMA, source.NewLocation(p.filePath, prev.End, prev.End))) // Recovery must always consume or skip the unexpected separator token. // Without this, inputs like `foo();` inside a braced item list keep // reporting the same missing-comma diagnostic forever. @@ -701,7 +844,17 @@ func parseBracedItemList[T any]( continue } } - end := p.expectClose(lbraceStart, token.RBRACE, "{") + p.reportMissingCommas(missingCommas) + if len(wrongSeparators) > 0 { + p.diag.Add(diagnostics.NewError("expected ',' between items, found ';'"). + WithCode(diagnostics.ErrExpectedToken). + WithPrimaryLabel(wrongSeparators[0].Location, ""). + Help("separate items with `,`", wrongSeparators...)) + } + var end *token.Token + if !isClosedEarly { + end = p.expectClose(lbraceStart, token.RBRACE, "{") + } var endPos source.Position if end != nil { endPos = end.End @@ -723,23 +876,165 @@ func (p *Parser) consume(kind token.Kind, msg string) *token.Token { } prev := p.stream[p.pos-1] loc := source.NewLocation(p.filePath, prev.End, prev.End) - p.diag.Add(missingTokenError(msg, kind, loc)) + p.diag.Add(p.missingTokenError(msg, kind, loc)) return nil } -// missingTokenError reports a token that should follow loc. Separators and -// closing delimiters are spelled exactly as their kind, so the diagnostic can -// show the line with the token inserted. Other kinds name a category -// ("identifier") or need spacing a blind insertion would get wrong, so they -// only say what is missing. -func missingTokenError(msg string, kind token.Kind, loc *source.Location) *diagnostics.Diagnostic { - d := diagnostics.NewError(msg).WithCode(diagnostics.ErrExpectedToken) +// missingTokenError reports a token that should follow loc. Where the guess +// is reliable the diagnostic shows the line with the token inserted; elsewhere +// it only says what is missing, because the real mistake is often a different +// token between the two pieces of code on that line. +func (p *Parser) missingTokenError(msg string, kind token.Kind, loc *source.Location) *diagnostics.Diagnostic { + if p.isMissingTokenLikely(kind, loc) { + return insertionError(msg, *loc.Start, p.filePath, string(kind), string(kind)) + } + return diagnostics.NewError(msg). + WithCode(diagnostics.ErrExpectedToken). + WithPrimaryLabel(loc, fmt.Sprintf("add missing `%s` here", string(kind))) +} + +// isMissingTokenLikely reports whether inserting kind at loc is very probably +// what the author meant, judged by the unexpected token that follows. A +// separator is trusted when the line ends there; a statement also ends before +// a closing brace; a closing delimiter is also trusted before another closing +// delimiter, or before a semicolon that itself ends the line. Nothing is +// trusted right after a semicolon or an opening delimiter, where the parser +// has already skipped past the real mistake. Kinds that name a category, such +// as an identifier, cannot be inserted as text at all. +func (p *Parser) isMissingTokenLikely(kind token.Kind, loc *source.Location) bool { + switch p.prev().Kind { + case token.SEMICOLON, token.LPAREN, token.LBRACK, token.LBRACE: + return false + } + next := p.current() + endsLine := func(tok token.Token) bool { return tok.Kind == token.EOF || tok.Start.Line > loc.Start.Line } + isCloser := func(k token.Kind) bool { + return k == token.RPAREN || k == token.RBRACK || k == token.RBRACE || k == token.GT + } + switch { + case kind == token.SEMICOLON: + return endsLine(next) || next.Kind == token.RBRACE + case kind == token.COMMA: + return endsLine(next) + case isCloser(kind): + after := p.next() + // An arrow only follows the `)` of a parameter list. + return endsLine(next) || isCloser(next.Kind) || kind == token.RPAREN && next.Kind == token.ARROW || + next.Kind == token.SEMICOLON && (endsLine(after) || isCloser(after.Kind)) + } + return false +} + +// listContinues reports whether another list item follows the one just read: +// either a comma was there, or startsItem says the next token begins an item, +// which means the comma between the two is missing. Each missing comma is +// added to missing; reportMissingCommas turns them into one diagnostic once +// the list is read, so `f(1 2 3)` shows a single line with both commas. +func (p *Parser) listContinues(startsItem bool, missing *[]diagnostics.CodeFix) bool { + if p.match(token.COMMA) { + // A comma right before the closer separates nothing. + if p.at(token.RPAREN) || p.at(token.GT) { + p.reportTrailingComma(p.prev()) + return false + } + return true + } + if !startsItem { + return false + } + at := p.prev().End + *missing = append(*missing, diagnostics.Fix.Insert(source.NewLocation(p.filePath, at, at), ",")) + return true +} + +func (p *Parser) reportTrailingComma(comma token.Token) { + loc := source.NewLocation(p.filePath, comma.Start, comma.End) + p.diag.Add(diagnostics.NewInfo("trailing comma is unnecessary"). + WithCode(diagnostics.InfoTrailingComma). + WithPrimaryLabel(loc, ""). + Help("remove this comma", diagnostics.Fix.Remove(loc))) +} + +func (p *Parser) reportMissingCommas(missing []diagnostics.CodeFix) { + if len(missing) == 0 { + return + } + help := "add missing `,`" + if len(missing) > 1 { + help = "add the missing commas" + } + p.diag.Add(diagnostics.NewError("expected ','"). + WithCode(diagnostics.ErrExpectedToken). + WithPrimaryLabel(missing[0].Location, ""). + Help(help, missing...)) +} + +// startsArgument reports whether the current token begins another argument of +// the list being read. On the line of the previous argument any value does. On +// a later line it may be the statement after a call left unclosed, so it +// counts only when the `)` of the list is still ahead. +func (p *Parser) startsArgument() bool { + if !p.startsValue() { + return false + } + if p.isOnLineOfPrev() { + return true + } + closer := p.closerAhead(p.pos) + return closer >= 0 && p.stream[closer].Kind == token.RPAREN +} + +// isSemicolonForComma reports whether the `;` at the current token of a value +// list was typed for a `,`. It was when the list still closes with its own +// `}`, which shows in what follows that brace: the end of the statement or of +// an enclosing list. A `}` followed by anything else closes the block around +// the statement, so the list's own brace is the one missing. +func (p *Parser) isSemicolonForComma() bool { + closer := p.closerAhead(p.pos) + if closer < 0 || p.stream[closer].Kind != token.RBRACE || closer+1 >= len(p.stream) { + return false + } + switch p.stream[closer+1].Kind { + case token.SEMICOLON, token.COMMA, token.RPAREN: + return true + } + return false +} + +// closerAhead returns the index of the token that closes the bracket the +// token at from sits in, skipping brackets that open and close on the way, or +// -1 when the file ends first. +func (p *Parser) closerAhead(from int) int { + depth := 0 + for i := from; i < len(p.stream); i++ { + switch p.stream[i].Kind { + case token.LPAREN, token.LBRACK, token.LBRACE: + depth++ + case token.RPAREN, token.RBRACK, token.RBRACE: + if depth == 0 { + return i + } + depth-- + } + } + return -1 +} + +// startsValue reports whether the current token can begin an expression. A +// `!` is left out: after a value it is as likely a mistyped `!=` as a new one. +func (p *Parser) startsValue() bool { + _, ok := nudLookup[p.current().Kind] + return ok && !p.at(token.BANG) +} + +// isTypeStart reports whether kind can begin a type. +func isTypeStart(kind token.Kind) bool { switch kind { - case token.SEMICOLON, token.COMMA, token.RPAREN, token.RBRACK, token.RBRACE, token.GT: - return d.WithPrimaryLabel(loc, ""). - WithHelp(fmt.Sprintf("add missing `%s`", string(kind)), diagnostics.Fix.Insert(loc, string(kind))) + case token.IDENT, token.AMP, token.QUESTION, token.QQ, token.ASTERISK, token.RAWPTR, + token.LBRACK, token.FN, token.STRUCT, token.IFACE, token.ENUM: + return true } - return d.WithPrimaryLabel(loc, fmt.Sprintf("add missing `%s` here", string(kind))) + return false } // advances token if matched diff --git a/internal/frontend/parser/parser_test.go b/internal/frontend/parser/parser_test.go index 979d910a..040d9b2a 100644 --- a/internal/frontend/parser/parser_test.go +++ b/internal/frontend/parser/parser_test.go @@ -2077,8 +2077,7 @@ func TestParseInterfaceMethodsUnexpectedSemicolonRecovers(t *testing.T) { if len(ifaceType.Methods) != 2 { t.Fatalf("methods: got %d want 2", len(ifaceType.Methods)) } - if !strings.Contains(diag.EmitAllToString(), "expected '}' after iface methods") && - !strings.Contains(diag.EmitAllToString(), "add missing `,`") { + if !strings.Contains(diag.EmitAllToString(), "expected ',' between items, found ';'") { t.Fatalf("expected separator recovery diagnostic, got:\n%s", diag.EmitAllToString()) } } @@ -2856,3 +2855,340 @@ func TestRedundantCommaAdviceCountsOnlyTheExtraOnes(t *testing.T) { } } } + +func TestMissingTokenAtAKnownSpotNamesTheRealToken(t *testing.T) { + for _, test := range []struct { + name string + source string + message string + insert string + }{ + {"colon before a let type", "fn main() { let a i32 = 1; }", "expected ':' before the type", ":"}, + {"equals before a let value", "fn main() { let a 5; }", "expected '=' before the value", " ="}, + {"equals after a let type", "fn main() { let a: i32 5; }", "expected '=' before the value", " ="}, + {"equals before a call value", "fn main() { let a foo(); }", "expected '=' before the value", " ="}, + {"comma between call arguments", "fn main() { add(1 2); }", "expected ','", ","}, + {"comma between parameters", "fn add(a: i32 b: i32) {}", "expected ','", ","}, + } { + t.Run(test.name, func(t *testing.T) { + _, diag := parseTestModule(test.source) + items := diag.Diagnostics() + if len(items) != 1 { + t.Fatalf("expected exactly one diagnostic, got %d:\n%s", len(items), diag.EmitAllToString()) + } + item := items[0] + if item.Message != test.message || len(item.Extras) != 1 || len(item.Extras[0].Text.Fixes) != 1 || + item.Extras[0].Text.Fixes[0].NewText != test.insert { + t.Fatalf("expected %q with a fix inserting %q, got: %#v", test.message, test.insert, item) + } + }) + } +} + +func TestMissingTokenFixIsShownOnlyWhereTheGuessIsReliable(t *testing.T) { + for _, test := range []struct { + name string + source string + code string + hasFix bool + }{ + {"semicolon at the end of a line", "fn main() {\n let total = 5\n return;\n}", diagnostics.ErrExpectedToken, true}, + {"semicolon before a closing brace", "fn main() { let total = 5 }", diagnostics.ErrExpectedToken, true}, + {"closer before a semicolon", "fn main() { let total = (5; }", diagnostics.ErrExpectedToken, true}, + {"closer with more code on the line", "fn main() { let xs = [1, 2, 3; }", diagnostics.ErrExpectedToken, false}, + {"semicolon with a value on the next line is still trusted", "fn main() {\n let a\n foo();\n}", diagnostics.ErrExpectedToken, true}, + } { + t.Run(test.name, func(t *testing.T) { + _, diag := parseTestModule(test.source) + for _, item := range diag.Diagnostics() { + if item.Code != test.code { + continue + } + gotFix := len(item.Extras) == 1 && len(item.Extras[0].Text.Fixes) == 1 + if gotFix != test.hasFix { + t.Fatalf("fix shown = %t, want %t: %#v", gotFix, test.hasFix, item) + } + if !gotFix && !strings.HasPrefix(item.Labels[0].Message, "add missing `") { + t.Fatalf("expected the short hint on the label, got: %#v", item.Labels) + } + return + } + t.Fatalf("expected a missing-token error, got:\n%s", diag.EmitAllToString()) + }) + } +} + +func TestLetWithoutSemicolonEndsAtItsLastToken(t *testing.T) { + module, diag := parseTestModule("fn main() {\n let a\n}") + if out := diag.EmitAllToString(); !strings.Contains(out, "expected ';' after statement") { + t.Fatalf("expected the missing semicolon error, got:\n%s", out) + } + found := false + for _, stmt := range module.Stmts { + ast.Inspect(stmt, func(node ast.Node) bool { + if let, ok := node.(*ast.LetDecl); ok { + found = true + if let.Location.End.Line != let.Location.Start.Line { + t.Fatalf("let ends on line %d, want its own line %d", let.Location.End.Line, let.Location.Start.Line) + } + } + return true + }) + } + if !found { + t.Fatal("let declaration not found") + } +} + +func TestMissingTokenRulesStaySilentWhenTheGuessWouldBeWrong(t *testing.T) { + for _, test := range []struct { + name string + source string + want string // message of the one diagnostic that must carry a fix, or "" for none at all + insert string + }{ + {"reference type without colon", "fn main() { let c &i32 = x; }", "expected ':' before the type", ":"}, + {"array type without colon", "fn main() { let c [3]i32 = xs; }", "expected ':' before the type", ":"}, + {"generic type without colon", "fn main() { let c Box = x; }", "expected ':' before the type", ":"}, + {"optional type without colon", "fn main() { let c ?i32 = none; }", "expected ':' before the type", ":"}, + {"const type without colon", "const N i32 = 3;", "expected ':' before the type", ":"}, + {"colon and equals both missing", "fn main() { let c i32 5; }", "", ""}, + {"field access is not a binding", "fn main() { let c.x = 5; }", "", ""}, + {"parameter without colon", "fn f(a i32) {}", "expected ':' after parameter name", ":"}, + {"name before a named parameter", "fn f(a b: i32) {}", "", ""}, + {"modifier before a lone name", "fn f(mut a, b: i32) {}", "", ""}, + {"modifier and name without colon", "fn f(mut a [3]i32) {}", "expected ':' after parameter name", ":"}, + {"not after an argument", "fn main() { f(a !b); }", "", ""}, + {"closer before a semicolon in the middle of a line", "fn main() { add(a; b); }", "", ""}, + } { + t.Run(test.name, func(t *testing.T) { + _, diag := parseTestModule(test.source) + for _, item := range diag.Diagnostics() { + for _, extra := range item.Extras { + for _, fix := range extra.Text.Fixes { + if item.Message != test.want || fix.NewText != test.insert { + t.Fatalf("unexpected fix %q on %q:\n%s", fix.NewText, item.Message, diag.EmitAllToString()) + } + return + } + } + } + if test.want != "" { + t.Fatalf("expected a fix on %q, got:\n%s", test.want, diag.EmitAllToString()) + } + }) + } +} + +func TestNoCommaFixAfterASkippedSemicolon(t *testing.T) { + _, diag := parseTestModule("fn main() {\n let c = [3]i32{1, 2, 3;\n}") + for _, item := range diag.Diagnostics() { + for _, extra := range item.Extras { + for _, fix := range extra.Text.Fixes { + if fix.NewText == "," { + t.Fatalf("a comma must not be offered after the stray `;`:\n%s", diag.EmitAllToString()) + } + } + } + } +} + +func TestMissingCommasInOneListAreOneError(t *testing.T) { + _, diag := parseTestModule("fn main() { add(1 2 3); }") + items := diag.Diagnostics() + if len(items) != 1 || items[0].Message != "expected ','" || len(items[0].Extras) != 1 || + items[0].Extras[0].Text.Message != "add the missing commas" || len(items[0].Extras[0].Text.Fixes) != 2 { + t.Fatalf("expected one error carrying both commas, got:\n%s", diag.EmitAllToString()) + } +} + +func TestMissingCommaInOtherLists(t *testing.T) { + for name, source := range map[string]string{ + "type parameters": "struct Pair { a: T, b: U }", + "function type": "type F = fn(i32 i32) -> i32;", + "parameter without colon": "fn f(a i32 b: i32) {}", + } { + _, diag := parseTestModule(source) + found := false + for _, item := range diag.Diagnostics() { + if item.Message == "expected ','" && len(item.Extras) == 1 && len(item.Extras[0].Text.Fixes) == 1 { + found = true + } + } + if !found { + t.Errorf("%s: expected a missing-comma fix, got:\n%s", name, diag.EmitAllToString()) + } + } +} + +func TestSemicolonClosesAValueListMissingItsBrace(t *testing.T) { + _, diag := parseTestModule("fn main() {\n let c = [3]i32{1, 2, 3;\n return;\n}") + items := diag.Diagnostics() + if len(items) != 1 || items[0].Message != "expected '}' after array literal" || len(items[0].Extras) != 1 || + items[0].Extras[0].Text.Fixes[0].NewText != "}" { + t.Fatalf("expected one error inserting `}`, got:\n%s", diag.EmitAllToString()) + } +} + +func TestUnclosedCallDoesNotTakeTheNextStatementAsAnArgument(t *testing.T) { + for name, source := range map[string]string{ + "call statement": "fn main() {\n f(1\n g(2);\n}", + "assignment statement": "fn main() {\n f(1, 2\n x = 3;\n}", + } { + mod, diag := parseTestModule(source) + items := diag.Diagnostics() + if len(items) != 1 || items[0].Message != "expected ')'" || len(items[0].Extras) != 1 || + items[0].Extras[0].Text.Fixes[0].NewText != ")" { + t.Errorf("%s: expected one error inserting `)`, got:\n%s", name, diag.EmitAllToString()) + } + if body := mod.Stmts[0].(*ast.FnDecl).Body; len(body.Stmts) != 2 { + t.Errorf("%s: got %d statements, want the call and the one after it", name, len(body.Stmts)) + } + } +} + +func TestMissingCommasAcrossLinesOfAClosedCall(t *testing.T) { + _, diag := parseTestModule("fn main() {\n add(\n 1\n 2\n 3\n );\n}") + items := diag.Diagnostics() + if len(items) != 1 || items[0].Message != "expected ','" || len(items[0].Extras) != 1 || + len(items[0].Extras[0].Text.Fixes) != 2 { + t.Fatalf("expected one error carrying both commas, got:\n%s", diag.EmitAllToString()) + } +} + +func TestSemicolonsTypedForCommasAreOneError(t *testing.T) { + for _, test := range []struct { + name string + source string + want []string // text each fix puts in place of a `;` + }{ + {"composite literal", "fn main() { let p: P = .{ x = 1; y = 2 }; }", []string{","}}, + {"array literal", "fn main() { let xs = [3]i32{1; 2; 3}; }", []string{",", ","}}, + {"one item per line", "fn main() {\n let p: P = .{\n x = 1;\n y = 2;\n };\n}", []string{",", ""}}, + {"literal as an argument", "fn main() { f(.{ x = 1; y = 2 }); }", []string{","}}, + {"struct fields", "struct P { x: i32; y: i32 }", []string{","}}, + {"enum variants", "enum E {\n A;\n B;\n}", []string{",", ""}}, + } { + t.Run(test.name, func(t *testing.T) { + _, diag := parseTestModule(test.source) + items := diag.Diagnostics() + if len(items) != 1 || items[0].Message != "expected ',' between items, found ';'" || len(items[0].Extras) != 1 { + t.Fatalf("expected one separator error, got:\n%s", diag.EmitAllToString()) + } + fixes := items[0].Extras[0].Text.Fixes + if len(fixes) != len(test.want) { + t.Fatalf("got %d fixes, want %d:\n%s", len(fixes), len(test.want), diag.EmitAllToString()) + } + for i, fix := range fixes { + if fix.NewText != test.want[i] { + t.Errorf("fix %d puts %q, want %q", i, fix.NewText, test.want[i]) + } + } + }) + } +} + +func TestParameterNameWithoutTypeIsOneErrorAndKeepsTheList(t *testing.T) { + for _, test := range []struct{ name, source, want string }{ + {"name before a named parameter", "fn f(a b: i32) {}", "expected a type after parameter name"}, + {"modifier before a lone name", "fn f(mut a, b: i32) {}", "expected a name and a type after `mut`"}, + {"modifier before a lone type name", "fn f(mut i32, b: i32) {}", "expected a name and a type after `mut`"}, + {"modifier before a lone last word", "fn f(b: i32, mut a) {}", "expected a name and a type after `mut`"}, + } { + mod, diag := parseTestModule(test.source) + items := diag.Diagnostics() + if len(items) != 1 || items[0].Message != test.want || len(items[0].Extras) != 0 { + t.Errorf("%s: expected only %q with no fix, got:\n%s", test.name, test.want, diag.EmitAllToString()) + } + params := mod.Stmts[0].(*ast.FnDecl).Params + if len(params) != 2 || params[0].Name == nil || params[1].Name == nil { + t.Errorf("%s: expected two named parameters, got %d", test.name, len(params)) + } + } +} + +func TestLoneNameAfterBindingNameOffersBothRepairs(t *testing.T) { + for name, source := range map[string]string{ + "before a semicolon": "fn main() { let c other; }", + "at the end of a line": "fn main() {\n let c other\n}", + "in a const": "const c other;", + } { + _, diag := parseTestModule(source) + var found *diagnostics.Diagnostic + for _, item := range diag.Diagnostics() { + if item.Message == "expected ':' or '=' after the name" { + found = item + } + } + if found == nil || len(found.Extras) != 2 || + found.Extras[0].Text.Message != "if `other` is a type, add `:`" || found.Extras[0].Text.Fixes[0].NewText != ":" || + found.Extras[1].Text.Message != "if `other` is a value, add `=`" || found.Extras[1].Text.Fixes[0].NewText != " =" { + t.Errorf("%s: expected both repairs, got:\n%s", name, diag.EmitAllToString()) + } + } +} + +func TestRepeatedParameterModifierIsOneError(t *testing.T) { + mod, diag := parseTestModule("fn f(mut mut a: i32) {}") + items := diag.Diagnostics() + if len(items) != 1 || items[0].Message != "`mut` is repeated" || len(items[0].Extras) != 1 || + len(items[0].Extras[0].Text.Fixes) != 1 || items[0].Extras[0].Text.Fixes[0].NewText != "" { + t.Fatalf("expected one error removing the extra `mut`, got:\n%s", diag.EmitAllToString()) + } + if params := mod.Stmts[0].(*ast.FnDecl).Params; len(params) != 1 || !params[0].IsMutable { + t.Fatalf("expected one mutable parameter, got %d", len(params)) + } +} + +func TestSingleRepairDiagnostics(t *testing.T) { + for _, test := range []struct { + name, source, message, text string + }{ + {"return type without arrow", "fn f() i32 { return 0; }", "expected '->' before the return type", " ->"}, + {"parameter list closed before the arrow", "fn f(a: i32 -> i32 { return a; }", "expected ')'", ")"}, + {"match arm without arrow", "fn f(e: E) { match e { E::A { return; } } }", "expected '=>' after match pattern", " =>"}, + {"array type without colon", "fn main() { let a [3]i32; }", "expected ':' before the type", ":"}, + {"optional type without colon", "fn main() { let a ?i32; }", "expected ':' before the type", ":"}, + {"colon in a struct literal", "fn main() { let p = P.{ x: 1, y: 2 }; }", "expected '=' after struct literal field name", " ="}, + {"literal closed by the call's paren", "fn main() { f(.{ x = 1 ); }", "expected '}' after composite literal", "}"}, + {"assignment in a condition", "fn main() { if a = 1 { return; } }", "expected '==' in condition, found '='", "=="}, + {"fields on one line without comma", "struct P { x: i32 y: i32 }", "expected ','", ","}, + {"literal fields on one line without comma", "fn main() { let p = P.{ x = 1 y = 2 }; }", "expected ','", ","}, + {"variants on separate lines without comma", "enum E {\n A\n B\n}", "expected ','", ","}, + } { + t.Run(test.name, func(t *testing.T) { + _, diag := parseTestModule(test.source) + items := diag.Diagnostics() + if len(items) != 1 || items[0].Message != test.message || len(items[0].Extras) != 1 || + items[0].Extras[0].Text.Fixes[0].NewText != test.text { + t.Fatalf("expected only %q with fix %q, got:\n%s", test.message, test.text, diag.EmitAllToString()) + } + }) + } +} + +func TestTrailingCommaBeforeAParenIsOnlyANote(t *testing.T) { + for name, source := range map[string]string{ + "call": "fn main() { f(1, 2,); }", + "parameters": "fn f(a: i32, b: i32,) {}", + } { + _, diag := parseTestModule(source) + items := diag.Diagnostics() + if diag.HasErrors() || len(items) != 1 || items[0].Message != "trailing comma is unnecessary" { + t.Errorf("%s: expected only the trailing comma note, got:\n%s", name, diag.EmitAllToString()) + } + } +} + +func TestBracedListRecoveryStillEndsOnTokensNoItemAccepts(t *testing.T) { + for name, source := range map[string]string{ + "function inside struct fields": "struct S { x: i32 fn f() {} }", + "statement inside a literal": "fn main() {\n let a = [3]i32{1, 2\n foo(x);\n}", + "pattern fields keep with": "fn f(e: E) { match e { E::B { v = x } => { return; } } }", + } { + _, diag := parseTestModule(source) + if !diag.HasErrors() { + t.Errorf("%s: expected an error", name) + } + } +} diff --git a/internal/frontend/parser/recover_literal.go b/internal/frontend/parser/recover_literal.go index f43a9638..0ed4e3bb 100644 --- a/internal/frontend/parser/recover_literal.go +++ b/internal/frontend/parser/recover_literal.go @@ -67,7 +67,7 @@ scan: p.diag.Add(diagnostics.NewError(message). WithCode(diagnostics.ErrInvalidExpression). WithPrimaryLabel(source.NewLocation(p.filePath, open.Start, open.End), "missing literal introducer"). - WithHelp(help)) + Help(help)) p.pos = end return reg(p, &ast.BadExpr{Location: source.NewLocation(p.filePath, start, p.prev().End)}) } diff --git a/internal/ir/cfg/analyze.go b/internal/ir/cfg/analyze.go index 578d6631..0e586b98 100644 --- a/internal/ir/cfg/analyze.go +++ b/internal/ir/cfg/analyze.go @@ -78,8 +78,8 @@ func reportMissingReturn(fn *ControlFlowGraph, diag *diagnostics.DiagnosticBag) diagnostic.WithSecondaryLabel(branch.Location, "this branch does not return a value") } } - diagnostic.WithNote("some branch completes without a `return`, execution can fall off end of function") - diagnostic.WithHelp("fulfill the return or add a fallback return on parent scope") + diagnostic.Note("some branch completes without a `return`, execution can fall off end of function") + diagnostic.Help("fulfill the return or add a fallback return on parent scope") diag.Add(diagnostic) } diff --git a/internal/pipeline/loader.go b/internal/pipeline/loader.go index 501ca1e1..66612980 100644 --- a/internal/pipeline/loader.go +++ b/internal/pipeline/loader.go @@ -138,7 +138,7 @@ func (l *moduleLoader) resolveImports(mod *module.Module, diag *diagnostics.Diag diag.Add(diagnostics.NewInfo("`"+rawPath+"` is imported automatically"). WithCode(diagnostics.InfoRedundantPreludeImport). WithPrimaryLabel(ast.LocOf(imp), "remove this import"). - WithNote("global symbols are always in scope without an import")) + Note("global symbols are always in scope without an import")) } resolvedImport := *resolved resolvedImport.Decl = imp diff --git a/internal/problems/problems.go b/internal/problems/problems.go index 63f49a73..c3dad148 100644 --- a/internal/problems/problems.go +++ b/internal/problems/problems.go @@ -21,7 +21,7 @@ func UnreachableCode(loc *source.Location) *diagnostics.Diagnostic { return diagnostics.NewWarning("unreachable code"). WithCode(diagnostics.WarnUnreachableCode). WithPrimaryLabel(loc, "this code is unreachable"). - WithHelp("remove this code or restructure control flow") + Help("remove this code or restructure control flow") } func Redeclaration(message string, current, previous *source.Location) *diagnostics.Diagnostic { diff --git a/internal/semantics/analysis/definite_init.go b/internal/semantics/analysis/definite_init.go index 878dc89b..b9bde7a8 100644 --- a/internal/semantics/analysis/definite_init.go +++ b/internal/semantics/analysis/definite_init.go @@ -201,7 +201,7 @@ func reportUninitializedAccess(at effectPlace, location *source.Location, curren diag.Add(diagnostics.NewError(msg). WithCode(diagnostics.ErrUninitializedVariable). WithPrimaryLabel(location, msg). - WithHelp(help)) + Help(help)) } func copyInitState(current initState) initState { diff --git a/internal/semantics/analysis/flow_thir.go b/internal/semantics/analysis/flow_thir.go index a66dcad3..891d528c 100644 --- a/internal/semantics/analysis/flow_thir.go +++ b/internal/semantics/analysis/flow_thir.go @@ -256,14 +256,14 @@ func optionalPayloadProofAt(location *source.Location) *diagnostics.Diagnostic { return diagnostics.NewError("optional payload use requires a presence proof"). WithPrimaryLabel(location, "payload is not proven present here"). WithCode(diagnostics.ErrOptionalPayloadProof). - WithHelp("guard this stable place with `value != none` or return after `value == none`") + Help("guard this stable place with `value != none` or return after `value == none`") } func unstableOptionalNarrowingAt(location *source.Location) *diagnostics.Diagnostic { return diagnostics.NewError("optional narrowing subject is not a stable place"). WithPrimaryLabel(location, "this expression can change between the test and use"). WithCode(diagnostics.ErrUnstableNarrowing). - WithHelp("bind the expression or index to a direct local before testing it") + Help("bind the expression or index to a direct local before testing it") } func (a *flowAnalyzer) AnalyzeBlock(*thir.Block) {} diff --git a/internal/semantics/analysis/ownership.go b/internal/semantics/analysis/ownership.go index e01d49ea..fe48bc64 100644 --- a/internal/semantics/analysis/ownership.go +++ b/internal/semantics/analysis/ownership.go @@ -358,7 +358,7 @@ func (a *analyzer) mergeState(nodeID cfg.SiteID, dst, src ownershipState, hasExi a.reportedJoin[nodeID] = true a.diagnostics.AddError(diagnostics.ErrInvalidAssignment, "ownership state differs across control-flow paths", node.cfgSite.Location, ""). - WithHelp("move or reinitialize ownership-tracked values on every path") + Help("move or reinitialize ownership-tracked values on every path") } for sym, site := range src.moved { if _, ok := dst.moved[sym]; ok { diff --git a/internal/semantics/analysis/ownership_expr.go b/internal/semantics/analysis/ownership_expr.go index 2bd15ed9..6df943bd 100644 --- a/internal/semantics/analysis/ownership_expr.go +++ b/internal/semantics/analysis/ownership_expr.go @@ -177,7 +177,7 @@ func (a *analyzer) reportPointerEscape(expr thir.Expr, origin *symbols.Symbol) { if origin.Location != nil { diag.WithSecondaryLabel(origin.Location, "local storage declared here") } - diag.WithHelp("allocate the value with an explicit allocator before returning a pointer to it") + diag.Help("allocate the value with an explicit allocator before returning a pointer to it") } func ownershipTrackedSymbol(sym *symbols.Symbol) bool { diff --git a/internal/semantics/resolver/resolver.go b/internal/semantics/resolver/resolver.go index e2d206a0..6f0fb54d 100644 --- a/internal/semantics/resolver/resolver.go +++ b/internal/semantics/resolver/resolver.go @@ -262,7 +262,7 @@ func (r *resolver) resolveExpr(scope *symbols.Scope, expr ast.Expr) { diagnostics.NewError(msg). WithCode(diagnostics.ErrUseBeforeDecl). WithPrimaryLabel(ast.LocOf(node), msg). - WithHelp("rename binding or use earlier value"), + Help("rename binding or use earlier value"), ) return } @@ -454,7 +454,7 @@ func (r *resolver) lookupImportedMember(qualifierNode, memberNode *ast.Ident, si if !resolved.Symbol.IsPub { r.ctx.Diagnostics.AddError(diagnostics.ErrSymbolNotExported, "`"+member+"` is not exported from `"+qualifier+"`", ast.LocOf(site), "use of unexported symbol"). WithSecondaryLabel(resolved.Symbol.Location, "defined here"). - WithNote("symbols with uppercase are exported otherwise private") + Note("symbols with uppercase are exported otherwise private") r.reportGlobalQualifier(resolved.Symbol, qualifier, site) return nil, false } @@ -477,5 +477,5 @@ func (r *resolver) reportGlobalQualifier(sym *symbols.Symbol, qualifier string, r.ctx.Diagnostics.Add(diagnostics.NewInfo("`"+sym.Name+"` is already in scope without `"+qualifier+"::`"). WithCode(diagnostics.InfoRedundantGlobalQualifier). WithPrimaryLabel(ast.LocOf(site), "drop the `"+qualifier+"::` prefix"). - WithNote("global symbols are always in scope")) + Note("global symbols are always in scope")) } diff --git a/internal/semantics/typechecker/assignability.go b/internal/semantics/typechecker/assignability.go index 0e9bebfb..b7725fa9 100644 --- a/internal/semantics/typechecker/assignability.go +++ b/internal/semantics/typechecker/assignability.go @@ -111,7 +111,7 @@ func (c *checker) addInterfaceHint(d *diagnostics.Diagnostic, dst, src typeinfo. } _, missing, _ := c.resolveInterfaceImplementations(iface, src) if len(missing) > 0 { - d.WithHelp(fmt.Sprintf("missing methods: %s", strings.Join(missing, ", "))) + d.Help(fmt.Sprintf("missing methods: %s", strings.Join(missing, ", "))) } } diff --git a/internal/semantics/typechecker/check_call.go b/internal/semantics/typechecker/check_call.go index 1843f99b..95d6f16e 100644 --- a/internal/semantics/typechecker/check_call.go +++ b/internal/semantics/typechecker/check_call.go @@ -353,7 +353,7 @@ func (c *checker) typeSelectorCall(scope *symbols.Scope, selector *ast.SelectorE if field, _, fieldOK := typeinfo.LookupStructField(baseType, selector.Name.Name); fieldOK { c.ctx.Diagnostics.AddError(diagnostics.ErrNotCallable, fmt.Sprintf("field `%s` is not callable", selector.Name.Name), ast.LocOf(selector.Name), ""). - WithHelp(fmt.Sprintf("field `%s` has type %s - access it without `()`", selector.Name.Name, typeinfo.TypeText(field.Type))) + Help(fmt.Sprintf("field `%s` has type %s - access it without `()`", selector.Name.Name, typeinfo.TypeText(field.Type))) return &typeinfo.InvalidType{} } methods := c.availableMethods(baseType) @@ -361,9 +361,9 @@ func (c *checker) typeSelectorCall(scope *symbols.Scope, selector *ast.SelectorE WithCode(diagnostics.ErrMethodNotFound). WithPrimaryLabel(ast.LocOf(selector.Name), "") if len(methods) > 0 { - d.WithHelp("available methods: " + strings.Join(methods, ", ")) + d.Help("available methods: " + strings.Join(methods, ", ")) } else if match, ok := diagnostics.NearestName(selector.Name.Name, availableFields(baseType)); ok { - d.WithHelp("did you mean field `" + match + "`?") + d.Help("did you mean field `" + match + "`?") } c.ctx.Diagnostics.Add(d) return &typeinfo.InvalidType{} @@ -399,7 +399,7 @@ func (c *checker) checkCall(scope *symbols.Scope, receiverExpr ast.Expr, callExp for i, p := range fnType.Params[displayOffset:] { paramDescs[i] = typeinfo.TypeText(p) } - d.WithHelp(fmt.Sprintf("expected parameters: (%s)", strings.Join(paramDescs, ", "))) + d.Help(fmt.Sprintf("expected parameters: (%s)", strings.Join(paramDescs, ", "))) } c.ctx.Diagnostics.Add(d) return diff --git a/internal/semantics/typechecker/check_expr.go b/internal/semantics/typechecker/check_expr.go index bdd44dc6..7ffa6bf6 100644 --- a/internal/semantics/typechecker/check_expr.go +++ b/internal/semantics/typechecker/check_expr.go @@ -611,7 +611,7 @@ func (c *checker) typeSelectorExpr(scope *symbols.Scope, node *ast.SelectorExpr) WithCode(diagnostics.ErrFieldNotFound). WithPrimaryLabel(ast.LocOf(node.Name), "") if match, ok := diagnostics.NearestName(node.Name.Name, append(availableFields(baseType), c.availableMethods(baseType)...)); ok { - d.WithHelp("did you mean `" + match + "`?") + d.Help("did you mean `" + match + "`?") } c.ctx.Diagnostics.Add(d) return &typeinfo.InvalidType{} @@ -849,7 +849,7 @@ func (c *checker) typeLiteralFields(scope *symbols.Scope, site ast.Node, fields valid = false c.ctx.Diagnostics.AddError(diagnostics.ErrMissingInitializer, "missing "+literal+" field `"+targetField.Name+"`", ast.LocOf(site), ""). - WithHelp(fmt.Sprintf("required fields: %s", strings.Join(required, ", "))) + Help(fmt.Sprintf("required fields: %s", strings.Join(required, ", "))) continue } ordered[index] = field.Value @@ -1038,7 +1038,7 @@ func (c *checker) typeNumber(node *ast.NumberLit, expected typeinfo.Type) typein if !ok { c.ctx.Diagnostics.AddError(diagnostics.ErrInvalidNumber, fmt.Sprintf("unsupported numeric literal suffix `%s`", node.ExplicitType), ast.LocOf(node), ""). - WithHelp("integer suffix widths must be between 1 and 8388608; float suffixes are limited to f32 and f64") + Help("integer suffix widths must be between 1 and 8388608; float suffixes are limited to f32 and f64") return &typeinfo.InvalidType{} } if !typeinfo.LiteralFitsType(node.Value, explicit) { @@ -1066,7 +1066,7 @@ func (c *checker) typeNumber(node *ast.NumberLit, expected typeinfo.Type) typein WithCode(diagnostics.ErrInvalidNumber). WithPrimaryLabel(ast.LocOf(node), "") if intType, ok := numberTarget.(*typeinfo.IntegerType); ok { - d.WithHelp(integerRangeHint(intType)) + d.Help(integerRangeHint(intType)) } c.ctx.Diagnostics.Add(d) return nil diff --git a/internal/semantics/typechecker/check_fn.go b/internal/semantics/typechecker/check_fn.go index a80b80c9..8a938fed 100644 --- a/internal/semantics/typechecker/check_fn.go +++ b/internal/semantics/typechecker/check_fn.go @@ -159,7 +159,8 @@ func (c *checker) checkFunctionShape(decl *ast.FnDecl, fnType *typeinfo.FuncType continue } paramType := fnType.Params[index] - if typeinfo.ContainsInvalid(paramType) { + // A parameter without a written type was reported by the parser. + if param.Type == nil || typeinfo.ContainsInvalid(paramType) { continue } if c.rejectUnsizedType(paramType, param.Type, "parameter") { @@ -344,6 +345,10 @@ func (c *checker) checkInterfaceDecl(decl *ast.InterfaceDecl) { } context := c.typeContextForDecl(decl) for _, param := range method.Params { + // A parameter without a written type was reported by the parser. + if param.Type == nil { + continue + } paramType := c.ctx.TypeResolver.Resolve(c.ctx.Diagnostics, c.module, param.Type, context) if c.rejectUnsizedType(paramType, param.Type, "interface method parameter") { continue @@ -614,8 +619,8 @@ func (c *checker) checkDeclAttributes(decl ast.Decl) { case ast.AttributeExtern: if fn != nil && fn.Body != nil { d := invalidAttributeError(attr, "attribute `#[extern]` requires a body-less function declaration") - d.WithHelp("remove body to declare extern function") - d.WithHelp("remove `#[extern]` to keep local definition") + d.Help("remove body to declare extern function") + d.Help("remove `#[extern]` to keep local definition") c.ctx.Diagnostics.Add(d) } } diff --git a/internal/semantics/typechecker/check_stmt.go b/internal/semantics/typechecker/check_stmt.go index 92413493..239d7260 100644 --- a/internal/semantics/typechecker/check_stmt.go +++ b/internal/semantics/typechecker/check_stmt.go @@ -138,7 +138,7 @@ func (c *checker) checkMatchStmt(scope *symbols.Scope, node *ast.MatchStmt, retu if typeinfo.OwnershipCapabilityOf(subjectType).NeedsDrop && !place.IsPlaceExpr(node.Subject) { c.ctx.Diagnostics.Add(invalidOperationError(node.Subject, "ownership-bearing match subject must be a named place"). - WithHelp("bind subject to a local before matching it")) + Help("bind subject to a local before matching it")) } evidence := Match{ SubjectID: node.Subject.ID(), @@ -336,7 +336,7 @@ func (c *checker) checkAssign(scope *symbols.Scope, node *ast.AssignStmt) { if sharedReference != nil { c.ctx.Diagnostics.AddError(diagnostics.ErrInvalidAssignment, "cannot assign through immutable reference", ast.LocOf(target), ""). - WithHelp(fmt.Sprintf("use `&mut %s` to modify referenced value", typeinfo.TypeText(sharedReference))) + Help(fmt.Sprintf("use `&mut %s` to modify referenced value", typeinfo.TypeText(sharedReference))) return } c.ctx.Diagnostics.AddError(diagnostics.ErrInvalidAssignment, @@ -677,14 +677,14 @@ func (c *checker) checkCallIteration(scope *symbols.Scope, node *ast.ForStmt, it switch { case !isCallExpression: c.ctx.Diagnostics.Add(invalidExpressionError(node.Iterable, "cannot iterate over "+typeinfo.TypeText(iterableType)). - WithHelp("use a range, array, slice, or an explicit call returning an optional item, such as `for item in producer()`"). - WithNote("iterator calls, including arguments, are evaluated on every attempt; a bare optional value is not a producer")) + Help("use a range, array, slice, or an explicit call returning an optional item, such as `for item in producer()`"). + Note("iterator calls, including arguments, are evaluated on every attempt; a bare optional value is not a producer")) case !isOptionalResultType || optional.Inner == nil: c.ctx.Diagnostics.Add(invalidExpressionError(node.Iterable, "iterator call must return an optional item"). - WithHelp("return an optional type, such as `?i32`: return an item to continue, or `none` to end the loop")) + Help("return an optional type, such as `?i32`: return an item to continue, or `none` to end the loop")) case node.Index != nil: c.ctx.Diagnostics.Add(invalidExpressionError(node.Index, "iterator loops provide an item, not an index"). - WithHelp("use `for item in producer()`; if you need an index, maintain a separate counter")) + Help("use `for item in producer()`; if you need an index, maintain a separate counter")) default: c.expandCallIteration(scope, node) } @@ -813,7 +813,7 @@ func (c *checker) rejectUnsizedType(typ typeinfo.Type, site ast.Node, context st diagnostic := invalidTypeError(site, fmt.Sprintf("%s requires a sized type; %s is unsized", context, typeinfo.TypeText(typ))) if _, ok := typeinfo.Underlying(typ).(*typeinfo.InterfaceType); ok { - diagnostic.WithHelp("use &Interface, &mut Interface, or *Interface instead of a bare interface value") + diagnostic.Help("use &Interface, &mut Interface, or *Interface instead of a bare interface value") } c.ctx.Diagnostics.Add(diagnostic) return true @@ -867,7 +867,7 @@ func (c *checker) rejectTemporaryBorrowEscape(scope *symbols.Scope, expr ast.Exp } diagnostic := c.ctx.Diagnostics.AddError(diagnostics.ErrInvalidExpression, fmt.Sprintf("reference to temporary cannot escape through %s", context), ast.LocOf(expr), "temporary borrow escapes here"). - WithHelp("pass the borrow directly to a call so it ends with the full expression") + Help("pass the borrow directly to a call so it ends with the full expression") if temporary != expr { diagnostic.WithSecondaryLabel(ast.LocOf(temporary), "temporary borrowed here") } diff --git a/internal/semantics/typechecker/errors.go b/internal/semantics/typechecker/errors.go index cf0fa6fb..2b726497 100644 --- a/internal/semantics/typechecker/errors.go +++ b/internal/semantics/typechecker/errors.go @@ -17,8 +17,8 @@ func explicitBoolCastRequiredError(expr ast.Expr, message string) *diagnostics.D e := diagnostics.NewError(message). WithPrimaryLabel(ast.LocOf(expr), ""). WithCode(diagnostics.ErrInvalidOperation). - WithNote("condition is a boolean type. It either can be `true` or `false`"). - WithHelp("use `as bool` for explicit conversion") + Note("condition is a boolean type. It either can be `true` or `false`"). + Help("use `as bool` for explicit conversion") return e } diff --git a/internal/semantics/typeresolution/instances.go b/internal/semantics/typeresolution/instances.go index a0685b1d..d74d4f77 100644 --- a/internal/semantics/typeresolution/instances.go +++ b/internal/semantics/typeresolution/instances.go @@ -69,7 +69,7 @@ func (r *Resolver) instantiateType(diag *diagnostics.DiagnosticBag, base *typein WithCode(diagnostics.ErrInvalidType). WithPrimaryLabel(ast.LocOf(node), "`"+applicationText+"` changes recursive arguments"). WithSecondaryLabel(ast.LocOf(origin.node), "`"+origin.applicationText+"` started this instantiation"). - WithHelp("use the same canonical type arguments at every recursive reference") + Help("use the same canonical type arguments at every recursive reference") diag.Add(diagnostic) } return &typeinfo.InvalidType{} diff --git a/internal/semantics/usage/usage.go b/internal/semantics/usage/usage.go index 9061fb57..7af2834c 100644 --- a/internal/semantics/usage/usage.go +++ b/internal/semantics/usage/usage.go @@ -96,7 +96,7 @@ func Analyze(diag *diagnostics.DiagnosticBag, module *module.Module, preludeID m } diag.AddWarning(diagnostics.WarnUnmodifiedMutable, fmt.Sprintf("mutable binding `%s` is never modified", sym.Name), sym.MutableLocation, ""). - WithHelp("remove unnecessary `mut`", diagnostics.Fix.Remove(sym.MutableLocation)) + Help("remove unnecessary `mut`", diagnostics.Fix.Remove(sym.MutableLocation)) } }) } diff --git a/x_test/negative_bang_after_argument/peeper.toml b/x_test/negative_bang_after_argument/peeper.toml new file mode 100644 index 00000000..7d5e6283 --- /dev/null +++ b/x_test/negative_bang_after_argument/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_bang_after_argument" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = ["expected ')'", "Compilation failed with 1 error(s)"] +stderr_excludes = ["expected ','", "= help: add missing", "wrong number of arguments"] diff --git a/x_test/negative_bang_after_argument/src/main.peep b/x_test/negative_bang_after_argument/src/main.peep new file mode 100644 index 00000000..baa8d3a1 --- /dev/null +++ b/x_test/negative_bang_after_argument/src/main.peep @@ -0,0 +1,12 @@ +fn Same(value: bool) -> bool { + return value; +} + +fn main() -> i32 { + let ready = true; + let done = false; + if Same(ready !done) { + return 1; + } + return 0; +} diff --git a/x_test/negative_let_missing_separator/peeper.toml b/x_test/negative_let_missing_separator/peeper.toml new file mode 100644 index 00000000..e0654081 --- /dev/null +++ b/x_test/negative_let_missing_separator/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_let_missing_separator" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = [" = help: add missing `=`\n | \n1 | const Limit = 5;\n", " = help: add missing `:`\n | \n2 | const Width: i32 = 6;\n", " = help: add missing `=`\n | \n3 | const Depth: i32 = 7;\n", " = help: add missing `=`\n | \n6 | let first: i32 = 5;\n", " = help: add missing `=`\n | \n7 | let second = 5 + 3;\n", " = help: add missing `:`\n | \n8 | let third: i32 = 1;\n", " = help: add missing `=`\n | \n9 | let values = [3]i32{1, 2, 3};\n", " = help: add missing `=`\n | \n10 | let target = &first;\n", "Compilation failed with 8 error(s)"] +stderr_excludes = ["\n\n\n", "expected ';'", "unknown identifier", "needs type or initializer", "module scope expects declaration"] diff --git a/x_test/negative_let_missing_separator/src/main.peep b/x_test/negative_let_missing_separator/src/main.peep new file mode 100644 index 00000000..4549dbc0 --- /dev/null +++ b/x_test/negative_let_missing_separator/src/main.peep @@ -0,0 +1,12 @@ +const Limit 5; +const Width i32 = 6; +const Depth: i32 7; + +fn main() -> i32 { + let first: i32 5; + let second 5 + 3; + let third i32 = 1; + let values [3]i32{1, 2, 3}; + let target &first; + return first + second + third + values[0] + Limit + Width + Depth; +} diff --git a/x_test/negative_missing_closers/peeper.toml b/x_test/negative_missing_closers/peeper.toml new file mode 100644 index 00000000..6e22c40e --- /dev/null +++ b/x_test/negative_missing_closers/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_missing_closers" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = [" = help: add missing `]`\n | \n11 | let first = values[0];\n", " = help: add missing `]`\n | \n12 | let second = Double(values[1]);\n", " = help: add missing `)`\n | \n13 | let third = (first + second);\n", " = help: add missing `)`\n | \n14 | let total = Sum(first, second)\n", " = help: add missing `)`\n | \n16 | Double(total)\n", "cannot assign to immutable binding `count`", "Compilation failed with 6 error(s)"] +stderr_excludes = ["\n\n\n", "expected ','", "wrong number of arguments", "unknown identifier", "used before"] diff --git a/x_test/negative_missing_closers/src/main.peep b/x_test/negative_missing_closers/src/main.peep new file mode 100644 index 00000000..efd66fdd --- /dev/null +++ b/x_test/negative_missing_closers/src/main.peep @@ -0,0 +1,19 @@ +fn Double(value: i32) -> i32 { + return value * 2; +} + +fn Sum(first: i32, second: i32) -> i32 { + return first + second; +} + +fn main() -> i32 { + let values = [3]i32{1, 2, 3}; + let first = values[0; + let second = Double(values[1); + let third = (first + second; + let total = Sum(first, second + let count = 3; + Double(total + count = 4; + return third; +} diff --git a/x_test/negative_missing_comma_in_lists/peeper.toml b/x_test/negative_missing_comma_in_lists/peeper.toml new file mode 100644 index 00000000..0e64127a --- /dev/null +++ b/x_test/negative_missing_comma_in_lists/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_missing_comma_in_lists" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = [" = help: add missing `,`\n | \n1 | struct Pair {\n", " = help: add missing `,`\n | \n6 | type Combine = fn(i32, i32) -> i32;\n", "expected ':' after parameter name", " = help: add missing `:`\n | \n8 | fn Scale(value: i32, factor: i32) -> i32 {\n", " = help: add the missing commas\n | \n17 | let total = Sum(1, 2, 3);\n", " = help: add the missing commas\n | \n19 | 1,\n20 | 2,\n", "Compilation failed with 5 error(s)"] +stderr_excludes = ["\n\n\n", "wrong number of arguments", "expected ')'"] diff --git a/x_test/negative_missing_comma_in_lists/src/main.peep b/x_test/negative_missing_comma_in_lists/src/main.peep new file mode 100644 index 00000000..cb5ab46b --- /dev/null +++ b/x_test/negative_missing_comma_in_lists/src/main.peep @@ -0,0 +1,24 @@ +struct Pair { + first: T, + second: U +} + +type Combine = fn(i32 i32) -> i32; + +fn Scale(value i32, factor: i32) -> i32 { + return value * factor; +} + +fn Sum(first: i32, second: i32, third: i32) -> i32 { + return first + second + third; +} + +fn main() -> i32 { + let total = Sum(1 2 3); + let spread = Sum( + 1 + 2 + 3 + ); + return Scale(total, spread); +} diff --git a/x_test/negative_missing_token_at_known_spot/peeper.toml b/x_test/negative_missing_token_at_known_spot/peeper.toml new file mode 100644 index 00000000..210581f7 --- /dev/null +++ b/x_test/negative_missing_token_at_known_spot/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_missing_token_at_known_spot" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = ["expected ','\n --> ", " = help: add missing `,`\n | \n1 | fn Add(left: i32, right: i32) -> i32 {\n", "expected ':' before the type", " = help: add missing `:`\n | \n6 | let first: i32 = 1;\n", "expected '=' before the value", " = help: add missing `=`\n | \n7 | let second = 2;\n", " = help: add missing `,`\n | \n8 | return Add(first, second);\n", "Compilation failed with 4 error(s)"] +stderr_excludes = ["\n\n\n", "unknown identifier", "used before"] diff --git a/x_test/negative_missing_token_at_known_spot/src/main.peep b/x_test/negative_missing_token_at_known_spot/src/main.peep new file mode 100644 index 00000000..b5e147ef --- /dev/null +++ b/x_test/negative_missing_token_at_known_spot/src/main.peep @@ -0,0 +1,9 @@ +fn Add(left: i32 right: i32) -> i32 { + return left + right; +} + +fn main() -> i32 { + let first i32 = 1; + let second 2; + return Add(first second); +} diff --git a/x_test/negative_missing_token_unreliable_guess/peeper.toml b/x_test/negative_missing_token_unreliable_guess/peeper.toml new file mode 100644 index 00000000..5e601a8c --- /dev/null +++ b/x_test/negative_missing_token_unreliable_guess/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_missing_token_unreliable_guess" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = ["expected ']' after array literal length", "add missing `]` here"] +stderr_excludes = ["= help: add missing", "\n\n\n"] diff --git a/x_test/negative_missing_token_unreliable_guess/src/main.peep b/x_test/negative_missing_token_unreliable_guess/src/main.peep new file mode 100644 index 00000000..bd5215dd --- /dev/null +++ b/x_test/negative_missing_token_unreliable_guess/src/main.peep @@ -0,0 +1,4 @@ +fn main() -> i32 { + let xs = [1, 2, 3; + return 0; +} diff --git a/x_test/negative_parameter_without_type/peeper.toml b/x_test/negative_parameter_without_type/peeper.toml new file mode 100644 index 00000000..52e853bc --- /dev/null +++ b/x_test/negative_parameter_without_type/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_parameter_without_type" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = ["expected a name and a type after `mut`\n --> ", "1 | fn Scale(mut value, factor: i32) -> i32 {\n | ^^^^^^^^^ write `mut name: Type`\n", "expected a type after parameter name\n --> ", "5 | fn Shift(value amount: i32) -> i32 {\n | ^ add `: Type` here\n", "`mut` is repeated", " = help: remove the extra `mut`\n | \n9 | fn Count(mut total: i32) -> i32 {\n", "14 | fn Keep(mut i32) -> i32 {\n | ^^^^^^^ write `mut name: Type`\n", "Compilation failed with 4 error(s)"] +stderr_excludes = ["\n\n\n", "is unsized", "unknown identifier", "missing parameter binding", "expected ','", "expected ')'"] diff --git a/x_test/negative_parameter_without_type/src/main.peep b/x_test/negative_parameter_without_type/src/main.peep new file mode 100644 index 00000000..663875e3 --- /dev/null +++ b/x_test/negative_parameter_without_type/src/main.peep @@ -0,0 +1,20 @@ +fn Scale(mut value, factor: i32) -> i32 { + return factor; +} + +fn Shift(value amount: i32) -> i32 { + return amount; +} + +fn Count(mut mut total: i32) -> i32 { + total = total + 1; + return total; +} + +fn Keep(mut i32) -> i32 { + return 0; +} + +fn main() -> i32 { + return Count(1); +} diff --git a/x_test/negative_semicolon_in_value_list/peeper.toml b/x_test/negative_semicolon_in_value_list/peeper.toml new file mode 100644 index 00000000..b274a66c --- /dev/null +++ b/x_test/negative_semicolon_in_value_list/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_semicolon_in_value_list" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = ["expected ',' between items, found ';'", " = help: separate items with `,`\n | \n2 | x: i32,\n", " = help: separate items with `,`\n | \n11 | let origin: Point = .{ x = 1, y = 2 };\n", " = help: separate items with `,`\n | \n12 | let values = [3]i32{1, 2, 3};\n", "expected '}' after array literal", " = help: add missing `}`\n | \n13 | let open = [3]i32{4, 5, 6};\n", "expected ':' or '=' after the name", " = help: if `i32` is a type, add `:`\n | \n14 | let size: i32\n | \n = help: if `i32` is a value, add `=`\n | \n14 | let size = i32\n", " = help: add missing `)`\n | \n15 | Double(origin.x)\n", "Compilation failed with 7 error(s)"] +stderr_excludes = ["\n\n\n", "wrong number of arguments", "module scope expects declaration", "unknown identifier", "expected ','\n"] diff --git a/x_test/negative_semicolon_in_value_list/src/main.peep b/x_test/negative_semicolon_in_value_list/src/main.peep new file mode 100644 index 00000000..815d9545 --- /dev/null +++ b/x_test/negative_semicolon_in_value_list/src/main.peep @@ -0,0 +1,18 @@ +struct Point { + x: i32; + y: i32 +} + +fn Double(value: i32) -> i32 { + return value * 2; +} + +fn main() -> i32 { + let origin: Point = .{ x = 1; y = 2 }; + let values = [3]i32{1; 2; 3}; + let open = [3]i32{4, 5, 6; + let size i32 + Double(origin.x + Double(values[0] + open[0]); + return 0; +} diff --git a/x_test/negative_single_repair_spots/peeper.toml b/x_test/negative_single_repair_spots/peeper.toml new file mode 100644 index 00000000..d8fc6312 --- /dev/null +++ b/x_test/negative_single_repair_spots/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_single_repair_spots" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = ["expected '->' before the return type", " = help: add missing `->`\n | \n15 | fn Width() -> i32 {\n", " = help: add missing `)`\n | \n19 | fn Add(first: i32, second: i32) -> i32 {\n", "expected '=>' after match pattern", " = help: add missing `=>`\n | \n25 | Kind::Empty => {\n", " = help: add missing `:`\n | \n36 | let slots: [3]i32;\n", " = help: add missing `:`\n | \n37 | let maybe: ?i32;\n", " = help: set a field with `=`\n | \n38 | let first: Point = .{ x = 1, y = 2 };\n", " = help: add missing `,`\n | \n39 | let second: Point = .{ x = 1, y = 2 };\n", " = help: add missing `}`\n | \n40 | let taken = Take(.{ x = 1, y = 2} );\n", "expected '==' in condition, found '='", " = help: compare with `==`\n | \n41 | if count == 1 {\n", "trailing comma is unnecessary", " = help: remove this comma\n | \n44 | return Add(taken, Width()) + first.x + second.x;\n", "Compilation failed with 9 error(s)"] +stderr_excludes = ["\n\n\n", "module scope expects declaration", "missing function body", "missing if body", "requires 'with'", "missing struct literal field", "wrong number of arguments", "expected expression"] diff --git a/x_test/negative_single_repair_spots/src/main.peep b/x_test/negative_single_repair_spots/src/main.peep new file mode 100644 index 00000000..7b8366ba --- /dev/null +++ b/x_test/negative_single_repair_spots/src/main.peep @@ -0,0 +1,45 @@ +struct Point { + x: i32, + y: i32 +} + +enum Kind { + Empty, + Full: { value: i32 } +} + +fn Take(point: Point) -> i32 { + return point.x; +} + +fn Width() i32 { + return 0; +} + +fn Add(first: i32, second: i32 -> i32 { + return first + second; +} + +fn Read(kind: Kind) -> i32 { + match kind { + Kind::Empty { + return 0; + } + Kind::Full with { value = inner } => { + return inner; + } + } +} + +fn main() -> i32 { + let count = 1; + let slots [3]i32; + let maybe ?i32; + let first: Point = .{ x: 1, y: 2 }; + let second: Point = .{ x = 1 y = 2 }; + let taken = Take(.{ x = 1, y = 2 ); + if count = 1 { + return 1; + } + return Add(taken, Width(),) + first.x + second.x; +} diff --git a/x_test/positive_multiline_lists_and_continuations/peeper.toml b/x_test/positive_multiline_lists_and_continuations/peeper.toml new file mode 100644 index 00000000..4b812aea --- /dev/null +++ b/x_test/positive_multiline_lists_and_continuations/peeper.toml @@ -0,0 +1,7 @@ +name = "positive_multiline_lists_and_continuations" +build = "program" + +[test] +mode = "run" +outcome = "exit_code" +exit_code = 0 diff --git a/x_test/positive_multiline_lists_and_continuations/src/main.peep b/x_test/positive_multiline_lists_and_continuations/src/main.peep new file mode 100644 index 00000000..64005a62 --- /dev/null +++ b/x_test/positive_multiline_lists_and_continuations/src/main.peep @@ -0,0 +1,41 @@ +struct Point { + x: i32, + y: i32 +} + +fn (self: &Point) sum() -> i32 { + return self.x + self.y; +} + +fn Add( + first: i32, + second: i32 +) -> i32 { + return first + + second; +} + +fn main() -> i32 { + let point: Point = .{ + x = 1, + y = 2 + }; + let values = [3]i32{ + 1, + 2, + 3 + }; + let total = Add( + point.x, + -point.y + ); + let chained = point + .sum(); + let later: i32 + = total; + let next = + values[0]; + Add(total, next); + Add(later, chained); + return total + later + chained + next - 2; +} From e0067b3ab4a73ffe4901716023bcd39cb1557d27 Mon Sep 17 00:00:00 2001 From: itsfuad Date: Fri, 9 Oct 2026 17:11:28 +0600 Subject: [PATCH 3/8] Report unknown type names once and silence follow-on errors A type name that is neither declared nor built in used to become a silent placeholder, so `let x: Nope;` compiled and other uses failed with unrelated messages. Report "unknown type" where the name is written, with a similar declared name when one is close. The unknown name becomes the invalid type and keeps its text for display. Checks that meet a type with an invalid part stay silent: the size check, array literal typing, and ownership tracking, which also stops the module-binding error after an unknown constant value. A type query treats a type whose only fault is an unknown name as showable. Reject type parameters on functions with one message; the language does not have generic functions yet. The entry-point test no longer covers that case because it is rejected earlier. Qualified names that do not resolve keep their placeholder. --- internal/pipeline/pipeline_test.go | 1 - internal/semantics/analysis/ownership_expr.go | 4 +- internal/semantics/binder/binder.go | 5 ++ internal/semantics/typechecker/check_expr.go | 2 +- internal/semantics/typechecker/check_stmt.go | 5 ++ .../semantics/typechecker/typechecker_test.go | 81 ++++++++++++++++++- internal/semantics/typeinfo/relations.go | 10 +++ internal/semantics/typeinfo/syntax.go | 4 +- internal/semantics/typeinfo/types.go | 14 +++- internal/semantics/typeresolution/syntax.go | 36 ++++++++- .../peeper.toml | 2 +- .../negative_unknown_const_value/peeper.toml | 8 ++ .../src/main.peep | 5 ++ x_test/negative_unknown_names/peeper.toml | 8 ++ x_test/negative_unknown_names/src/main.peep | 14 ++++ x_test/negative_unknown_type_name/peeper.toml | 8 ++ .../negative_unknown_type_name/src/main.peep | 43 ++++++++++ 17 files changed, 235 insertions(+), 15 deletions(-) create mode 100644 x_test/negative_unknown_const_value/peeper.toml create mode 100644 x_test/negative_unknown_const_value/src/main.peep create mode 100644 x_test/negative_unknown_names/peeper.toml create mode 100644 x_test/negative_unknown_names/src/main.peep create mode 100644 x_test/negative_unknown_type_name/peeper.toml create mode 100644 x_test/negative_unknown_type_name/src/main.peep diff --git a/internal/pipeline/pipeline_test.go b/internal/pipeline/pipeline_test.go index f0cd64e8..c57d1374 100644 --- a/internal/pipeline/pipeline_test.go +++ b/internal/pipeline/pipeline_test.go @@ -446,7 +446,6 @@ func TestPipelineRequiresBuildEntrypoint(t *testing.T) { fn main() -> ExitCode { return 0; }`}, {name: "extern", src: `#[extern] fn main();`}, - {name: "generic", src: `fn main() {}`}, {name: "method", src: `struct App {} fn (self: App) main() {}`}, } diff --git a/internal/semantics/analysis/ownership_expr.go b/internal/semantics/analysis/ownership_expr.go index 6df943bd..e3e38c1d 100644 --- a/internal/semantics/analysis/ownership_expr.go +++ b/internal/semantics/analysis/ownership_expr.go @@ -186,7 +186,9 @@ func ownershipTrackedSymbol(sym *symbols.Symbol) bool { } func ownershipTrackedType(t typeinfo.Type) bool { - if t == nil || typeinfo.OwnershipCapabilityOf(t).Copy == typeinfo.CopyImplicit { + // A type with an invalid part was already reported; anything ownership + // says about its values would only repeat that. + if t == nil || typeinfo.ContainsInvalid(t) || typeinfo.OwnershipCapabilityOf(t).Copy == typeinfo.CopyImplicit { return false } return true diff --git a/internal/semantics/binder/binder.go b/internal/semantics/binder/binder.go index a707baeb..15b6600e 100644 --- a/internal/semantics/binder/binder.go +++ b/internal/semantics/binder/binder.go @@ -1,6 +1,7 @@ package binder import ( + "compiler/internal/diagnostics" "compiler/internal/frontend/ast" "compiler/internal/graph" "compiler/internal/module" @@ -60,6 +61,10 @@ func (b *binder) bindFunctionDecl(fn *ast.FnDecl) { if b == nil || b.module == nil || fn == nil || fn.Name == nil { return } + if len(fn.TypeParams) > 0 { + b.ctx.Diagnostics.AddError(diagnostics.ErrInvalidDeclaration, + "functions cannot take type parameters yet", fn.TypeParams[0].Location, "") + } fnType := b.ctx.TypeResolver.ResolveFunction(b.ctx.Diagnostics, b.module, fn, typeresolution.Context{}) sym := b.module.SymbolIndex.Symbol(fn.Name) if fn.Receiver != nil { diff --git a/internal/semantics/typechecker/check_expr.go b/internal/semantics/typechecker/check_expr.go index 7ffa6bf6..43358340 100644 --- a/internal/semantics/typechecker/check_expr.go +++ b/internal/semantics/typechecker/check_expr.go @@ -954,7 +954,7 @@ func (c *checker) typeArrayLit(scope *symbols.Scope, node *ast.ArrayLit) typeinf return &typeinfo.InvalidType{} } arrayType := c.ctx.TypeResolver.Resolve(c.ctx.Diagnostics, c.module, node.Type, typeresolution.Context{}) - if typeinfo.IsInvalidOrUnknown(arrayType) { + if typeinfo.IsInvalidOrUnknown(arrayType) || typeinfo.ContainsInvalid(arrayType) { return &typeinfo.InvalidType{} } array, ok := typeinfo.Underlying(arrayType).(*typeinfo.ArrayType) diff --git a/internal/semantics/typechecker/check_stmt.go b/internal/semantics/typechecker/check_stmt.go index 239d7260..693bcf63 100644 --- a/internal/semantics/typechecker/check_stmt.go +++ b/internal/semantics/typechecker/check_stmt.go @@ -810,6 +810,11 @@ func (c *checker) rejectUnsizedType(typ typeinfo.Type, site ast.Node, context st if typeinfo.IsSizedType(typ) { return false } + // A type with an invalid part is rejected by the error already reported + // for that part; saying it has no size would only repeat it. + if typeinfo.ContainsInvalid(typ) { + return true + } diagnostic := invalidTypeError(site, fmt.Sprintf("%s requires a sized type; %s is unsized", context, typeinfo.TypeText(typ))) if _, ok := typeinfo.Underlying(typ).(*typeinfo.InterfaceType); ok { diff --git a/internal/semantics/typechecker/typechecker_test.go b/internal/semantics/typechecker/typechecker_test.go index 45ea193c..479ba717 100644 --- a/internal/semantics/typechecker/typechecker_test.go +++ b/internal/semantics/typechecker/typechecker_test.go @@ -2299,13 +2299,12 @@ func TestDynamicArrayLiteralRejectsReferenceElementWithoutBinding(t *testing.T) } } -func TestDynamicArrayLiteralRejectsNonLowerableElement(t *testing.T) { +func TestDynamicArrayLiteralReportsUnknownElementTypeOnce(t *testing.T) { diag := checkTypeSource(t, `fn main() { []void{}; }`) - if !hasTypeCode(diag, diagnostics.ErrInvalidType) || - !strings.Contains(diag.EmitAllToString(), "dynamic array element type is not lowerable") { - t.Fatalf("expected non-lowerable element diagnostic, got:\n%s", diag.EmitAllToString()) + if len(diag.Diagnostics()) != 1 || !strings.Contains(diag.EmitAllToString(), "unknown type `void`") { + t.Fatalf("expected only the unknown element type diagnostic, got:\n%s", diag.EmitAllToString()) } } @@ -3669,3 +3668,77 @@ func TestRuntimeStringConstructionAndConcatenationRejectUnsupportedForms(t *test }) } } + +func TestUnknownTypeNameIsReportedWhereverItIsWritten(t *testing.T) { + for name, source := range map[string]string{ + "local": "fn main() { let a: Nope; }", + "parameter": "fn f(a: Nope) {}", + "return": "fn f() -> Nope {}", + "struct field": "struct S { a: Nope }", + "enum payload": "enum E { A: { v: Nope }, B }", + "alias": "type A = Nope;", + "optional": "fn main() { let a: ?Nope = none; }", + "array element": "fn main() { let a: [2]Nope; }", + "reference target": "fn f(a: &Nope) {}", + "type argument": "struct Box { v: T } fn main() { let a: Box; }", + "applied name": "fn main() { let a: Nope; }", + } { + diag := checkTypeSource(t, source) + if !hasTypeCode(diag, diagnostics.ErrUndefinedSymbol) || + !strings.Contains(diag.EmitAllToString(), "unknown type `Nope`") { + t.Errorf("%s: expected unknown type diagnostic, got:\n%s", name, diag.EmitAllToString()) + } + } +} + +func TestDeclaredTypeNamesAreNotReportedAsUnknown(t *testing.T) { + diag := checkTypeSource(t, `fn Read(a: Later) -> i32 { return a.v; } +struct Later { v: i32 } +struct Node { next: ?*Node, v: i32 } +struct Box { value: T } +fn Keep(value: T) -> T { return value; } +fn main() { + let a: Box = .{ value = 1 }; + let b: usize = 1; + let c: str = "x"; +}`) + if strings.Contains(diag.EmitAllToString(), "unknown type") { + t.Fatalf("declared, generic and built-in names must resolve, got:\n%s", diag.EmitAllToString()) + } +} + +func TestUnknownTypeNameIsReportedOnceAndSilencesLaterChecks(t *testing.T) { + for name, source := range map[string]string{ + "value used later": "fn main() { let a: Nope = 1; let b = a + 1; }", + "struct with the field": "struct S { a: Nope, b: i32 } fn main() { let s: S = .{ a = 1, b = 2 }; let t = s; let u = s; }", + "module constant": "const c: Nope = 1;", + "parameter": "fn f(a: Nope) -> i32 { return 0; }", + "cast target": "fn main() { let a = 1 as Nope; }", + } { + diag := checkTypeSource(t, source) + items := diag.Diagnostics() + if len(items) != 1 || items[0].Message != "unknown type `Nope`" { + t.Errorf("%s: expected only the unknown type diagnostic, got:\n%s", name, diag.EmitAllToString()) + } + } +} + +func TestUnknownTypeNameSuggestsADeclaredType(t *testing.T) { + diag := checkTypeSource(t, "struct Point { x: i32 } fn f(p: Pont) {}") + if !strings.Contains(diag.EmitAllToString(), "unknown type `Pont`") || + !strings.Contains(diag.EmitAllToString(), "did you mean `Point`?") { + t.Fatalf("expected a suggestion for the misspelled type, got:\n%s", diag.EmitAllToString()) + } + diag = checkTypeSource(t, "struct Point { x: i32 } fn f(p: Zzzzzz) {}") + if strings.Contains(diag.EmitAllToString(), "did you mean") { + t.Fatalf("an unrelated name must get no suggestion, got:\n%s", diag.EmitAllToString()) + } +} + +func TestFunctionTypeParametersAreRejectedOnce(t *testing.T) { + diag := checkTypeSource(t, "fn keep(value: T) -> T { return value; }") + items := diag.Diagnostics() + if len(items) != 1 || items[0].Message != "functions cannot take type parameters yet" { + t.Fatalf("expected only the type parameter diagnostic, got:\n%s", diag.EmitAllToString()) + } +} diff --git a/internal/semantics/typeinfo/relations.go b/internal/semantics/typeinfo/relations.go index dc625440..f25b70a9 100644 --- a/internal/semantics/typeinfo/relations.go +++ b/internal/semantics/typeinfo/relations.go @@ -179,6 +179,16 @@ func ContainsInvalid(t Type) bool { }) } +// ContainsMalformed reports whether t has an invalid part other than an +// unknown type name. A type whose only fault is such a name can still be +// shown as written. +func ContainsMalformed(t Type) bool { + return containsType(t, typeTraversal{followDefined: true, followCallable: true}, func(candidate Type, _ bool) bool { + invalid, ok := candidate.(*InvalidType) + return ok && invalid.Name == "" + }) +} + func ContainsReference(t Type) bool { return containsType(t, typeTraversal{followDefined: true}, func(candidate Type, _ bool) bool { _, ok := candidate.(*RefType) diff --git a/internal/semantics/typeinfo/syntax.go b/internal/semantics/typeinfo/syntax.go index fc596c05..14ceb21a 100644 --- a/internal/semantics/typeinfo/syntax.go +++ b/internal/semantics/typeinfo/syntax.go @@ -22,6 +22,7 @@ const ( SyntaxInvalidArrayLength SyntaxInvalidApplication SyntaxAnonymousInterface + SyntaxUnknownName ) type SyntaxIssue struct { @@ -304,7 +305,8 @@ func resolveTypeName(node ast.TypeExpr, context SyntaxContext) Type { if signed, bits, ok := token.ParseIntegerBuiltin(name, context.Target); ok { return &IntegerType{IsSigned: signed, Bits: bits} } - return &NamedType{Name: name} + context.recordIssue(SyntaxUnknownName, node, name, 0, 0) + return &InvalidType{Name: name} } func applyTypeArguments(node ast.TypeExpr, base Type, arguments []Type, context SyntaxContext) Type { diff --git a/internal/semantics/typeinfo/types.go b/internal/semantics/typeinfo/types.go index 317ff32d..8e57ac04 100644 --- a/internal/semantics/typeinfo/types.go +++ b/internal/semantics/typeinfo/types.go @@ -18,7 +18,12 @@ type Type interface { VisitRuntimeType(RuntimeTypeVisitor) } -type InvalidType struct{} +// InvalidType stands for a type whose error was already reported, so checks +// that meet it stay silent. Name is the type name as written when the error +// was an unknown name; it is kept only so the type can still be shown. +type InvalidType struct { + Name string +} type UnknownType struct{} @@ -211,7 +216,12 @@ type EnumType struct { Cases []VariantCase } -func (*InvalidType) Text() string { return "" } +func (t *InvalidType) Text() string { + if t.Name != "" { + return t.Name + } + return "" +} func (*UnknownType) Text() string { return "" } func (t *IntegerType) Text() string { diff --git a/internal/semantics/typeresolution/syntax.go b/internal/semantics/typeresolution/syntax.go index 2645ef67..61987f99 100644 --- a/internal/semantics/typeresolution/syntax.go +++ b/internal/semantics/typeresolution/syntax.go @@ -2,6 +2,7 @@ package typeresolution import ( "fmt" + "slices" "compiler/internal/diagnostics" "compiler/internal/frontend/ast" @@ -55,7 +56,7 @@ func (r *Resolver) Query(mod *module.Module, node ast.TypeExpr, context Context) result := QueryResult{Status: QueryAvailable} syntax := syntaxResolver{resolver: r, module: mod, query: &result} result.Type = typeinfo.TypeFromSyntax(node, syntax.context(context, r.compilerTarget(), nil)) - if typeinfo.ContainsInvalid(result.Type) { + if typeinfo.ContainsMalformed(result.Type) { result.Status = QueryInvalid } if result.Status == QueryLoading { @@ -72,7 +73,14 @@ func (r *Resolver) ResolveFunction(diag *diagnostics.DiagnosticBag, mod *module. syntax := syntaxResolver{resolver: r, module: mod, diag: diag} compilerTarget := r.compilerTarget() fnType := typeinfo.FuncTypeFromDecl(fn, syntax.context(context, compilerTarget, &issues)) - reportSyntaxIssues(diag, compilerTarget.IndexBits, issues) + // A function's own type parameters are not bound to types yet. Their + // uses keep the name placeholder, which the signature checks reject. + issues = slices.DeleteFunc(issues, func(issue typeinfo.SyntaxIssue) bool { + return issue.Kind == typeinfo.SyntaxUnknownName && slices.ContainsFunc(fn.TypeParams, func(param ast.TypeParam) bool { + return param.Name != nil && param.Name.Name == issue.Name + }) + }) + reportSyntaxIssues(diag, mod, compilerTarget.IndexBits, issues) return fnType } @@ -81,7 +89,7 @@ func (r *Resolver) resolve(diag *diagnostics.DiagnosticBag, mod *module.Module, issues := make([]typeinfo.SyntaxIssue, 0, 1) compilerTarget := r.compilerTarget() typ := typeinfo.TypeFromSyntax(node, syntax.context(context, compilerTarget, &issues)) - reportSyntaxIssues(diag, compilerTarget.IndexBits, issues) + reportSyntaxIssues(diag, mod, compilerTarget.IndexBits, issues) return typ } @@ -92,7 +100,7 @@ func (r *Resolver) compilerTarget() target.Info { return target.Host() } -func reportSyntaxIssues(diag *diagnostics.DiagnosticBag, indexBits int, issues []typeinfo.SyntaxIssue) { +func reportSyntaxIssues(diag *diagnostics.DiagnosticBag, mod *module.Module, indexBits int, issues []typeinfo.SyntaxIssue) { if diag == nil { return } @@ -113,6 +121,11 @@ func reportSyntaxIssues(diag *diagnostics.DiagnosticBag, indexBits int, issues [ diag.AddError(diagnostics.ErrInvalidType, fmt.Sprintf("type `%s` expects %d type %s, got %d", issue.Name, issue.Want, word, issue.Got), ast.LocOf(issue.Node), "use exact explicit type arguments") + case typeinfo.SyntaxUnknownName: + unknown := diag.AddError(diagnostics.ErrUndefinedSymbol, "unknown type `"+issue.Name+"`", ast.LocOf(issue.Node), "") + if match, ok := diagnostics.NearestName(issue.Name, declaredTypeNames(mod)); ok { + unknown.Help("did you mean `" + match + "`?") + } case typeinfo.SyntaxAnonymousInterface: diag.AddError(diagnostics.ErrInvalidType, "anonymous interface types are not supported yet", ast.LocOf(issue.Node), @@ -121,6 +134,21 @@ func reportSyntaxIssues(diag *diagnostics.DiagnosticBag, indexBits int, issues [ } } +// declaredTypeNames lists the type names an unqualified name in mod can +// resolve to, for suggesting the one a misspelled name was meant to be. +func declaredTypeNames(mod *module.Module) []string { + if mod == nil || mod.ModuleScope == nil { + return nil + } + var names []string + for _, sym := range mod.ModuleScope.Symbols() { + if sym != nil && sym.Kind == symbols.SymbolType { + names = append(names, sym.Name) + } + } + return names +} + func (r syntaxResolver) context(context Context, compilerTarget target.Info, issues *[]typeinfo.SyntaxIssue) typeinfo.SyntaxContext { return typeinfo.SyntaxContext{ Target: compilerTarget, diff --git a/x_test/negative_struct_literal_value_prefix/peeper.toml b/x_test/negative_struct_literal_value_prefix/peeper.toml index b35b6d55..10dc17ba 100644 --- a/x_test/negative_struct_literal_value_prefix/peeper.toml +++ b/x_test/negative_struct_literal_value_prefix/peeper.toml @@ -4,5 +4,5 @@ build = "program" [test] mode = "check" outcome = "failure" -stderr_contains = ["composite literal type must be struct"] +stderr_contains = ["unknown type `point`"] stderr_excludes = ["panic:"] diff --git a/x_test/negative_unknown_const_value/peeper.toml b/x_test/negative_unknown_const_value/peeper.toml new file mode 100644 index 00000000..e374e49e --- /dev/null +++ b/x_test/negative_unknown_const_value/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_unknown_const_value" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = ["unknown identifier `nothing`", "Compilation failed with 1 error(s)"] +stderr_excludes = ["ownership-tracked module bindings are not supported"] diff --git a/x_test/negative_unknown_const_value/src/main.peep b/x_test/negative_unknown_const_value/src/main.peep new file mode 100644 index 00000000..d56c2420 --- /dev/null +++ b/x_test/negative_unknown_const_value/src/main.peep @@ -0,0 +1,5 @@ +const limit = nothing; + +fn main() -> i32 { + return 0; +} diff --git a/x_test/negative_unknown_names/peeper.toml b/x_test/negative_unknown_names/peeper.toml new file mode 100644 index 00000000..3a4c0b37 --- /dev/null +++ b/x_test/negative_unknown_names/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_unknown_names" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = ["unknown identifier `missing_value`", "unknown identifier `MissingFn`", "unknown member `z`", "unknown method `total`", "unknown variant `C` in enum `Kind`", "unknown struct literal field `w`", "unknown member `q`", "Compilation failed with 7 error(s)"] +stderr_excludes = ["\n\n\n", "panic:"] diff --git a/x_test/negative_unknown_names/src/main.peep b/x_test/negative_unknown_names/src/main.peep new file mode 100644 index 00000000..cd900b17 --- /dev/null +++ b/x_test/negative_unknown_names/src/main.peep @@ -0,0 +1,14 @@ +struct Point { x: i32, y: i32 } +enum Kind { A, B } +fn (self: &Point) sum() -> i32 { return self.x + self.y; } +fn main() -> i32 { + let p: Point = .{ x = 1, y = 2 }; + let a = missing_value; + let b = MissingFn(1); + let c = p.z; + let d = p.total(); + let e = Kind::C; + let f: Point = .{ x = 1, y = 2, w = 3 }; + p.q = 4; + return 0; +} diff --git a/x_test/negative_unknown_type_name/peeper.toml b/x_test/negative_unknown_type_name/peeper.toml new file mode 100644 index 00000000..bdbbba4b --- /dev/null +++ b/x_test/negative_unknown_type_name/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_unknown_type_name" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = ["unknown type `Missing`", "7 | inner: Missing,\n | ^^^^^^^\n", "unknown type `Absent`", "unknown type `Pont`", " = help: did you mean `Point`?", "functions cannot take type parameters yet", "unknown type `Nope`", "unknown type `Gone`", "unknown type `Lost`", "Compilation failed with 7 error(s)"] +stderr_excludes = ["unknown type `Later`", "unknown type `T`", "is unsized", "not lowerable", "cannot be used as", "ownership-tracked", "used after move", "panic:"] diff --git a/x_test/negative_unknown_type_name/src/main.peep b/x_test/negative_unknown_type_name/src/main.peep new file mode 100644 index 00000000..dc9f7ec3 --- /dev/null +++ b/x_test/negative_unknown_type_name/src/main.peep @@ -0,0 +1,43 @@ +struct Point { + x: i32, + y: i32 +} + +struct Holder { + inner: Missing, + count: i32 +} + +struct Later { + value: i32 +} + +const origin: Holder = .{ inner = 1, count = 2 }; + +fn Read(source: Later) -> i32 { + return source.value; +} + +fn Take(input: Absent) -> i32 { + return 0; +} + +fn Give() -> Pont { + return .{ x = 1, y = 2 }; +} + +fn Keep(value: T) -> T { + return value; +} + +fn main() -> i32 { + let first: Nope = 1; + let next = first + 1; + let second: ?Gone = none; + let third: [2]Lost; + let holder: Holder = .{ inner = 1, count = 2 }; + let moved = holder; + let again = holder; + let known: Later = .{ value = 1 }; + return Read(known) - 1; +} From 20faec1b80767077f8234d84c8bbabfad4d78010 Mon Sep 17 00:00:00 2001 From: itsfuad Date: Fri, 9 Oct 2026 17:11:29 +0600 Subject: [PATCH 4/8] Add shift guard and operand order regression tests Cover a shift count equal to the operand width, valid narrow and odd width shift counts, the generated shift guard for each count width, and a left operand read before a call on the right changes it. --- internal/backend/llvm/shift_guard_test.go | 109 ++++++++++++++++++ .../llvm_shift_guard_exact_bound/peeper.toml | 6 + .../src/main.peep | 9 ++ .../llvm_shift_guard_narrow_valid/peeper.toml | 6 + .../src/main.peep | 32 +++++ .../peeper.toml | 7 ++ .../src/main.peep | 16 +++ 7 files changed, 185 insertions(+) create mode 100644 internal/backend/llvm/shift_guard_test.go create mode 100644 x_test/llvm_shift_guard_exact_bound/peeper.toml create mode 100644 x_test/llvm_shift_guard_exact_bound/src/main.peep create mode 100644 x_test/llvm_shift_guard_narrow_valid/peeper.toml create mode 100644 x_test/llvm_shift_guard_narrow_valid/src/main.peep create mode 100644 x_test/runtime_mir_rvalue_eval_order_binary_capture/peeper.toml create mode 100644 x_test/runtime_mir_rvalue_eval_order_binary_capture/src/main.peep diff --git a/internal/backend/llvm/shift_guard_test.go b/internal/backend/llvm/shift_guard_test.go new file mode 100644 index 00000000..95321d3f --- /dev/null +++ b/internal/backend/llvm/shift_guard_test.go @@ -0,0 +1,109 @@ +package llvm + +import ( + "fmt" + "strings" + "testing" + + "compiler/internal/diagnostics" + "compiler/internal/ir" + "compiler/internal/ir/mir" +) + +func TestGenerateLLVMIRShiftGuardWidths(t *testing.T) { + for _, target := range []struct { + name string + bits int + }{ + {name: "amd64", bits: 64}, + {name: "386", bits: 32}, + } { + for _, tt := range []struct { + name string + leftBits int + countBits int + leftSigned bool + countSigned bool + }{ + {name: "narrow unsigned", leftBits: 32, countBits: 5}, + {name: "narrow signed", leftBits: 32, countBits: 5, countSigned: true}, + {name: "signed operand", leftBits: 32, countBits: 5, leftSigned: true}, + {name: "one bit", leftBits: 1, countBits: 1}, + {name: "two bits signed count", leftBits: 2, countBits: 1, countSigned: true}, + {name: "odd width", leftBits: 3, countBits: 2}, + {name: "wide operand", leftBits: 128, countBits: 5}, + {name: "wide unsigned count", leftBits: 8, countBits: 64}, + {name: "wide signed count", leftBits: 8, countBits: 64, countSigned: true}, + {name: "equal width signed count", leftBits: 32, countBits: 32, countSigned: true}, + } { + for _, op := range []string{"<<", ">>"} { + t.Run(target.name+"/"+tt.name+"/"+op, func(t *testing.T) { + types := newLLVMTypeFixture(target.bits) + leftType := types.table.Intern(ir.Type{Kind: ir.TypeInteger, Bits: tt.leftBits, IsSigned: tt.leftSigned}) + countType := types.table.Intern(ir.Type{Kind: ir.TypeInteger, Bits: tt.countBits, IsSigned: tt.countSigned}) + mod := &mir.Module{ + Name: "test", Types: types.table, + Funcs: []*mir.Function{{ + Name: "apply", + Params: []ir.Param{{Name: "left", Type: leftType}, {Name: "right", Type: countType}}, + ReturnType: leftType, + Blocks: []*mir.Block{{ + ID: 0, + Instrs: []mir.Instr{&mir.Assign{Name: "result", Value: &mir.Binary{ + Op: op, Left: &mir.RefName{Name: "left", Type: leftType}, Right: &mir.RefName{Name: "right", Type: countType}, Type: leftType, + }}}, + Term: &mir.Ret{Value: &mir.RefName{Name: "result", Type: leftType}}, + }}, + }}, + } + targetInfo := testLinuxAMD64 + if target.bits == 32 { + targetInfo = testLinux386 + } + diag := diagnostics.NewDiagnosticBag() + out := GenerateLLVMIR(mod, diag, targetInfo, false) + if diag.HasErrors() || out == "" { + t.Fatalf("shift lowering failed:\n%s", out) + } + + count := "%right" + if tt.countBits < tt.leftBits { + extension := "zext" + if tt.countSigned { + extension = "sext" + } + cast := fmt.Sprintf(" = %s i%d %%right to i%d", extension, tt.countBits, tt.leftBits) + if strings.Count(out, cast) != 1 { + t.Fatalf("expected one lossless count extension %q:\n%s", cast, out) + } + before, _, _ := strings.Cut(out, cast) + count = strings.TrimSpace(before[strings.LastIndex(before, "\n")+1:]) + } + guardText := fmt.Sprintf("icmp uge i%d %s, %d", max(tt.leftBits, tt.countBits), count, tt.leftBits) + guard := strings.Index(out, guardText) + trap := strings.Index(out, "call void @llvm.trap()") + ready := strings.Index(out, "\nshift_ready_") + opcode := "shl" + if op == ">>" { + opcode = "lshr" + if tt.leftSigned { + opcode = "ashr" + } + } + shift := strings.Index(out, fmt.Sprintf(" = %s i%d %%left,", opcode, tt.leftBits)) + if guard < 0 || trap < guard || ready < trap || shift < ready { + t.Fatalf("expected guard %q before trap/ready/shift:\n%s", guardText, out) + } + if tt.countBits > tt.leftBits { + cast := strings.Index(out, fmt.Sprintf("trunc i%d %%right to i%d", tt.countBits, tt.leftBits)) + if cast < ready || shift < cast { + t.Fatalf("wide count must only truncate after guard succeeds:\n%s", out) + } + } else if !strings.Contains(out, fmt.Sprintf(" = %s i%d %%left, %s", opcode, tt.leftBits, count)) { + t.Fatalf("shift must reuse validated count:\n%s", out) + } + }) + } + } + } +} diff --git a/x_test/llvm_shift_guard_exact_bound/peeper.toml b/x_test/llvm_shift_guard_exact_bound/peeper.toml new file mode 100644 index 00000000..53555ec4 --- /dev/null +++ b/x_test/llvm_shift_guard_exact_bound/peeper.toml @@ -0,0 +1,6 @@ +name = "llvm_shift_guard_exact_bound" +build = "program" + +[test] +mode = "run" +outcome = "failure" diff --git a/x_test/llvm_shift_guard_exact_bound/src/main.peep b/x_test/llvm_shift_guard_exact_bound/src/main.peep new file mode 100644 index 00000000..a31b0927 --- /dev/null +++ b/x_test/llvm_shift_guard_exact_bound/src/main.peep @@ -0,0 +1,9 @@ +fn shift(count: u6) -> u32 { + return 1u32 << count; +} + +fn main() -> i32 { + let result = shift(32u6); + if result == 0u32 { return 0; } + return 0; +} diff --git a/x_test/llvm_shift_guard_narrow_valid/peeper.toml b/x_test/llvm_shift_guard_narrow_valid/peeper.toml new file mode 100644 index 00000000..4e49bfe2 --- /dev/null +++ b/x_test/llvm_shift_guard_narrow_valid/peeper.toml @@ -0,0 +1,6 @@ +name = "llvm_shift_guard_narrow_valid" +build = "program" + +[test] +mode = "run" +outcome = "success" diff --git a/x_test/llvm_shift_guard_narrow_valid/src/main.peep b/x_test/llvm_shift_guard_narrow_valid/src/main.peep new file mode 100644 index 00000000..fc27e883 --- /dev/null +++ b/x_test/llvm_shift_guard_narrow_valid/src/main.peep @@ -0,0 +1,32 @@ +fn left(count: u5) -> u32 { + return 1u32 << count; +} + +fn right(count: u5) -> u32 { + return 2147483648u32 >> count; +} + +fn signed_right(count: u5) -> i32 { + return -2147483648i32 >> count; +} + +fn signed_count(count: i5) -> u32 { + return 1u32 << count; +} + +fn odd_width(count: u2) -> u3 { + return 1u3 << count; +} + +fn main() -> i32 { + if left(0u5) != 1u32 { return 1; } + if left(1u5) != 2u32 { return 2; } + if left(31u5) != 2147483648u32 { return 3; } + if right(0u5) != 2147483648u32 { return 4; } + if right(31u5) != 1u32 { return 5; } + if signed_right(31u5) != -1i32 { return 6; } + if signed_count(0i5) != 1u32 { return 7; } + if signed_count(15i5) != 32768u32 { return 8; } + if odd_width(2u2) != 4u3 { return 9; } + return 0; +} diff --git a/x_test/runtime_mir_rvalue_eval_order_binary_capture/peeper.toml b/x_test/runtime_mir_rvalue_eval_order_binary_capture/peeper.toml new file mode 100644 index 00000000..d9ffb539 --- /dev/null +++ b/x_test/runtime_mir_rvalue_eval_order_binary_capture/peeper.toml @@ -0,0 +1,7 @@ +name = "runtime_mir_rvalue_eval_order_binary_capture" +build = "program" + +[test] +mode = "run" +outcome = "exit_code" +exit_code = 0 diff --git a/x_test/runtime_mir_rvalue_eval_order_binary_capture/src/main.peep b/x_test/runtime_mir_rvalue_eval_order_binary_capture/src/main.peep new file mode 100644 index 00000000..dc4c9d7a --- /dev/null +++ b/x_test/runtime_mir_rvalue_eval_order_binary_capture/src/main.peep @@ -0,0 +1,16 @@ +#[extern("memset")] +fn Memset(destination: rawptr, value: i32, size: usize) -> rawptr; + +fn Clear(value: rawptr) -> i32 { + Memset(value, 0, 4); + return 1; +} + +fn main() -> i32 { + let mut x: i32 = 10; + if x + Clear(@x) != 11 || x != 0 { return 1; } + + x = 10; + if x - Clear(@x) != 9 || x != 0 { return 2; } + return 0; +} From 0b0d7a365ca82efdb41fa5a3a55b2b2df0f59f42 Mon Sep 17 00:00:00 2001 From: itsfuad Date: Fri, 9 Oct 2026 18:12:14 +0600 Subject: [PATCH 5/8] Point at the token found and mark the open bracket Where a separator or a closing bracket is expected but inserting it is not a reliable repair, mark the token that was found and propose nothing. The old label told the reader to add the token at a place that was often wrong. When a closing bracket is missing and its bracket was opened on an earlier line, also mark where it was opened. --- internal/frontend/parser/parser.go | 37 +++++++++++++---- internal/frontend/parser/parser_test.go | 41 ++++++++++++++++++- .../peeper.toml | 8 ++++ .../src/main.peep | 23 +++++++++++ .../peeper.toml | 2 +- 5 files changed, 99 insertions(+), 12 deletions(-) create mode 100644 x_test/negative_found_token_and_open_bracket/peeper.toml create mode 100644 x_test/negative_found_token_and_open_bracket/src/main.peep diff --git a/internal/frontend/parser/parser.go b/internal/frontend/parser/parser.go index b2471172..1819d618 100644 --- a/internal/frontend/parser/parser.go +++ b/internal/frontend/parser/parser.go @@ -702,7 +702,7 @@ func (p *Parser) expectClose(openPos source.Position, kind token.Kind, name stri } prev := p.prev() loc := source.NewLocation(p.filePath, prev.End, prev.End) - p.diag.Add(p.missingTokenError(fmt.Sprintf("expected '%s'", string(kind)), kind, loc)) + p.diag.Add(p.withOpener(p.missingTokenError(fmt.Sprintf("expected '%s'", string(kind)), kind, loc), openPos, name)) if p.isMissingTokenLikely(kind, loc) { // The closer belongs right here, so the tokens after it are not part // of the bracket; skipping ahead to find one would drop them. @@ -715,6 +715,19 @@ func (p *Parser) expectClose(openPos source.Position, kind token.Kind, name stri return nil } +// withOpener marks the bracket a missing closer belongs to, when it was opened +// on a line above the one the diagnostic points at. On the same line the +// opener is in view already; further up it is the part of the mistake the +// reader cannot see. +func (p *Parser) withOpener(d *diagnostics.Diagnostic, openPos source.Position, name string) *diagnostics.Diagnostic { + if openPos.Line >= p.prev().End.Line { + return d + } + openEnd := openPos + openEnd.Column++ + return d.WithSecondaryLabel(source.NewLocation(p.filePath, openPos, openEnd), "this `"+name+"` is still open") +} + // consumeRedundant skips a run of kind, named noun in messages, and reports it // when it holds more than one token. isFirstNeeded says the run starts with a // token the grammar wants there, such as the comma after a list item, so the @@ -811,14 +824,14 @@ func parseBracedItemList[T any]( // `[3]i32{1, 2, 3;`: the statement ended where the list should // have closed. Leaving the `;` for the statement keeps the rest of // the block from being read as values. - p.diag.Add(insertionError(itemMsg, p.prev().End, p.filePath, "}", "}")) + p.diag.Add(p.withOpener(insertionError(itemMsg, p.prev().End, p.filePath, "}", "}"), lbraceStart, "{")) isClosedEarly = true break } if p.at(token.RPAREN) || p.at(token.RBRACK) { // `f(.{ x = 1 );`: the closer of a bracket around the list can // only follow the list's own `}`. - p.diag.Add(insertionError(itemMsg, p.prev().End, p.filePath, "}", "}")) + p.diag.Add(p.withOpener(insertionError(itemMsg, p.prev().End, p.filePath, "}", "}"), lbraceStart, "{")) isClosedEarly = true break } @@ -881,16 +894,22 @@ func (p *Parser) consume(kind token.Kind, msg string) *token.Token { } // missingTokenError reports a token that should follow loc. Where the guess -// is reliable the diagnostic shows the line with the token inserted; elsewhere -// it only says what is missing, because the real mistake is often a different -// token between the two pieces of code on that line. +// is reliable the diagnostic shows the line with the token inserted. Where a +// separator or closer is expected but the guess is not reliable, it points at +// the token found instead and proposes nothing: the real mistake is often a +// different token between the two pieces of code on that line. Any other kind +// has one place it can go, so the diagnostic says to add it there. func (p *Parser) missingTokenError(msg string, kind token.Kind, loc *source.Location) *diagnostics.Diagnostic { if p.isMissingTokenLikely(kind, loc) { return insertionError(msg, *loc.Start, p.filePath, string(kind), string(kind)) } - return diagnostics.NewError(msg). - WithCode(diagnostics.ErrExpectedToken). - WithPrimaryLabel(loc, fmt.Sprintf("add missing `%s` here", string(kind))) + d := diagnostics.NewError(msg).WithCode(diagnostics.ErrExpectedToken) + switch kind { + case token.SEMICOLON, token.COMMA, token.RPAREN, token.RBRACK, token.RBRACE, token.GT: + found := p.current() + return d.WithPrimaryLabel(source.NewLocation(p.filePath, found.Start, found.End), "found `"+found.Literal+"`") + } + return d.WithPrimaryLabel(loc, fmt.Sprintf("add missing `%s` here", string(kind))) } // isMissingTokenLikely reports whether inserting kind at loc is very probably diff --git a/internal/frontend/parser/parser_test.go b/internal/frontend/parser/parser_test.go index 040d9b2a..aff4c153 100644 --- a/internal/frontend/parser/parser_test.go +++ b/internal/frontend/parser/parser_test.go @@ -2908,8 +2908,8 @@ func TestMissingTokenFixIsShownOnlyWhereTheGuessIsReliable(t *testing.T) { if gotFix != test.hasFix { t.Fatalf("fix shown = %t, want %t: %#v", gotFix, test.hasFix, item) } - if !gotFix && !strings.HasPrefix(item.Labels[0].Message, "add missing `") { - t.Fatalf("expected the short hint on the label, got: %#v", item.Labels) + if !gotFix && !strings.HasPrefix(item.Labels[0].Message, "found `") { + t.Fatalf("expected the label to name the token found, got: %#v", item.Labels) } return } @@ -3192,3 +3192,40 @@ func TestBracedListRecoveryStillEndsOnTokensNoItemAccepts(t *testing.T) { } } } + +func TestUnreliableSeparatorOrCloserPointsAtTheTokenFound(t *testing.T) { + for _, test := range []struct{ name, source, message, label string }{ + {"two values in a row", "fn main() { let b = a c; }", "expected ';' after statement", "found `c`"}, + {"two calls on a line", "fn main() { f(a) g(a); }", "expected ';' after expression", "found `g`"}, + {"not after an argument", "fn main() { f(a !b); }", "expected ')'", "found `!`"}, + } { + _, diag := parseTestModule(test.source) + items := diag.Diagnostics() + if len(items) == 0 || items[0].Message != test.message || len(items[0].Extras) != 0 || + len(items[0].Labels) != 1 || items[0].Labels[0].Message != test.label { + t.Errorf("%s: expected %q marked %q with no fix, got:\n%s", test.name, test.message, test.label, diag.EmitAllToString()) + } + } +} + +func TestMissingCloserMarksAnOpenerFromAnEarlierLine(t *testing.T) { + for _, test := range []struct { + name, source string + wantOpener bool + }{ + {"call opened above", "fn main() {\n f(\n 1\n ;\n}", true}, + {"literal opened above", "fn main() {\n let a = [2]i32{\n 1, 2\n ;\n}", true}, + {"call opened on the same line", "fn main() {\n f(1;\n}", false}, + } { + _, diag := parseTestModule(test.source) + items := diag.Diagnostics() + if len(items) != 1 { + t.Errorf("%s: expected one error, got:\n%s", test.name, diag.EmitAllToString()) + continue + } + hasOpener := len(items[0].Labels) == 2 && strings.Contains(items[0].Labels[1].Message, "is still open") + if hasOpener != test.wantOpener { + t.Errorf("%s: opener marker = %v, want %v:\n%s", test.name, hasOpener, test.wantOpener, diag.EmitAllToString()) + } + } +} diff --git a/x_test/negative_found_token_and_open_bracket/peeper.toml b/x_test/negative_found_token_and_open_bracket/peeper.toml new file mode 100644 index 00000000..e4bd7274 --- /dev/null +++ b/x_test/negative_found_token_and_open_bracket/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_found_token_and_open_bracket" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = ["expected ';' after statement", "12 | let twice = count count;\n | ^^^^^ found `count`\n", "13 | if Same(ready !ready) {\n | ^ found `!`\n", "16 | let total = Add(\n | - this `(` is still open\n", " = help: add missing `)`\n | \n19 | 3)\n", " = help: add missing `)`\n | \n21 | let near = Add(1, 2, 3);\n", "Compilation failed with 4 error(s)"] +stderr_excludes = ["add missing `;` here", "add missing `)` here", "expected ','", "wrong number of arguments", "21 | let near = Add(1, 2, 3;\n | - this"] diff --git a/x_test/negative_found_token_and_open_bracket/src/main.peep b/x_test/negative_found_token_and_open_bracket/src/main.peep new file mode 100644 index 00000000..f21ea78d --- /dev/null +++ b/x_test/negative_found_token_and_open_bracket/src/main.peep @@ -0,0 +1,23 @@ +fn Add(first: i32, second: i32, third: i32) -> i32 { + return first + second + third; +} + +fn Same(value: bool) -> bool { + return value; +} + +fn main() -> i32 { + let count = 1; + let ready = true; + let twice = count count; + if Same(ready !ready) { + return 1; + } + let total = Add( + 1, + 2, + 3 + ; + let near = Add(1, 2, 3; + return total + near + twice; +} diff --git a/x_test/negative_missing_token_unreliable_guess/peeper.toml b/x_test/negative_missing_token_unreliable_guess/peeper.toml index 5e601a8c..df419179 100644 --- a/x_test/negative_missing_token_unreliable_guess/peeper.toml +++ b/x_test/negative_missing_token_unreliable_guess/peeper.toml @@ -4,5 +4,5 @@ build = "program" [test] mode = "check" outcome = "failure" -stderr_contains = ["expected ']' after array literal length", "add missing `]` here"] +stderr_contains = ["expected ']' after array literal length", "found `,`"] stderr_excludes = ["= help: add missing", "\n\n\n"] From 0fadf117423e0af0f074d86b4856e493362dcc5b Mon Sep 17 00:00:00 2001 From: itsfuad Date: Fri, 9 Oct 2026 19:05:30 +0600 Subject: [PATCH 6/8] Print each source line once when a diagnostic has several labels The printer drew every label as its own block: the line above it, the labelled lines, then the underline. A second label on the next line repeated that line as context, two labels on one line printed the line twice, and labels over the same lines printed all of them again. Lay the labels out by line instead. Each source line is printed once with a row for every label on it, the line above is shown only when it is not on screen yet, and `...` appears only where lines are really left out. The location header names the primary label instead of the first label in file order. A diagnostic with one label prints as before. --- internal/diagnostics/emitter.go | 254 ++++++++++-------- internal/diagnostics/emitter_test.go | 102 +++++++ .../peeper.toml | 4 +- .../src/main.peep | 4 +- 4 files changed, 255 insertions(+), 109 deletions(-) diff --git a/internal/diagnostics/emitter.go b/internal/diagnostics/emitter.go index 46fe4733..f3e1556b 100644 --- a/internal/diagnostics/emitter.go +++ b/internal/diagnostics/emitter.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "slices" - "sort" "strings" "sync" "unicode/utf8" @@ -220,20 +219,18 @@ func (e *Emitter) printUnderlineIndent(extraPadding int) { fmt.Fprint(e.writer, strings.Repeat(" ", indent+extraPadding)) } -func (e *Emitter) printPrevNonEmptyLine(filepath string, line int) { - if line <= 1 { - return +// contextLine returns the line above line, which is shown so the marked line +// is read in its setting. There is none when that line is blank or already on +// screen. +func (e *Emitter) contextLine(filepath string, line, lastPrinted int) (string, bool) { + if line <= 1 || line-1 <= lastPrinted { + return "", false } prevLine, err := e.cache.GetLine(filepath, line-1) - if err != nil { - return - } - if strings.TrimSpace(prevLine) == "" { - return + if err != nil || strings.TrimSpace(prevLine) == "" { + return "", false } - e.printGutter(line - 1) - e.highlighter.HighlightWithColor(expandTabs(prevLine), e.writer) - fmt.Fprintln(e.writer) + return prevLine, true } func (e *Emitter) printLocationHeader(filepath string, line int, col int) { @@ -291,68 +288,33 @@ func (e *Emitter) Emit(diag *Diagnostic) { // Step 1: Print Main Diagnostic Header Block e.printDiagnosticHeader(diag) - var lastFile string - var lastLine int - - // Step 2: Print Sequential Location Cards - if len(diag.Labels) > 0 { - // Clone and sort labels by line number so we read the file top-to-bottom - sortedLabels := make([]Label, len(diag.Labels)) - copy(sortedLabels, diag.Labels) - sort.SliceStable(sortedLabels, func(i, j int) bool { - locI, locJ := sortedLabels[i].Location, sortedLabels[j].Location - if locI == nil || locJ == nil || locI.Start == nil || locJ.Start == nil { - return false - } - fileI, fileJ := "", "" - if locI.Filename != nil { - fileI = *locI.Filename - } - if locJ.Filename != nil { - fileJ = *locJ.Filename - } - - if fileI != fileJ { - return fileI < fileJ - } - return locI.Start.Line < locJ.Start.Line - }) - - for _, label := range sortedLabels { - if label.Location == nil || label.Location.Filename == nil { - continue - } - filepath := *label.Location.Filename - if filepath == "" { - filepath = diag.FilePath + // Step 2: Print the marked source lines, file by file + if files := snippetFiles(diag); len(files) > 0 { + for index, file := range files { + if index > 0 { + fmt.Fprintln(e.writer) } - - line := label.Location.Start.Line - col := label.Location.Start.Column - - if filepath != lastFile { - // CASE 1: Different file -> Print full file path header - if lastFile != "" { + e.printLocationHeader(file.path, file.headerLine, file.headerColumn) + e.printPipeOnly() + // Each source line is printed once, and `...` stands for lines + // really left out between two printed ones. + lastPrinted := 0 + for _, line := range file.lines { + context, hasContext := e.contextLine(file.path, line.number, lastPrinted) + firstShown := line.number + if hasContext { + firstShown-- + } + if lastPrinted > 0 && firstShown > lastPrinted+1 { + e.logger.Fprintf(e.writer, colors.GREY, "%*s\n", e.currentLineNumWidth, "...") + } + if hasContext { + e.printGutter(line.number - 1) + e.highlighter.HighlightWithColor(expandTabs(context), e.writer) fmt.Fprintln(e.writer) } - e.printLocationHeader(filepath, line, col) - e.printBlankGutter() - fmt.Fprintln(e.writer) - - lastFile = filepath - } else if line > lastLine+1 { - // CASE 2: Same file, skip in lines -> Print aligned '...' without the pipe - // This aligns the dots perfectly with where the line numbers sit - e.logger.Fprintf(e.writer, colors.GREY, "%*s\n", e.currentLineNumWidth, "...") - } - - // Clean context code block with customizable tilde/caret markings - e.printPeeperSnippetBlock(filepath, label, diag.Severity) - - // Track the end line of this snippet to know where we left off - lastLine = line - if label.Location.End != nil { - lastLine = label.Location.End.Line + e.printSnippetLine(file.path, line, diag.Severity) + lastPrinted = line.number } } // Keep the margin unbroken when help or notes follow the snippet. @@ -379,59 +341,139 @@ func (e *Emitter) Emit(diag *Diagnostic) { fmt.Fprintln(e.writer) } -func (e *Emitter) printPeeperSnippetBlock(filepath string, label Label, severity Severity) { - startLine := label.Location.Start.Line - endLine := startLine - if label.Location.End != nil { - endLine = label.Location.End.Line - } - - e.printPrevNonEmptyLine(filepath, startLine) +// snippetMark is the stretch of one source line that a label covers. +type snippetMark struct { + startColumn int + endColumn int // 0 when the label runs on past the end of the line + style LabelStyle + message string +} - for l := startLine; l <= endLine; l++ { - sourceLine, err := e.cache.GetLine(filepath, l) - if err != nil { +type snippetLine struct { + number int + marks []snippetMark +} + +// snippetFile holds the marked lines of one file in the order they are read, +// and the position its `-->` header names. +type snippetFile struct { + path string + headerLine int + headerColumn int + lines []snippetLine +} + +// snippetFiles arranges the labels of diag for printing: the file of the +// primary label first, then the others by name; within a file the lines in +// order, and on a line the marks from left to right with the primary one +// ahead of a secondary one that starts at the same column. The header of a +// file names its primary label, or its first mark when it has none. +func snippetFiles(diag *Diagnostic) []snippetFile { + var files []snippetFile + primaryPath := "" + for _, label := range diag.Labels { + if label.Location == nil || label.Location.Filename == nil || label.Location.Start == nil { continue } + path := *label.Location.Filename + if path == "" { + path = diag.FilePath + } + fileIndex := slices.IndexFunc(files, func(file snippetFile) bool { return file.path == path }) + if fileIndex < 0 { + fileIndex = len(files) + files = append(files, snippetFile{path: path}) + } + file := &files[fileIndex] + start, end := label.Location.Start, label.Location.End + if end == nil { + end = start + } + if label.Style == Primary && primaryPath == "" { + primaryPath = path + file.headerLine, file.headerColumn = start.Line, start.Column + } + for number := start.Line; number <= end.Line; number++ { + mark := snippetMark{startColumn: 1, style: label.Style} + if number == start.Line { + mark.startColumn = start.Column + } + if number == end.Line { + mark.message = label.Message + if label.Location.End != nil { + mark.endColumn = end.Column + } + } + lineIndex := slices.IndexFunc(file.lines, func(line snippetLine) bool { return line.number == number }) + if lineIndex < 0 { + lineIndex = len(file.lines) + file.lines = append(file.lines, snippetLine{number: number}) + } + file.lines[lineIndex].marks = append(file.lines[lineIndex].marks, mark) + } + } + for index := range files { + file := &files[index] + slices.SortFunc(file.lines, func(a, b snippetLine) int { return a.number - b.number }) + for _, line := range file.lines { + slices.SortStableFunc(line.marks, func(a, b snippetMark) int { + if a.startColumn != b.startColumn { + return a.startColumn - b.startColumn + } + return int(a.style) - int(b.style) + }) + } + if file.headerLine == 0 { + file.headerLine, file.headerColumn = file.lines[0].number, file.lines[0].marks[0].startColumn + } + } + slices.SortStableFunc(files, func(a, b snippetFile) int { + switch { + case a.path == primaryPath: + return -1 + case b.path == primaryPath: + return 1 + } + return strings.Compare(a.path, b.path) + }) + return files +} - e.printCurrentGutter(l) - e.highlighter.HighlightWithColor(expandTabs(sourceLine), e.writer) - fmt.Fprintln(e.writer) +// printSnippetLine prints one source line and, beneath it, a row for each +// label that covers part of it. +func (e *Emitter) printSnippetLine(filepath string, line snippetLine, severity Severity) { + sourceLine, err := e.cache.GetLine(filepath, line.number) + if err != nil { + return + } + e.printCurrentGutter(line.number) + e.highlighter.HighlightWithColor(expandTabs(sourceLine), e.writer) + fmt.Fprintln(e.writer) + for _, mark := range line.marks { e.printBlankGutter() - startCol := 1 - if l == startLine { - startCol = label.Location.Start.Column - } - - endCol := len(sourceLine) + 1 - if l == endLine && label.Location.End != nil { - endCol = label.Location.End.Column + endColumn := mark.endColumn + if endColumn == 0 { + endColumn = len(sourceLine) + 1 } - - padding := visualColumnToPosition(sourceLine, startCol) - length := visualColumnToPosition(sourceLine, endCol) - padding + padding := visualColumnToPosition(sourceLine, mark.startColumn) + length := visualColumnToPosition(sourceLine, endColumn) - padding if length <= 0 { length = 1 } - var underlineColor colors.COLOR - var underlineChar string - - if label.Style == Primary { + underlineColor := colors.BLUE + underlineChar := "-" // Soft line for secondary context + if mark.style == Primary { underlineColor = e.getSeverityColor(severity) underlineChar = "^" // Sharp pointer for the main error - } else { - underlineColor = colors.BLUE - underlineChar = "-" // Soft line for secondary context } fmt.Fprint(e.writer, strings.Repeat(" ", padding)) e.logger.Fprint(e.writer, underlineColor, strings.Repeat(underlineChar, length)) - - if l == endLine && label.Message != "" { - e.logger.Fprintf(e.writer, underlineColor, " %s", label.Message) + if mark.message != "" { + e.logger.Fprintf(e.writer, underlineColor, " %s", mark.message) } fmt.Fprintln(e.writer) } diff --git a/internal/diagnostics/emitter_test.go b/internal/diagnostics/emitter_test.go index 808b8931..85308938 100644 --- a/internal/diagnostics/emitter_test.go +++ b/internal/diagnostics/emitter_test.go @@ -5,6 +5,8 @@ import ( "strings" "testing" + "compiler/internal/source" + "compiler/pkg/colors" "compiler/pkg/peeper" ) @@ -34,3 +36,103 @@ func TestEmitterHandlesMultiplePrimaryLabelsWithoutPanic(t *testing.T) { t.Fatalf("expected second label to remain visible in output, got:\n%s", text) } } + +// emitPlain prints diag over the given sources with colours off. +func emitPlain(t *testing.T, sources map[string]string, diag *Diagnostic) string { + t.Helper() + prevFormat := colors.CurrentLogFormat() + colors.SetLogFormat(colors.LogFormatNormal) + defer colors.SetLogFormat(prevFormat) + + var out bytes.Buffer + emitter := NewEmitter(&out) + for path, text := range sources { + emitter.cache.AddSource(path, text) + } + emitter.Emit(diag) + return out.String() +} + +func spanLoc(file string, startLine, startCol, endLine, endCol int) *source.Location { + return source.NewLocation(file, source.Position{Line: startLine, Column: startCol}, source.Position{Line: endLine, Column: endCol}) +} + +const snippetSource = "one\ntwo\nthree\nfour\nfive\nsix\nseven\neight" + +func TestEmitterPrintsEachSourceLineOnce(t *testing.T) { + path := "main" + peeper.SourceExt + for _, test := range []struct { + name string + primary *source.Location + secondary *source.Location + want string + }{ + { + name: "labels on neighbouring lines", + primary: spanLoc(path, 4, 1, 4, 5), + secondary: spanLoc(path, 3, 1, 3, 6), + want: "[X]: bad\n --> " + path + ":4:1\n | \n" + + "2 | two\n3 | three\n | ----- context\n4 | four\n | ^^^^ here\n\n", + }, + { + name: "two labels on one line in column order", + primary: spanLoc(path, 3, 4, 3, 6), + secondary: spanLoc(path, 3, 1, 3, 3), + want: "[X]: bad\n --> " + path + ":3:4\n | \n" + + "2 | two\n3 | three\n | -- context\n | ^^ here\n\n", + }, + { + name: "labels over the same lines", + primary: spanLoc(path, 3, 1, 4, 5), + secondary: spanLoc(path, 3, 1, 4, 5), + want: "[X]: bad\n --> " + path + ":3:1\n | \n" + + "2 | two\n3 | three\n | ^^^^^\n | -----\n4 | four\n | ^^^^ here\n | ---- context\n\n", + }, + { + name: "secondary label above the primary keeps the primary in the header", + primary: spanLoc(path, 7, 1, 7, 6), + secondary: spanLoc(path, 2, 1, 2, 4), + want: "[X]: bad\n --> " + path + ":7:1\n | \n" + + "1 | one\n2 | two\n | --- context\n...\n6 | six\n7 | seven\n | ^^^^^ here\n\n", + }, + { + name: "one line between two labels is context, not a gap", + primary: spanLoc(path, 5, 1, 5, 5), + secondary: spanLoc(path, 3, 1, 3, 6), + want: "[X]: bad\n --> " + path + ":5:1\n | \n" + + "2 | two\n3 | three\n | ----- context\n4 | four\n5 | five\n | ^^^^ here\n\n", + }, + } { + t.Run(test.name, func(t *testing.T) { + diag := NewError("bad").WithCode("X"). + WithPrimaryLabel(test.primary, "here"). + WithSecondaryLabel(test.secondary, "context") + if got := emitPlain(t, map[string]string{path: snippetSource}, diag); got != test.want { + t.Fatalf("got:\n%s\nwant:\n%s", got, test.want) + } + }) + } +} + +func TestEmitterSingleLabelLayout(t *testing.T) { + path := "main" + peeper.SourceExt + diag := NewError("bad").WithCode("X"). + WithPrimaryLabel(spanLoc(path, 4, 1, 4, 5), "here"). + Help("fix it") + want := "[X]: bad\n --> " + path + ":4:1\n | \n3 | three\n4 | four\n | ^^^^ here\n | \n = help: fix it\n\n" + if got := emitPlain(t, map[string]string{path: snippetSource}, diag); got != want { + t.Fatalf("got:\n%s\nwant:\n%s", got, want) + } +} + +func TestEmitterGivesEachFileItsHeaderWithThePrimaryFileFirst(t *testing.T) { + first, second := "a"+peeper.SourceExt, "z"+peeper.SourceExt + diag := NewError("bad").WithCode("X"). + WithPrimaryLabel(spanLoc(second, 2, 1, 2, 4), "here"). + WithSecondaryLabel(spanLoc(first, 3, 1, 3, 6), "context") + want := "[X]: bad\n --> " + second + ":2:1\n | \n1 | one\n2 | two\n | ^^^ here\n\n" + + " --> " + first + ":3:1\n | \n2 | two\n3 | three\n | ----- context\n\n" + if got := emitPlain(t, map[string]string{first: snippetSource, second: snippetSource}, diag); got != want { + t.Fatalf("got:\n%s\nwant:\n%s", got, want) + } +} diff --git a/x_test/negative_found_token_and_open_bracket/peeper.toml b/x_test/negative_found_token_and_open_bracket/peeper.toml index e4bd7274..1583d0dd 100644 --- a/x_test/negative_found_token_and_open_bracket/peeper.toml +++ b/x_test/negative_found_token_and_open_bracket/peeper.toml @@ -4,5 +4,5 @@ build = "program" [test] mode = "check" outcome = "failure" -stderr_contains = ["expected ';' after statement", "12 | let twice = count count;\n | ^^^^^ found `count`\n", "13 | if Same(ready !ready) {\n | ^ found `!`\n", "16 | let total = Add(\n | - this `(` is still open\n", " = help: add missing `)`\n | \n19 | 3)\n", " = help: add missing `)`\n | \n21 | let near = Add(1, 2, 3);\n", "Compilation failed with 4 error(s)"] -stderr_excludes = ["add missing `;` here", "add missing `)` here", "expected ','", "wrong number of arguments", "21 | let near = Add(1, 2, 3;\n | - this"] +stderr_contains = ["expected ';' after statement", "12 | let twice = count count;\n | ^^^^^ found `count`\n", "13 | if Same(ready !ready) {\n | ^ found `!`\n", "16 | let total = Add(\n | - this `(` is still open\n", " = help: add missing `)`\n | \n19 | 3)\n", " = help: add missing `)`\n | \n21 | let near = Add(1, 2, 3);\n", " --> ", ":23:13\n", "22 | let next = Add(1,\n | - this `(` is still open\n23 | 2, 3;\n | ^\n", "Compilation failed with 5 error(s)"] +stderr_excludes = ["add missing `;` here", "add missing `)` here", "expected ','", "wrong number of arguments", "21 | let near = Add(1, 2, 3;\n | - this", "22 | let next = Add(1,\n22 | let next = Add(1,\n"] diff --git a/x_test/negative_found_token_and_open_bracket/src/main.peep b/x_test/negative_found_token_and_open_bracket/src/main.peep index f21ea78d..6b031c8f 100644 --- a/x_test/negative_found_token_and_open_bracket/src/main.peep +++ b/x_test/negative_found_token_and_open_bracket/src/main.peep @@ -19,5 +19,7 @@ fn main() -> i32 { 3 ; let near = Add(1, 2, 3; - return total + near + twice; + let next = Add(1, + 2, 3; + return total + near + next + twice; } From 1b40b20e64b88ede5ab1ddf293ecc6db8055cdd1 Mon Sep 17 00:00:00 2001 From: itsfuad Date: Fri, 9 Oct 2026 19:21:02 +0600 Subject: [PATCH 7/8] Drop carriage returns when splitting source text into lines A source file with CRLF line ends kept the `\r` on every line read for diagnostics, so printed source lines and fixed lines ended with a stray carriage return. Trim it in the two places that split source text: the diagnostic source cache and GetSourceLines. --- internal/diagnostics/emitter.go | 4 ++++ internal/diagnostics/emitter_test.go | 12 ++++++++++++ internal/source/location.go | 6 ++++-- internal/source/source_test.go | 17 +++++++++++++++++ 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/internal/diagnostics/emitter.go b/internal/diagnostics/emitter.go index f3e1556b..1892b6a3 100644 --- a/internal/diagnostics/emitter.go +++ b/internal/diagnostics/emitter.go @@ -97,6 +97,10 @@ func NewSourceCache() *SourceCache { func (sc *SourceCache) AddSource(filepath, content string) { lines := strings.Split(content, "\n") + // A file with CRLF line ends must not carry the `\r` into printed lines. + for index, line := range lines { + lines[index] = strings.TrimSuffix(line, "\r") + } sc.mu.Lock() sc.files[filepath] = lines sc.mu.Unlock() diff --git a/internal/diagnostics/emitter_test.go b/internal/diagnostics/emitter_test.go index 85308938..6a93bb5a 100644 --- a/internal/diagnostics/emitter_test.go +++ b/internal/diagnostics/emitter_test.go @@ -136,3 +136,15 @@ func TestEmitterGivesEachFileItsHeaderWithThePrimaryFileFirst(t *testing.T) { t.Fatalf("got:\n%s\nwant:\n%s", got, want) } } + +func TestEmitterDropsCarriageReturnsOfCRLFSources(t *testing.T) { + path := "main" + peeper.SourceExt + loc := spanLoc(path, 2, 4, 2, 4) + diag := NewError("bad").WithCode("X"). + WithPrimaryLabel(loc, ""). + Help("end it", Fix.Insert(loc, ";")) + want := "[X]: bad\n --> " + path + ":2:4\n | \n1 | one\n2 | two\n | ^\n | \n = help: end it\n | \n2 | two;\n\n" + if got := emitPlain(t, map[string]string{path: "one\r\ntwo\r\nthree\r\n"}, diag); got != want { + t.Fatalf("got:\n%q\nwant:\n%q", got, want) + } +} diff --git a/internal/source/location.go b/internal/source/location.go index fbd85d64..3fe24f1a 100644 --- a/internal/source/location.go +++ b/internal/source/location.go @@ -75,6 +75,8 @@ func (l *Location) GetText(cache SourceCache) string { return result.String() } +// GetSourceLines reads a file as lines without their line ends, whether the +// file uses LF or CRLF. func GetSourceLines(filepath string) ([]string, error) { content, err := os.ReadFile(filepath) if err != nil { @@ -87,12 +89,12 @@ func GetSourceLines(filepath string) ([]string, error) { start := 0 for i := range content { if content[i] == '\n' { - lines = append(lines, string(content[start:i])) + lines = append(lines, strings.TrimSuffix(string(content[start:i]), "\r")) start = i + 1 } } if start < len(content) { - lines = append(lines, string(content[start:])) + lines = append(lines, strings.TrimSuffix(string(content[start:]), "\r")) } return lines, nil } diff --git a/internal/source/source_test.go b/internal/source/source_test.go index 394c48d4..5ffc0d7d 100644 --- a/internal/source/source_test.go +++ b/internal/source/source_test.go @@ -86,3 +86,20 @@ func TestGetSourceLinesRangeUsesCache(t *testing.T) { t.Fatalf("unexpected cached lines: %#v", lines) } } + +func TestGetSourceLinesDropsLineEndsOfLFAndCRLFFiles(t *testing.T) { + for name, content := range map[string]string{ + "lf": "one\ntwo\n", + "crlf": "one\r\ntwo\r\n", + "crlf without last end": "one\r\ntwo", + } { + path := filepath.Join(t.TempDir(), "lines.txt") + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + lines, err := GetSourceLines(path) + if err != nil || len(lines) != 2 || lines[0] != "one" || lines[1] != "two" { + t.Errorf("%s: got %q, %v", name, lines, err) + } + } +} From d656a9868c526790bd81f5ed7ac5e96568fc8ed5 Mon Sep 17 00:00:00 2001 From: itsfuad Date: Fri, 9 Oct 2026 22:19:07 +0600 Subject: [PATCH 8/8] Place markers by display width and report a non-ASCII name once The printer placed a marker one column per character, so it sat too far left after East Asian wide characters and emoji, which take two terminal cells, and too far right after combining marks, which take none. Count cells for marker position, underline length and tab stops. A label that runs to the end of a line counted bytes; count characters. A name with letters outside ASCII was one "illegal character" error per character and broke the statement around it. Report the whole word once and read it as a name so parsing continues. Text outside ASCII in strings, character literals and comments stays allowed. --- internal/diagnostics/emitter.go | 79 ++++++++----------- internal/diagnostics/emitter_test.go | 34 ++++++++ internal/frontend/lexer/lexer.go | 33 ++++++++ internal/frontend/lexer/lexer_test.go | 23 ++++++ .../peeper.toml | 8 ++ .../src/main.peep | 7 ++ 6 files changed, 140 insertions(+), 44 deletions(-) create mode 100644 x_test/negative_wide_text_marker_and_name/peeper.toml create mode 100644 x_test/negative_wide_text_marker_and_name/src/main.peep diff --git a/internal/diagnostics/emitter.go b/internal/diagnostics/emitter.go index 1892b6a3..2db25b1e 100644 --- a/internal/diagnostics/emitter.go +++ b/internal/diagnostics/emitter.go @@ -7,6 +7,7 @@ import ( "slices" "strings" "sync" + "unicode" "unicode/utf8" "compiler/internal/source" @@ -21,6 +22,26 @@ const ( TAB_WIDTH = 4 ) +// cellWidth is the number of terminal cells ch takes when it is printed at +// visualPos: a tab runs to the next tab stop, a combining mark or a format +// character sits on the cell before it, and East Asian wide characters and +// emoji take two cells. Terminals differ on rarer cases, such as emoji joined +// into one picture, so a marker after those can still be a cell or two off. +func cellWidth(ch rune, visualPos int) int { + switch { + case ch == '\t': + return TAB_WIDTH - (visualPos % TAB_WIDTH) + case unicode.In(ch, unicode.Mn, unicode.Me, unicode.Cf): + return 0 + case unicode.In(ch, unicode.Han, unicode.Hiragana, unicode.Katakana, unicode.Hangul), + ch >= 0x3000 && ch <= 0x303F, // CJK symbols and punctuation + ch >= 0xFF01 && ch <= 0xFF60, ch >= 0xFFE0 && ch <= 0xFFE6, // fullwidth forms + ch >= 0x1F300 && ch <= 0x1FAFF: // emoji and pictographs + return 2 + } + return 1 +} + // expandTabs replaces tab characters with spaces to align with tab stops. // This ensures consistent visual alignment between source lines and diagnostic markers. func expandTabs(line string) string { @@ -30,59 +51,30 @@ func expandTabs(line string) string { var result strings.Builder col := 0 for _, ch := range line { + width := cellWidth(ch, col) if ch == '\t' { - spaces := TAB_WIDTH - (col % TAB_WIDTH) - result.WriteString(strings.Repeat(" ", spaces)) - col += spaces + result.WriteString(strings.Repeat(" ", width)) } else { result.WriteRune(ch) - col++ } + col += width } return result.String() } -// visualColumnToPosition converts a character column (where tabs=1) to a visual position -// in an expanded line (where tabs are expanded to TAB_WIDTH spaces). -// This is used to align carets with source code when tabs are present. +// visualColumnToPosition converts a character column to the terminal cell +// where that character is drawn, so a marker lines up with the source above +// it. A column past the end of the line counts one cell for each column. func visualColumnToPosition(line string, column int) int { - if column <= 0 { - return 0 - } - if !strings.ContainsRune(line, '\t') { - // No tabs, column is already the correct position (1-indexed to 0-indexed) - return column - 1 - } - // Iterate through characters, tracking both character column and visual position - charCol := 1 // 1-indexed character column - visualPos := 0 // 0-indexed visual position in expanded line - + visualPos, charCol := 0, 1 for _, ch := range line { if charCol >= column { - // Found the character at or past our target column - // For tabs, we need to check if the column falls within the tab expansion - if ch == '\t' { - // This tab starts at charCol and expands to visual positions - // The tab spans from charCol to charCol (it's one character) - // but visually it's from visualPos to visualPos+spaces-1 - // Since column == charCol, we return visualPos - return visualPos - } return visualPos } - - if ch == '\t' { - spaces := TAB_WIDTH - (visualPos % TAB_WIDTH) - visualPos += spaces - charCol++ - } else { - visualPos++ - charCol++ - } + visualPos += cellWidth(ch, visualPos) + charCol++ } - - // If we get here, the column is beyond the end of the line - return visualPos + return visualPos + max(column-charCol, 0) } // SourceCache caches source file contents for error reporting @@ -459,7 +451,7 @@ func (e *Emitter) printSnippetLine(filepath string, line snippetLine, severity S endColumn := mark.endColumn if endColumn == 0 { - endColumn = len(sourceLine) + 1 + endColumn = utf8.RuneCountInString(sourceLine) + 1 } padding := visualColumnToPosition(sourceLine, mark.startColumn) length := visualColumnToPosition(sourceLine, endColumn) - padding @@ -610,14 +602,13 @@ func columnOffset(sourceLine string, column int) (int, bool) { if current == column { return offset, true } + width := cellWidth(ch, visual) if ch == '\t' { - spaces := TAB_WIDTH - (visual % TAB_WIDTH) - offset += spaces - visual += spaces + offset += width } else { offset += utf8.RuneLen(ch) - visual++ } + visual += width current++ } return offset, current == column diff --git a/internal/diagnostics/emitter_test.go b/internal/diagnostics/emitter_test.go index 6a93bb5a..fbcd67f4 100644 --- a/internal/diagnostics/emitter_test.go +++ b/internal/diagnostics/emitter_test.go @@ -148,3 +148,37 @@ func TestEmitterDropsCarriageReturnsOfCRLFSources(t *testing.T) { t.Fatalf("got:\n%q\nwant:\n%q", got, want) } } + +func TestMarkerSitsUnderTheCharacterWhateverItsWidth(t *testing.T) { + path := "main" + peeper.SourceExt + for _, test := range []struct { + name, line string + startCol int + endCol int + wantMarker string + }{ + {"plain text", `a = "ab" + 1;`, 12, 13, " | ^ here"}, + {"wide characters before", `a = "日本" + 1;`, 12, 13, " | ^ here"}, + {"emoji before", `a = "😀b" + 1;`, 12, 13, " | ^ here"}, + {"combining mark before", "a = \"éb\" + 1;", 13, 14, " | ^ here"}, + {"wide characters under the marker", `a = 日本 + 1;`, 5, 7, " | ^^^^ here"}, + {"tab then wide characters", "\ta = \"日本\" + 1;", 13, 14, " | ^ here"}, + } { + t.Run(test.name, func(t *testing.T) { + diag := NewError("bad").WithCode("X").WithPrimaryLabel(spanLoc(path, 1, test.startCol, 1, test.endCol), "here") + got := emitPlain(t, map[string]string{path: test.line}, diag) + if !strings.Contains(got, "\n"+test.wantMarker+"\n") { + t.Fatalf("want marker row %q, got:\n%s", test.wantMarker, got) + } + }) + } +} + +func TestLabelToTheEndOfALineCountsCharactersNotBytes(t *testing.T) { + path := "main" + peeper.SourceExt + diag := NewError("bad").WithCode("X").WithPrimaryLabel(spanLoc(path, 1, 1, 2, 2), "here") + got := emitPlain(t, map[string]string{path: "éé\nb"}, diag) + if !strings.Contains(got, "1 | éé\n | ^^\n2 | b\n") { + t.Fatalf("expected a two-cell underline beneath the first line, got:\n%s", got) + } +} diff --git a/internal/frontend/lexer/lexer.go b/internal/frontend/lexer/lexer.go index 48b28cd3..88951d57 100644 --- a/internal/frontend/lexer/lexer.go +++ b/internal/frontend/lexer/lexer.go @@ -5,6 +5,7 @@ import ( "regexp" "strconv" "strings" + "unicode" "unicode/utf8" "compiler/internal/diagnostics" @@ -254,11 +255,43 @@ func (l *Lexer) reportEscapeError(start source.Position, err error) { ) } +// nonASCIIWord returns the word at the start of text when it is written like a +// name but holds a letter, mark or digit outside ASCII, and "" otherwise. +func nonASCIIWord(text string) string { + end, isASCII := 0, true + for index, ch := range text { + isWordStart := ch == '_' || unicode.IsLetter(ch) + if !isWordStart && (index == 0 || !unicode.IsDigit(ch) && !unicode.IsMark(ch)) { + break + } + isASCII = isASCII && ch < utf8.RuneSelf + end = index + utf8.RuneLen(ch) + } + if isASCII { + return "" + } + return text[:end] +} + func (l *Lexer) Tokenize() []token.Token { for !l.isAtEOF() { matched := false rem := l.remainder() + if word := nonASCIIWord(rem); word != "" { + // One error for the whole word, and a name token in its place so + // the parser reads on as if the name were allowed. + start := l.pos + l.advanceBy(word) + l.diag.Add( + diagnostics.NewError(fmt.Sprintf("name `%s` uses characters that are not allowed in names", word)). + WithCode(diagnostics.ErrUnexpectedCharacter). + WithPrimaryLabel(source.NewLocation(l.file, start, l.pos), "use only `A-Z`, `a-z`, `0-9` and `_`"), + ) + l.push(token.Token{Kind: token.IDENT, Literal: word, Start: start, End: l.pos}) + continue + } + for _, p := range regexPatterns { loc := p.regex.FindStringIndex(rem) if loc != nil && loc[0] == 0 { diff --git a/internal/frontend/lexer/lexer_test.go b/internal/frontend/lexer/lexer_test.go index ad4ed322..06c9ca85 100644 --- a/internal/frontend/lexer/lexer_test.go +++ b/internal/frontend/lexer/lexer_test.go @@ -1,6 +1,7 @@ package lexer import ( + "strings" "testing" "compiler/internal/diagnostics" @@ -152,3 +153,25 @@ fn add(a: i32, b: i32): i32 { New("benchmark"+peeper.SourceExt, src, nil).Tokenize() } } + +func TestNameWithNonASCIICharactersIsOneErrorAndOneToken(t *testing.T) { + for _, word := range []string{"名前", "größe", "নাম", "naïve_1", "_é"} { + diag := diagnostics.NewDiagnosticBag() + stream := New("names"+peeper.SourceExt, "let "+word+" = 1;", diag).Tokenize() + items := diag.Diagnostics() + if len(items) != 1 || !strings.Contains(items[0].Message, "`"+word+"`") { + t.Errorf("%s: expected one error naming the word, got %d:\n%s", word, len(items), diag.EmitAllToString()) + } + if len(stream) < 2 || stream[1].Kind != token.IDENT || stream[1].Literal != word { + t.Errorf("%s: expected the word as one name token, got %#v", word, stream) + } + } +} + +func TestASCIINamesAndNonASCIITextElsewhereAreNotReported(t *testing.T) { + diag := diagnostics.NewDiagnosticBag() + New("ok"+peeper.SourceExt, "// 日本語\nlet name_1 = \"বাংলা 😀\"; let c = 'λ'; let s = c\"é\";", diag).Tokenize() + if diag.HasErrors() { + t.Fatalf("unexpected errors:\n%s", diag.EmitAllToString()) + } +} diff --git a/x_test/negative_wide_text_marker_and_name/peeper.toml b/x_test/negative_wide_text_marker_and_name/peeper.toml new file mode 100644 index 00000000..afc09690 --- /dev/null +++ b/x_test/negative_wide_text_marker_and_name/peeper.toml @@ -0,0 +1,8 @@ +name = "negative_wide_text_marker_and_name" +build = "program" + +[test] +mode = "check" +outcome = "failure" +stderr_contains = ["3 | let first: str = \"日本語 😀\" + 1;\n | ^\n", "4 | let second = \"한국어\" \"x\";\n | ^^^ found `x`\n", "name `名前` uses characters that are not allowed in names", "5 | let 名前 = 3;\n | ^^^^ use only `A-Z`, `a-z`, `0-9` and `_`\n", "Compilation failed with 4 error(s)"] +stderr_excludes = ["illegal character", "unknown identifier", "\n\n\n"] diff --git a/x_test/negative_wide_text_marker_and_name/src/main.peep b/x_test/negative_wide_text_marker_and_name/src/main.peep new file mode 100644 index 00000000..da737900 --- /dev/null +++ b/x_test/negative_wide_text_marker_and_name/src/main.peep @@ -0,0 +1,7 @@ +fn main() -> i32 { + // 日本語のコメント + let first: str = "日本語 😀" + 1; + let second = "한국어" "x"; + let 名前 = 3; + return 名前; +}