From 2588a7d15dab4be7743f57f3603ec0e5556cfb85 Mon Sep 17 00:00:00 2001 From: Lili Zoey Zyli Date: Wed, 16 Oct 2024 00:31:44 +0200 Subject: [PATCH] More accurately provide spans to errors in the `GodotClass` macro --- godot-macros/src/class/data_models/field.rs | 22 +++- .../src/class/data_models/field_export.rs | 65 +++++---- .../src/class/data_models/field_var.rs | 19 ++- .../src/class/data_models/property.rs | 1 + godot-macros/src/class/derive_godot_class.rs | 123 ++++++++++++------ 5 files changed, 162 insertions(+), 68 deletions(-) diff --git a/godot-macros/src/class/data_models/field.rs b/godot-macros/src/class/data_models/field.rs index 0924fae09..a96ec8454 100644 --- a/godot-macros/src/class/data_models/field.rs +++ b/godot-macros/src/class/data_models/field.rs @@ -6,17 +6,19 @@ */ use crate::class::{FieldExport, FieldVar}; -use proc_macro2::{Ident, TokenStream}; +use proc_macro2::{Ident, Span, TokenStream}; +use quote::ToTokens; pub struct Field { pub name: Ident, pub ty: venial::TypeExpr, - pub default_val: Option, + pub default_val: Option, pub var: Option, pub export: Option, pub is_onready: bool, #[cfg(feature = "register-docs")] pub attributes: Vec, + pub span: Span, } impl Field { @@ -30,6 +32,7 @@ impl Field { is_onready: false, #[cfg(feature = "register-docs")] attributes: field.attributes.clone(), + span: field.span(), } } } @@ -43,4 +46,19 @@ pub struct Fields { /// Deprecation warnings. pub deprecations: Vec, + + /// Errors during macro evaluation that shouldn't abort the execution of the macro. + pub errors: Vec, +} + +#[derive(Clone)] +pub struct FieldDefault { + pub default_val: TokenStream, + pub span: Span, +} + +impl ToTokens for FieldDefault { + fn to_tokens(&self, tokens: &mut TokenStream) { + self.default_val.to_tokens(tokens) + } } diff --git a/godot-macros/src/class/data_models/field_export.rs b/godot-macros/src/class/data_models/field_export.rs index f3b0578af..f1ba38e4c 100644 --- a/godot-macros/src/class/data_models/field_export.rs +++ b/godot-macros/src/class/data_models/field_export.rs @@ -5,15 +5,32 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use proc_macro2::{Ident, TokenStream}; +use proc_macro2::{Ident, Span, TokenStream}; use quote::quote; use std::collections::{HashMap, HashSet}; use crate::util::{KvParser, ListParser}; use crate::ParseResult; +pub struct FieldExport { + pub export_type: ExportType, + pub span: Span, +} + +impl FieldExport { + pub(crate) fn new_from_kv(parser: &mut KvParser) -> ParseResult { + let span = parser.span(); + let export_type = ExportType::new_from_kv(parser)?; + Ok(Self { export_type, span }) + } + + pub fn to_export_hint(&self) -> Option { + self.export_type.to_export_hint() + } +} + /// Store info from `#[export]` attribute. -pub enum FieldExport { +pub enum ExportType { /// ### GDScript annotations /// - `@export` /// @@ -121,7 +138,7 @@ pub enum FieldExport { ColorNoAlpha, } -impl FieldExport { +impl ExportType { /// Parse an `#[export(...)]` attribute. /// /// The translation from GDScript annotations to rust attributes is given by: @@ -253,10 +270,10 @@ impl FieldExport { return Ok(Self::ColorNoAlpha); } - Ok(FieldExport::Default) + Ok(Self::Default) } - fn new_range_list(mut parser: ListParser) -> ParseResult { + fn new_range_list(mut parser: ListParser) -> ParseResult { const FLAG_OPTIONS: [&str; 7] = [ "or_greater", "or_less", @@ -299,7 +316,7 @@ impl FieldExport { parser.finish()?; - Ok(FieldExport::Range { + Ok(Self::Range { min, max, step, @@ -374,12 +391,12 @@ macro_rules! quote_export_func { } } -impl FieldExport { +impl ExportType { pub fn to_export_hint(&self) -> Option { match self { - FieldExport::Default => None, + Self::Default => None, - FieldExport::Range { + Self::Range { min, max, step, @@ -416,7 +433,7 @@ impl FieldExport { }) } - FieldExport::Enum { variants } => { + Self::Enum { variants } => { let variants = variants.iter().map(ValueWithKey::to_tuple_expression); quote_export_func! { @@ -424,14 +441,14 @@ impl FieldExport { } } - FieldExport::ExpEasing { + Self::ExpEasing { attenuation, positive_only, } => quote_export_func! { export_exp_easing(#attenuation, #positive_only) }, - FieldExport::Flags { bits } => { + Self::Flags { bits } => { let bits = bits.iter().map(ValueWithKey::to_tuple_expression); quote_export_func! { @@ -439,47 +456,47 @@ impl FieldExport { } } - FieldExport::Layers { + Self::Layers { dimension: LayerDimension::_2d, kind: LayerKind::Physics, } => quote_export_func! { export_flags_2d_physics() }, - FieldExport::Layers { + Self::Layers { dimension: LayerDimension::_2d, kind: LayerKind::Render, } => quote_export_func! { export_flags_2d_render() }, - FieldExport::Layers { + Self::Layers { dimension: LayerDimension::_2d, kind: LayerKind::Navigation, } => quote_export_func! { export_flags_2d_navigation() }, - FieldExport::Layers { + Self::Layers { dimension: LayerDimension::_3d, kind: LayerKind::Physics, } => quote_export_func! { export_flags_3d_physics() }, - FieldExport::Layers { + Self::Layers { dimension: LayerDimension::_3d, kind: LayerKind::Render, } => quote_export_func! { export_flags_3d_render() }, - FieldExport::Layers { + Self::Layers { dimension: LayerDimension::_3d, kind: LayerKind::Navigation, } => quote_export_func! { export_flags_3d_navigation() }, - FieldExport::File { + Self::File { global: false, kind: FileKind::Dir, } => quote_export_func! { export_dir() }, - FieldExport::File { + Self::File { global: true, kind: FileKind::Dir, } => quote_export_func! { export_global_dir() }, - FieldExport::File { + Self::File { global, kind: FileKind::File { filter }, } => { @@ -488,12 +505,12 @@ impl FieldExport { quote_export_func! { export_file_inner(#global, #filter) } } - FieldExport::Multiline => quote_export_func! { export_multiline() }, + Self::Multiline => quote_export_func! { export_multiline() }, - FieldExport::PlaceholderText { placeholder } => quote_export_func! { + Self::PlaceholderText { placeholder } => quote_export_func! { export_placeholder(#placeholder) }, - FieldExport::ColorNoAlpha => quote_export_func! { export_color_no_alpha() }, + Self::ColorNoAlpha => quote_export_func! { export_color_no_alpha() }, } } } diff --git a/godot-macros/src/class/data_models/field_var.rs b/godot-macros/src/class/data_models/field_var.rs index 267ae0de9..a79304dd9 100644 --- a/godot-macros/src/class/data_models/field_var.rs +++ b/godot-macros/src/class/data_models/field_var.rs @@ -5,7 +5,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use proc_macro2::{Ident, TokenStream}; +use proc_macro2::{Ident, Span, TokenStream}; use quote::{format_ident, quote}; use crate::class::{ @@ -16,12 +16,13 @@ use crate::util::KvParser; use crate::{util, ParseResult}; /// Store info from `#[var]` attribute. -#[derive(Default, Clone, Debug)] +#[derive(Clone, Debug)] pub struct FieldVar { pub getter: GetterSetter, pub setter: GetterSetter, pub hint: FieldHint, pub usage_flags: UsageFlags, + pub span: Span, } impl FieldVar { @@ -34,6 +35,7 @@ impl FieldVar { /// - `hint_string = expr` /// - `usage_flags = pub(crate) fn new_from_kv(parser: &mut KvParser) -> ParseResult { + let span = parser.span(); let mut getter = GetterSetter::parse(parser, "get")?; let mut setter = GetterSetter::parse(parser, "set")?; @@ -71,10 +73,23 @@ impl FieldVar { setter, hint, usage_flags, + span, }) } } +impl Default for FieldVar { + fn default() -> Self { + Self { + getter: Default::default(), + setter: Default::default(), + hint: Default::default(), + usage_flags: Default::default(), + span: Span::call_site(), + } + } +} + #[derive(Default, Clone, Eq, PartialEq, Debug)] pub enum GetterSetter { /// Getter/setter should be omitted, field is write/read only. diff --git a/godot-macros/src/class/data_models/property.rs b/godot-macros/src/class/data_models/property.rs index cae0defc9..be4077964 100644 --- a/godot-macros/src/class/data_models/property.rs +++ b/godot-macros/src/class/data_models/property.rs @@ -70,6 +70,7 @@ pub fn make_property_impl(class_name: &Ident, fields: &Fields) -> TokenStream { setter, hint, mut usage_flags, + .. } = var; let export_hint; diff --git a/godot-macros/src/class/derive_godot_class.rs b/godot-macros/src/class/derive_godot_class.rs index 4fc7efa84..281433e88 100644 --- a/godot-macros/src/class/derive_godot_class.rs +++ b/godot-macros/src/class/derive_godot_class.rs @@ -6,13 +6,13 @@ */ use proc_macro2::{Ident, Punct, TokenStream}; -use quote::{format_ident, quote}; +use quote::{format_ident, quote, quote_spanned}; use crate::class::{ - make_property_impl, make_virtual_callback, BeforeKind, Field, FieldExport, FieldVar, Fields, - SignatureInfo, + make_property_impl, make_virtual_callback, BeforeKind, Field, FieldDefault, FieldExport, + FieldVar, Fields, SignatureInfo, }; -use crate::util::{bail, ident, path_ends_with_complex, require_api_version, KvParser}; +use crate::util::{bail, error, ident, path_ends_with_complex, require_api_version, KvParser}; use crate::{handle_mutually_exclusive_keys, util, ParseResult}; pub fn derive_godot_class(item: venial::Item) -> ParseResult { @@ -28,6 +28,8 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult { let mut deprecations = std::mem::take(&mut struct_cfg.deprecations); deprecations.append(&mut fields.deprecations); + let errors = fields.errors.iter().map(|error| error.to_compile_error()); + let class_name = &class.name; let class_name_str: String = struct_cfg .rename @@ -61,14 +63,18 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult { let prv = quote! { ::godot::private }; let godot_exports_impl = make_property_impl(class_name, &fields); - let godot_withbase_impl = if let Some(Field { name, .. }) = &fields.base_field { - quote! { + let godot_withbase_impl = if let Some(Field { name, ty, .. }) = &fields.base_field { + // Apply the span of the field's type so that errors show up on the field's type. + quote_spanned! { ty.span()=> impl ::godot::obj::WithBaseField for #class_name { - fn to_gd(&self) -> ::godot::obj::Gd { - self.#name.to_gd().cast() + fn to_gd(&self) -> ::godot::obj::Gd<#class_name> { + // By not referencing the base field directly here we ensure that the user only gets one error when the base + // field's type is wrong. + let base = <#class_name as ::godot::obj::WithBaseField>::base_field(self); + base.to_gd().cast() } - fn base_field(&self) -> &::godot::obj::Base<::Base> { + fn base_field(&self) -> &::godot::obj::Base<<#class_name as ::godot::obj::GodotClass>::Base> { &self.#name } } @@ -148,6 +154,7 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult { #user_class_impl #init_expecter #( #deprecations )* + #( #errors )* ::godot::sys::plugin_add!(__GODOT_PLUGIN_REGISTRY in #prv; #prv::ClassPlugin { class_name: #class_name_obj, @@ -222,8 +229,8 @@ impl ClassAttributes { } fn make_godot_init_impl(class_name: &Ident, fields: &Fields) -> TokenStream { - let base_init = if let Some(Field { name, .. }) = &fields.base_field { - quote! { #name: base, } + let base_init = if let Some(Field { name, ty, .. }) = &fields.base_field { + quote_spanned! { ty.span()=> #name: base, } } else { TokenStream::new() }; @@ -233,14 +240,16 @@ fn make_godot_init_impl(class_name: &Ident, fields: &Fields) -> TokenStream { let value_expr = field .default_val .clone() - .unwrap_or_else(|| quote! { ::std::default::Default::default() }); + .map(|field| field.default_val) + // Use quote_spanned with the field's span so that errors show up on the field and not the derive macro. + .unwrap_or_else(|| quote_spanned! { field.span=> ::std::default::Default::default() }); quote! { #field_name: #value_expr, } }); quote! { impl ::godot::obj::cap::GodotDefault for #class_name { - fn __godot_user_init(base: ::godot::obj::Base) -> Self { + fn __godot_user_init(base: ::godot::obj::Base<<#class_name as ::godot::obj::GodotClass>::Base>) -> Self { Self { #( #rest_init )* #base_init @@ -357,7 +366,7 @@ fn parse_struct_attributes(class: &venial::Struct) -> ParseResult ::godot::__deprecated::emit_deprecated_warning!(class_editor_plugin); }); } @@ -373,11 +382,11 @@ fn parse_struct_attributes(class: &venial::Struct) -> ParseResult ::godot::__deprecated::emit_deprecated_warning!(class_hidden); }); } @@ -421,6 +430,7 @@ fn parse_fields( let mut all_fields = vec![]; let mut base_field = Option::::None; let mut deprecations = vec![]; + let mut errors = vec![]; // Attributes on struct fields for (named_field, _punct) in named_fields { @@ -449,7 +459,10 @@ fn parse_fields( // #[init(val = expr)] if let Some(default) = parser.handle_expr("val")? { - field.default_val = Some(default); + field.default_val = Some(FieldDefault { + default_val: default, + span: parser.span(), + }); } // Deprecated #[init(default = expr)] @@ -460,36 +473,49 @@ fn parse_fields( "Cannot use both `val` and `default` keys in #[init]; prefer using `val`" ); } - field.default_val = Some(default); - deprecations.push(quote! { + field.default_val = Some(FieldDefault { + default_val: default, + span: parser.span(), + }); + deprecations.push(quote_spanned! { parser.span()=> ::godot::__deprecated::emit_deprecated_warning!(init_default); }) } // #[init(node = "NodePath")] if let Some(node_path) = parser.handle_expr("node")? { + let mut is_well_formed = true; if !field.is_onready { - return bail!( + is_well_formed = false; + errors.push(error!( parser.span(), "The key `node` in attribute #[init] requires field of type `OnReady`\n\ Help: The syntax #[init(node = \"NodePath\")] is equivalent to \ #[init(val = OnReady::node(\"NodePath\"))], \ which can only be assigned to fields of type `OnReady`" - ); + )); } if field.default_val.is_some() { - return bail!( + is_well_formed = false; + errors.push(error!( parser.span(), - "The key `node` in attribute #[init] is mutually exclusive with the key `default`\n\ + "The key `node` in attribute #[init] is mutually exclusive with the keys `default` and `val`\n\ Help: The syntax #[init(node = \"NodePath\")] is equivalent to \ #[init(val = OnReady::node(\"NodePath\"))], \ both aren't allowed since they would override each other" - ); + )); } - field.default_val = Some(quote! { - OnReady::node(#node_path) + let default_val = if is_well_formed { + quote! { OnReady::node(#node_path) } + } else { + quote! { todo!() } + }; + + field.default_val = Some(FieldDefault { + default_val, + span: parser.span(), }); } @@ -524,25 +550,41 @@ fn parse_fields( // Extra validation; eventually assign to base_fields or all_fields. if is_base { - if field.is_onready - || field.var.is_some() - || field.export.is_some() - || field.default_val.is_some() - { - return bail!( - named_field, - "base field cannot have type `OnReady` or attributes #[var], #[export] or #[init]" - ); + if field.is_onready { + errors.push(error!( + field.ty.clone(), + "base field cannot have type `OnReady`" + )); + } + + if let Some(var) = field.var.as_ref() { + errors.push(error!( + var.span, + "base field cannot have the attribute #[var]" + )); + } + + if let Some(export) = field.export.as_ref() { + errors.push(error!( + export.span, + "base field cannot have the attribute #[export]" + )); + } + + if let Some(default_val) = field.default_val.as_ref() { + errors.push(error!( + default_val.span, + "base field cannot have the attribute #[init]" + )); } if let Some(prev_base) = base_field.replace(field) { // Ensure at most one Base. - return bail!( + errors.push(error!( // base_field.unwrap().name, named_field, - "at most 1 field can have type Base; previous is `{}`", - prev_base.name - ); + "at most 1 field can have type Base; previous is `{}`", prev_base.name + )); } } else { all_fields.push(field); @@ -553,6 +595,7 @@ fn parse_fields( all_fields, base_field, deprecations, + errors, }) }