From 91e0e2493af11cc6e9de01a51c93ea959bc19341 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Tue, 6 Aug 2024 11:45:42 +0200 Subject: [PATCH] Provide better errors on unknown type in cyclic definitions. Let's consider the following case: ``` type Var = Integer type Vars = List ``` This incorrectly reports an infinite cycle; due to the inability to properly type-check `Var` which is also a dependent var of `Vars`. Yet the real issue here being that `Integer` is an unknown type. This commit also upgrades miette to 7.2.0, so that we can also display a better error output when the problem is actually a cycle. --- Cargo.lock | 104 +++++++++--------- Cargo.toml | 1 + crates/aiken-lang/Cargo.toml | 2 +- crates/aiken-lang/src/ast.rs | 2 +- crates/aiken-lang/src/tipo/environment.rs | 93 ++++++++-------- crates/aiken-lang/src/tipo/error.rs | 11 +- crates/aiken-lsp/Cargo.toml | 2 +- crates/aiken-project/Cargo.toml | 2 +- crates/aiken-project/src/blueprint/error.rs | 2 +- ...lueprint__validator__tests__free_vars.snap | 1 + ...ests__opaque_singleton_multi_variants.snap | 1 + ...tor__tests__opaque_singleton_variants.snap | 1 + crates/aiken-project/src/error.rs | 15 ++- crates/aiken-project/src/lib.rs | 6 +- ...export__tests__cannot_export_generics.snap | 1 + ...t__export__tests__illegal_opaque_type.snap | 1 + crates/aiken/Cargo.toml | 2 +- 17 files changed, 119 insertions(+), 128 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 17bc2973..39dbb7d4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -62,10 +62,10 @@ dependencies = [ "ignore", "indoc", "inquire", - "miette", + "miette 7.2.0", "num-bigint", "ordinal", - "owo-colors", + "owo-colors 3.5.0", "pallas-codec", "pallas-primitives", "pallas-traverse", @@ -88,10 +88,10 @@ dependencies = [ "indoc", "insta", "itertools", - "miette", + "miette 7.2.0", "num-bigint", "ordinal", - "owo-colors", + "owo-colors 3.5.0", "pallas-primitives", "petgraph", "pretty_assertions", @@ -113,8 +113,8 @@ dependencies = [ "itertools", "lsp-server", "lsp-types", - "miette", - "owo-colors", + "miette 7.2.0", + "owo-colors 3.5.0", "serde", "serde_json", "thiserror", @@ -142,10 +142,10 @@ dependencies = [ "indoc", "insta", "itertools", - "miette", + "miette 7.2.0", "notify", "num-bigint", - "owo-colors", + "owo-colors 3.5.0", "pallas-addresses", "pallas-codec", "pallas-crypto", @@ -546,7 +546,7 @@ dependencies = [ "anstyle", "clap_lex", "strsim", - "terminal_size 0.3.0", + "terminal_size", "unicase", "unicode-width", ] @@ -1425,17 +1425,6 @@ version = "2.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f518f335dce6725a761382244631d86cf0ccb2863413590b31338feb467f9c3" -[[package]] -name = "is-terminal" -version = "0.4.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f23ff5ef2b80d608d61efee834934d862cd92461afc0560dedf493e4c033738b" -dependencies = [ - "hermit-abi 0.3.9", - "libc", - "windows-sys 0.52.0", -] - [[package]] name = "is_ci" version = "1.2.0" @@ -1634,17 +1623,28 @@ name = "miette" version = "5.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "59bb584eaeeab6bd0226ccf3509a69d7936d148cf3d036ad350abe35e8c6856e" +dependencies = [ + "miette-derive 5.10.0", + "once_cell", + "thiserror", + "unicode-width", +] + +[[package]] +name = "miette" +version = "7.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4edc8853320c2a0dab800fbda86253c8938f6ea88510dc92c5f1ed20e794afc1" dependencies = [ "backtrace", "backtrace-ext", - "is-terminal", - "miette-derive", - "once_cell", - "owo-colors", - "supports-color 2.1.0", + "cfg-if", + "miette-derive 7.2.0", + "owo-colors 4.0.0", + "supports-color 3.0.0", "supports-hyperlinks", "supports-unicode", - "terminal_size 0.1.17", + "terminal_size", "textwrap", "thiserror", "unicode-width", @@ -1661,6 +1661,17 @@ dependencies = [ "syn 2.0.60", ] +[[package]] +name = "miette-derive" +version = "7.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dcf09caffaac8068c346b6df2a7fc27a177fd20b39421a39ce0a211bde679a6c" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.60", +] + [[package]] name = "mime" version = "0.3.17" @@ -1904,6 +1915,12 @@ dependencies = [ "supports-color 1.3.1", ] +[[package]] +name = "owo-colors" +version = "4.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "caff54706df99d2a78a5a4e3455ff45448d81ef1bb63c22cd14052ca0e993a3f" + [[package]] name = "pallas-addresses" version = "0.22.0" @@ -2764,31 +2781,24 @@ dependencies = [ [[package]] name = "supports-color" -version = "2.1.0" +version = "3.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d6398cde53adc3c4557306a96ce67b302968513830a77a95b2b17305d9719a89" +checksum = "9829b314621dfc575df4e409e79f9d6a66a3bd707ab73f23cb4aa3a854ac854f" dependencies = [ - "is-terminal", "is_ci", ] [[package]] name = "supports-hyperlinks" -version = "2.1.0" +version = "3.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f84231692eb0d4d41e4cdd0cabfdd2e6cd9e255e65f80c9aa7c98dd502b4233d" -dependencies = [ - "is-terminal", -] +checksum = "2c0a1e5168041f5f3ff68ff7d95dcb9c8749df29f6e7e89ada40dd4c9de404ee" [[package]] name = "supports-unicode" -version = "2.1.0" +version = "3.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f850c19edd184a205e883199a261ed44471c81e39bd95b1357f5febbef00e77a" -dependencies = [ - "is-terminal", -] +checksum = "b7401a30af6cb5818bb64852270bb722533397edcfc7344954a38f420819ece2" [[package]] name = "syn" @@ -2851,16 +2861,6 @@ dependencies = [ "windows-sys 0.52.0", ] -[[package]] -name = "terminal_size" -version = "0.1.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "633c1a546cee861a1a6d0dc69ebeca693bf4296661ba7852b9d21d159e0506df" -dependencies = [ - "libc", - "winapi", -] - [[package]] name = "terminal_size" version = "0.3.0" @@ -2873,9 +2873,9 @@ dependencies = [ [[package]] name = "textwrap" -version = "0.15.2" +version = "0.16.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7b3e525a49ec206798b40326a44121291b530c963cfb01018f63e135bac543d" +checksum = "23d434d3f8967a09480fb04132ebe0a3e088c173e6d0ee7897abbdf4eab0f8b9" dependencies = [ "smawk", "unicode-linebreak", @@ -3153,7 +3153,7 @@ dependencies = [ "indoc", "itertools", "k256", - "miette", + "miette 5.10.0", "num-bigint", "num-integer", "num-traits", diff --git a/Cargo.toml b/Cargo.toml index a0397c13..13864e29 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,7 @@ x86_64-unknown-linux-gnu = "ubuntu-22.04" [workspace.dependencies] walkdir = "2.3.2" insta = { version = "1.30.0", features = ["yaml", "json"] } +miette = { version = "7.2.0", features = ["fancy"] } pallas-addresses = "0.22.0" pallas-codec = { version = "0.22.0", features = ["num-bigint"] } pallas-crypto = "0.22.0" diff --git a/crates/aiken-lang/Cargo.toml b/crates/aiken-lang/Cargo.toml index 6d75c769..799c980f 100644 --- a/crates/aiken-lang/Cargo.toml +++ b/crates/aiken-lang/Cargo.toml @@ -19,7 +19,7 @@ hex = "0.4.3" indexmap = "1.9.2" indoc = "2.0.1" itertools = "0.10.5" -miette = "5.9.0" +miette.workspace = true num-bigint = "0.4.3" ordinal = "0.3.2" owo-colors = { version = "3.5.0", features = ["supports-colors"] } diff --git a/crates/aiken-lang/src/ast.rs b/crates/aiken-lang/src/ast.rs index 3f462192..881fed84 100644 --- a/crates/aiken-lang/src/ast.rs +++ b/crates/aiken-lang/src/ast.rs @@ -1921,7 +1921,7 @@ pub struct Span { impl From for miette::SourceSpan { fn from(span: Span) -> Self { - Self::new(span.start.into(), (span.end - span.start).into()) + Self::new(span.start.into(), span.end - span.start) } } diff --git a/crates/aiken-lang/src/tipo/environment.rs b/crates/aiken-lang/src/tipo/environment.rs index 2e8febf9..87760a9a 100644 --- a/crates/aiken-lang/src/tipo/environment.rs +++ b/crates/aiken-lang/src/tipo/environment.rs @@ -1,5 +1,5 @@ use super::{ - error::{Error, Snippet, Warning}, + error::{Error, Warning}, exhaustive::{simplify, Matrix, PatternStack}, hydrator::Hydrator, AccessorsMap, RecordAccessor, Type, TypeConstructor, TypeInfo, TypeVar, ValueConstructor, @@ -972,7 +972,7 @@ impl<'a> Environment<'a> { ) -> Result<(), Error> { let known_types_before = names.keys().copied().collect::>(); - let mut error = None; + let mut errors = vec![]; let mut remaining_definitions = vec![]; // in case we failed at registering a type-definition, we backtrack and @@ -989,60 +989,57 @@ impl<'a> Environment<'a> { // a cycle). for def in definitions { if let Err(e) = self.register_type(def, module, hydrators, names) { - error = Some(e); + let type_name = match def { + Definition::TypeAlias(TypeAlias { alias, .. }) => { + names.remove(alias.as_str()); + Some(alias) + } + Definition::DataType(DataType { name, .. }) => Some(name), + _ => None, + }; + errors.push((type_name, e)); remaining_definitions.push(def); - if let Definition::TypeAlias(TypeAlias { alias, .. }) = def { - names.remove(alias.as_str()); - } }; } - match error { - None => Ok(()), - Some(e) => { - let known_types_after = names.keys().copied().collect::>(); - if known_types_before == known_types_after { - let unknown_name = match e { - Error::UnknownType { ref name, .. } => name, - _ => "", - }; - let mut is_cyclic = false; - let unknown_types = remaining_definitions - .into_iter() - .filter_map(|def| match def { - Definition::TypeAlias(TypeAlias { - alias, location, .. - }) => { - is_cyclic = is_cyclic || alias == unknown_name; - Some(Snippet { - location: location.to_owned(), - }) - } - Definition::DataType(DataType { name, location, .. }) => { - is_cyclic = is_cyclic || name == unknown_name; - Some(Snippet { - location: location.to_owned(), - }) - } - Definition::Fn { .. } - | Definition::Validator { .. } - | Definition::Use { .. } - | Definition::ModuleConstant { .. } - | Definition::Test { .. } => None, - }) - .collect::>(); + if errors.is_empty() { + return Ok(()); + } - if is_cyclic { - Err(Error::CyclicTypeDefinitions { - errors: unknown_types, - }) - } else { - Err(e) - } + let known_types_after = names.keys().copied().collect::>(); + if known_types_before == known_types_after { + let (type_definitions, mut unknowns): (Vec<_>, Vec<_>) = errors.into_iter().unzip(); + + let first_error = unknowns.first().cloned(); + + unknowns.retain(|err| { + if let Error::UnknownType { ref name, .. } = err { + !type_definitions.contains(&Some(name)) } else { - self.register_types(remaining_definitions, module, hydrators, names) + false } + }); + + if unknowns.is_empty() { + let cycle = remaining_definitions + .iter() + .filter_map(|def| match def { + Definition::TypeAlias(TypeAlias { location, .. }) + | Definition::DataType(DataType { location, .. }) => Some(*location), + Definition::Fn { .. } + | Definition::Validator { .. } + | Definition::Use { .. } + | Definition::ModuleConstant { .. } + | Definition::Test { .. } => None, + }) + .collect::>(); + + Err(Error::CyclicTypeDefinitions { cycle }) + } else { + Err(first_error.unwrap()) } + } else { + self.register_types(remaining_definitions, module, hydrators, names) } } diff --git a/crates/aiken-lang/src/tipo/error.rs b/crates/aiken-lang/src/tipo/error.rs index a07f035d..041573cf 100644 --- a/crates/aiken-lang/src/tipo/error.rs +++ b/crates/aiken-lang/src/tipo/error.rs @@ -16,13 +16,6 @@ use owo_colors::{ }; use std::{collections::HashMap, fmt::Display, rc::Rc}; -#[derive(Debug, thiserror::Error, Diagnostic, Clone)] -#[error("Something is possibly wrong here...")] -pub struct Snippet { - #[label] - pub location: Span, -} - #[derive(Debug, Clone, thiserror::Error)] #[error( "I don't know some of the labels used in this expression. I've highlighted them just below." @@ -100,8 +93,8 @@ pub enum Error { #[diagnostic(url("https://aiken-lang.org/language-tour/custom-types#type-aliases"))] #[diagnostic(code("cycle"))] CyclicTypeDefinitions { - #[related] - errors: Vec, + #[label(collection, "part of a cycle")] + cycle: Vec, }, #[error( diff --git a/crates/aiken-lsp/Cargo.toml b/crates/aiken-lsp/Cargo.toml index 4ae6b397..7502b6b2 100644 --- a/crates/aiken-lsp/Cargo.toml +++ b/crates/aiken-lsp/Cargo.toml @@ -15,7 +15,7 @@ indoc = "2.0.1" itertools = "0.10.5" lsp-server = "0.7.0" lsp-types = "0.94.0" -miette = "5.5.0" +miette.workspace = true owo-colors = { version = "3.5.0", features = ["supports-colors"] } serde = "1.0.152" serde_json = "1.0.94" diff --git a/crates/aiken-project/Cargo.toml b/crates/aiken-project/Cargo.toml index 0fd6c645..257bb6a7 100644 --- a/crates/aiken-project/Cargo.toml +++ b/crates/aiken-project/Cargo.toml @@ -26,7 +26,7 @@ hex = "0.4.3" ignore = "0.4.20" indexmap = "1.9.2" itertools = "0.10.5" -miette = { version = "5.9.0", features = ["fancy"] } +miette.workspace = true notify = "6.1.1" num-bigint = "0.4.4" owo-colors = { version = "3.5.0", features = ["supports-colors"] } diff --git a/crates/aiken-project/src/blueprint/error.rs b/crates/aiken-project/src/blueprint/error.rs index 4ca1bb6f..da3bf8a2 100644 --- a/crates/aiken-project/src/blueprint/error.rs +++ b/crates/aiken-project/src/blueprint/error.rs @@ -19,7 +19,7 @@ pub enum Error { #[label("invalid validator's boundary")] location: Span, #[source_code] - source_code: NamedSource, + source_code: NamedSource, }, #[error("Invalid or missing project's blueprint file.")] diff --git a/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__free_vars.snap b/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__free_vars.snap index 8f64d017..708547f5 100644 --- a/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__free_vars.snap +++ b/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__free_vars.snap @@ -20,5 +20,6 @@ Schema { source_code: NamedSource { name: "", source: "", + language: None, , } diff --git a/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__opaque_singleton_multi_variants.snap b/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__opaque_singleton_multi_variants.snap index e90fb15b..0b14e043 100644 --- a/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__opaque_singleton_multi_variants.snap +++ b/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__opaque_singleton_multi_variants.snap @@ -20,5 +20,6 @@ Schema { source_code: NamedSource { name: "", source: "", + language: None, , } diff --git a/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__opaque_singleton_variants.snap b/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__opaque_singleton_variants.snap index 2fc9e2ff..ba397f99 100644 --- a/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__opaque_singleton_variants.snap +++ b/crates/aiken-project/src/blueprint/snapshots/aiken_project__blueprint__validator__tests__opaque_singleton_variants.snap @@ -51,5 +51,6 @@ Schema { source_code: NamedSource { name: "", source: "", + language: None, , } diff --git a/crates/aiken-project/src/error.rs b/crates/aiken-project/src/error.rs index 5f0ebe33..05e030e3 100644 --- a/crates/aiken-project/src/error.rs +++ b/crates/aiken-project/src/error.rs @@ -16,7 +16,6 @@ use owo_colors::{ use std::{ fmt::{Debug, Display}, io, - ops::Deref, path::{Path, PathBuf}, }; use zip::result::ZipError; @@ -62,7 +61,7 @@ pub enum Error { TomlLoading { path: PathBuf, src: String, - named: Box, + named: Box>, location: Option, help: String, }, @@ -77,7 +76,7 @@ pub enum Error { Parse { path: PathBuf, src: String, - named: NamedSource, + named: Box>, #[source] error: Box, }, @@ -86,7 +85,7 @@ pub enum Error { Type { path: PathBuf, src: String, - named: NamedSource, + named: NamedSource, #[source] error: tipo::error::Error, }, @@ -156,7 +155,7 @@ impl Error { errors.push(Error::Parse { path: path.into(), src: src.to_string(), - named: NamedSource::new(path.display().to_string(), src.to_string()), + named: NamedSource::new(path.display().to_string(), src.to_string()).into(), error: error.into(), }); } @@ -451,11 +450,11 @@ impl Diagnostic for Error { Error::ExportNotFound { .. } => None, Error::Blueprint(e) => e.source_code(), Error::NoDefaultEnvironment { .. } => None, - Error::Parse { named, .. } => Some(named), + Error::Parse { named, .. } => Some(named.as_ref()), Error::Type { named, .. } => Some(named), Error::StandardIo(_) => None, Error::MissingManifest { .. } => None, - Error::TomlLoading { named, .. } => Some(named.deref()), + Error::TomlLoading { named, .. } => Some(named.as_ref()), Error::Format { .. } => None, Error::TestFailure { .. } => None, Error::Http(_) => None, @@ -538,7 +537,7 @@ pub enum Warning { Type { path: PathBuf, src: String, - named: NamedSource, + named: NamedSource, #[source] warning: tipo::error::Warning, }, diff --git a/crates/aiken-project/src/lib.rs b/crates/aiken-project/src/lib.rs index 5e6bd222..74c086eb 100644 --- a/crates/aiken-project/src/lib.rs +++ b/crates/aiken-project/src/lib.rs @@ -762,7 +762,7 @@ where .map(|(path, src, named, error)| Error::Parse { path, src, - named, + named: named.into(), error, }) .collect(); @@ -812,10 +812,6 @@ where &mut self.data_types, )?; - if name == "foo" { - println!("{:#?}", checked_module.ast); - } - if our_modules.contains(checked_module.name.as_str()) { self.warnings.extend(warnings); } diff --git a/crates/aiken-project/src/snapshots/aiken_project__export__tests__cannot_export_generics.snap b/crates/aiken-project/src/snapshots/aiken_project__export__tests__cannot_export_generics.snap index 32ab8881..d8fddbfc 100644 --- a/crates/aiken-project/src/snapshots/aiken_project__export__tests__cannot_export_generics.snap +++ b/crates/aiken-project/src/snapshots/aiken_project__export__tests__cannot_export_generics.snap @@ -20,5 +20,6 @@ Schema { source_code: NamedSource { name: "", source: "", + language: None, , } diff --git a/crates/aiken-project/src/snapshots/aiken_project__export__tests__illegal_opaque_type.snap b/crates/aiken-project/src/snapshots/aiken_project__export__tests__illegal_opaque_type.snap index b1f8008f..5c6ae132 100644 --- a/crates/aiken-project/src/snapshots/aiken_project__export__tests__illegal_opaque_type.snap +++ b/crates/aiken-project/src/snapshots/aiken_project__export__tests__illegal_opaque_type.snap @@ -20,5 +20,6 @@ Schema { source_code: NamedSource { name: "", source: "", + language: None, , } diff --git a/crates/aiken/Cargo.toml b/crates/aiken/Cargo.toml index f93858f9..1042a80c 100644 --- a/crates/aiken/Cargo.toml +++ b/crates/aiken/Cargo.toml @@ -34,7 +34,7 @@ hex = "0.4.3" ignore = "0.4.20" indoc = "2.0" inquire = "0.6.2" -miette = { version = "5.5.0", features = ["fancy"] } +miette.workspace = true num-bigint = "0.4.3" ordinal = "0.3.2" owo-colors = { version = "3.5.0", features = ["supports-colors"] }