Skip to content

Commit

Permalink
feat: improve error reporting for cross-schema errors
Browse files Browse the repository at this point in the history
Type resolution that goes across a single file now shows code snippets
for both the use side and declaration side to better pinpoint the
problem.
  • Loading branch information
dnaka91 committed Nov 2, 2023
1 parent 62feaa0 commit c380ace
Show file tree
Hide file tree
Showing 49 changed files with 524 additions and 144 deletions.
4 changes: 2 additions & 2 deletions crates/stef-benches/benches/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ fn main() {
#[divan::bench(consts = [1, 10, 100, 1000])]
fn validate_large_schema<const N: usize>(bencher: Bencher) {
let schema = stef_benches::generate_schema(N);
let schema = stef_parser::Schema::parse(&schema).unwrap();
let schema = stef_parser::Schema::parse(&schema, None).unwrap();
stef_compiler::validate_schema(&schema).unwrap();

bencher.bench(|| stef_compiler::validate_schema(black_box(&schema)))
Expand All @@ -19,7 +19,7 @@ fn validate_large_schema<const N: usize>(bencher: Bencher) {
#[divan::bench(consts = [1, 10, 100, 1000])]
fn resolve_large_schema<const N: usize>(bencher: Bencher) {
let schema = stef_benches::generate_schema(N);
let schema = stef_parser::Schema::parse(&schema).unwrap();
let schema = stef_parser::Schema::parse(&schema, None).unwrap();
stef_compiler::validate_schema(&schema).unwrap();

let list = &[("bench", black_box(&schema))];
Expand Down
10 changes: 5 additions & 5 deletions crates/stef-benches/benches/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,23 +41,23 @@ fn basic(bencher: Bencher) {
type SampleTyped = SampleStruct<bool, string>;
"#};

stef_parser::Schema::parse(input).unwrap();
stef_parser::Schema::parse(input, None).unwrap();

bencher.bench(|| stef_parser::Schema::parse(black_box(input)));
bencher.bench(|| stef_parser::Schema::parse(black_box(input), None));
}

#[divan::bench(consts = [1, 10, 100, 1000])]
fn large_schema<const N: usize>(bencher: Bencher) {
let schema = stef_benches::generate_schema(N);
stef_parser::Schema::parse(&schema).unwrap();
stef_parser::Schema::parse(&schema, None).unwrap();

bencher.bench(|| stef_parser::Schema::parse(black_box(&schema)))
bencher.bench(|| stef_parser::Schema::parse(black_box(&schema), None))
}

#[divan::bench(consts = [1, 10, 100, 1000])]
fn print<const N: usize>(bencher: Bencher) {
let schema = stef_benches::generate_schema(N);
let schema = stef_parser::Schema::parse(&schema).unwrap();
let schema = stef_parser::Schema::parse(&schema, None).unwrap();

bencher.bench(|| black_box(&schema).to_string())
}
2 changes: 1 addition & 1 deletion crates/stef-build/src/definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use stef_parser::{

use super::{decode, encode};

pub fn compile_schema(Schema { definitions }: &Schema<'_>) -> TokenStream {
pub fn compile_schema(Schema { definitions, .. }: &Schema<'_>) -> TokenStream {
let definitions = definitions.iter().map(compile_definition);

quote! {
Expand Down
7 changes: 3 additions & 4 deletions crates/stef-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,13 @@ pub fn compile(schemas: &[impl AsRef<str>], _includes: &[impl AsRef<Path>]) -> R
for (path, input) in &inputs {
let stem = path.file_stem().unwrap().to_str().unwrap();

let schema = Schema::parse(input).map_err(|e| Error::Parse {
report: e.with_source_code(NamedSource::new(path.display().to_string(), input.clone())),
let schema = Schema::parse(input, Some(path)).map_err(|e| Error::Parse {
report: Report::new(e),
file: path.clone(),
})?;

stef_compiler::validate_schema(&schema).map_err(|e| Error::Compile {
report: Report::new(e)
.with_source_code(NamedSource::new(path.display().to_string(), input.clone())),
report: Report::new(e),
file: path.clone(),
})?;

Expand Down
16 changes: 13 additions & 3 deletions crates/stef-build/tests/compiler.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
use std::fs;
use std::{
fs,
path::{Path, PathBuf},
};

use insta::{assert_snapshot, glob, with_settings};
use stef_parser::Schema;

fn strip_path(path: &Path) -> PathBuf {
path.strip_prefix(concat!(env!("CARGO_MANIFEST_DIR"), "/tests/inputs"))
.or_else(|_| path.strip_prefix(concat!(env!("CARGO_MANIFEST_DIR"), "/tests/inputs_extra")))
.unwrap()
.to_owned()
}

#[test]
fn compile_schema() {
glob!("inputs/*.stef", |path| {
let input = fs::read_to_string(path).unwrap();
let value = Schema::parse(input.as_str()).unwrap();
let value = Schema::parse(input.as_str(), Some(&strip_path(path))).unwrap();
let value = stef_build::compile_schema(&value);
let value = prettyplease::unparse(&syn::parse2(value.clone()).unwrap());

Expand All @@ -24,7 +34,7 @@ fn compile_schema() {
fn compile_schema_extra() {
glob!("inputs_extra/*.stef", |path| {
let input = fs::read_to_string(path).unwrap();
let value = Schema::parse(input.as_str()).unwrap();
let value = Schema::parse(input.as_str(), Some(&strip_path(path))).unwrap();
let value = stef_build::compile_schema(&value);
let value = prettyplease::unparse(&syn::parse2(value.clone()).unwrap());

Expand Down
18 changes: 10 additions & 8 deletions crates/stef-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ fn check(patterns: Vec<String>) -> Result<()> {
.into_diagnostic()
.wrap_err_with(|| format!("Failed reading {entry:?}"))?;

if let Err(e) = Schema::parse(&buf).wrap_err("Failed parsing schema file") {
if let Err(e) = Schema::parse(&buf, Some(&entry)).wrap_err("Failed parsing schema file")
{
eprintln!("{e:?}");
}
}
Expand All @@ -68,13 +69,14 @@ fn format(patterns: Vec<String>) -> Result<()> {
{
let entry = entry.into_diagnostic().wrap_err("Failed reading entry")?;
let buf = fs::read_to_string(&entry).into_diagnostic()?;
let schema = match Schema::parse(&buf).wrap_err("Failed parsing schema file") {
Ok(schema) => schema,
Err(e) => {
eprintln!("{e:?}");
continue;
}
};
let schema =
match Schema::parse(&buf, Some(&entry)).wrap_err("Failed parsing schema file") {
Ok(schema) => schema,
Err(e) => {
eprintln!("{e:?}");
continue;
}
};

let formatted = schema.to_string();

Expand Down
125 changes: 92 additions & 33 deletions crates/stef-compiler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,20 @@
#![warn(clippy::pedantic)]
#![allow(clippy::missing_errors_doc, clippy::module_name_repetitions)]

use std::{
error::Error,
fmt::{self, Display},
};

pub use ids::{DuplicateFieldId, DuplicateId, DuplicateVariantId};
use miette::Diagnostic;
use miette::{Diagnostic, NamedSource};
use stef_parser::{Definition, Schema};
use thiserror::Error;

use self::{
generics::InvalidGenericType,
names::{DuplicateFieldName, DuplicateName},
resolve::{ResolveImport, ResolveLocal, ResolveRemote},
resolve::ResolveError,
};

mod generics;
Expand All @@ -21,7 +26,7 @@ mod names;
mod resolve;

#[derive(Debug, Diagnostic, Error)]
pub enum Error {
pub enum ValidationError {
#[error("duplicate ID found")]
#[diagnostic(transparent)]
DuplicateId(#[from] DuplicateId),
Expand All @@ -31,47 +36,26 @@ pub enum Error {
#[error("invalid generic type found")]
#[diagnostic(transparent)]
InvalidGeneric(#[from] InvalidGenericType),
#[error("type resolution failed")]
#[diagnostic(transparent)]
Resolve(#[from] resolve::ResolveError),
}

impl From<DuplicateFieldId> for Error {
impl From<DuplicateFieldId> for ValidationError {
fn from(v: DuplicateFieldId) -> Self {
Self::DuplicateId(v.into())
}
}

impl From<DuplicateFieldName> for Error {
impl From<DuplicateFieldName> for ValidationError {
fn from(v: DuplicateFieldName) -> Self {
Self::DuplicateName(v.into())
}
}

impl From<ResolveLocal> for Error {
fn from(v: ResolveLocal) -> Self {
Self::Resolve(v.into())
}
}

impl From<ResolveImport> for Error {
fn from(v: ResolveImport) -> Self {
Self::Resolve(v.into())
}
}

impl From<ResolveRemote> for Error {
fn from(v: ResolveRemote) -> Self {
Self::Resolve(v.into())
}
}

pub fn validate_schema(value: &Schema<'_>) -> Result<(), Error> {
pub fn validate_schema(value: &Schema<'_>) -> Result<(), ValidationError> {
names::validate_names_in_module(&value.definitions)?;
value.definitions.iter().try_for_each(validate_definition)
}

fn validate_definition(value: &Definition<'_>) -> Result<(), Error> {
fn validate_definition(value: &Definition<'_>) -> Result<(), ValidationError> {
match value {
Definition::Module(m) => {
names::validate_names_in_module(&m.definitions)?;
Expand All @@ -93,20 +77,95 @@ fn validate_definition(value: &Definition<'_>) -> Result<(), Error> {
Ok(())
}

pub fn resolve_schemas(values: &[(&str, &Schema<'_>)]) -> Result<(), Error> {
#[derive(Debug)]
pub struct ResolutionError {
source_code: NamedSource,
cause: resolve::ResolveError,
}

impl Error for ResolutionError {
fn source(&self) -> Option<&(dyn Error + 'static)> {
Some(&self.cause)
}
}

impl Display for ResolutionError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str("type resolution failed")
}
}

impl Diagnostic for ResolutionError {
fn code<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
self.cause.code()
}

fn severity(&self) -> Option<miette::Severity> {
self.cause.severity()
}

fn help<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
self.cause.help()
}

fn url<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
self.cause.url()
}

fn source_code(&self) -> Option<&dyn miette::SourceCode> {
Some(&self.source_code)
}

fn labels(&self) -> Option<Box<dyn Iterator<Item = miette::LabeledSpan> + '_>> {
self.cause.labels()
}

fn related<'a>(&'a self) -> Option<Box<dyn Iterator<Item = &'a dyn Diagnostic> + 'a>> {
self.cause.related()
}

fn diagnostic_source(&self) -> Option<&dyn Diagnostic> {
self.cause.diagnostic_source()
}
}

pub fn resolve_schemas(values: &[(&str, &Schema<'_>)]) -> Result<(), ResolutionError> {
let modules = values
.iter()
.map(|(name, value)| (*name, resolve::resolve_types(name, value)))
.map(|(name, schema)| (*name, resolve::resolve_types(name, schema)))
.collect::<Vec<_>>();

for (_, module) in &modules {
for (schema, module) in modules
.iter()
.enumerate()
.map(|(i, (_, module))| (values[i].1, module))
{
let mut missing = Vec::new();
resolve::resolve_module_types(module, &mut missing);

let imports = resolve::resolve_module_imports(module, &modules)?;
let imports =
resolve::resolve_module_imports(module, &modules).map_err(|e| ResolutionError {
source_code: NamedSource::new(
schema
.path
.as_ref()
.map_or_else(|| "<unknown>".to_owned(), |p| p.display().to_string()),
schema.source.to_owned(),
),
cause: ResolveError::Import(e),
})?;

for ty in missing {
resolve::resolve_type_remotely(ty, &imports)?;
resolve::resolve_type_remotely(ty, &imports).map_err(|e| ResolutionError {
source_code: NamedSource::new(
schema
.path
.as_ref()
.map_or_else(|| "<unknown>".to_owned(), |p| p.display().to_string()),
schema.source.to_owned(),
),
cause: e,
})?;
}
}

Expand Down
Loading

0 comments on commit c380ace

Please sign in to comment.