Skip to content

Improve parser diagnostics and report unknown type names - #162

Merged
itsfuad merged 8 commits into
mainfrom
fix/code-hint-target-name
Oct 9, 2026
Merged

itsfuad merged 8 commits into
mainfrom
fix/code-hint-target-name

Conversation

@itsfuad

@itsfuad itsfuad commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Summary

  • What changed:
    • Parser: names the missing token and shows the repaired line where the repair is certain (:/= in let and const, : after a parameter name, ->, =>, , in lists, closing brackets). Reports a token typed for another with a replacement (; for ,, : for = in a struct literal, = for == in a condition). Where the repair cannot be told it shows both (let a b;) or points at the token found and proposes nothing.
    • Type system: an undeclared type name is now an error ("unknown type", with a similar declared name when one is close). Before, let x: Nope; compiled.
    • Diagnostics: WithHelp/WithNote renamed to Help/Note; new HelpWithChoices; fixed lines are drawn beneath help lines; the printer prints each source line once when a diagnostic has several labels.
    • Tests: regression tests for the shift guard and operand evaluation order.
  • Why this change is needed: Many parser errors proposed a wrong repair or cascaded into unrelated errors, undeclared type names were accepted, and diagnostics with more than one label repeated source lines.
  • Owning phase/package/component: internal/frontend/parser, internal/diagnostics, internal/semantics/typeinfo, typeresolution, typechecker, binder, analysis (ownership).
  • Scope boundaries / non-goals: No bracket matching before parsing. Later phases still run after syntax errors. Qualified type names that do not resolve (pkg::Missing) are unchanged. No redesign of the diagnostic format.

Production ownership

  • Current non-test production consumer(s) of each new production symbol:
    • Diagnostic.HelpWithChoices, Choice: Parser.parseBindingFields.
    • Fix.Replace (existing, first callers): parseBracedItemList, parseCompositeLiteral, parseIfStmt.
    • Parser: listContinues, reportMissingCommas, reportTrailingComma, startsArgument, startsValue, closerAhead, isSemicolonForComma, isBraceClosedBeforeArrow, parseReturnType, parseParamModifiers, startsNamedParam, withOpener, insertionError, isMissingTokenLikely, hasBeforeStatementEnd, isLoneNameAfterBindingName, isTypeWithoutColon, isOnLineOfPrev, isTypeStart: all called from the list, binding, parameter, match and close-bracket parsers.
    • typeinfo.SyntaxUnknownName, InvalidType.Name: resolveTypeName, reported by typeresolution.reportSyntaxIssues. typeinfo.ContainsMalformed: Resolver.Query. declaredTypeNames: reportSyntaxIssues.
    • Printer: snippetFiles, printSnippetLine, contextLine: Emitter.Emit.
  • Execution path showing where the new behavior is reached in production: peeper check|build|run → parser → binder/typechecker (type syntax resolution) → DiagnosticBag.EmitAll → Emitter.Emit.
  • Existing implementation reused, extended, replaced, or intentionally left unchanged: Reuses the SyntaxIssue list, InvalidType, ContainsInvalid, diagnostics.NearestName, and the existing fix drawing. rejectUnsizedType, typeArrayLit and ownershipTrackedType now stay silent on a type with an invalid part.
  • Wrappers, aliases, duplicate paths, compatibility shims, or experimental scaffolding removed: WithHelp/WithNote are renamed with every caller, no alias kept. printPeeperSnippetBlock and printPrevNonEmptyLine are replaced and deleted.
  • Any new production symbol without a current production consumer: None.

Design and behavior

  • Previous behavior: A missing-token error often said "add missing X here" at a place that was wrong, or cascaded (for example .{ x = 1; y = 2 } gave 7 errors after an earlier draft of this work, 1–2 before). An unclosed call could take the next statement as an argument or drop it. An unknown type name became a silent placeholder. A diagnostic with two labels printed shared lines twice and could put the header on the secondary label.
  • New behavior: One error per mistake in the covered cases, with a fixed line only where the repair is reliable. Unknown type names are reported once per use and later checks stay silent about them. Each source line is printed once per diagnostic and the header names the primary label.
  • Key invariants preserved: Valid programs parse and check as before (all positive fixtures pass). Braced-list recovery always consumes a token no item accepts (regression test added). A diagnostic with one label prints byte for byte as before.
  • Intentional behavioral changes:
    • A trailing comma before ) or > in calls, parameters, function types, type parameters and attribute arguments is now the same note as in literals (S0001) instead of an error, so such code compiles.
    • Undeclared type names are rejected.
    • Functions with type parameters get one error, "functions cannot take type parameters yet"; the spec says generic functions are not part of the current language surface.
    • Ownership checks do not run on values whose type has an invalid part.
  • Error / edge-case handling: Rules that guess are limited to cases where the guess is reliable (end of line, before another closer, list still closes ahead); otherwise the diagnostic names the token found. A value on a later line counts as the next argument only when the list's ) is still ahead.
  • Why this implementation belongs in this layer rather than another phase/package: Token-level repairs are decided in the parser, which is the only phase that sees the tokens. Whether a name is a type is decided where type syntax is resolved. Label layout belongs to the printer.

Validation

Run on the final tree (0fadf11), locally on Linux amd64.

gofmt -l cmd internal pkg scripts
(no output)

go build ./... && go vet ./...
(no output)

go test ./...
all packages ok

go run ./scripts/bundle.go
ok

PEEPER_BIN=$PWD/build/bin/peeper go test ./x_test/ -count=1
ok  	compiler/x_test

go run mvdan.cc/unparam@latest ./...
(no output)

go run honnef.co/go/tools/cmd/staticcheck@latest ./...
internal/diagnostics/emitter.go: func (*Emitter).printUnderlineIndent is unused (U1000)   [kept on purpose, pre-existing]

git diff --check
(no output)
  • Targeted tests added or updated: Parser tests for every new rule, including inputs that must stay silent; printer tests for each label layout; typechecker tests for unknown type names, suggestions and generic functions. New fixtures: negative_missing_token_at_known_spot, negative_missing_token_unreliable_guess, negative_missing_comma_in_lists, negative_semicolon_in_value_list, negative_parameter_without_type, negative_single_repair_spots, negative_let_missing_separator, negative_missing_closers, negative_bang_after_argument, negative_found_token_and_open_bracket, negative_unknown_names, negative_unknown_type_name, negative_unknown_const_value, positive_multiline_lists_and_continuations, llvm_shift_guard_exact_bound, llvm_shift_guard_narrow_valid, runtime_mir_rvalue_eval_order_binary_capture. Updated: negative_struct_literal_value_prefix (now "unknown type point").
  • Existing regression coverage exercised: All existing fixtures and Go tests.
  • Manual verification, if applicable: About 50 deliberately broken programs were compiled with this branch and with 6ff091e, and every diagnostic was read. More errors than before appear only where a real follow-on error or an unknown type is now reported.
  • Checks not run: Race detector, cross-platform native builds and distribution tests were not run locally; CI runs them.

Diff sanity

  • Unrelated production changes: None.
  • Duplicated logic introduced: expectClose calls isMissingTokenLikely after missingTokenError has called it with the same arguments. The typechecker's two parameter loops each skip a parameter with no written type.
  • Dead or unreachable code introduced: None.
  • Temporary/debug code remaining: None.
  • Comments/docs that became stale because of this change: None found.

Risks and follow-up

  • Known risks or assumptions: Rules that guess a missing token can be too eager; each has tests for valid and near-miss inputs. Layout of every multi-label diagnostic changed.
  • Behavior intentionally changed: See "Intentional behavioral changes".
  • Behavior intentionally not changed: fn Add(i32 i32) still suggests :; Id(a !b) gets no repair; an unclosed bracket before { (if f(a {) still cascades.
  • Remaining gaps: Qualified type names that do not resolve are still accepted silently. const c: [2]Nope = [2]Nope{1, 2}; reports only the first Nope. "not all control paths return a value" draws the same span as a primary and a secondary label. Inside a function with type parameters, let x: T also says "unknown type T".
  • Linked issues / follow-up work: Related to Add advanced generics for functions and methods #90 (generic functions and methods): this adds a clear rejection message only.
  • Anything that should block merge: None known. No independent review has been done yet.

Replace the code-hint block with fixes attached to help and note lines.
A fix is built with Fix.Remove, Fix.Insert or Fix.Replace and drawn as
one source line: the line as it will read with added text marked, or
the current line with removed text marked. Fixes that cannot be shown
faithfully on single lines are not drawn.

Attach fixes to unneeded mut, missing separators and closing brackets,
redundant separators, trailing and match-arm commas, redundant optional
markers and leading zeros. Other missing tokens keep their label.

Keep the margin unbroken before help lines and end every diagnostic
with one blank line.

Known gap: a missing-token fix follows the parser's guess, so it can
show a wrong line when more code follows on the same line.
Name the token that is missing and show the repaired line where the
repair is certain: ':' or '=' in let and const, ':' after a parameter
name, '->' before a return type, '=>' in a match arm, ',' in calls,
parameters, function types, type parameters, attribute arguments and
braced lists, and a closing bracket at the end of a line or before
another closer. Several missing commas in one list are one error.

Report a token typed for another with a replacement: ';' for ',' in
lists, ':' for '=' in a struct literal, '=' for '==' in a condition.
Where the repair cannot be told, show both: `let a b;` offers ':' and
'='. One word after `mut` is reported without guessing whether it is
the name or the type, a repeated `mut` is reported once, and a
parameter name without a type keeps the rest of the list.

An unclosed call no longer takes the next statement as an argument or
drops it. A ';' in a value list closes the list only when its own '}'
is missing. A trailing comma before ')' or '>' is the same note as in
literals instead of an error.

Rename Diagnostic.WithHelp and WithNote to Help and Note, and add
HelpWithChoices for repairs that depend on what the author meant.
A type name that is neither declared nor built in used to become a
silent placeholder, so `let x: Nope;` compiled and other uses failed
with unrelated messages. Report "unknown type" where the name is
written, with a similar declared name when one is close.

The unknown name becomes the invalid type and keeps its text for
display. Checks that meet a type with an invalid part stay silent: the
size check, array literal typing, and ownership tracking, which also
stops the module-binding error after an unknown constant value. A type
query treats a type whose only fault is an unknown name as showable.

Reject type parameters on functions with one message; the language
does not have generic functions yet. The entry-point test no longer
covers that case because it is rejected earlier.

Qualified names that do not resolve keep their placeholder.
Cover a shift count equal to the operand width, valid narrow and odd
width shift counts, the generated shift guard for each count width,
and a left operand read before a call on the right changes it.
Where a separator or a closing bracket is expected but inserting it is
not a reliable repair, mark the token that was found and propose
nothing. The old label told the reader to add the token at a place
that was often wrong.

When a closing bracket is missing and its bracket was opened on an
earlier line, also mark where it was opened.
The printer drew every label as its own block: the line above it, the
labelled lines, then the underline. A second label on the next line
repeated that line as context, two labels on one line printed the line
twice, and labels over the same lines printed all of them again.

Lay the labels out by line instead. Each source line is printed once
with a row for every label on it, the line above is shown only when it
is not on screen yet, and `...` appears only where lines are really
left out. The location header names the primary label instead of the
first label in file order. A diagnostic with one label prints as
before.
A source file with CRLF line ends kept the `\r` on every line read for
diagnostics, so printed source lines and fixed lines ended with a
stray carriage return. Trim it in the two places that split source
text: the diagnostic source cache and GetSourceLines.
The printer placed a marker one column per character, so it sat too
far left after East Asian wide characters and emoji, which take two
terminal cells, and too far right after combining marks, which take
none. Count cells for marker position, underline length and tab stops.
A label that runs to the end of a line counted bytes; count characters.

A name with letters outside ASCII was one "illegal character" error
per character and broke the statement around it. Report the whole word
once and read it as a name so parsing continues. Text outside ASCII in
strings, character literals and comments stays allowed.
@itsfuad
itsfuad merged commit 23f03d4 into main Oct 9, 2026
17 checks passed
@itsfuad
itsfuad deleted the fix/code-hint-target-name branch October 9, 2026 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant