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/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/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..a48b24f6 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,81 +193,48 @@ func (d *Diagnostic) markInternalCompilerError(message string) *Diagnostic { return d } -func (d *Diagnostic) WithCodeHint(loc *source.Location, code string, labels ...CodeHintLabel) *Diagnostic { - if loc == nil { +// 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 } - 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 + if color == "" { + color = colors.WHITE } - 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, - }, + Kind: ExtraText, + Text: DiagnosticText{Kind: kind, Message: message, Color: color, Fixes: fixes}, }) 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) Note(message string, fixes ...CodeFix) *Diagnostic { + return d.WithText("note", message, colors.CYAN, fixes...) } -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) Help(help string, fixes ...CodeFix) *Diagnostic { + return d.WithText("help", help, colors.GREEN, fixes...) } -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...) +// 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 } -func (d *Diagnostic) WithText(kind, message string, color colors.COLOR) *Diagnostic { - if message == "" { - return d - } - if color == "" { - color = colors.WHITE +// 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)) } - d.Extras = append(d.Extras, DiagnosticExtra{ - Kind: ExtraText, - Text: DiagnosticText{Kind: kind, Message: message, Color: color}, - }) return d } -func (d *Diagnostic) WithNote(message string) *Diagnostic { - return d.WithText("note", message, colors.CYAN) -} - -func (d *Diagnostic) WithHelp(help string) *Diagnostic { - return d.WithText("help", help, colors.GREEN) -} - const internalCompilerFailureMessage = "internal compiler failure" type PresentationOptions struct { diff --git a/internal/diagnostics/diagnostic_test.go b/internal/diagnostics/diagnostic_test.go index c635fd3a..5be8d4bb 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 TestHelpCarriesItsFixes(t *testing.T) { d := NewError("immutable") loc := testLoc("main"+peeper.SourceExt, 2, 5) - d.WithCodeReplacement(loc, "maybe", "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)) } - 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) } } @@ -105,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() @@ -121,29 +118,257 @@ 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, ""). + Help("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) + } +} - loc := source.NewLocation(samplePath, source.Position{Line: 1, Column: 1}, source.Position{Line: 1, Column: 6}) - diag := NewError("replace"). +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) + } +} + +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), ""). + 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) + } +} - 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) + } +} + +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, ""). + 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)) } - if !strings.Contains(text, "\n | \n") { - t.Fatalf("expected blank gutter spacer lines, got:\n%s", text) + 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/diagnostics/emitter.go b/internal/diagnostics/emitter.go index 1d11ed40..2db25b1e 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" - "sort" + "slices" "strings" "sync" + "unicode" + "unicode/utf8" "compiler/internal/source" "compiler/pkg/colors" @@ -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 @@ -97,6 +89,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() @@ -162,15 +158,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 +202,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) @@ -242,20 +215,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 err != nil || strings.TrimSpace(prevLine) == "" { + return "", false } - if strings.TrimSpace(prevLine) == "" { - return - } - 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) { @@ -287,6 +258,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,215 +280,201 @@ 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) - 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 } } - 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) { - startLine := label.Location.Start.Line - endLine := startLine - if label.Location.End != nil { - endLine = label.Location.End.Line - } +// 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 +} - e.printPrevNonEmptyLine(filepath, startLine) +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 +} - for l := startLine; l <= endLine; l++ { - sourceLine, err := e.cache.GetLine(filepath, l) - if err != nil { +// 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 = utf8.RuneCountInString(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) } } -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 +493,192 @@ 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 +// 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 } - if e.printInlineReplacementHint(ctx, hint) { - return + color := text.Color + if color == "" { + color = colors.WHITE } - lines := e.codeHintRenderableLines(hint) - if len(lines) == 0 { - return + 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) +} - e.printBlankGutter() - fmt.Fprintln(e.writer) +// 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 +} - labelsByLine := make(map[int][]CodeHintLabel) - for _, label := range hint.Labels { - if label.Line <= 0 { - continue - } - labelsByLine[label.Line] = append(labelsByLine[label.Line], label) +// fixedLine is a source line together with every fix that changes it. +type fixedLine struct { + file string + number int + source string + spans []fixSpan +} + +// 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 + } + e.printPipeOnly() + for _, line := range lines { + e.printFixedLine(line) } + return true +} - 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() +// 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 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) + if loc.End != nil && loc.End.Line != loc.Start.Line || strings.ContainsAny(fix.NewText, "\r\n") { + return nil, false } - fmt.Fprintln(e.writer) - - if labels := labelsByLine[i+1]; len(labels) > 0 { - for _, label := range labels { - e.printCodeHintLabelLine(label, ctx.severity) + 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 } } } + return lines, true } -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) - - oldFrag := lines[0].Code - start := visualColumnToPosition(sourceLine, ctx.startCol) - if start < 0 || start > len(expandedSourceLine) { - return false - } - end := visualColumnToPosition(sourceLine, ctx.endCol) - - 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) +// 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 end < start || end > len(expandedSourceLine) { - 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) + width := cellWidth(ch, visual) + if ch == '\t' { + offset += width + } else { + offset += utf8.RuneLen(ch) + } + visual += width + current++ + } + return offset, current == column +} + +// 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 != "" + }) - if oldAbsStart < 0 { - oldAbsStart = 0 - } - if oldAbsStart > len(expandedSourceLine) { - oldAbsStart = len(expandedSourceLine) + 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++ + } } - - 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) + e.highlighter.HighlightWithColor(text[position:], e.writer) 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) - +// 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(oldBytes) && prefix < len(newBytes) && oldBytes[prefix] == newBytes[prefix] { + for prefix < len(before) && prefix < len(after) && before[prefix] == after[prefix] { prefix++ } - suffix := 0 - for suffix < len(oldBytes)-prefix && suffix < len(newBytes)-prefix && - oldBytes[len(oldBytes)-1-suffix] == newBytes[len(newBytes)-1-suffix] { + for suffix < len(before)-prefix && suffix < len(after)-prefix && + before[len(before)-1-suffix] == after[len(after)-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 - } - - e.highlighter.HighlightWithColor(line[:start], e.writer) - e.logger.Fprint(e.writer, spanColor, line[start:end]) - e.highlighter.HighlightWithColor(line[end:], e.writer) -} - -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 - } - } - return pathpkg.ToSlash(rel) -} - -func (e *Emitter) hasRenderableCodeHint(hint *CodeHint) bool { - return len(e.codeHintRenderableLines(hint)) > 0 -} - -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, - }) - } - 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) - } - fmt.Fprintln(e.writer) -} - -func (e *Emitter) printText(text DiagnosticText) { - if text.Message == "" { - return - } - color := text.Color - if color == "" { - color = colors.WHITE - } - - 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) -} - -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/diagnostics/emitter_test.go b/internal/diagnostics/emitter_test.go index 808b8931..fbcd67f4 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,149 @@ 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) + } +} + +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) + } +} + +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/internal/frontend/parser/parse_expr.go b/internal/frontend/parser/parse_expr.go index e6ebef3f..8fd4b5fd 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, ""). + 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.Help("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) { @@ -359,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 @@ -483,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 @@ -557,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 @@ -629,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) @@ -648,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 a07a6e65..74f5fbf0 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) @@ -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() @@ -347,9 +366,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, ""). + Help("remove this comma", diagnostics.Fix.Remove(loc))) } } armListEnd := p.current().End @@ -369,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 eb83df67..f34bdf1e 100644 --- a/internal/frontend/parser/parse_types.go +++ b/internal/frontend/parser/parse_types.go @@ -170,10 +170,12 @@ 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 `?`"). - WithNote("nested optional types are the same as a single optional type")) + WithPrimaryLabel(loc, ""). + Help("remove redundant `?`", diagnostics.Fix.Remove(loc)). + Note("nested optional types are the same as a single optional type")) } return inner } @@ -274,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 { @@ -289,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 @@ -346,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 { @@ -364,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 @@ -395,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 { @@ -424,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 { @@ -473,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 } @@ -492,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 { @@ -538,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 } @@ -549,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 4841fd2b..1819d618 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) @@ -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,9 +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(diagnostics.NewError( - fmt.Sprintf("expected '%s'", string(kind)), - ).WithCode(diagnostics.ErrExpectedToken).WithPrimaryLabel(loc, fmt.Sprintf("add missing `%s` here", string(kind)))) + 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. + return nil + } p.synchronize(kind) if p.current().Kind == kind { return p.advance() @@ -609,7 +715,24 @@ 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) { +// 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 +// 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 +744,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, "").Help(advice, diagnostics.Fix.Remove(removed)) + } else { + d.WithPrimaryLabel(loc, advice) + } + p.diag.Add(d) } } @@ -631,6 +769,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) @@ -640,11 +779,16 @@ 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 + isClosedEarly := false + var wrongSeparators, missingCommas []diagnostics.CodeFix 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 +796,7 @@ func parseBracedItemList[T any]( break } item, ok := parseItem() + isAfterItem = true if ok { items = append(items, item) } else { @@ -659,15 +804,49 @@ func parseBracedItemList[T any]( } if p.at(token.COMMA) { if p.next().Kind == token.RBRACE { - 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")) + 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) + 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(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(p.withOpener(insertionError(itemMsg, p.prev().End, p.filePath, "}", "}"), lbraceStart, "{")) + 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(diagnostics.NewError(itemMsg).WithCode(diagnostics.ErrExpectedToken).WithPrimaryLabel(source.NewLocation(p.filePath, prev.End, prev.End), "add missing `,` here")) + 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. @@ -678,7 +857,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 @@ -700,12 +889,173 @@ 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(p.missingTokenError(msg, kind, loc)) return nil } +// missingTokenError reports a token that should follow loc. Where the guess +// 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)) + } + 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 +// 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.IDENT, token.AMP, token.QUESTION, token.QQ, token.ASTERISK, token.RAWPTR, + token.LBRACK, token.FN, token.STRUCT, token.IFACE, token.ENUM: + return true + } + return false +} + // 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..aff4c153 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 `,` here") { + if !strings.Contains(diag.EmitAllToString(), "expected ',' between items, found ';'") { t.Fatalf("expected separator recovery diagnostic, got:\n%s", diag.EmitAllToString()) } } @@ -2687,8 +2686,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 +2709,523 @@ 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()) + } + } +} + +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, "found `") { + t.Fatalf("expected the label to name the token found, 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) + } + } +} + +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/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/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/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..e3e38c1d 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 { @@ -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/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..43358340 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 @@ -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) @@ -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..693bcf63 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) } @@ -810,10 +810,15 @@ 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 { - 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 +872,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/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/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/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/internal/semantics/usage/usage.go b/internal/semantics/usage/usage.go index c5257f84..7af2834c 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, ""). + Help("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/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) + } + } +} 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/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/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_found_token_and_open_bracket/peeper.toml b/x_test/negative_found_token_and_open_bracket/peeper.toml new file mode 100644 index 00000000..1583d0dd --- /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", " --> ", ":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 new file mode 100644 index 00000000..6b031c8f --- /dev/null +++ b/x_test/negative_found_token_and_open_bracket/src/main.peep @@ -0,0 +1,25 @@ +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; + let next = Add(1, + 2, 3; + return total + near + next + twice; +} 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_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_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_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/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..df419179 --- /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", "found `,`"] +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/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; +} 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 名前; +} 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; +} 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; +} 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; +}