Require that turbo_tasks::value_trait items be annotated with turbo-tasks::function

crates/next-api/src/route.rs

@@ -49,14 +49,19 @@ pub enum Route {
 
 #[turbo_tasks::value_trait]
 pub trait Endpoint {
+    #[turbo_tasks::function]
     fn output(self: Vc<Self>) -> Vc<EndpointOutput>;
     // fn write_to_disk(self: Vc<Self>) -> Vc<EndpointOutputPaths>;
+    #[turbo_tasks::function]
     fn server_changed(self: Vc<Self>) -> Vc<Completion>;
+    #[turbo_tasks::function]
     fn client_changed(self: Vc<Self>) -> Vc<Completion>;
     /// The entry modules for the modules graph.
+    #[turbo_tasks::function]
     fn entries(self: Vc<Self>) -> Vc<GraphEntries>;
     /// Additional entry modules for the module graph.
     /// This may read the module graph and return additional modules.
+    #[turbo_tasks::function]
     fn additional_entries(self: Vc<Self>, _graph: Vc<ModuleGraph>) -> Vc<GraphEntries> {
         GraphEntries::empty()
     }

turbopack/crates/turbo-tasks-env/src/lib.rs

@@ -49,9 +49,11 @@ pub trait ProcessEnv {
     // Instead we should use only `read_prefix` to read all env vars with a specific
     // prefix.
     /// Reads all env variables into a Map
+    #[turbo_tasks::function]
     fn read_all(self: Vc<Self>) -> Vc<EnvMap>;
 
     /// Reads a single env variable. Ignores casing.
+    #[turbo_tasks::function]
     fn read(self: Vc<Self>, name: RcStr) -> Vc<Option<RcStr>> {
         case_insensitive_read(self.read_all(), name)
     }

turbopack/crates/turbo-tasks-fs/src/lib.rs

@@ -195,14 +195,21 @@ fn create_semaphore() -> tokio::sync::Semaphore {
 #[turbo_tasks::value_trait]
 pub trait FileSystem: ValueToString {
     /// Returns the path to the root of the file system.
+    #[turbo_tasks::function]
     fn root(self: Vc<Self>) -> Vc<FileSystemPath> {
         FileSystemPath::new_normalized(self, RcStr::default())
     }
+    #[turbo_tasks::function]
     fn read(self: Vc<Self>, fs_path: Vc<FileSystemPath>) -> Vc<FileContent>;
+    #[turbo_tasks::function]
     fn read_link(self: Vc<Self>, fs_path: Vc<FileSystemPath>) -> Vc<LinkContent>;
+    #[turbo_tasks::function]
     fn raw_read_dir(self: Vc<Self>, fs_path: Vc<FileSystemPath>) -> Vc<RawDirectoryContent>;
+    #[turbo_tasks::function]
     fn write(self: Vc<Self>, fs_path: Vc<FileSystemPath>, content: Vc<FileContent>) -> Vc<()>;
+    #[turbo_tasks::function]
     fn write_link(self: Vc<Self>, fs_path: Vc<FileSystemPath>, target: Vc<LinkContent>) -> Vc<()>;
+    #[turbo_tasks::function]
     fn metadata(self: Vc<Self>, fs_path: Vc<FileSystemPath>) -> Vc<FileMeta>;
 }
 

turbopack/crates/turbo-tasks-macros-tests/tests/function/fail_attribute_invalid_args_inherent_impl.stderr

@@ -3,3 +3,17 @@ error: unexpected token, expected one of: "fs", "network", "operation", "local"
    |
 14 |     #[turbo_tasks::function(invalid_argument)]
    |                             ^^^^^^^^^^^^^^^^
+
+warning: unused import: `Vc`
+ --> tests/function/fail_attribute_invalid_args_inherent_impl.rs:4:31
+  |
+4 | use turbo_tasks::{ResolvedVc, Vc};
+  |                               ^^
+  |
+  = note: `#[warn(unused_imports)]` on by default
+
+error[E0599]: no method named `return_contains_resolved_vc` found for struct `Vc<ExampleStruct>` in the current scope
+  --> tests/function/fail_attribute_invalid_args_inherent_impl.rs:23:34
+   |
+23 |     let _ = ExampleStruct.cell().return_contains_resolved_vc();
+   |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^ method not found in `Vc<ExampleStruct>`

turbopack/crates/turbo-tasks-macros-tests/tests/value_trait/fail_missing_function_annotation.rs

@@ -0,0 +1,41 @@
+#![feature(arbitrary_self_types)]
+#![feature(arbitrary_self_types_pointers)]
+
+use turbo_tasks::Vc;
+
+#[turbo_tasks::value_trait]
+trait MyTrait {
+    fn item(&self) -> bool;
+
+    #[turbo_tasks::function]
+    fn item2(&self) -> Vc<bool>;
+
+    #[turbo_tasks::function(operation)]
+    fn item3(&self) -> Vc<bool>;
+}
+
+#[turbo_tasks::value]
+struct MyStruct;
+
+#[turbo_tasks::value_impl]
+impl MyTrait for MyStruct {
+    fn item(&self) -> bool {
+        true
+    }
+
+    #[turbo_tasks::function]
+    fn item2(&self) -> Vc<bool> {
+        Vc::cell(true)
+    }
+
+    #[turbo_tasks::function]
+    fn item3(&self) -> Vc<bool> {
+        Vc::cell(true)
+    }
+}
+
+fn expects_my_trait(_x: impl MyTrait) {}
+
+fn main() {
+    expects_my_trait(MyStruct {});
+}

turbopack/crates/turbo-tasks-macros-tests/tests/value_trait/fail_missing_function_annotation.stderr

@@ -0,0 +1,19 @@
+error: trait items cannot be operations
+  --> tests/value_trait/fail_missing_function_annotation.rs:13:29
+   |
+13 |     #[turbo_tasks::function(operation)]
+   |                             ^^^^^^^^^
+
+error: #[turbo_tasks::function] attribute missing
+ --> tests/value_trait/fail_missing_function_annotation.rs:8:5
+  |
+8 |     fn item(&self) -> bool;
+  |     ^^^^^^^^^^^^^^^^^^^^^^^
+
+error: #[turbo_tasks::function] attribute missing
+  --> tests/value_trait/fail_missing_function_annotation.rs:22:5
+   |
+22 | /     fn item(&self) -> bool {
+23 | |         true
+24 | |     }
+   | |_____^

turbopack/crates/turbo-tasks-macros-tests/tests/value_trait/pass_trait_items.rs

@@ -0,0 +1,43 @@
+#![feature(arbitrary_self_types)]
+#![feature(arbitrary_self_types_pointers)]
+
+use turbo_tasks::Vc;
+
+#[turbo_tasks::value_trait]
+trait MyTrait {
+    #[turbo_tasks::function]
+    fn item(&self) -> Vc<bool>;
+
+    #[turbo_tasks::function]
+    fn item2(&self) -> Vc<bool>;
+
+    #[turbo_tasks::function]
+    fn item3(&self) -> Vc<bool>;
+}
+
+#[turbo_tasks::value]
+struct MyStruct;
+
+#[turbo_tasks::value_impl]
+impl MyTrait for MyStruct {
+    #[turbo_tasks::function]
+    fn item(&self) -> Vc<bool> {
+        Vc::cell(true)
+    }
+
+    #[turbo_tasks::function]
+    fn item2(&self) -> Vc<bool> {
+        Vc::cell(true)
+    }
+
+    #[turbo_tasks::function]
+    fn item3(&self) -> Vc<bool> {
+        Vc::cell(true)
+    }
+}
+
+fn expects_my_trait(_x: impl MyTrait) {}
+
+fn main() {
+    expects_my_trait(MyStruct {});
+}

turbopack/crates/turbo-tasks-macros/src/func.rs

@@ -1195,3 +1195,48 @@ pub fn inline_inputs_identifier_filter(arg_ident: &Ident) -> bool {
     // filter out underscore-prefixed (unused) arguments, we don't need to cache these
     !arg_ident.to_string().starts_with('_')
 }
+
+/// Returns true if this attribute is a turbo_tasks attribute with the given name.
+fn is_attribute(attr: &Attribute, name: &str) -> bool {
+    let path = &attr.path();
+    if path.leading_colon.is_some() {
+        return false;
+    }
+    let mut iter = path.segments.iter();
+    match iter.next() {
+        Some(seg) if seg.arguments.is_empty() && seg.ident == "turbo_tasks" => match iter.next() {
+            Some(seg) if seg.arguments.is_empty() && seg.ident == name => iter.next().is_none(),
+            _ => false,
+        },
+        _ => false,
+    }
+}
+
+/// Parses a `turbo_tasks::function` attribute out of the given attributes and then returns the
+/// remaining attributes.
+pub fn split_function_attributes<'a>(
+    item: &'a impl Spanned,
+    attrs: &'a [Attribute],
+) -> (syn::Result<FunctionArguments>, Vec<&'a Attribute>) {
+    let (func_attrs_vec, attrs): (Vec<_>, Vec<_>) = attrs
+        .iter()
+        // TODO(alexkirsz) Replace this with function
+        .partition(|attr| is_attribute(attr, "function"));
+    let func_args = if let Some(func_attr) = func_attrs_vec.first() {
+        if func_attrs_vec.len() == 1 {
+            parse_with_optional_parens::<FunctionArguments>(func_attr)
+        } else {
+            Err(syn::Error::new(
+                // Report the error on the second annotation.
+                func_attrs_vec[1].span(),
+                "Only one #[turbo_tasks::function] attribute is allowed per method",
+            ))
+        }
+    } else {
+        Err(syn::Error::new(
+            item.span(),
+            "#[turbo_tasks::function] attribute missing",
+        ))
+    };
+    (func_args, attrs)
+}

turbopack/crates/turbo-tasks-macros/src/value_impl_macro.rs

@@ -2,11 +2,10 @@ use proc_macro::TokenStream;
 use proc_macro2::{Ident, TokenStream as TokenStream2};
 use quote::{ToTokens, quote};
 use syn::{
-    Attribute, Error, Expr, ExprLit, Generics, ImplItem, ImplItemFn, ItemImpl, Lit, LitStr, Meta,
+    Error, Expr, ExprLit, Generics, ImplItem, ImplItemFn, ItemImpl, Lit, LitStr, Meta,
     MetaNameValue, Path, Token, Type,
     parse::{Parse, ParseStream},
     parse_macro_input, parse_quote,
-    spanned::Spanned,
 };
 use turbo_tasks_macros_shared::{
     get_inherent_impl_function_id_ident, get_inherent_impl_function_ident, get_path_ident,
@@ -15,51 +14,9 @@ use turbo_tasks_macros_shared::{
 };
 
 use crate::func::{
-    DefinitionContext, FunctionArguments, NativeFn, TurboFn, filter_inline_attributes,
-    parse_with_optional_parens,
+    DefinitionContext, NativeFn, TurboFn, filter_inline_attributes, split_function_attributes,
 };
 
-fn is_attribute(attr: &Attribute, name: &str) -> bool {
-    let path = &attr.path();
-    if path.leading_colon.is_some() {
-        return false;
-    }
-    let mut iter = path.segments.iter();
-    match iter.next() {
-        Some(seg) if seg.arguments.is_empty() && seg.ident == "turbo_tasks" => match iter.next() {
-            Some(seg) if seg.arguments.is_empty() && seg.ident == name => iter.next().is_none(),
-            _ => false,
-        },
-        _ => false,
-    }
-}
-
-fn split_function_attributes<'a>(
-    item: &'a ImplItem,
-    attrs: &'a [Attribute],
-) -> (syn::Result<FunctionArguments>, Vec<&'a Attribute>) {
-    let (func_attrs_vec, attrs): (Vec<_>, Vec<_>) = attrs
-        .iter()
-        // TODO(alexkirsz) Replace this with function
-        .partition(|attr| is_attribute(attr, "function"));
-    let func_args = if let Some(func_attr) = func_attrs_vec.first() {
-        if func_attrs_vec.len() == 1 {
-            parse_with_optional_parens::<FunctionArguments>(func_attr)
-        } else {
-            Err(syn::Error::new(
-                func_attr.span(),
-                "Only one #[turbo_tasks::function] attribute is allowed per method",
-            ))
-        }
-    } else {
-        Err(syn::Error::new(
-            item.span(),
-            "#[turbo_tasks::function] attribute missing",
-        ))
-    };
-    (func_args, attrs)
-}
-
 struct ValueImplArguments {
     ident: Option<LitStr>,
 }
@@ -121,9 +78,11 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
             {
                 let ident = &sig.ident;
                 let (func_args, attrs) = split_function_attributes(item, attrs);
-                let func_args = func_args
-                    .inspect_err(|err| errors.push(err.to_compile_error()))
-                    .unwrap_or_default();
+                let Ok(func_args) =
+                    func_args.inspect_err(|err| errors.push(err.to_compile_error()))
+                else {
+                    continue;
+                };
                 let local = func_args.local.is_some();
                 let is_self_used = func_args.operation.is_some() || is_self_used(block);
 
@@ -225,9 +184,12 @@ pub fn value_impl(args: TokenStream, input: TokenStream) -> TokenStream {
                 let ident = &sig.ident;
 
                 let (func_args, attrs) = split_function_attributes(item, attrs);
-                let func_args = func_args
-                    .inspect_err(|err| errors.push(err.to_compile_error()))
-                    .unwrap_or_default();
+                let Ok(func_args) =
+                    func_args.inspect_err(|err| errors.push(err.to_compile_error()))
+                else {
+                    continue;
+                };
+
                 let local = func_args.local.is_some();
                 let is_self_used = func_args.operation.is_some() || is_self_used(block);
 

turbopack/crates/turbo-tasks-macros/src/value_trait_macro.rs

@@ -10,6 +10,7 @@ use turbo_tasks_macros_shared::{
 
 use crate::func::{
     DefinitionContext, FunctionArguments, NativeFn, TurboFn, filter_inline_attributes,
+    split_function_attributes,
 };
 
 pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
@@ -68,6 +69,7 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
     let mut trait_methods: Vec<TokenStream2> = Vec::new();
     let mut native_functions = Vec::new();
     let mut items = Vec::with_capacity(raw_items.len());
+    let mut errors = Vec::new();
 
     for item in raw_items.iter() {
         let TraitItem::Fn(TraitItemFn {
@@ -85,10 +87,18 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
         };
 
         let ident = &sig.ident;
+        // This effectively parses and removes the function annotation ensuring that that macro
+        // doesn't run after us.
+        let (func_args, attrs) = split_function_attributes(item, attrs);
+        let Ok(func_args) = func_args.inspect_err(|err| errors.push(err.to_compile_error())) else {
+            continue;
+        };
+        if let Some(span) = func_args.operation {
+            span.unwrap()
+                .error("trait items cannot be operations")
+                .emit();
+        }
 
-        // Value trait method declarations don't have `#[turbo_tasks::function]`
-        // annotations on them, though their `impl`s do. It may make sense to require it
-        // in the future when defining a default implementation.
         let Some(turbo_fn) = TurboFn::new(
             sig,
             DefinitionContext::ValueTrait,
@@ -114,7 +124,7 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
                 Ident::new(&format!("{trait_ident}_{ident}_inline"), ident.span());
             let (inline_signature, inline_block) =
                 turbo_fn.inline_signature_and_block(default, is_self_used);
-            let inline_attrs = filter_inline_attributes(&attrs[..]);
+            let inline_attrs = filter_inline_attributes(attrs.iter().copied());
 
             let native_function = NativeFn {
                 function_path_string: format!("{trait_ident}::{ident}"),
@@ -183,7 +193,7 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
         items.push(TraitItem::Fn(TraitItemFn {
             sig: turbo_fn.trait_signature(),
             default,
-            attrs: attrs.clone(),
+            attrs: attrs.iter().map(|a| (*a).clone()).collect(),
             semi_token: Default::default(),
         }));
     }
@@ -258,6 +268,8 @@ pub fn value_trait(args: TokenStream, input: TokenStream) -> TokenStream {
         )*
 
         #value_debug_impl
+
+        #(#errors)*
     };
     expanded.into()
 }