Emit a deprecation warning on an encounter of a String field when deriving Properties

packages/yew-macro/Makefile.toml

@@ -5,12 +5,17 @@ command = "cargo"
 # test target can be optionally specified like `cargo make test html_macro`,
 args = ["test", "${@}"]
 
+[tasks.test-overwrite]
+extend = "test"
+env = { TRYBUILD = "overwrite" }
+
 [tasks.test-lint]
 clear = true
 toolchain = "nightly"
 command = "cargo"
-args = ["test", "test_html_lints", "--features", "lints"]
+env = { RUSTFLAGS = "--cfg yew_lints" }
+args = ["test", "lints"]
 
-[tasks.test-overwrite]
-extend = "test"
-env = { TRYBUILD = "overwrite" }
+[tasks.test-lint-overwrite]
+extend = "test-lint"
+env = { RUSTFLAGS = "--cfg yew_lints", TRYBUILD = "overwrite" }

packages/yew-macro/src/derive_props/field.rs

@@ -32,6 +32,26 @@ pub struct PropField {
 }
 
 impl PropField {
+    #[cfg(not(yew_lints))]
+    pub fn lint(&self) {}
+
+    #[cfg(yew_lints)]
+    pub fn lint(&self) {
+        match &self.ty {
+            Type::Path(TypePath { qself: None, path }) => {
+                if is_path_a_string(path) {
+                    proc_macro_error::emit_warning!(
+                    path.span(),
+                    "storing string values with `String` is not recommended, prefer `AttrValue`.\n\
+                     for further info visit \
+                     https://yew.rs/docs/concepts/function-components/properties#anti-patterns"
+                )
+                }
+            }
+            _ => (),
+        }
+    }
+
     /// All required property fields are wrapped in an `Option`
     pub fn is_required(&self) -> bool {
         matches!(self.attr, PropAttr::Required { .. })
@@ -293,22 +313,16 @@ impl<'a> PropFieldCheck<'a> {
     }
 }
 
-fn is_path_segments_an_option(path_segments: impl Iterator<Item = String>) -> bool {
-    fn is_option_path_seg(seg_index: usize, path: &str) -> u8 {
-        match (seg_index, path) {
-            (0, "core") => 0b001,
-            (0, "std") => 0b001,
-            (0, "Option") => 0b111,
-            (1, "option") => 0b010,
-            (2, "Option") => 0b100,
-            _ => 0,
-        }
-    }
-
-    path_segments
-        .enumerate()
-        .fold(0, |flags, (i, ps)| flags | is_option_path_seg(i, &ps))
-        == 0b111
+fn is_path_segments_an_option<'a, T>(mut iter: impl Iterator<Item = &'a T>) -> bool
+where
+    T: 'a + ?Sized + PartialEq<str>,
+{
+    iter.next().map_or(false, |first| {
+        first == "Option"
+            || (first == "std" || first == "core")
+                && iter.next().map_or(false, |second| second == "option")
+                && iter.next().map_or(false, |third| third == "Option")
+    })
 }
 
 /// Returns true when the [`Path`] seems like an [`Option`] type.
@@ -320,7 +334,31 @@ fn is_path_segments_an_option(path_segments: impl Iterator<Item = String>) -> bo
 ///
 /// Users can define their own [`Option`] type and this will return true - this is unavoidable.
 fn is_path_an_option(path: &Path) -> bool {
-    is_path_segments_an_option(path.segments.iter().take(3).map(|ps| ps.ident.to_string()))
+    is_path_segments_an_option(path.segments.iter().map(|ps| &ps.ident))
+}
+
+#[cfg(any(yew_lints, test))]
+fn is_path_segments_a_string<'a, T>(mut iter: impl Iterator<Item = &'a T>) -> bool
+where
+    T: 'a + ?Sized + PartialEq<str>,
+{
+    iter.next().map_or(false, |first| {
+        first == "String"
+            || (first == "std" || first == "alloc")
+                && iter.next().map_or(false, |second| second == "string")
+                && iter.next().map_or(false, |third| third == "String")
+    })
+}
+
+/// returns true when the [`Path`] seems like a [`String`] type.
+///
+/// This function considers the following paths as Strings:
+/// - std::string::String
+/// - alloc::string::String
+/// - String
+#[cfg(yew_lints)]
+fn is_path_a_string(path: &Path) -> bool {
+    is_path_segments_a_string(path.segments.iter().map(|ps| &ps.ident))
 }
 
 impl TryFrom<Field> for PropField {
@@ -379,22 +417,33 @@ impl PartialEq for PropField {
 
 #[cfg(test)]
 mod tests {
-    use crate::derive_props::field::is_path_segments_an_option;
+    use std::iter::once;
+
+    use crate::derive_props::field::{is_path_segments_a_string, is_path_segments_an_option};
 
     #[test]
     fn all_std_and_core_option_path_seg_return_true() {
         assert!(is_path_segments_an_option(
-            vec!["core".to_owned(), "option".to_owned(), "Option".to_owned()].into_iter()
-        ));
-        assert!(is_path_segments_an_option(
-            vec!["std".to_owned(), "option".to_owned(), "Option".to_owned()].into_iter()
+            ["core", "option", "Option"].into_iter()
         ));
         assert!(is_path_segments_an_option(
-            vec!["Option".to_owned()].into_iter()
+            ["std", "option", "Option"].into_iter()
         ));
+        assert!(is_path_segments_an_option(once("Option")));
         // why OR instead of XOR
         assert!(is_path_segments_an_option(
-            vec!["Option".to_owned(), "Vec".to_owned(), "Option".to_owned()].into_iter()
+            ["Option", "Vec", "Option"].into_iter()
+        ));
+    }
+
+    #[test]
+    fn all_std_and_alloc_string_seg_return_true() {
+        assert!(is_path_segments_a_string(
+            ["alloc", "string", "String"].into_iter()
+        ));
+        assert!(is_path_segments_a_string(
+            ["std", "string", "String"].into_iter()
         ));
+        assert!(is_path_segments_a_string(once("String")));
     }
 }

packages/yew-macro/src/derive_props/mod.rs

@@ -79,17 +79,18 @@ impl ToTokens for DerivePropsInput {
         let Self {
             generics,
             props_name,
+            prop_fields,
+            preserved_attrs,
             ..
         } = self;
 
+        for field in prop_fields {
+            field.lint();
+        }
+
         // The wrapper is a new struct which wraps required props in `Option`
         let wrapper_name = format_ident!("{}Wrapper", props_name, span = Span::mixed_site());
-        let wrapper = PropsWrapper::new(
-            &wrapper_name,
-            generics,
-            &self.prop_fields,
-            &self.preserved_attrs,
-        );
+        let wrapper = PropsWrapper::new(&wrapper_name, generics, prop_fields, preserved_attrs);
         tokens.extend(wrapper.into_token_stream());
 
         // The builder will only build if all required props have been set
@@ -101,7 +102,7 @@ impl ToTokens for DerivePropsInput {
             self,
             &wrapper_name,
             &check_all_props_name,
-            &self.preserved_attrs,
+            preserved_attrs,
         );
         let generic_args = to_arguments(generics);
         tokens.extend(builder.into_token_stream());

packages/yew-macro/src/lib.rs

@@ -105,6 +105,7 @@ fn is_ide_completion() -> bool {
     }
 }
 
+#[proc_macro_error::proc_macro_error]
 #[proc_macro_derive(Properties, attributes(prop_or, prop_or_else, prop_or_default))]
 pub fn derive_props(input: TokenStream) -> TokenStream {
     let input = parse_macro_input!(input as DerivePropsInput);

packages/yew-macro/tests/derive_props_lints/fail.rs

@@ -0,0 +1,14 @@
+use yew::prelude::*;
+extern crate alloc;
+
+#[derive(Properties, PartialEq)]
+struct Props {
+    suboptimal1: String,
+    suboptimal2: std::string::String,
+    suboptimal3: alloc::string::String
+}
+
+fn main() {
+    compile_error!("This macro call exists to deliberately fail\
+                    the compilation of the test so we can verify output of lints");
+}

packages/yew-macro/tests/derive_props_lints/fail.stderr

@@ -0,0 +1,6 @@
+error: This macro call exists to deliberately failthe compilation of the test so we can verify output of lints
+  --> tests/derive_props_lints/fail.rs:12:5
+   |
+12 | /     compile_error!("This macro call exists to deliberately fail\
+13 | |                     the compilation of the test so we can verify output of lints");
+   | |__________________________________________________________________________________^

packages/yew-macro/tests/derive_props_lints_test.rs

@@ -0,0 +1,7 @@
+#[allow(dead_code)]
+#[cfg(yew_lints)]
+#[rustversion::attr(nightly, test)]
+fn test_derive_props_lints() {
+    let t = trybuild::TestCases::new();
+    t.compile_fail("tests/derive_props_lints/fail.rs");
+}