diff --git a/src/compiler.rs b/src/compiler.rs index db4b3c7..3148bf3 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -26,7 +26,7 @@ use std::path::{Path, PathBuf}; use miette::Context; use serde_json::Value; -use crate::error::ParseDiagnostic; +use crate::error::{ParseDiagnostic, SpanWithSource}; use crate::import::{ImportContext, import_protocol, import_schema}; use crate::model::json::{build_lookup, protocol_to_json, schema_to_json}; use crate::model::protocol::Message; @@ -65,9 +65,9 @@ struct CompileOutput { warnings: Vec, /// Original source text, retained for error diagnostics in type-specific /// serialization logic. - source: String, + source: &'static str, /// Name used for the source in diagnostics (e.g., file path or `""`). - source_name: String, + source_name: &'static str, } impl IdlCompiler { @@ -104,13 +104,19 @@ impl IdlCompiler { let dir = dir.canonicalize().unwrap_or(dir); let canonical_path = path.canonicalize().ok(); - self.compile(&source, &source_name, &dir, canonical_path) + let source = source.leak(); + let source_name = source_name.leak(); + self.compile(source, source_name, &dir, canonical_path) } /// Compile an IDL source string using the current working directory as the /// import base. This is the shared implementation behind /// `Idl::convert_str_named` and `Idl2Schemata::extract_str_named`. - fn compile_str(&mut self, source: &str, name: &str) -> miette::Result { + fn compile_str( + &mut self, + source: &'static str, + name: &'static str, + ) -> miette::Result { let cwd = std::env::current_dir() .map_err(|e| miette::miette!("{e}")) .context("determine current directory")?; @@ -126,8 +132,8 @@ impl IdlCompiler { /// are available via [`drain_warnings`](Self::drain_warnings). fn compile( &mut self, - source: &str, - source_name: &str, + source: &'static str, + source_name: &'static str, input_dir: &Path, input_path: Option, ) -> miette::Result { @@ -163,8 +169,8 @@ impl IdlCompiler { idl_file, registry, warnings, - source: source.to_string(), - source_name: source_name.to_string(), + source, + source_name, }) } } @@ -281,13 +287,17 @@ impl Idl { /// Compile an IDL source string to JSON. Uses `""` as the source /// name in diagnostics. - pub fn convert_str(&mut self, source: &str) -> miette::Result { + pub fn convert_str(&mut self, source: &'static str) -> miette::Result { self.convert_str_named(source, "") } /// Compile an IDL source string to JSON with a custom source name for /// diagnostics. - pub fn convert_str_named(&mut self, source: &str, name: &str) -> miette::Result { + pub fn convert_str_named( + &mut self, + source: &'static str, + name: &'static str, + ) -> miette::Result { let compiled = self.inner.compile_str(source, name)?; self.convert_impl(compiled) } @@ -320,8 +330,7 @@ impl Idl { let span_len = source.len().min(1); return Err(ParseDiagnostic { - src: miette::NamedSource::new(source_name, source), - span: (0, span_len).into(), + span: SpanWithSource::new(0, span_len, source_name, source), message: "IDL file contains neither a protocol nor a schema declaration" .to_string(), label: Some("this file".to_string()), @@ -465,7 +474,7 @@ impl Idl2Schemata { } /// Extract named schemas from an IDL source string. - pub fn extract_str(&mut self, source: &str) -> miette::Result { + pub fn extract_str(&mut self, source: &'static str) -> miette::Result { self.extract_str_named(source, "") } @@ -473,8 +482,8 @@ impl Idl2Schemata { /// name for diagnostics. pub fn extract_str_named( &mut self, - source: &str, - name: &str, + source: &'static str, + name: &'static str, ) -> miette::Result { let compiled = self.inner.compile_str(source, name)?; Ok(Self::extract_impl(compiled)) @@ -565,7 +574,7 @@ struct CompileContext { /// in the IDL source. Used to enrich error messages for unresolved /// references from `.avsc`/`.avpr` imports, which lack source spans of /// their own. - json_import_spans: Vec<(String, Option)>, + json_import_spans: Vec<(String, Option)>, } impl CompileContext { @@ -592,8 +601,8 @@ impl CompileContext { /// declaration items (imports and local types) in source order, and we /// process them sequentially, so the registry reflects declaration order. fn parse_and_resolve( - source: &str, - source_name: &str, + source: &'static str, + source_name: &'static str, input_dir: &Path, input_path: Option, ctx: &mut CompileContext, @@ -631,7 +640,7 @@ fn parse_and_resolve( // Process declaration items in source order: resolve imports when // encountered, register local types when encountered. Any import-derived // warnings are appended to `ctx.warnings` by `process_decl_items`. - process_decl_items(&decl_items, ctx, input_dir, source, source_name)?; + process_decl_items(&decl_items, ctx, input_dir)?; // For protocol files, rebuild the types list from the registry (which now // includes imported types in declaration order) and prepend imported @@ -659,19 +668,16 @@ fn process_decl_items( decl_items: &[DeclItem], ctx: &mut CompileContext, current_dir: &Path, - source: &str, - source_name: &str, ) -> miette::Result<()> { for item in decl_items { match item { DeclItem::Import(import) => { - resolve_single_import(import, ctx, current_dir, source, source_name)?; + resolve_single_import(import, ctx, current_dir)?; } DeclItem::Type(schema, span, field_spans) => { if let Err(msg) = ctx.registry.register(schema.as_ref().clone()) { - if let Some(span) = span { + if let Some(span) = span.as_ref() { return Err(ParseDiagnostic { - src: miette::NamedSource::new(source_name, source.to_string()), span: *span, message: msg, label: None, @@ -705,7 +711,6 @@ fn process_decl_items( ); let effective_span = field_spans.get(&field_name).copied().or(*span); effective_span.map(|span| ParseDiagnostic { - src: miette::NamedSource::new(source_name, source.to_string()), span, message: msg, label: None, @@ -724,7 +729,6 @@ fn process_decl_items( let effective_span = field_spans.get(&first_field).copied().or(*span); if let Some(span) = effective_span { return Err(ParseDiagnostic { - src: miette::NamedSource::new(source_name, source.to_string()), span, message: first_msg, label: None, @@ -747,15 +751,12 @@ fn resolve_single_import( import: &crate::reader::ImportEntry, ctx: &mut CompileContext, current_dir: &Path, - source: &str, - source_name: &str, ) -> miette::Result<()> { let resolved_path = match ctx.import_ctx.resolve_import(&import.path, current_dir) { Ok(p) => p, Err(e) => { if let Some(span) = import.span { return Err(ParseDiagnostic { - src: miette::NamedSource::new(source_name, source.to_string()), span, message: format!("{e}"), label: None, @@ -780,17 +781,8 @@ fn resolve_single_import( match import.kind { ImportKind::Protocol => { - let imported_messages = - import_protocol(&resolved_path, &mut ctx.registry).map_err(|e| { - wrap_import_error( - e, - import.span, - source, - source_name, - &resolved_path, - "protocol", - ) - })?; + let imported_messages = import_protocol(&resolved_path, &mut ctx.registry) + .map_err(|e| wrap_import_error(e, import.span, &resolved_path, "protocol"))?; ctx.messages.extend(imported_messages); // Track the import so unresolved references from this .avpr can @@ -799,16 +791,8 @@ fn resolve_single_import( .push((resolved_path.display().to_string(), import.span)); } ImportKind::Schema => { - import_schema(&resolved_path, &mut ctx.registry).map_err(|e| { - wrap_import_error( - e, - import.span, - source, - source_name, - &resolved_path, - "schema", - ) - })?; + import_schema(&resolved_path, &mut ctx.registry) + .map_err(|e| wrap_import_error(e, import.span, &resolved_path, "schema"))?; // Track the import so unresolved references from this .avsc can // be attributed to the import statement in error diagnostics. @@ -818,11 +802,12 @@ fn resolve_single_import( ImportKind::Idl => { let imported_source = fs::read_to_string(&resolved_path) .map_err(|e| miette::miette!("{e}")) - .with_context(|| format!("read imported IDL {}", resolved_path.display()))?; + .with_context(|| format!("read imported IDL {}", resolved_path.display())) + .map(String::leak)?; - let imported_name = resolved_path.display().to_string(); + let imported_name = resolved_path.display().to_string().leak(); let (imported_idl, nested_decl_items, import_warnings) = - parse_idl_named(&imported_source, &imported_name) + parse_idl_named(imported_source, imported_name) .with_context(|| format!("parse imported IDL {}", resolved_path.display()))?; // Propagate warnings from the imported file, wrapping each with the @@ -845,14 +830,7 @@ fn resolve_single_import( // IDL imports use their own source text for span tracking, so // `ctx.json_import_spans` is passed through to capture any nested // JSON imports within the imported IDL file. - process_decl_items( - &nested_decl_items, - ctx, - &import_dir, - &imported_source, - &imported_name, - ) - .with_context(|| { + process_decl_items(&nested_decl_items, ctx, &import_dir).with_context(|| { format!("resolve nested imports from `{}`", resolved_path.display()) })?; } @@ -871,15 +849,12 @@ fn resolve_single_import( /// diagnostic -- context layers are shown as plain text. fn wrap_import_error( error: miette::Report, - span: Option, - source: &str, - source_name: &str, + span: Option, resolved_path: &Path, kind: &str, ) -> miette::Report { if let Some(span) = span { let diag = ParseDiagnostic { - src: miette::NamedSource::new(source_name, source.to_string()), span, message: format!("import {} {}", kind, resolved_path.display()), label: None, @@ -1023,9 +998,9 @@ fn suggest_similar_name(unresolved: &str, registry: &SchemaRegistry) -> Option)], + source: &'static str, + source_name: &'static str, + json_import_spans: &[(String, Option)], ) -> miette::Result<()> { let mut unresolved = registry.validate_references(); @@ -1071,7 +1046,10 @@ fn validate_all_references( // Sort by source span offset so the first error in the file is reported // first. References without a span (from JSON imports) sort to the end. - unresolved.sort_by_key(|(_, span)| span.map_or(usize::MAX, |s| s.offset())); + unresolved.sort_by_key(|(_, span)| { + span.as_ref() + .map_or(("", usize::MAX), |s| (s.name, s.offset)) + }); if unresolved.is_empty() { return Ok(()); @@ -1110,7 +1088,6 @@ fn validate_all_references( if let Some(span) = first_import_span { return Err(ParseDiagnostic { - src: miette::NamedSource::new(source_name, source.to_string()), span, message, label: Some("this import contains undefined type references".to_string()), @@ -1140,7 +1117,6 @@ fn validate_all_references( let span = span.expect("partitioned into Some"); let help = suggest_similar_name(&name, registry); ParseDiagnostic { - src: miette::NamedSource::new(source_name, source.to_string()), span, message: format!("Undefined name: {name}"), label: None, @@ -1154,7 +1130,7 @@ fn validate_all_references( // statement spans so the user can see which import brought them in. // Fall back to a zero-length span at offset 0 if no import span is // available. Include "did you mean?" suggestions where applicable. - let fallback_span: miette::SourceSpan = (0, 0).into(); + let fallback_span = SpanWithSource::new(0, 0, source_name, source); for (name, _) in &without_span { let (span, label) = if let Some((path, Some(import_span))) = json_import_spans.first() { ( @@ -1177,7 +1153,6 @@ fn validate_all_references( }; related.push(ParseDiagnostic { - src: miette::NamedSource::new(source_name, source.to_string()), span, message: format!("Undefined name: {name}"), label, @@ -1188,7 +1163,6 @@ fn validate_all_references( let first_help = suggest_similar_name(&first_name, registry); Err(ParseDiagnostic { - src: miette::NamedSource::new(source_name, source.to_string()), span: first_span, message: format!("Undefined name: {first_name}"), label: None, diff --git a/src/error.rs b/src/error.rs index 3c27163..fdbe248 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,4 +1,64 @@ -use miette::{Diagnostic, LabeledSpan, NamedSource, SourceSpan}; +use miette::{Diagnostic, LabeledSpan, MietteError, MietteSpanContents, SourceCode, SourceSpan}; + +/// A source span paired with the file it points into. +/// +/// Stored in `ParseDiagnostic`, `Warning`, and `AvroSchema::Reference` so that +/// every diagnostic — including those from imported files — can render the +/// correct source excerpt and filename. +/// +#[derive(Debug, Copy, Clone, PartialEq)] +pub struct SpanWithSource { + pub offset: usize, + pub length: usize, + /// Absolute path of the file that contains this span. + pub name: &'static str, + /// Full source text of that file. + pub content: &'static str, +} + +impl SpanWithSource { + pub fn new( + offset: usize, + length: usize, + file_name: &'static str, + content: &'static str, + ) -> Self { + SpanWithSource { + offset, + length, + name: file_name, + content, + } + } + + pub(crate) fn source_span(&self) -> SourceSpan { + SourceSpan::new(self.offset.into(), self.length) + } +} + +impl SourceCode for SpanWithSource { + fn read_span<'a>( + &'a self, + span: &SourceSpan, + context_lines_before: usize, + context_lines_after: usize, + ) -> Result + 'a>, MietteError> { + let inner = SourceCode::read_span( + &self.content, + span, + context_lines_before, + context_lines_after, + )?; + Ok(Box::new(MietteSpanContents::new_named( + self.name.to_string(), + inner.data(), + *inner.span(), + inner.line(), + inner.column(), + inner.line_count(), + ))) + } +} /// A parse error with source location information for rich diagnostics. /// @@ -12,8 +72,7 @@ use miette::{Diagnostic, LabeledSpan, NamedSource, SourceSpan}; /// source-underline label. #[derive(Debug)] pub struct ParseDiagnostic { - pub src: NamedSource, - pub span: SourceSpan, + pub span: SpanWithSource, pub message: String, /// Shorter label for the source-underline annotation. When `None`, falls /// back to `message`. @@ -53,8 +112,8 @@ pub(crate) fn render_diagnostic(report: &miette::Report) -> String { } impl miette::Diagnostic for ParseDiagnostic { - fn source_code(&self) -> Option<&dyn miette::SourceCode> { - Some(&self.src) + fn source_code(&self) -> Option<&dyn SourceCode> { + Some(&self.span) } fn help<'a>(&'a self) -> Option> { @@ -67,7 +126,7 @@ impl miette::Diagnostic for ParseDiagnostic { let label_text = self.label.clone().unwrap_or_else(|| self.message.clone()); Some(Box::new(std::iter::once(LabeledSpan::new_with_span( Some(label_text), - self.span, + self.span.source_span(), )))) } diff --git a/src/main.rs b/src/main.rs index d30eecb..8d36c85 100644 --- a/src/main.rs +++ b/src/main.rs @@ -206,8 +206,9 @@ fn run_idl( io::stdin() .read_to_string(&mut source) .map_err(|e| miette::miette!("{e}: read IDL from stdin"))?; - let source_name = input.as_deref().unwrap_or(""); - builder.convert_str_named(&source, source_name) + let source_name = input.map(|s| &*String::leak(s)).unwrap_or(""); + let source = source.leak(); + builder.convert_str_named(source, source_name) } }; diff --git a/src/model/schema.rs b/src/model/schema.rs index 8921c35..9963f9e 100644 --- a/src/model/schema.rs +++ b/src/model/schema.rs @@ -1,10 +1,11 @@ -use miette::SourceSpan; use serde_json::Value; use std::borrow::Cow; use std::collections::HashMap; use std::fmt; use std::str::FromStr; +use crate::error::SpanWithSource; + /// The eight Avro primitive type names. /// /// Both `is_invalid_type_name` in `reader.rs` and `is_schema_type_name` in @@ -370,10 +371,10 @@ pub enum AvroSchema { name: std::string::String, namespace: Option, properties: HashMap, - /// Source location of this reference in the `.avdl` input, used for - /// error diagnostics when the reference cannot be resolved. `None` for - /// references created from JSON imports (no source location available). - span: Option, + /// Source location and file of this reference in the `.avdl` input, + /// used for error diagnostics when the reference cannot be resolved. + /// `None` for references created from JSON imports. + span: Option, }, } diff --git a/src/reader.rs b/src/reader.rs index 4721ed6..737922c 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -39,7 +39,7 @@ use serde_json::Value; use std::collections::HashMap; use crate::doc_comments::extract_doc_comment; -use crate::error::ParseDiagnostic; +use crate::error::{ParseDiagnostic, SpanWithSource}; use crate::generated::idllexer::IdlLexer; use crate::generated::idlparser::*; use crate::model::protocol::{Message, Protocol}; @@ -62,10 +62,8 @@ use miette::{Context, Result}; /// the offending token underlined. pub(crate) struct Warning { pub(crate) message: String, - /// The source text and file name, for rich diagnostic rendering. - pub(crate) source: Option>, - /// Byte range of the token that triggered the warning. - pub(crate) span: Option, + /// Source (file, source-code, offsets) of the problematic token + pub(crate) span: Option, } /// Custom `Debug` implementation that shows a compact representation instead of @@ -77,12 +75,13 @@ impl std::fmt::Debug for Warning { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("Warning") .field("message", &self.message) - .field("file", &self.source.as_ref().map(|s| s.name().to_string())) + .field("file", &self.span.as_ref().map(|s| s.name)) .field( "span", &self .span - .map(|s| format!("{}..{}", s.offset(), s.offset() + s.len())), + .as_ref() + .map(|s| format!("{}..{}", s.offset, s.offset + s.length)), ) .finish() } @@ -100,7 +99,7 @@ impl Warning { fn out_of_place_doc_comment( line: isize, column: isize, - src: &SourceInfo<'_>, + src: &SourceInfo, token_start: isize, token_stop: isize, ) -> Self { @@ -124,8 +123,7 @@ impl Warning { // get_column() is 0-based, so we add 1 to match. column + 1, ), - source: Some(miette::NamedSource::new(src.name, src.source.to_string())), - span: Some(miette::SourceSpan::new(offset.into(), length)), + span: Some(src.span(offset, length)), } } @@ -137,7 +135,7 @@ impl Warning { /// warning lets the user know their annotations had no effect. fn annotations_dropped_on_union<'a>( annotation_keys: &[&str], - src: &SourceInfo<'_>, + src: &SourceInfo, ctx: &impl antlr4rust::parser_rule_context::ParserRuleContext<'a>, ) -> Self { let start_token = ctx.start(); @@ -164,8 +162,7 @@ impl Warning { message: format!( "Annotations on union types are not supported and will be ignored: {keys_display}" ), - source: Some(miette::NamedSource::new(src.name, src.source.to_string())), - span: Some(miette::SourceSpan::new(byte_offset.into(), length)), + span: Some(src.span(byte_offset, length)), } } } @@ -186,11 +183,11 @@ impl miette::Diagnostic for Warning { } fn source_code(&self) -> Option<&dyn miette::SourceCode> { - self.source.as_ref().map(|s| s as &dyn miette::SourceCode) + self.span.as_ref().map(|s| s as &dyn miette::SourceCode) } fn labels(&self) -> Option + '_>> { - let span = self.span?; + let span = self.span.as_ref()?; // Derive a short label from the message content rather than hardcoding // a single label for all warning types. let label = if self.message.contains("out-of-place documentation comment") { @@ -203,7 +200,7 @@ impl miette::Diagnostic for Warning { "here" }; Some(Box::new(std::iter::once( - miette::LabeledSpan::new_with_span(Some(label.to_string()), span), + miette::LabeledSpan::new_with_span(Some(label.to_string()), span.source_span()), ))) } } @@ -1800,7 +1797,7 @@ struct CollectingErrorListener { /// Original source text for line/column-to-byte-offset conversion. /// `None` for parser errors where the offending token always provides /// byte offsets directly. - source: Option>, + source: Option<&'static str>, } impl<'a, T: Recognizer<'a>> ErrorListener<'a, T> for CollectingErrorListener { @@ -1832,7 +1829,7 @@ impl<'a, T: Recognizer<'a>> ErrorListener<'a, T> for CollectingErrorListener { .unwrap_or_else(|| { // Lexer errors have no offending token. Fall back to computing // the byte offset from (line, column) using the source text. - if let Some(ref src) = self.source { + if let Some(src) = self.source { let offset = line_col_to_byte_offset(src, line, column); (offset, 1) } else { @@ -1907,9 +1904,9 @@ pub enum IdlFile { pub struct ImportEntry { pub kind: ImportKind, pub path: String, - /// Byte range of the import statement in the originating IDL source, + /// Source location of the import statement in the originating IDL source, /// enabling source-highlighted diagnostics when import resolution fails. - pub span: Option, + pub span: Option, } /// The kind of import statement. @@ -1930,20 +1927,20 @@ pub enum DeclItem { Import(ImportEntry), /// A locally-defined named type (record, enum, or fixed). /// - /// The optional `SourceSpan` records the byte range of the declaration in - /// the originating IDL source, enabling source-highlighted diagnostics when - /// registration fails (e.g., duplicate type name). + /// The optional `SpanWithSource` records the source location of the + /// declaration, enabling source-highlighted diagnostics when registration + /// fails (e.g., duplicate type name). /// - /// The `HashMap` maps field names to their default - /// value's source span for record types. When a field has a default value, - /// the span covers the `jsonValue` node; otherwise it falls back to the - /// `variableDeclaration` node. This enables per-field diagnostics that + /// The `HashMap` maps field names to their default + /// value's source location for record types. When a field has a default + /// value, the span covers the `jsonValue` node; otherwise it falls back to + /// the `variableDeclaration` node. This enables per-field diagnostics that /// highlight the offending default value when validating Reference-typed /// fields in the compiler. Type( Box, - Option, - HashMap, + Option, + HashMap, ), } @@ -1955,11 +1952,11 @@ pub enum DeclItem { /// Tests that need to verify CRLF-specific behavior should call /// [`parse_idl_named`] directly. #[cfg(test)] -pub fn parse_idl_for_test(input: &str) -> Result<(IdlFile, Vec, Vec)> { +pub fn parse_idl_for_test(input: &'static str) -> Result<(IdlFile, Vec, Vec)> { let normalized; let input = if input.contains("\r\n") { normalized = input.replace("\r\n", "\n"); - normalized.as_str() + normalized.leak() } else { input }; @@ -1970,8 +1967,8 @@ pub fn parse_idl_for_test(input: &str) -> Result<(IdlFile, Vec, Vec Result<(IdlFile, Vec, Vec)> { // The ANTLR grammar's `idlFile` rule includes `('\u001a' .*?)? EOF` // to treat the ASCII SUB character (U+001A) as an end-of-file marker, @@ -1992,12 +1989,11 @@ pub fn parse_idl_named( // warnings instead, matching Java's behavior of not treating lexer // errors as fatal. (Java also doesn't install a custom listener on // the lexer — it just lets ConsoleErrorListener print to stderr.) - let source_rc: Rc = Rc::from(input); let lexer_errors: Rc>> = Rc::new(RefCell::new(Vec::new())); lexer.remove_error_listeners(); lexer.add_error_listener(Box::new(CollectingErrorListener { errors: Rc::clone(&lexer_errors), - source: Some(Rc::clone(&source_rc)), + source: Some(input), })); let token_stream = CommonTokenStream::new(lexer); @@ -2034,8 +2030,7 @@ pub fn parse_idl_named( .iter() .map(|e| Warning { message: e.message.clone(), - source: Some(miette::NamedSource::new(source_name, input.to_string())), - span: Some(miette::SourceSpan::new(e.offset.into(), e.length)), + span: Some(SpanWithSource::new(e.offset, e.length, source_name, input)), }) .collect(); @@ -2073,8 +2068,7 @@ pub fn parse_idl_named( }; return Err(ParseDiagnostic { - src: miette::NamedSource::new(source_name, input.to_string()), - span: miette::SourceSpan::new(unterm.offset.into(), unterm.length), + span: SpanWithSource::new(unterm.offset, unterm.length, source_name, input), message, label: Some("unterminated string literal".to_string()), help: Some( @@ -2097,8 +2091,7 @@ pub fn parse_idl_named( let related: Vec = errors_to_report[1..] .iter() .map(|e| ParseDiagnostic { - src: miette::NamedSource::new(source_name, input.to_string()), - span: miette::SourceSpan::new(e.offset.into(), e.length), + span: SpanWithSource::new(e.offset, e.length, source_name, input), message: e.message.clone(), label: e.label.clone(), help: e.help.clone(), @@ -2106,8 +2099,7 @@ pub fn parse_idl_named( }) .collect(); return Err(ParseDiagnostic { - src: miette::NamedSource::new(source_name, input.to_string()), - span: miette::SourceSpan::new(first.offset.into(), first.length), + span: SpanWithSource::new(first.offset, first.length, source_name, input), message: first.message.clone(), label: first.label.clone(), help: first.help.clone(), @@ -2177,9 +2169,9 @@ type TS<'input> = CommonTokenStream<'input, IdlLexer<'input, InputStream<&'input /// /// Also tracks which doc comment token indices have been consumed by /// declarations, so that orphaned doc comments can be detected after the walk. -struct SourceInfo<'a> { - source: &'a str, - name: &'a str, +struct SourceInfo { + source: &'static str, + name: &'static str, /// Token indices of doc comments consumed by `extract_doc_from_context`. /// After the full tree walk, any `DocComment` token NOT in this set is /// orphaned and should generate a warning. @@ -2191,6 +2183,12 @@ struct SourceInfo<'a> { warnings: RefCell>, } +impl SourceInfo { + fn span(&self, offset: usize, length: usize) -> SpanWithSource { + SpanWithSource::new(offset, length, self.name, self.source) + } +} + /// Compute `(offset, length)` from ANTLR's inclusive start/stop byte offsets. /// /// ANTLR tokens report byte positions where both `start` and `stop` are @@ -2215,7 +2213,7 @@ fn span_from_offsets(start: isize, stop: isize) -> (usize, usize) { /// `SourceSpan` that miette can render as an underlined region in the error /// output. fn make_diagnostic<'input>( - src: &SourceInfo<'_>, + src: &SourceInfo, ctx: &impl antlr4rust::parser_rule_context::ParserRuleContext<'input>, message: impl Into, ) -> miette::Report { @@ -2240,8 +2238,7 @@ fn make_diagnostic<'input>( let message = message.into(); ParseDiagnostic { - src: miette::NamedSource::new(src.name, src.source.to_string()), - span: miette::SourceSpan::new(offset.into(), length), + span: src.span(offset, length), message, label: None, help: None, @@ -2254,7 +2251,7 @@ fn make_diagnostic<'input>( /// context node. Useful when the error relates to a specific token field /// (e.g. `ctx.size`, `ctx.typeName`) rather than the whole context. fn make_diagnostic_from_token( - src: &SourceInfo<'_>, + src: &SourceInfo, token: &impl Token, message: impl Into, ) -> miette::Report { @@ -2262,8 +2259,7 @@ fn make_diagnostic_from_token( let message = message.into(); ParseDiagnostic { - src: miette::NamedSource::new(src.name, src.source.to_string()), - span: miette::SourceSpan::new(offset.into(), length), + span: src.span(offset, length), message, label: None, help: None, @@ -2272,7 +2268,7 @@ fn make_diagnostic_from_token( .into() } -/// Extract a `SourceSpan` from a parse tree context's start token. +/// Extract the span (offset and length) from a parse tree context's start token. /// /// Returns `None` if the token has no valid position (e.g., synthetic tokens). /// Used to attach spans to `DeclItem::Type` and `DeclItem::Import` entries so @@ -2280,13 +2276,13 @@ fn make_diagnostic_from_token( /// source-highlighted diagnostics. fn span_from_context<'input>( ctx: &impl antlr4rust::parser_rule_context::ParserRuleContext<'input>, -) -> Option { +) -> Option<(usize, usize)> { let start_token = ctx.start(); let (offset, length) = span_from_offsets(start_token.get_start(), start_token.get_stop()); // A zero-length span at offset 0 means no valid position was available. if length > 0 { - Some(miette::SourceSpan::new(offset.into(), length)) + Some((offset, length)) } else { None } @@ -2472,7 +2468,7 @@ const MESSAGE_PROPS: PropertyContext = PropertyContext { fn walk_schema_properties<'input>( props: &[Rc>], token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, pctx: PropertyContext, ) -> Result { let mut result = SchemaProperties::new(); @@ -2592,7 +2588,7 @@ fn walk_schema_properties<'input>( fn walk_idl_file<'input>( ctx: &IdlFileContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, namespace: &mut Option, decl_items: &mut Vec, ) -> Result { @@ -2625,9 +2621,9 @@ fn walk_idl_file<'input>( .clone() .downcast_rc::>() { - collect_single_import(&import_ctx, decl_items); + collect_single_import(&import_ctx, decl_items, src); } else if let Ok(ns_ctx) = child.downcast_rc::>() { - let span = span_from_context(&*ns_ctx); + let span = span_from_context(&*ns_ctx).map(|(o, l)| src.span(o, l)); let (schema, field_spans) = walk_named_schema_no_register(&ns_ctx, token_stream, src, namespace)?; local_schemas.push(schema.clone()); @@ -2661,7 +2657,7 @@ fn walk_idl_file<'input>( fn walk_protocol<'input>( ctx: &ProtocolDeclarationContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, namespace: &mut Option, decl_items: &mut Vec, ) -> Result { @@ -2710,12 +2706,12 @@ fn walk_protocol<'input>( .clone() .downcast_rc::>() { - collect_single_import(&import_ctx, decl_items); + collect_single_import(&import_ctx, decl_items, src); } else if let Ok(ns_ctx) = child .clone() .downcast_rc::>() { - let span = span_from_context(&*ns_ctx); + let span = span_from_context(&*ns_ctx).map(|(o, l)| src.span(o, l)); let (schema, field_spans) = walk_named_schema_no_register(&ns_ctx, token_stream, src, namespace)?; decl_items.push(DeclItem::Type(Box::new(schema), span, field_spans)); @@ -2746,9 +2742,9 @@ fn walk_protocol<'input>( fn walk_named_schema_no_register<'input>( ctx: &NamedSchemaDeclarationContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, namespace: &mut Option, -) -> Result<(AvroSchema, HashMap)> { +) -> Result<(AvroSchema, HashMap)> { if let Some(fixed_ctx) = ctx.fixedDeclaration() { Ok(( walk_fixed(&fixed_ctx, token_stream, src, namespace.as_deref())?, @@ -2782,9 +2778,9 @@ fn walk_named_schema_no_register<'input>( fn walk_record<'input>( ctx: &RecordDeclarationContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, namespace: &mut Option, -) -> Result<(AvroSchema, HashMap)> { +) -> Result<(AvroSchema, HashMap)> { let doc = extract_doc_from_context(ctx, token_stream, src); let props = walk_schema_properties( &ctx.schemaProperty_all(), @@ -2831,7 +2827,7 @@ fn walk_record<'input>( .ok_or_else(|| make_diagnostic(src, ctx, "missing record body"))?; let mut fields = Vec::new(); - let mut field_spans: HashMap = HashMap::new(); + let mut field_spans: HashMap = HashMap::new(); let mut seen_field_names: HashSet = HashSet::new(); for field_ctx in body.fieldDeclaration_all() { let mut field_fields = walk_field_declaration( @@ -2878,8 +2874,8 @@ fn walk_record<'input>( .jsonValue() .and_then(|jv| span_from_context(&*jv)) .or_else(|| span_from_context(&**var_ctx)); - if let Some(span) = default_span { - field_spans.insert(field.name.clone(), span); + if let Some((offset, length)) = default_span { + field_spans.insert(field.name.clone(), src.span(offset, length)); } } fields.append(&mut field_fields); @@ -2914,7 +2910,7 @@ fn walk_record<'input>( fn walk_field_declaration<'input>( ctx: &FieldDeclarationContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, namespace: Option<&str>, enclosing_name: Option<&str>, ) -> Result> { @@ -2955,7 +2951,7 @@ fn walk_variable<'input>( field_type: &AvroSchema, default_doc: Option<&str>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, _namespace: Option<&str>, enclosing_name: Option<&str>, ) -> Result { @@ -3037,7 +3033,7 @@ fn walk_variable<'input>( fn walk_enum<'input>( ctx: &EnumDeclarationContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, enclosing_namespace: Option<&str>, ) -> Result { let doc = extract_doc_from_context(ctx, token_stream, src); @@ -3121,7 +3117,7 @@ fn walk_enum<'input>( fn walk_fixed<'input>( ctx: &FixedDeclarationContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, enclosing_namespace: Option<&str>, ) -> Result { let doc = extract_doc_from_context(ctx, token_stream, src); @@ -3189,7 +3185,7 @@ fn walk_fixed<'input>( fn walk_full_type<'input>( ctx: &FullTypeContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, namespace: Option<&str>, ) -> Result { let props = walk_schema_properties(&ctx.schemaProperty_all(), token_stream, src, BARE_PROPS)?; @@ -3241,7 +3237,7 @@ fn walk_full_type<'input>( fn walk_plain_type<'input>( ctx: &PlainTypeContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, namespace: Option<&str>, ) -> Result { if let Some(array_ctx) = ctx.arrayType() { @@ -3264,7 +3260,7 @@ fn walk_plain_type<'input>( fn walk_nullable_type<'input>( ctx: &NullableTypeContextAll<'input>, _token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, namespace: Option<&str>, ) -> Result { let base_type = if let Some(prim_ctx) = ctx.primitiveType() { @@ -3274,7 +3270,7 @@ fn walk_nullable_type<'input>( // so the Reference carries them separately, enabling correct namespace // shortening during JSON serialization. let type_name = identifier_text(&ref_ctx); - let ref_span = span_from_context(&*ref_ctx); + let ref_span = span_from_context(&*ref_ctx).map(|(o, l)| src.span(o, l)); if let Some((ns, name)) = type_name.rsplit_once('.') { AvroSchema::Reference { name: name.to_string(), @@ -3317,7 +3313,7 @@ fn walk_nullable_type<'input>( /// Walk a primitive type keyword and return the corresponding `AvroSchema`. fn walk_primitive_type<'input>( ctx: &PrimitiveTypeContextAll<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, ) -> Result { let type_tok = ctx .typeName @@ -3425,7 +3421,7 @@ fn walk_primitive_type<'input>( fn walk_array_type<'input>( ctx: &ArrayTypeContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, namespace: Option<&str>, ) -> Result { let element_ctx = ctx @@ -3442,7 +3438,7 @@ fn walk_array_type<'input>( fn walk_map_type<'input>( ctx: &MapTypeContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, namespace: Option<&str>, ) -> Result { let value_ctx = ctx @@ -3459,7 +3455,7 @@ fn walk_map_type<'input>( fn walk_union_type<'input>( ctx: &UnionTypeContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, namespace: Option<&str>, ) -> Result { let mut types = Vec::new(); @@ -3511,7 +3507,7 @@ fn walk_union_type<'input>( fn walk_message<'input>( ctx: &MessageDeclarationContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, namespace: Option<&str>, ) -> Result<(String, Message)> { let doc = extract_doc_from_context(ctx, token_stream, src); @@ -3599,7 +3595,7 @@ fn walk_message<'input>( let mut error_schemas = Vec::new(); for error_id_ctx in &ctx.errors { let error_name = identifier_text(error_id_ctx); - let error_span = span_from_context(&**error_id_ctx); + let error_span = span_from_context(&**error_id_ctx).map(|(o, l)| src.span(o, l)); if let Some((ns, name)) = error_name.rsplit_once('.') { error_schemas.push(AvroSchema::Reference { name: name.to_string(), @@ -3644,7 +3640,7 @@ fn walk_message<'input>( fn walk_result_type<'input>( ctx: &ResultTypeContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, namespace: Option<&str>, ) -> Result { // If there's a Void token, return Null. @@ -3666,7 +3662,7 @@ fn walk_result_type<'input>( fn walk_json_value<'input>( ctx: &JsonValueContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, ) -> Result { if let Some(obj_ctx) = ctx.jsonObject() { return walk_json_object(&obj_ctx, token_stream, src); @@ -3682,7 +3678,7 @@ fn walk_json_value<'input>( fn walk_json_literal<'input>( ctx: &JsonLiteralContextAll<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, ) -> Result { let tok = ctx .literal @@ -3720,7 +3716,7 @@ fn walk_json_literal<'input>( fn walk_json_object<'input>( ctx: &JsonObjectContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, ) -> Result { let mut map = serde_json::Map::new(); for pair_ctx in ctx.jsonPair_all() { @@ -3743,7 +3739,7 @@ fn walk_json_object<'input>( fn walk_json_array<'input>( ctx: &JsonArrayContextAll<'input>, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, ) -> Result { let mut elements = Vec::new(); for val_ctx in ctx.jsonValue_all() { @@ -4430,7 +4426,7 @@ fn json_value_as_u32(v: &Value) -> Option { fn extract_doc_from_context<'input, T>( ctx: &T, token_stream: &TS<'input>, - src: &SourceInfo<'_>, + src: &SourceInfo, ) -> Option where T: antlr4rust::parser_rule_context::ParserRuleContext<'input>, @@ -4455,7 +4451,7 @@ where fn collect_orphaned_doc_comment_warnings<'input, S>( token_stream: &S, consumed_indices: &HashSet, - src: &SourceInfo<'_>, + src: &SourceInfo, ) -> Vec where S: TokenStream<'input>, @@ -4487,6 +4483,7 @@ where fn collect_single_import<'input>( import_ctx: &ImportStatementContextAll<'input>, decl_items: &mut Vec, + src: &SourceInfo, ) { let kind_tok = import_ctx.importType.as_ref(); let location_tok = import_ctx.location.as_ref(); @@ -4502,7 +4499,7 @@ fn collect_single_import<'input>( decl_items.push(DeclItem::Import(ImportEntry { kind: import_kind, path: get_string_from_literal(loc.get_text()), - span: span_from_context(import_ctx), + span: span_from_context(import_ctx).map(|(o, l)| src.span(o, l)), })); } } @@ -5360,7 +5357,7 @@ mod tests { // ------------------------------------------------------------------ /// Helper: parse an IDL protocol with a single record, return its first field's schema. - fn parse_first_field_schema(idl: &str) -> AvroSchema { + fn parse_first_field_schema(idl: &'static str) -> AvroSchema { let (_idl_file, decl_items, _warnings) = parse_idl_for_test(idl).expect("IDL should parse successfully"); // Find the record among declaration items. @@ -5618,7 +5615,7 @@ mod tests { /// Helper: parse an IDL protocol and return the first named type schema /// (record, enum, or fixed). - fn parse_first_named_type(idl: &str) -> AvroSchema { + fn parse_first_named_type(idl: &'static str) -> AvroSchema { let (_idl_file, decl_items, _warnings) = parse_idl_for_test(idl).expect("IDL should parse successfully"); for item in &decl_items { diff --git a/src/resolve.rs b/src/resolve.rs index cd97d76..8b55d43 100644 --- a/src/resolve.rs +++ b/src/resolve.rs @@ -15,8 +15,8 @@ // named type. use indexmap::IndexMap; -use miette::SourceSpan; +use crate::error::SpanWithSource; use crate::model::schema::{AvroSchema, make_full_name}; // ============================================================================== @@ -185,7 +185,7 @@ impl SchemaRegistry { /// /// The caller is responsible for deduplication and ordering -- see /// `validate_all_references` in `compiler.rs`. - pub fn validate_references(&self) -> Vec<(String, Option)> { + pub fn validate_references(&self) -> Vec<(String, Option)> { let mut unresolved = Vec::new(); for schema in self.schemas.values() { collect_unresolved_refs(schema, &self.schemas, &mut unresolved); @@ -205,7 +205,7 @@ impl SchemaRegistry { /// /// The caller is responsible for deduplication and ordering -- see /// `validate_all_references` in `compiler.rs`. - pub fn validate_schema(&self, schema: &AvroSchema) -> Vec<(String, Option)> { + pub fn validate_schema(&self, schema: &AvroSchema) -> Vec<(String, Option)> { let mut unresolved = Vec::new(); collect_unresolved_refs(schema, &self.schemas, &mut unresolved); unresolved @@ -225,7 +225,7 @@ impl SchemaRegistry { fn collect_unresolved_refs( schema: &AvroSchema, known: &IndexMap, - unresolved: &mut Vec<(String, Option)>, + unresolved: &mut Vec<(String, Option)>, ) { match schema { AvroSchema::Reference { @@ -274,7 +274,7 @@ mod tests { /// Extract just the names from unresolved reference tuples, for concise /// test assertions. - fn names(unresolved: Vec<(String, Option)>) -> Vec { + fn names(unresolved: Vec<(String, Option)>) -> Vec { unresolved.into_iter().map(|(name, _)| name).collect() } diff --git a/tests/error_reporting.rs b/tests/error_reporting.rs index bd20ac8..d525887 100644 --- a/tests/error_reporting.rs +++ b/tests/error_reporting.rs @@ -29,7 +29,7 @@ use miette::{Diagnostic, GraphicalReportHandler, GraphicalTheme}; /// output for errors without source location info. /// /// Returns `None` if compilation succeeds. -fn compile_error(input: &str) -> Option { +fn compile_error(input: &'static str) -> Option { compile_error_with_width(input, 80) } @@ -38,7 +38,7 @@ fn compile_error(input: &str) -> Option { /// Useful when error messages embed absolute paths that would be split across /// lines at the default 80-column width, making post-processing (e.g., CWD /// redaction) unreliable. -fn compile_error_with_width(input: &str, width: usize) -> Option { +fn compile_error_with_width(input: &'static str, width: usize) -> Option { match Idl::new().convert_str(input) { Ok(_) => None, Err(e) => { @@ -84,7 +84,7 @@ fn compile_file_error(path: &std::path::Path) -> Option { /// compilation. /// /// Panics if compilation fails, since warning tests require a successful parse. -fn compile_warnings(input: &str) -> Vec { +fn compile_warnings(input: &'static str) -> Vec { let output = Idl::new() .convert_str(input) .expect("warning test input should compile successfully"); @@ -320,6 +320,49 @@ fn test_error_import_bad_avpr_json() { insta::assert_snapshot!(error); } +/// Referencing an unkown type should produce an error that +/// includes the source span of the `unknown.type` in the calling `.avdl` +/// file. +#[test] +#[cfg_attr(windows, ignore)] +fn test_error_file_unknown_type() { + let avdl_path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/testdata/test_import_unknown_type.avdl"); + let error = compile_file_error(&avdl_path).expect("should produce an error for unknown type"); + + // Redact the absolute path prefix so the snapshot is portable. + let manifest_dir = env!("CARGO_MANIFEST_DIR"); + let error = error.replace(manifest_dir, "[ROOT]"); + insta::assert_snapshot!(error); + + assert!( + error.contains("unknown_type.avdl"), + "error should mention the imported file containing the syntax error" + ); +} + +/// Importing a `.avdl` file with a syntax error should produce an error that +/// includes the source span of the syntax error in the imported file, and +/// mentions both the calling file and the imported file. +#[test] +#[cfg_attr(windows, ignore)] +fn test_error_import_syntax_error() { + let avdl_path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/testdata/test_import_syntax_error.avdl"); + let error = compile_file_error(&avdl_path) + .expect("should produce an error for syntax error in imported file"); + + // Redact the absolute path prefix so the snapshot is portable. + let manifest_dir = env!("CARGO_MANIFEST_DIR"); + let error = error.replace(manifest_dir, "[ROOT]"); + insta::assert_snapshot!(error); + + assert!( + error.contains("syntax_error.avdl"), + "error should mention the imported file containing the syntax error" + ); +} + // ============================================================================== // Mutation Error Tests (Slight Variations of Valid IDL) // ============================================================================== diff --git a/tests/integration.rs b/tests/integration.rs index 16bffd7..eb98977 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -602,7 +602,7 @@ fn test_extra_schema_syntax() { // ============================================================================== /// Helper: parse an inline `.avdl` string via the builder and return JSON. -fn parse_inline_to_json(avdl_input: &str) -> Value { +fn parse_inline_to_json(avdl_input: &'static str) -> Value { let output = Idl::new() .convert_str(avdl_input) .unwrap_or_else(|e| panic!("failed to parse inline avdl: {e}")); diff --git a/tests/snapshots/error_reporting__error_file_unknown_type.snap b/tests/snapshots/error_reporting__error_file_unknown_type.snap new file mode 100644 index 0000000..e4a183a --- /dev/null +++ b/tests/snapshots/error_reporting__error_file_unknown_type.snap @@ -0,0 +1,12 @@ +--- +source: tests/error_reporting.rs +expression: error +--- + x Undefined name: unknown.type + ,-[[ROOT]/tests/testdata/unknown_type.avdl:3:5] + 2 | record Foo { + 3 | unknown.type field; + : ^^^^^^|^^^^^ + : `-- Undefined name: unknown.type + 4 | } + `---- diff --git a/tests/snapshots/error_reporting__error_import_syntax_error.snap b/tests/snapshots/error_reporting__error_import_syntax_error.snap new file mode 100644 index 0000000..b4cc8d4 --- /dev/null +++ b/tests/snapshots/error_reporting__error_import_syntax_error.snap @@ -0,0 +1,13 @@ +--- +source: tests/error_reporting.rs +expression: error +--- + x parse imported IDL [ROOT]/tests/testdata/syntax_error.avdl + `-> line 6:2 unexpected '}' expected ';' or ',' + ,-[[ROOT]/tests/testdata/syntax_error.avdl:6:3] + 5 | string name + 6 | } + : | + : `-- line 6:2 unexpected '}' expected ';' or ',' + 7 | } + `---- diff --git a/tests/testdata/syntax_error.avdl b/tests/testdata/syntax_error.avdl new file mode 100644 index 0000000..8dee6d1 --- /dev/null +++ b/tests/testdata/syntax_error.avdl @@ -0,0 +1,7 @@ +@namespace("test.error") +protocol SyntaxError { + // Missing semicolon after field declaration + record BadRecord { + string name + } +} diff --git a/tests/testdata/test_import_syntax_error.avdl b/tests/testdata/test_import_syntax_error.avdl new file mode 100644 index 0000000..6b13658 --- /dev/null +++ b/tests/testdata/test_import_syntax_error.avdl @@ -0,0 +1,8 @@ +@namespace("test") +protocol ImportError { + import idl "syntax_error.avdl"; + + record MyRecord { + string value; + } +} diff --git a/tests/testdata/test_import_unknown_type.avdl b/tests/testdata/test_import_unknown_type.avdl new file mode 100644 index 0000000..e0e9bee --- /dev/null +++ b/tests/testdata/test_import_unknown_type.avdl @@ -0,0 +1,8 @@ +@namespace("test") +protocol ImportError { + import idl "unknown_type.avdl"; + + record MyRecord { + string value; + } +} diff --git a/tests/testdata/unknown_type.avdl b/tests/testdata/unknown_type.avdl new file mode 100644 index 0000000..28be8f1 --- /dev/null +++ b/tests/testdata/unknown_type.avdl @@ -0,0 +1,6 @@ +protocol Test { + record Foo { + unknown.type field; + } +} +