Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ test:
@cargo nextest run --no-fail-fast --hide-progress-bar --status-level none --failure-output final

shot:
@# See AGENTS.md for diagnostic guidelines
@cargo nextest run --no-fail-fast --hide-progress-bar --status-level none --failure-output final || true
@cargo insta accept
@cargo nextest run --no-fail-fast --hide-progress-bar --status-level none --failure-output final

coverage-lines:
@cargo llvm-cov --package plotnik-lib --text --show-missing-lines 2>/dev/null | grep '\.rs: [0-9]' | sed 's|.*/crates/|crates/|'
Expand Down
92 changes: 55 additions & 37 deletions crates/plotnik-lib/src/diagnostics/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,37 +129,57 @@ impl DiagnosticKind {
matches!(self, Self::UnnamedDef)
}

/// Default hint for this kind, automatically included in diagnostics.
/// Call sites can add additional hints for context-specific information.
pub fn default_hint(&self) -> Option<&'static str> {
match self {
Self::ExpectedSubtype => Some("e.g., `expression/binary_expression`"),
Self::ExpectedTypeName => Some("e.g., `::MyType` or `::string`"),
Self::ExpectedFieldName => Some("e.g., `!value`"),
Self::EmptyTree => Some("use `(_)` to match any named node, or `_` for any node"),
Self::TreeSitterSequenceSyntax => Some("use `{...}` for sequences"),
Self::MixedAltBranches => {
Some("use all labels for a tagged union, or none for a merged struct")
}
Self::RecursionNoEscape => {
Some("add a non-recursive branch to terminate: `[Base: ... Rec: (Self)]`")
}
Self::DirectRecursion => {
Some("recursive references must consume input before recursing")
}
_ => None,
}
}

/// Base message for this diagnostic kind, used when no custom message is provided.
pub fn fallback_message(&self) -> &'static str {
match self {
// Unclosed delimiters - clear about what's missing
// Unclosed delimiters
Self::UnclosedTree => "missing closing `)`",
Self::UnclosedSequence => "missing closing `}`",
Self::UnclosedAlternation => "missing closing `]`",

// Expected token errors - specific about what's needed
// Expected token errors
Self::ExpectedExpression => "expected an expression",
Self::ExpectedTypeName => "expected type name after `::`",
Self::ExpectedCaptureName => "expected name after `@`",
Self::ExpectedTypeName => "expected type name",
Self::ExpectedCaptureName => "expected capture name",
Self::ExpectedFieldName => "expected field name",
Self::ExpectedSubtype => "expected subtype after `/`",

// Invalid syntax - explain what's wrong
Self::EmptyTree => "empty parentheses are not allowed",
Self::BareIdentifier => "bare identifier is not a valid expression",
Self::InvalidSeparator => "separators are not needed",
Self::InvalidFieldEquals => "use `:` for field constraints, not `=`",
Self::InvalidSupertypeSyntax => "supertype syntax not allowed on references",
Self::InvalidTypeAnnotationSyntax => "use `::` for type annotations, not `:`",
Self::ErrorTakesNoArguments => "`(ERROR)` cannot have child nodes",
Self::ExpectedSubtype => "expected subtype name",

// Invalid syntax
Self::EmptyTree => "empty `()` is not allowed",
Self::BareIdentifier => "bare identifier is not valid",
Self::InvalidSeparator => "unexpected separator",
Self::InvalidFieldEquals => "use `:` instead of `=`",
Self::InvalidSupertypeSyntax => "references cannot have supertypes",
Self::InvalidTypeAnnotationSyntax => "use `::` for type annotations",
Self::ErrorTakesNoArguments => "`(ERROR)` cannot have children",
Self::RefCannotHaveChildren => "references cannot have children",
Self::ErrorMissingOutsideParens => {
"`ERROR` and `MISSING` must be wrapped in parentheses"
}
Self::UnsupportedPredicate => "predicates like `#match?` are not supported",
Self::ErrorMissingOutsideParens => "special node requires parentheses",
Self::UnsupportedPredicate => "predicates are not supported",
Self::UnexpectedToken => "unexpected token",
Self::CaptureWithoutTarget => "`@` must follow an expression to capture",
Self::LowercaseBranchLabel => "branch labels must be capitalized",
Self::CaptureWithoutTarget => "capture has no target",
Self::LowercaseBranchLabel => "branch label must start with uppercase",

// Naming convention violations
Self::CaptureNameHasDots => "capture names cannot contain `.`",
Expand All @@ -172,23 +192,25 @@ impl DiagnosticKind {
Self::FieldNameHasHyphens => "field names cannot contain `-`",
Self::FieldNameUppercase => "field names must be lowercase",
Self::TypeNameInvalidChars => "type names cannot contain `.` or `-`",
Self::TreeSitterSequenceSyntax => "Tree-sitter sequence syntax",
Self::TreeSitterSequenceSyntax => "tree-sitter sequence syntax",

// Semantic errors
Self::DuplicateDefinition => "name already defined",
Self::DuplicateDefinition => "duplicate definition",
Self::UndefinedReference => "undefined reference",
Self::MixedAltBranches => "cannot mix labeled and unlabeled branches",
Self::RecursionNoEscape => "infinite recursion: cycle has no escape path",
Self::RecursionNoEscape => "infinite recursion: no escape path",
Self::DirectRecursion => "infinite recursion: cycle consumes no input",
Self::FieldSequenceValue => "field must match exactly one node",
Self::FieldSequenceValue => "field cannot match a sequence",

// Type inference
Self::IncompatibleTypes => "incompatible types in alternation branches",
Self::IncompatibleTypes => "incompatible types",
Self::MultiCaptureQuantifierNoName => {
"quantified expression with multiple captures requires `@name`"
"quantified expression with multiple captures requires a struct capture"
}
Self::UnusedBranchLabels => "branch labels have no effect without capture",
Self::StrictDimensionalityViolation => "quantifier requires row capture",
Self::StrictDimensionalityViolation => {
"quantifier with captures requires a struct capture"
}
Self::DuplicateCaptureInScope => "duplicate capture in scope",
Self::IncompatibleCaptureTypes => "incompatible capture types",
Self::IncompatibleStructShapes => "incompatible struct shapes",
Expand All @@ -201,7 +223,7 @@ impl DiagnosticKind {
Self::InvalidChildType => "node type not valid as child",

// Structural
Self::UnnamedDef => "definitions must be named",
Self::UnnamedDef => "definition must be named",
}
}

Expand All @@ -212,9 +234,7 @@ impl DiagnosticKind {
Self::RefCannotHaveChildren => {
"`{}` is a reference and cannot have children".to_string()
}
Self::FieldSequenceValue => {
"field `{}` must match exactly one node, not a sequence".to_string()
}
Self::FieldSequenceValue => "field `{}` cannot match a sequence".to_string(),

// Semantic errors with name context
Self::DuplicateDefinition => "`{}` is already defined".to_string(),
Expand Down Expand Up @@ -249,15 +269,13 @@ impl DiagnosticKind {
}

// Type annotation specifics
Self::InvalidTypeAnnotationSyntax => {
"type annotations use `::`, not `:` — {}".to_string()
}
Self::InvalidTypeAnnotationSyntax => "use `::` for type annotations: {}".to_string(),

// Named def
Self::UnnamedDef => "definitions must be named — {}".to_string(),
// Named def (no custom message needed; suggestion goes in hint)
Self::UnnamedDef => self.fallback_message().to_string(),

// Standard pattern: fallback + context
_ => format!("{}; {{}}", self.fallback_message()),
_ => format!("{}: {{}}", self.fallback_message()),
}
}

Expand Down
6 changes: 5 additions & 1 deletion crates/plotnik-lib/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,11 @@ impl<'a> DiagnosticBuilder<'a> {
self
}

pub fn emit(self) {
pub fn emit(mut self) {
// Prepend default hint if one exists for this kind
if let Some(default_hint) = self.message.kind.default_hint() {
self.message.hints.insert(0, default_hint.to_string());
}
self.diagnostics.messages.push(self.message);
}
}
4 changes: 2 additions & 2 deletions crates/plotnik-lib/src/diagnostics/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ fn builder_with_fix() {

let result = diagnostics.printer(&map).render();
insta::assert_snapshot!(result, @r"
error: use `:` for field constraints, not `=`; fixable
error: use `:` instead of `=`: fixable
|
1 | hello world
| ^^^^^
Expand Down Expand Up @@ -207,7 +207,7 @@ fn printer_zero_width_span() {

let result = diagnostics.printer(&map).render();
insta::assert_snapshot!(result, @r"
error: expected an expression; zero width error
error: expected an expression: zero width error
|
1 | hello
| ^
Expand Down
25 changes: 22 additions & 3 deletions crates/plotnik-lib/src/parser/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ impl<'src, 'diag> Parser<'src, 'diag> {
true
}

fn bump_as_error(&mut self) {
pub(super) fn bump_as_error(&mut self) {
if !self.eof() {
self.start_node(SyntaxKind::Error);
self.bump();
Expand Down Expand Up @@ -298,11 +298,32 @@ impl<'src, 'diag> Parser<'src, 'diag> {
.emit();
}

pub(super) fn error_with_hint(&mut self, kind: DiagnosticKind, hint: impl Into<String>) {
let Some((range, suppression)) = self.get_error_ranges() else {
return;
};
self.diagnostics
.report(self.source_id, kind, range)
.hint(hint)
.suppression_range(suppression)
.emit();
}

pub(super) fn error_and_bump(&mut self, kind: DiagnosticKind) {
self.error(kind);
self.bump_as_error();
}

pub(super) fn error_and_bump_with_hint(
&mut self,
kind: DiagnosticKind,
hint: impl Into<String>,
) {
self.error_with_hint(kind, hint);
self.bump_as_error();
}

#[allow(dead_code)]
pub(super) fn error_and_bump_msg(&mut self, kind: DiagnosticKind, message: impl Into<String>) {
self.error_msg(kind, message);
self.bump_as_error();
Expand Down Expand Up @@ -359,7 +380,6 @@ impl<'src, 'diag> Parser<'src, 'diag> {
pub(super) fn error_unclosed_delimiter(
&mut self,
kind: DiagnosticKind,
message: impl Into<String>,
related_msg: impl Into<String>,
open_range: TextRange,
) {
Expand All @@ -371,7 +391,6 @@ impl<'src, 'diag> Parser<'src, 'diag> {
let full_range = TextRange::new(open_range.start(), current.end());
self.diagnostics
.report(self.source_id, kind, full_range)
.message(message)
.related_to(self.source_id, open_range, related_msg)
.emit();
}
Expand Down
4 changes: 2 additions & 2 deletions crates/plotnik-lib/src/parser/grammar/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl Parser<'_, '_> {
return false;
}

self.error_and_bump_msg(
self.error_and_bump_with_hint(
DiagnosticKind::UnexpectedToken,
"try `(node)`, `[a b]`, `{a b}`, `\"literal\"`, or `_`",
);
Expand Down Expand Up @@ -65,7 +65,7 @@ impl Parser<'_, '_> {
self.error_and_bump(DiagnosticKind::ErrorMissingOutsideParens);
}
_ => {
self.error_and_bump_msg(DiagnosticKind::UnexpectedToken, "not a valid expression");
self.error_and_bump(DiagnosticKind::UnexpectedToken);
}
}

Expand Down
22 changes: 11 additions & 11 deletions crates/plotnik-lib/src/parser/grammar/fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@ impl Parser<'_, '_> {
self.bump();
self.validate_type_name(text, span);
} else {
self.error_msg(
DiagnosticKind::ExpectedTypeName,
"e.g., `::MyType` or `::string`",
);
self.error(DiagnosticKind::ExpectedTypeName);
}

self.finish_node();
Expand Down Expand Up @@ -84,7 +81,7 @@ impl Parser<'_, '_> {
self.expect(SyntaxKind::Negation, "'!' for negated field");

if !self.currently_is(SyntaxKind::Id) {
self.error_msg(DiagnosticKind::ExpectedFieldName, "e.g., `!value`");
self.error(DiagnosticKind::ExpectedFieldName);
self.finish_node();
return;
}
Expand All @@ -110,10 +107,13 @@ impl Parser<'_, '_> {
}

// Bare identifiers are not valid expressions; trees require parentheses
self.error_and_bump_msg(
DiagnosticKind::BareIdentifier,
"wrap in parentheses: `(identifier)`",
);
let span = self.current_span();
let text = token_text(self.source, &self.tokens[self.pos]);
self.diagnostics
.report(self.source_id, DiagnosticKind::BareIdentifier, span)
.fix("wrap in parentheses", format!("({})", text))
.emit();
self.bump_as_error();
}

/// Field constraint: `field_name: expr`
Expand All @@ -134,7 +134,7 @@ impl Parser<'_, '_> {
if self.currently_is_one_of(EXPR_FIRST_TOKENS) {
self.parse_expr_no_suffix();
} else {
self.error_msg(DiagnosticKind::ExpectedExpression, "after `field:`");
self.error(DiagnosticKind::ExpectedExpression);
}

self.finish_node();
Expand All @@ -158,7 +158,7 @@ impl Parser<'_, '_> {
if self.currently_is_one_of(EXPR_FIRST_TOKENS) {
self.parse_expr();
} else {
self.error_msg(DiagnosticKind::ExpectedExpression, "after `field =`");
self.error(DiagnosticKind::ExpectedExpression);
}

self.finish_node();
Expand Down
7 changes: 2 additions & 5 deletions crates/plotnik-lib/src/parser/grammar/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl Parser<'_, '_> {
let def_text = &self.source[usize::from(start)..usize::from(end)];
self.diagnostics
.report(self.source_id, DiagnosticKind::UnnamedDef, span)
.message(format!("give it a name like `Name = {}`", def_text.trim()))
.hint(format!("give it a name like `Name = {}`", def_text.trim()))
.emit();
}
}
Expand Down Expand Up @@ -83,10 +83,7 @@ impl Parser<'_, '_> {
if self.currently_is_one_of(EXPR_FIRST_TOKENS) {
self.parse_expr();
} else {
self.error_msg(
DiagnosticKind::ExpectedExpression,
"after `=` in definition",
);
self.error(DiagnosticKind::ExpectedExpression);
}

self.finish_node();
Expand Down
Loading