From f3e0ced34080afba025410ef340b4521514b8911 Mon Sep 17 00:00:00 2001 From: Dominik Nakamura Date: Tue, 24 Oct 2023 17:24:27 +0900 Subject: [PATCH] feat: verify that generics are unique and used All defined generics in structs or enums must have a unique name and be used in some way inside the definition of the element. These new verifications in the compiler ensure the rules are upheld. --- crates/stef-build/src/decode.rs | 4 +- crates/stef-build/src/definition.rs | 4 +- crates/stef-build/src/encode.rs | 4 +- ...compiler__compile@struct-many-ws.stef.snap | 29 ++- crates/stef-compiler/src/generics.rs | 172 ++++++++++++++++++ crates/stef-compiler/src/lib.rs | 13 +- crates/stef-parser/src/lib.rs | 4 +- crates/stef-parser/src/parser/generics.rs | 9 +- .../tests/inputs/struct-many-ws.stef | 5 +- .../parser__parse@enum-generics.stef.snap | 32 +++- .../parser__parse@enum-min-ws.stef.snap | 8 +- .../parser__parse@struct-generics.stef.snap | 16 +- .../parser__parse@struct-many-ws.stef.snap | 63 +++++-- .../parser__parse@struct-min-ws.stef.snap | 8 +- .../parser__parse@types-ref.stef.snap | 16 +- .../parser__print@struct-many-ws.stef.snap | 3 +- crates/stef-playground/src/lib.rs | 7 +- 17 files changed, 348 insertions(+), 49 deletions(-) create mode 100644 crates/stef-compiler/src/generics.rs diff --git a/crates/stef-build/src/decode.rs b/crates/stef-build/src/decode.rs index 52f9d97..627b8f1 100644 --- a/crates/stef-build/src/decode.rs +++ b/crates/stef-build/src/decode.rs @@ -226,7 +226,9 @@ fn compile_field_assigns(fields: &Fields<'_>) -> TokenStream { fn compile_generics(Generics(types): &Generics<'_>) -> (TokenStream, TokenStream) { (!types.is_empty()) .then(|| { - let types = types.iter().map(|ty| Ident::new(ty, Span::call_site())); + let types = types + .iter() + .map(|ty| Ident::new(ty.get(), Span::call_site())); let types2 = types.clone(); ( diff --git a/crates/stef-build/src/definition.rs b/crates/stef-build/src/definition.rs index d3fc0bb..0c078b8 100644 --- a/crates/stef-build/src/definition.rs +++ b/crates/stef-build/src/definition.rs @@ -202,7 +202,9 @@ fn compile_comment(Comment(lines): &Comment<'_>) -> TokenStream { fn compile_generics(Generics(types): &Generics<'_>) -> Option { (!types.is_empty()).then(|| { - let types = types.iter().map(|ty| Ident::new(ty, Span::call_site())); + let types = types + .iter() + .map(|ty| Ident::new(ty.get(), Span::call_site())); quote! { <#(#types,)*> } }) } diff --git a/crates/stef-build/src/encode.rs b/crates/stef-build/src/encode.rs index e9fc36c..60b53aa 100644 --- a/crates/stef-build/src/encode.rs +++ b/crates/stef-build/src/encode.rs @@ -215,7 +215,9 @@ fn compile_variant_fields(fields: &Fields<'_>) -> TokenStream { fn compile_generics(Generics(types): &Generics<'_>) -> (TokenStream, TokenStream) { (!types.is_empty()) .then(|| { - let types = types.iter().map(|ty| Ident::new(ty, Span::call_site())); + let types = types + .iter() + .map(|ty| Ident::new(ty.get(), Span::call_site())); let types2 = types.clone(); ( diff --git a/crates/stef-build/tests/snapshots/compiler__compile@struct-many-ws.stef.snap b/crates/stef-build/tests/snapshots/compiler__compile@struct-many-ws.stef.snap index 15bf327..2f5f9be 100644 --- a/crates/stef-build/tests/snapshots/compiler__compile@struct-many-ws.stef.snap +++ b/crates/stef-build/tests/snapshots/compiler__compile@struct-many-ws.stef.snap @@ -1,6 +1,6 @@ --- source: crates/stef-build/tests/compiler.rs -expression: "/// Some comment\n struct Sample<\n A,\n B\n > {\n\n a: u32 @1,\n b: bool @2,\n\n }" +expression: "/// Some comment\n struct Sample<\n T\n > {\n\n a: u32 @1,\n b: bool @2,\n c: T @3,\n }" input_file: crates/stef-parser/tests/inputs/struct-many-ws.stef --- #[allow(unused_imports)] @@ -8,15 +8,15 @@ use ::stef::buf::{Decode, Encode}; /// Some comment #[derive(Clone, Debug, PartialEq)] #[allow(clippy::module_name_repetitions, clippy::option_option)] -pub struct Sample { +pub struct Sample { pub a: u32, pub b: bool, + pub c: T, } #[automatically_derived] -impl ::stef::Encode for Sample +impl ::stef::Encode for Sample where - A: ::stef::buf::Encode, - B: ::stef::buf::Encode, + T: ::stef::buf::Encode, { #[allow( clippy::borrow_deref_ref, @@ -39,24 +39,32 @@ where ::stef::buf::encode_bool(w, self.b); }, ); + ::stef::buf::encode_field( + w, + 3, + |w| { + (self.c).encode(w); + }, + ); ::stef::buf::encode_u32(w, ::stef::buf::END_MARKER); } } #[automatically_derived] -impl ::stef::Decode for Sample +impl ::stef::Decode for Sample where - A: ::std::fmt::Debug + ::stef::buf::Decode, - B: ::std::fmt::Debug + ::stef::buf::Decode, + T: ::std::fmt::Debug + ::stef::buf::Decode, { #[allow(clippy::type_complexity, clippy::too_many_lines)] fn decode(r: &mut impl ::stef::Buf) -> ::stef::buf::Result { let mut a: Option = None; let mut b: Option = None; + let mut c: Option = None; loop { match ::stef::buf::decode_id(r)? { ::stef::buf::END_MARKER => break, 1 => a = Some(::stef::buf::decode_u32(r)?), 2 => b = Some(::stef::buf::decode_bool(r)?), + 3 => c = Some(T::decode(r)?), _ => continue, } } @@ -71,6 +79,11 @@ where id: 2, name: Some("b"), })?, + c: c + .ok_or(::stef::buf::Error::MissingField { + id: 3, + name: Some("c"), + })?, }) } } diff --git a/crates/stef-compiler/src/generics.rs b/crates/stef-compiler/src/generics.rs new file mode 100644 index 0000000..2c123ea --- /dev/null +++ b/crates/stef-compiler/src/generics.rs @@ -0,0 +1,172 @@ +use std::{collections::HashMap, ops::Range}; + +use miette::Diagnostic; +use stef_parser::{DataType, Enum, ExternalType, Fields, Generics, Span, Spanned, Struct}; +use thiserror::Error; + +#[derive(Debug, Diagnostic, Error)] +pub enum InvalidGenericType { + #[error("duplicate generic type name found")] + #[diagnostic(transparent)] + Duplicate(#[from] DuplicateGenericName), + #[error("unused generic type argument found")] + #[diagnostic(transparent)] + Unused(#[from] UnusedGeneric), +} + +#[derive(Debug, Diagnostic, Error)] +#[error("duplicate generic type name `{name}`")] +#[diagnostic(help("the names of each generic type must be unique"))] +pub struct DuplicateGenericName { + pub name: String, + #[label("first declared here")] + pub first: Range, + #[label("used here again")] + pub second: Range, +} + +#[derive(Debug, Diagnostic, Error)] +#[error("unused generic type argument `{name}`")] +#[diagnostic(help("each declared generic must be used in some way"))] +pub struct UnusedGeneric { + pub name: String, + #[label("declared here")] + pub definition: Range, +} + +/// Ensure all generics in a struct are unqiue and used. +pub fn validate_struct_generics(value: &Struct<'_>) -> Result<(), InvalidGenericType> { + validate_duplicate_generics(&value.generics)?; + + let mut unvisited = value + .generics + .0 + .iter() + .map(|gen| (gen.get(), gen.span())) + .collect::>(); + + validate_field_generics(&value.fields, &mut unvisited); + + unvisited.into_iter().next().map_or(Ok(()), |(name, span)| { + Err(UnusedGeneric { + name: name.to_owned(), + definition: span.into(), + } + .into()) + }) +} + +/// Ensure all generics in an enum are unqiue and used. +pub fn validate_enum_generics(value: &Enum<'_>) -> Result<(), InvalidGenericType> { + validate_duplicate_generics(&value.generics)?; + + let mut unvisited = value + .generics + .0 + .iter() + .map(|gen| (gen.get(), gen.span())) + .collect::>(); + + for variant in &value.variants { + validate_field_generics(&variant.fields, &mut unvisited); + } + + unvisited.into_iter().next().map_or(Ok(()), |(name, span)| { + Err(UnusedGeneric { + name: name.to_owned(), + definition: span.into(), + } + .into()) + }) +} + +/// Ensure all generic type arguments are unique within a struct or enum. +fn validate_duplicate_generics(value: &Generics<'_>) -> Result<(), DuplicateGenericName> { + let mut visited = HashMap::with_capacity(value.0.len()); + value + .0 + .iter() + .find_map(|name| { + visited + .insert(name.get(), name.span()) + .map(|first| DuplicateGenericName { + name: name.get().to_owned(), + first: first.into(), + second: name.span().into(), + }) + }) + .map_or(Ok(()), Err) +} + +/// Iterate over all the fields and mark any generic types as used when disvored as type for a +/// field. +fn validate_field_generics(value: &Fields<'_>, unvisited: &mut HashMap<&str, Span>) { + match &value { + Fields::Named(named) => { + for field in named { + visit_externals(&field.ty, &mut |external| { + if external.path.is_empty() && external.generics.is_empty() { + unvisited.remove(external.name); + } + }); + } + } + Fields::Unnamed(unnamed) => { + for field in unnamed { + visit_externals(&field.ty, &mut |external| { + if external.path.is_empty() && external.generics.is_empty() { + unvisited.remove(external.name); + } + }); + } + } + Fields::Unit => {} + } +} + +/// Iterate recursively through the data type and invoke the closure on each discovered external +/// type. +fn visit_externals(value: &DataType<'_>, visit: &mut impl FnMut(&ExternalType<'_>)) { + match value { + DataType::Bool + | DataType::U8 + | DataType::U16 + | DataType::U32 + | DataType::U64 + | DataType::U128 + | DataType::I8 + | DataType::I16 + | DataType::I32 + | DataType::I64 + | DataType::I128 + | DataType::F32 + | DataType::F64 + | DataType::String + | DataType::StringRef + | DataType::Bytes + | DataType::BytesRef + | DataType::NonZero(_) + | DataType::BoxString + | DataType::BoxBytes => {} + DataType::Vec(ty) + | DataType::HashSet(ty) + | DataType::Option(ty) + | DataType::Array(ty, _) => visit_externals(ty, visit), + DataType::HashMap(kv) => { + visit_externals(&kv.0, visit); + visit_externals(&kv.1, visit); + } + DataType::Tuple(types) => { + for ty in types { + visit_externals(ty, visit); + } + } + DataType::External(ty) => { + visit(ty); + + for ty in &ty.generics { + visit_externals(ty, visit); + } + } + } +} diff --git a/crates/stef-compiler/src/lib.rs b/crates/stef-compiler/src/lib.rs index f6d0dc6..69af040 100644 --- a/crates/stef-compiler/src/lib.rs +++ b/crates/stef-compiler/src/lib.rs @@ -1,15 +1,19 @@ #![forbid(unsafe_code)] #![deny(rust_2018_idioms, clippy::all)] #![warn(clippy::pedantic)] -#![allow(clippy::missing_errors_doc)] +#![allow(clippy::missing_errors_doc, clippy::module_name_repetitions)] pub use ids::{DuplicateFieldId, DuplicateId, DuplicateVariantId}; use miette::Diagnostic; use stef_parser::{Definition, Schema}; use thiserror::Error; -use self::names::{DuplicateFieldName, DuplicateName}; +use self::{ + generics::InvalidGenericType, + names::{DuplicateFieldName, DuplicateName}, +}; +mod generics; mod ids; mod names; @@ -21,6 +25,9 @@ pub enum Error { #[error("duplicate name found")] #[diagnostic(transparent)] DuplicateName(#[from] DuplicateName), + #[error("invalid generic type found")] + #[diagnostic(transparent)] + InvalidGeneric(#[from] InvalidGenericType), } impl From for Error { @@ -47,10 +54,12 @@ fn validate_definition(value: &Definition<'_>) -> Result<(), Error> { Definition::Struct(s) => { ids::validate_struct_ids(s)?; names::validate_struct_names(s)?; + generics::validate_struct_generics(s)?; } Definition::Enum(e) => { ids::validate_enum_ids(e)?; names::validate_enum_names(e)?; + generics::validate_enum_generics(e)?; } Definition::TypeAlias(_) | Definition::Const(_) | Definition::Import(_) => {} } diff --git a/crates/stef-parser/src/lib.rs b/crates/stef-parser/src/lib.rs index c09a683..74a5215 100644 --- a/crates/stef-parser/src/lib.rs +++ b/crates/stef-parser/src/lib.rs @@ -808,7 +808,7 @@ impl<'a> Display for ExternalType<'a> { /// /// ``` #[derive(Clone, Debug, Default, Eq, PartialEq)] -pub struct Generics<'a>(pub Vec<&'a str>); +pub struct Generics<'a>(pub Vec>); impl<'a> Display for Generics<'a> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -852,7 +852,7 @@ impl Display for Id { /// An arbitrary name of any element, which additionally carries a span into the schema to mark its /// location. -#[derive(Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq)] pub struct Name<'a> { /// Raw string value. value: &'a str, diff --git a/crates/stef-parser/src/parser/generics.rs b/crates/stef-parser/src/parser/generics.rs index ae32b6d..3fd9a3a 100644 --- a/crates/stef-parser/src/parser/generics.rs +++ b/crates/stef-parser/src/parser/generics.rs @@ -11,7 +11,7 @@ use winnow::{ }; use super::{ws, Input, Result}; -use crate::{highlight, Generics}; +use crate::{highlight, Generics, Name}; /// Encountered an invalid `<...>` generics declaration. #[derive(Debug, ParserError)] @@ -62,10 +62,15 @@ pub(super) fn parse<'i>(input: &mut Input<'i>) -> Result, ParseErro }) } -fn parse_name<'i>(input: &mut Input<'i>) -> Result<&'i str, Cause> { +fn parse_name<'i>(input: &mut Input<'i>) -> Result, Cause> { (one_of('A'..='Z'), alphanumeric0) .recognize() + .with_span() .parse_next(input) + .map(|(name, span)| Name { + value: name, + span: span.into(), + }) .map_err(|e| { e.map(|()| Cause::InvalidName { at: input.location(), diff --git a/crates/stef-parser/tests/inputs/struct-many-ws.stef b/crates/stef-parser/tests/inputs/struct-many-ws.stef index 16985c0..431ea6b 100644 --- a/crates/stef-parser/tests/inputs/struct-many-ws.stef +++ b/crates/stef-parser/tests/inputs/struct-many-ws.stef @@ -1,11 +1,10 @@ /// Some comment struct Sample< - A, - B + T > { a: u32 @1, b: bool @2, - + c: T @3, } diff --git a/crates/stef-parser/tests/snapshots/parser__parse@enum-generics.stef.snap b/crates/stef-parser/tests/snapshots/parser__parse@enum-generics.stef.snap index f14a078..36b41fe 100644 --- a/crates/stef-parser/tests/snapshots/parser__parse@enum-generics.stef.snap +++ b/crates/stef-parser/tests/snapshots/parser__parse@enum-generics.stef.snap @@ -18,10 +18,34 @@ Schema { name: "Sample", generics: Generics( [ - "A", - "B", - "C", - "D", + Name { + value: "A", + span: Span { + start: 36, + end: 37, + }, + }, + Name { + value: "B", + span: Span { + start: 39, + end: 40, + }, + }, + Name { + value: "C", + span: Span { + start: 42, + end: 43, + }, + }, + Name { + value: "D", + span: Span { + start: 45, + end: 46, + }, + }, ], ), variants: [ diff --git a/crates/stef-parser/tests/snapshots/parser__parse@enum-min-ws.stef.snap b/crates/stef-parser/tests/snapshots/parser__parse@enum-min-ws.stef.snap index 9e9aba3..b49bd3d 100644 --- a/crates/stef-parser/tests/snapshots/parser__parse@enum-min-ws.stef.snap +++ b/crates/stef-parser/tests/snapshots/parser__parse@enum-min-ws.stef.snap @@ -16,7 +16,13 @@ Schema { name: "Sample", generics: Generics( [ - "T", + Name { + value: "T", + span: Span { + start: 12, + end: 13, + }, + }, ], ), variants: [ diff --git a/crates/stef-parser/tests/snapshots/parser__parse@struct-generics.stef.snap b/crates/stef-parser/tests/snapshots/parser__parse@struct-generics.stef.snap index 5a2221f..329aba0 100644 --- a/crates/stef-parser/tests/snapshots/parser__parse@struct-generics.stef.snap +++ b/crates/stef-parser/tests/snapshots/parser__parse@struct-generics.stef.snap @@ -18,8 +18,20 @@ Schema { name: "KeyValue", generics: Generics( [ - "K", - "V", + Name { + value: "K", + span: Span { + start: 44, + end: 45, + }, + }, + Name { + value: "V", + span: Span { + start: 47, + end: 48, + }, + }, ], ), fields: Named( diff --git a/crates/stef-parser/tests/snapshots/parser__parse@struct-many-ws.stef.snap b/crates/stef-parser/tests/snapshots/parser__parse@struct-many-ws.stef.snap index 9b59673..1221cc6 100644 --- a/crates/stef-parser/tests/snapshots/parser__parse@struct-many-ws.stef.snap +++ b/crates/stef-parser/tests/snapshots/parser__parse@struct-many-ws.stef.snap @@ -18,8 +18,13 @@ Schema { name: "Sample", generics: Generics( [ - "A", - "B", + Name { + value: "T", + span: Span { + start: 58, + end: 59, + }, + }, ], ), fields: Named( @@ -31,21 +36,21 @@ Schema { name: Name { value: "a", span: Span { - start: 95, - end: 96, + start: 80, + end: 81, }, }, ty: U32, id: Id { value: 1, span: Span { - start: 108, - end: 110, + start: 93, + end: 95, }, }, span: Span { - start: 95, - end: 110, + start: 80, + end: 95, }, }, NamedField { @@ -55,21 +60,51 @@ Schema { name: Name { value: "b", span: Span { - start: 118, - end: 119, + start: 103, + end: 104, }, }, ty: Bool, id: Id { value: 2, span: Span { - start: 131, - end: 133, + start: 116, + end: 118, + }, + }, + span: Span { + start: 103, + end: 118, + }, + }, + NamedField { + comment: Comment( + [], + ), + name: Name { + value: "c", + span: Span { + start: 126, + end: 127, + }, + }, + ty: External( + ExternalType { + path: [], + name: "T", + generics: [], + }, + ), + id: Id { + value: 3, + span: Span { + start: 139, + end: 141, }, }, span: Span { - start: 118, - end: 133, + start: 126, + end: 141, }, }, ], diff --git a/crates/stef-parser/tests/snapshots/parser__parse@struct-min-ws.stef.snap b/crates/stef-parser/tests/snapshots/parser__parse@struct-min-ws.stef.snap index 96a8fa5..bcfce38 100644 --- a/crates/stef-parser/tests/snapshots/parser__parse@struct-min-ws.stef.snap +++ b/crates/stef-parser/tests/snapshots/parser__parse@struct-min-ws.stef.snap @@ -16,7 +16,13 @@ Schema { name: "Sample", generics: Generics( [ - "T", + Name { + value: "T", + span: Span { + start: 14, + end: 15, + }, + }, ], ), fields: Named( diff --git a/crates/stef-parser/tests/snapshots/parser__parse@types-ref.stef.snap b/crates/stef-parser/tests/snapshots/parser__parse@types-ref.stef.snap index a17047f..2d217cf 100644 --- a/crates/stef-parser/tests/snapshots/parser__parse@types-ref.stef.snap +++ b/crates/stef-parser/tests/snapshots/parser__parse@types-ref.stef.snap @@ -137,8 +137,20 @@ Schema { name: "KeyValue", generics: Generics( [ - "K", - "V", + Name { + value: "K", + span: Span { + start: 133, + end: 134, + }, + }, + Name { + value: "V", + span: Span { + start: 136, + end: 137, + }, + }, ], ), fields: Named( diff --git a/crates/stef-parser/tests/snapshots/parser__print@struct-many-ws.stef.snap b/crates/stef-parser/tests/snapshots/parser__print@struct-many-ws.stef.snap index 2d163bc..1192a8a 100644 --- a/crates/stef-parser/tests/snapshots/parser__print@struct-many-ws.stef.snap +++ b/crates/stef-parser/tests/snapshots/parser__print@struct-many-ws.stef.snap @@ -4,9 +4,10 @@ expression: "Schema :: parse(input.as_str()).unwrap()" input_file: crates/stef-parser/tests/inputs/struct-many-ws.stef --- /// Some comment -struct Sample { +struct Sample { a: u32 @1, b: bool @2, + c: T @3, } diff --git a/crates/stef-playground/src/lib.rs b/crates/stef-playground/src/lib.rs index 3b4e4eb..40e08d7 100644 --- a/crates/stef-playground/src/lib.rs +++ b/crates/stef-playground/src/lib.rs @@ -77,10 +77,9 @@ mod schemas { include!(concat!(env!("OUT_DIR"), "/struct-generics.rs")); } - // TODO: fix unused generic parameters - // mod struct_many_ws { - // include!(concat!(env!("OUT_DIR"), "/struct-many-ws.rs")); - // } + mod struct_many_ws { + include!(concat!(env!("OUT_DIR"), "/struct-many-ws.rs")); + } mod struct_min_ws { include!(concat!(env!("OUT_DIR"), "/struct-min-ws.rs"));