diff --git a/crates/next-custom-transforms/src/transforms/server_actions.rs b/crates/next-custom-transforms/src/transforms/server_actions.rs index 3568bac6adc70..a134ac8369f83 100644 --- a/crates/next-custom-transforms/src/transforms/server_actions.rs +++ b/crates/next-custom-transforms/src/transforms/server_actions.rs @@ -1,7 +1,7 @@ use std::{ collections::{BTreeMap, HashSet}, convert::{TryFrom, TryInto}, - mem::take, + mem::{replace, take}, }; use hex::encode as hex_encode; @@ -46,12 +46,23 @@ enum DirectiveLocation { FunctionBody, } +#[derive(Clone, Debug)] +enum ThisStatus { + Allowed, + Forbidden { directive: Directive }, +} + #[derive(Clone, Debug)] enum ServerActionsErrorKind { ExportedSyncFunction { span: Span, in_action_file: bool, }, + ForbiddenExpression { + span: Span, + expr: String, + directive: Directive, + }, InlineSyncFunction { span: Span, directive: Directive, @@ -113,6 +124,7 @@ pub fn server_actions(file_name: &FileName, config: Config, comment in_callee: false, has_action: false, has_cache: false, + this_status: ThisStatus::Allowed, reference_index: 0, in_module_level: true, @@ -163,6 +175,7 @@ struct ServerActions { in_callee: bool, has_action: bool, has_cache: bool, + this_status: ThisStatus, reference_index: u32, in_module_level: bool, @@ -897,24 +910,20 @@ impl VisitMut for ServerActions { } fn visit_mut_export_default_decl(&mut self, decl: &mut ExportDefaultDecl) { - let old = self.in_exported_expr; - let old_default = self.in_default_export_decl; - self.in_exported_expr = true; - self.in_default_export_decl = true; + let old_in_exported_expr = replace(&mut self.in_exported_expr, true); + let old_in_default_export_decl = replace(&mut self.in_default_export_decl, true); self.rewrite_default_fn_expr_to_proxy_expr = None; decl.decl.visit_mut_with(self); - self.in_exported_expr = old; - self.in_default_export_decl = old_default; + self.in_exported_expr = old_in_exported_expr; + self.in_default_export_decl = old_in_default_export_decl; } fn visit_mut_export_default_expr(&mut self, expr: &mut ExportDefaultExpr) { - let old = self.in_exported_expr; - let old_default = self.in_default_export_decl; - self.in_exported_expr = true; - self.in_default_export_decl = true; + let old_in_exported_expr = replace(&mut self.in_exported_expr, true); + let old_in_default_export_decl = replace(&mut self.in_default_export_decl, true); expr.expr.visit_mut_with(self); - self.in_exported_expr = old; - self.in_default_export_decl = old_default; + self.in_exported_expr = old_in_exported_expr; + self.in_default_export_decl = old_in_default_export_decl; } fn visit_mut_fn_expr(&mut self, f: &mut FnExpr) { @@ -931,18 +940,20 @@ impl VisitMut for ServerActions { let declared_idents_until = self.declared_idents.len(); let current_names = take(&mut self.names); + if let Some(directive) = &directive { + self.this_status = ThisStatus::Forbidden { + directive: directive.clone(), + }; + } + // Visit children { - let old_in_module = self.in_module_level; - let old_should_track_names = self.should_track_names; - let old_in_exported_expr = self.in_exported_expr; - let old_in_default_export_decl = self.in_default_export_decl; - let old_fn_decl_ident = self.fn_decl_ident.clone(); - self.in_module_level = false; - self.should_track_names = directive.is_some() || self.should_track_names; - self.in_exported_expr = false; - self.in_default_export_decl = false; - self.fn_decl_ident = None; + let old_in_module = replace(&mut self.in_module_level, false); + let should_track_names = directive.is_some() || self.should_track_names; + let old_should_track_names = replace(&mut self.should_track_names, should_track_names); + let old_in_exported_expr = replace(&mut self.in_exported_expr, false); + let old_in_default_export_decl = replace(&mut self.in_default_export_decl, false); + let old_fn_decl_ident = self.fn_decl_ident.take(); f.visit_mut_children_with(self); self.in_module_level = old_in_module; self.should_track_names = old_should_track_names; @@ -1070,12 +1081,14 @@ impl VisitMut for ServerActions { } fn visit_mut_fn_decl(&mut self, f: &mut FnDecl) { + let old_this_status = replace(&mut self.this_status, ThisStatus::Allowed); let old_in_exported_expr = self.in_exported_expr; if self.in_module_level && self.exported_local_ids.contains(&f.ident.to_id()) { self.in_exported_expr = true } let old_fn_decl_ident = self.fn_decl_ident.replace(f.ident.clone()); f.visit_mut_children_with(self); + self.this_status = old_this_status; self.in_exported_expr = old_in_exported_expr; self.fn_decl_ident = old_fn_decl_ident; } @@ -1091,19 +1104,22 @@ impl VisitMut for ServerActions { }, ); + if let Some(directive) = &directive { + self.this_status = ThisStatus::Forbidden { + directive: directive.clone(), + }; + } + let declared_idents_until = self.declared_idents.len(); let current_names = take(&mut self.names); { // Visit children - let old_in_module = self.in_module_level; - let old_should_track_names = self.should_track_names; - let old_in_exported_expr = self.in_exported_expr; - let old_in_default_export_decl = self.in_default_export_decl; - self.in_module_level = false; - self.should_track_names = directive.is_some() || self.should_track_names; - self.in_exported_expr = false; - self.in_default_export_decl = false; + let old_in_module = replace(&mut self.in_module_level, false); + let should_track_names = directive.is_some() || self.should_track_names; + let old_should_track_names = replace(&mut self.should_track_names, should_track_names); + let old_in_exported_expr = replace(&mut self.in_exported_expr, false); + let old_in_default_export_decl = replace(&mut self.in_default_export_decl, false); { for n in &mut a.params { collect_idents_in_pat(n, &mut self.declared_idents); @@ -1203,13 +1219,18 @@ impl VisitMut for ServerActions { } PropOrSpread::Prop(box Prop::Method(MethodProp { key, .. })) => { let key = key.clone(); + if let PropName::Ident(ident_name) = &key { self.arrow_or_fn_expr_ident = Some(ident_name.clone().into()); } + + let old_this_status = replace(&mut self.this_status, ThisStatus::Allowed); self.rewrite_expr_to_proxy_expr = None; self.in_exported_expr = false; n.visit_mut_children_with(self); self.in_exported_expr = old_in_exported_expr; + self.this_status = old_this_status; + if let Some(expr) = &self.rewrite_expr_to_proxy_expr { *n = PropOrSpread::Prop(Box::new(Prop::KeyValue(KeyValueProp { key, @@ -1217,6 +1238,7 @@ impl VisitMut for ServerActions { }))); self.rewrite_expr_to_proxy_expr = None; } + return; } _ => {} @@ -1237,9 +1259,26 @@ impl VisitMut for ServerActions { self.in_exported_expr = old_in_exported_expr; } + fn visit_mut_call_expr(&mut self, n: &mut CallExpr) { + if let Callee::Expr(box Expr::Ident(Ident { sym, .. })) = &mut n.callee { + if sym == "jsxDEV" || sym == "_jsxDEV" { + // Do not visit the 6th arg in a generated jsxDEV call, which is a `this` + // expression, to avoid emitting an error for using `this` if it's + // inside of a server function. https://github.com/facebook/react/blob/9106107/packages/react/src/jsx/ReactJSXElement.js#L429 + if n.args.len() > 4 { + for arg in &mut n.args[0..4] { + arg.visit_mut_with(self); + } + return; + } + } + } + + n.visit_mut_children_with(self); + } + fn visit_mut_callee(&mut self, n: &mut Callee) { - let old_in_callee = self.in_callee; - self.in_callee = true; + let old_in_callee = replace(&mut self.in_callee, true); n.visit_mut_children_with(self); self.in_callee = old_in_callee; } @@ -2027,6 +2066,28 @@ impl VisitMut for ServerActions { self.arrow_or_fn_expr_ident = old_arrow_or_fn_expr_ident; } + fn visit_mut_this_expr(&mut self, n: &mut ThisExpr) { + if let ThisStatus::Forbidden { directive } = &self.this_status { + emit_error(ServerActionsErrorKind::ForbiddenExpression { + span: n.span, + expr: "this".into(), + directive: directive.clone(), + }); + } + } + + fn visit_mut_ident(&mut self, n: &mut Ident) { + if n.sym == *"arguments" { + if let ThisStatus::Forbidden { directive } = &self.this_status { + emit_error(ServerActionsErrorKind::ForbiddenExpression { + span: n.span, + expr: "arguments".into(), + directive: directive.clone(), + }); + } + } + } + noop_visit_mut_type!(); } @@ -2771,6 +2832,23 @@ fn emit_error(error_kind: ServerActionsErrorKind) { } }, ), + ServerActionsErrorKind::ForbiddenExpression { + span, + expr, + directive, + } => ( + span, + formatdoc! { + r#" + {subject} cannot use `{expr}`. + "#, + subject = if let Directive::UseServer = directive { + "Server Actions" + } else { + "\"use cache\" functions" + } + }, + ), ServerActionsErrorKind::InlineUseCacheInClientComponent { span } => ( span, formatdoc! { diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/input.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/input.js new file mode 100644 index 0000000000000..7e524f5ed5927 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/input.js @@ -0,0 +1,50 @@ +async function a() { + 'use cache' + // this is not allowed here + this.foo() + // arguments is not allowed here + console.log(arguments) + + const b = async () => { + // this is not allowed here + this.foo() + // arguments is not allowed here + console.log(arguments) + } + + function c() { + // this is allowed here + this.foo() + // arguments is allowed here + console.log(arguments) + + const d = () => { + // this is allowed here + this.foo() + // arguments is allowed here + console.log(arguments) + } + + const e = async () => { + 'use server' + // this is not allowed here + this.foo() + // arguments is not allowed here + console.log(arguments) + } + } +} + +export const api = { + result: null, + product: { + async fetch() { + 'use cache' + + // this is not allowed here + this.result = await fetch('https://example.com').then((res) => res.json()) + // arguments is not allowed here + console.log(arguments) + }, + }, +} diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/output.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/output.js new file mode 100644 index 0000000000000..881e15a7e6333 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/output.js @@ -0,0 +1,55 @@ +/*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ /* __next_internal_action_entry_do_not_use__ {"006a88810ecce4a4e8b59d53b8327d7e98bbf251d7":"$$RSC_SERVER_ACTION_0","8069348c79fce073bae2f70f139565a2fda1c74c74":"$$RSC_SERVER_CACHE_2","80951c375b4a6a6e89d67b743ec5808127cfde405d":"$$RSC_SERVER_CACHE_1"} */ import { registerServerReference } from "private-next-rsc-server-reference"; +import { encryptActionBoundArgs, decryptActionBoundArgs } from "private-next-rsc-action-encryption"; +import { cache as $$cache__ } from "private-next-rsc-cache-wrapper"; +export const /*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ $$RSC_SERVER_ACTION_0 = async function e() { + // this is not allowed here + this.foo(); + // arguments is not allowed here + console.log(arguments); +}; +export var $$RSC_SERVER_CACHE_1 = $$cache__("default", "80951c375b4a6a6e89d67b743ec5808127cfde405d", 0, async function a() { + // this is not allowed here + this.foo(); + // arguments is not allowed here + console.log(arguments); + const b = async ()=>{ + // this is not allowed here + this.foo(); + // arguments is not allowed here + console.log(arguments); + }; + function c() { + // this is allowed here + this.foo(); + // arguments is allowed here + console.log(arguments); + const d = ()=>{ + // this is allowed here + this.foo(); + // arguments is allowed here + console.log(arguments); + }; + const e = registerServerReference($$RSC_SERVER_ACTION_0, "006a88810ecce4a4e8b59d53b8327d7e98bbf251d7", null); + } +}); +Object.defineProperty($$RSC_SERVER_CACHE_1, "name", { + "value": "a", + "writable": false +}); +var a = registerServerReference($$RSC_SERVER_CACHE_1, "80951c375b4a6a6e89d67b743ec5808127cfde405d", null); +export var $$RSC_SERVER_CACHE_2 = $$cache__("default", "8069348c79fce073bae2f70f139565a2fda1c74c74", 0, /*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ async function fetch1() { + // this is not allowed here + this.result = await fetch('https://example.com').then((res)=>res.json()); + // arguments is not allowed here + console.log(arguments); +}); +Object.defineProperty($$RSC_SERVER_CACHE_2, "name", { + "value": "fetch", + "writable": false +}); +export const api = { + result: null, + product: { + fetch: registerServerReference($$RSC_SERVER_CACHE_2, "8069348c79fce073bae2f70f139565a2fda1c74c74", null) + } +}; diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/output.stderr b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/output.stderr new file mode 100644 index 0000000000000..4051705fbb63b --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/output.stderr @@ -0,0 +1,63 @@ + x "use cache" functions cannot use `this`. + | + ,-[input.js:4:1] + 3 | // this is not allowed here + 4 | this.foo() + : ^^^^ + 5 | // arguments is not allowed here + `---- + x "use cache" functions cannot use `arguments`. + | + ,-[input.js:6:1] + 5 | // arguments is not allowed here + 6 | console.log(arguments) + : ^^^^^^^^^ + `---- + x "use cache" functions cannot use `this`. + | + ,-[input.js:10:1] + 9 | // this is not allowed here + 10 | this.foo() + : ^^^^ + 11 | // arguments is not allowed here + `---- + x "use cache" functions cannot use `arguments`. + | + ,-[input.js:12:1] + 11 | // arguments is not allowed here + 12 | console.log(arguments) + : ^^^^^^^^^ + 13 | } + `---- + x Server Actions cannot use `this`. + | + ,-[input.js:31:1] + 30 | // this is not allowed here + 31 | this.foo() + : ^^^^ + 32 | // arguments is not allowed here + `---- + x Server Actions cannot use `arguments`. + | + ,-[input.js:33:1] + 32 | // arguments is not allowed here + 33 | console.log(arguments) + : ^^^^^^^^^ + 34 | } + `---- + x "use cache" functions cannot use `this`. + | + ,-[input.js:45:1] + 44 | // this is not allowed here + 45 | this.result = await fetch('https://example.com').then((res) => res.json()) + : ^^^^ + 46 | // arguments is not allowed here + `---- + x "use cache" functions cannot use `arguments`. + | + ,-[input.js:47:1] + 46 | // arguments is not allowed here + 47 | console.log(arguments) + : ^^^^^^^^^ + 48 | }, + `---- diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/input.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/input.js new file mode 100644 index 0000000000000..66ac0246c493b --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/input.js @@ -0,0 +1,29 @@ +'use cache' + +// not exported! +async function a() { + // this is allowed here + this.foo() + // arguments is allowed here + console.log(arguments) + + const b = async () => { + 'use server' + // this is not allowed here + this.foo() + // arguments is not allowed here + console.log(arguments) + } +} + +export const obj = { + foo() { + return 42 + }, + bar() { + // this is allowed here + this.foo() + // arguments is allowed here + console.log(arguments) + }, +} diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/output.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/output.js new file mode 100644 index 0000000000000..d9446d59b1c2f --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/output.js @@ -0,0 +1,28 @@ +/* __next_internal_action_entry_do_not_use__ {"006a88810ecce4a4e8b59d53b8327d7e98bbf251d7":"$$RSC_SERVER_ACTION_0"} */ import { registerServerReference } from "private-next-rsc-server-reference"; +import { encryptActionBoundArgs, decryptActionBoundArgs } from "private-next-rsc-action-encryption"; +import { cache as $$cache__ } from "private-next-rsc-cache-wrapper"; +export const /*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ $$RSC_SERVER_ACTION_0 = async function b() { + // this is not allowed here + this.foo(); + // arguments is not allowed here + console.log(arguments); +}; +// not exported! +async function a() { + // this is allowed here + this.foo(); + // arguments is allowed here + console.log(arguments); + const b = registerServerReference($$RSC_SERVER_ACTION_0, "006a88810ecce4a4e8b59d53b8327d7e98bbf251d7", null); +} +export const obj = { + foo () { + return 42; + }, + bar () { + // this is allowed here + this.foo(); + // arguments is allowed here + console.log(arguments); + } +}; diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/output.stderr b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/output.stderr new file mode 100644 index 0000000000000..1963f4078d5f3 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/output.stderr @@ -0,0 +1,16 @@ + x Server Actions cannot use `this`. + | + ,-[input.js:13:1] + 12 | // this is not allowed here + 13 | this.foo() + : ^^^^ + 14 | // arguments is not allowed here + `---- + x Server Actions cannot use `arguments`. + | + ,-[input.js:15:1] + 14 | // arguments is not allowed here + 15 | console.log(arguments) + : ^^^^^^^^^ + 16 | } + `---- diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/24/input.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/24/input.js new file mode 100644 index 0000000000000..e5e254581cb8d --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/24/input.js @@ -0,0 +1,38 @@ +'use cache' + +// exported! +export async function a() { + // this is not allowed here + this.foo() + // arguments is not allowed here + console.log(arguments) + + const b = async () => { + // this is not allowed here + this.foo() + // arguments is not allowed here + console.log(arguments) + } + + function c() { + // this is allowed here + this.foo() + // arguments is allowed here + console.log(arguments) + + const d = () => { + // this is allowed here + this.foo() + // arguments is allowed here + console.log(arguments) + } + + const e = async () => { + 'use server' + // this is not allowed here + this.foo() + // arguments is not allowed here + console.log(arguments) + } + } +} diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/24/output.js b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/24/output.js new file mode 100644 index 0000000000000..b98e7e1aef798 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/24/output.js @@ -0,0 +1,40 @@ +/* __next_internal_action_entry_do_not_use__ {"006a88810ecce4a4e8b59d53b8327d7e98bbf251d7":"$$RSC_SERVER_ACTION_0","80951c375b4a6a6e89d67b743ec5808127cfde405d":"$$RSC_SERVER_CACHE_1"} */ import { registerServerReference } from "private-next-rsc-server-reference"; +import { encryptActionBoundArgs, decryptActionBoundArgs } from "private-next-rsc-action-encryption"; +import { cache as $$cache__ } from "private-next-rsc-cache-wrapper"; +export const /*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ $$RSC_SERVER_ACTION_0 = async function e() { + // this is not allowed here + this.foo(); + // arguments is not allowed here + console.log(arguments); +}; +export var $$RSC_SERVER_CACHE_1 = $$cache__("default", "80951c375b4a6a6e89d67b743ec5808127cfde405d", 0, /*#__TURBOPACK_DISABLE_EXPORT_MERGING__*/ async function a() { + // this is not allowed here + this.foo(); + // arguments is not allowed here + console.log(arguments); + const b = async ()=>{ + // this is not allowed here + this.foo(); + // arguments is not allowed here + console.log(arguments); + }; + function c() { + // this is allowed here + this.foo(); + // arguments is allowed here + console.log(arguments); + const d = ()=>{ + // this is allowed here + this.foo(); + // arguments is allowed here + console.log(arguments); + }; + const e = registerServerReference($$RSC_SERVER_ACTION_0, "006a88810ecce4a4e8b59d53b8327d7e98bbf251d7", null); + } +}); +Object.defineProperty($$RSC_SERVER_CACHE_1, "name", { + "value": "a", + "writable": false +}); +// exported! +export var a = registerServerReference($$RSC_SERVER_CACHE_1, "80951c375b4a6a6e89d67b743ec5808127cfde405d", null); diff --git a/crates/next-custom-transforms/tests/errors/server-actions/server-graph/24/output.stderr b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/24/output.stderr new file mode 100644 index 0000000000000..c25781805f2e3 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/24/output.stderr @@ -0,0 +1,47 @@ + x "use cache" functions cannot use `this`. + | + ,-[input.js:6:1] + 5 | // this is not allowed here + 6 | this.foo() + : ^^^^ + 7 | // arguments is not allowed here + `---- + x "use cache" functions cannot use `arguments`. + | + ,-[input.js:8:1] + 7 | // arguments is not allowed here + 8 | console.log(arguments) + : ^^^^^^^^^ + `---- + x "use cache" functions cannot use `this`. + | + ,-[input.js:12:1] + 11 | // this is not allowed here + 12 | this.foo() + : ^^^^ + 13 | // arguments is not allowed here + `---- + x "use cache" functions cannot use `arguments`. + | + ,-[input.js:14:1] + 13 | // arguments is not allowed here + 14 | console.log(arguments) + : ^^^^^^^^^ + 15 | } + `---- + x Server Actions cannot use `this`. + | + ,-[input.js:33:1] + 32 | // this is not allowed here + 33 | this.foo() + : ^^^^ + 34 | // arguments is not allowed here + `---- + x Server Actions cannot use `arguments`. + | + ,-[input.js:35:1] + 34 | // arguments is not allowed here + 35 | console.log(arguments) + : ^^^^^^^^^ + 36 | } + `----