From 559c93d821833397d6e26eb7a52e5fd28915929f Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Thu, 21 Nov 2024 15:41:17 +0100 Subject: [PATCH 1/5] Forbid `this` and `arguments` in server functions --- .../src/transforms/server_actions.rs | 74 +++++++++++++++++++ .../server-actions/server-graph/22/input.js | 50 +++++++++++++ .../server-actions/server-graph/22/output.js | 55 ++++++++++++++ .../server-graph/22/output.stderr | 63 ++++++++++++++++ .../server-actions/server-graph/23/input.js | 29 ++++++++ .../server-actions/server-graph/23/output.js | 28 +++++++ .../server-graph/23/output.stderr | 16 ++++ .../server-actions/server-graph/24/input.js | 38 ++++++++++ .../server-actions/server-graph/24/output.js | 40 ++++++++++ .../server-graph/24/output.stderr | 47 ++++++++++++ 10 files changed, 440 insertions(+) create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/input.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/output.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/output.stderr create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/input.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/output.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/output.stderr create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/24/input.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/24/output.js create mode 100644 crates/next-custom-transforms/tests/errors/server-actions/server-graph/24/output.stderr diff --git a/crates/next-custom-transforms/src/transforms/server_actions.rs b/crates/next-custom-transforms/src/transforms/server_actions.rs index 3568bac6adc70..dde3236f61e8d 100644 --- a/crates/next-custom-transforms/src/transforms/server_actions.rs +++ b/crates/next-custom-transforms/src/transforms/server_actions.rs @@ -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, @@ -931,6 +944,12 @@ 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; @@ -1070,12 +1089,15 @@ impl VisitMut for ServerActions { } fn visit_mut_fn_decl(&mut self, f: &mut FnDecl) { + let old_this_status = self.this_status.clone(); + 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,6 +1113,12 @@ 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); @@ -1203,13 +1231,19 @@ 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 = self.this_status.clone(); + 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 +1251,7 @@ impl VisitMut for ServerActions { }))); self.rewrite_expr_to_proxy_expr = None; } + return; } _ => {} @@ -2027,6 +2062,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 +2828,23 @@ fn emit_error(error_kind: ServerActionsErrorKind) { } }, ), + ServerActionsErrorKind::ForbiddenExpression { + span, + expr, + directive, + } => ( + span, + formatdoc! { + r#" + {subject} can not 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..f793892aa77a1 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/22/output.stderr @@ -0,0 +1,63 @@ + x "use cache" functions can not 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 can not use `arguments`. + | + ,-[input.js:6:1] + 5 | // arguments is not allowed here + 6 | console.log(arguments) + : ^^^^^^^^^ + `---- + x "use cache" functions can not 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 can not use `arguments`. + | + ,-[input.js:12:1] + 11 | // arguments is not allowed here + 12 | console.log(arguments) + : ^^^^^^^^^ + 13 | } + `---- + x Server Actions can not use `this`. + | + ,-[input.js:31:1] + 30 | // this is not allowed here + 31 | this.foo() + : ^^^^ + 32 | // arguments is not allowed here + `---- + x Server Actions can not use `arguments`. + | + ,-[input.js:33:1] + 32 | // arguments is not allowed here + 33 | console.log(arguments) + : ^^^^^^^^^ + 34 | } + `---- + x "use cache" functions can not 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 can not 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..f578b358d8d0f --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/23/output.stderr @@ -0,0 +1,16 @@ + x Server Actions can not use `this`. + | + ,-[input.js:13:1] + 12 | // this is not allowed here + 13 | this.foo() + : ^^^^ + 14 | // arguments is not allowed here + `---- + x Server Actions can not 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..d7a8ce1daede4 --- /dev/null +++ b/crates/next-custom-transforms/tests/errors/server-actions/server-graph/24/output.stderr @@ -0,0 +1,47 @@ + x "use cache" functions can not 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 can not use `arguments`. + | + ,-[input.js:8:1] + 7 | // arguments is not allowed here + 8 | console.log(arguments) + : ^^^^^^^^^ + `---- + x "use cache" functions can not 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 can not use `arguments`. + | + ,-[input.js:14:1] + 13 | // arguments is not allowed here + 14 | console.log(arguments) + : ^^^^^^^^^ + 15 | } + `---- + x Server Actions can not use `this`. + | + ,-[input.js:33:1] + 32 | // this is not allowed here + 33 | this.foo() + : ^^^^ + 34 | // arguments is not allowed here + `---- + x Server Actions can not use `arguments`. + | + ,-[input.js:35:1] + 34 | // arguments is not allowed here + 35 | console.log(arguments) + : ^^^^^^^^^ + 36 | } + `---- From 0419d486efcc276540a42738f0f2cdf2f58d99b7 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Fri, 22 Nov 2024 10:46:25 +0100 Subject: [PATCH 2/5] Ignore `this` expression in generated `jsxDEV` calls --- .../src/transforms/server_actions.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/next-custom-transforms/src/transforms/server_actions.rs b/crates/next-custom-transforms/src/transforms/server_actions.rs index dde3236f61e8d..50e5d0e1daf26 100644 --- a/crates/next-custom-transforms/src/transforms/server_actions.rs +++ b/crates/next-custom-transforms/src/transforms/server_actions.rs @@ -1272,6 +1272,24 @@ 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` exression, + // 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; From 21859fc9295f357eb6001bbb157516e7dc3fb5ec Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Sat, 23 Nov 2024 21:36:33 +0100 Subject: [PATCH 3/5] Update server_actions.rs Co-authored-by: Janka Uryga --- crates/next-custom-transforms/src/transforms/server_actions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/next-custom-transforms/src/transforms/server_actions.rs b/crates/next-custom-transforms/src/transforms/server_actions.rs index 50e5d0e1daf26..f0806e4f5a1a4 100644 --- a/crates/next-custom-transforms/src/transforms/server_actions.rs +++ b/crates/next-custom-transforms/src/transforms/server_actions.rs @@ -1275,7 +1275,7 @@ impl VisitMut for ServerActions { 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` exression, + // 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 { From 51cacc4ed899dbf746c37275e579abd83d8b7b15 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Sat, 23 Nov 2024 21:43:30 +0100 Subject: [PATCH 4/5] Fix typo Co-authored-by: Janka Uryga --- .../src/transforms/server_actions.rs | 2 +- .../server-actions/server-graph/22/output.stderr | 16 ++++++++-------- .../server-actions/server-graph/23/output.stderr | 4 ++-- .../server-actions/server-graph/24/output.stderr | 12 ++++++------ 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/crates/next-custom-transforms/src/transforms/server_actions.rs b/crates/next-custom-transforms/src/transforms/server_actions.rs index f0806e4f5a1a4..56a97c396f560 100644 --- a/crates/next-custom-transforms/src/transforms/server_actions.rs +++ b/crates/next-custom-transforms/src/transforms/server_actions.rs @@ -2854,7 +2854,7 @@ fn emit_error(error_kind: ServerActionsErrorKind) { span, formatdoc! { r#" - {subject} can not use `{expr}`. + {subject} cannot use `{expr}`. "#, subject = if let Directive::UseServer = directive { "Server Actions" 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 index f793892aa77a1..4051705fbb63b 100644 --- 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 @@ -1,4 +1,4 @@ - x "use cache" functions can not use `this`. + x "use cache" functions cannot use `this`. | ,-[input.js:4:1] 3 | // this is not allowed here @@ -6,14 +6,14 @@ : ^^^^ 5 | // arguments is not allowed here `---- - x "use cache" functions can not use `arguments`. + 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 can not use `this`. + x "use cache" functions cannot use `this`. | ,-[input.js:10:1] 9 | // this is not allowed here @@ -21,7 +21,7 @@ : ^^^^ 11 | // arguments is not allowed here `---- - x "use cache" functions can not use `arguments`. + x "use cache" functions cannot use `arguments`. | ,-[input.js:12:1] 11 | // arguments is not allowed here @@ -29,7 +29,7 @@ : ^^^^^^^^^ 13 | } `---- - x Server Actions can not use `this`. + x Server Actions cannot use `this`. | ,-[input.js:31:1] 30 | // this is not allowed here @@ -37,7 +37,7 @@ : ^^^^ 32 | // arguments is not allowed here `---- - x Server Actions can not use `arguments`. + x Server Actions cannot use `arguments`. | ,-[input.js:33:1] 32 | // arguments is not allowed here @@ -45,7 +45,7 @@ : ^^^^^^^^^ 34 | } `---- - x "use cache" functions can not use `this`. + x "use cache" functions cannot use `this`. | ,-[input.js:45:1] 44 | // this is not allowed here @@ -53,7 +53,7 @@ : ^^^^ 46 | // arguments is not allowed here `---- - x "use cache" functions can not use `arguments`. + x "use cache" functions cannot use `arguments`. | ,-[input.js:47:1] 46 | // arguments is not allowed here 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 index f578b358d8d0f..1963f4078d5f3 100644 --- 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 @@ -1,4 +1,4 @@ - x Server Actions can not use `this`. + x Server Actions cannot use `this`. | ,-[input.js:13:1] 12 | // this is not allowed here @@ -6,7 +6,7 @@ : ^^^^ 14 | // arguments is not allowed here `---- - x Server Actions can not use `arguments`. + x Server Actions cannot use `arguments`. | ,-[input.js:15:1] 14 | // arguments is not allowed here 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 index d7a8ce1daede4..c25781805f2e3 100644 --- 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 @@ -1,4 +1,4 @@ - x "use cache" functions can not use `this`. + x "use cache" functions cannot use `this`. | ,-[input.js:6:1] 5 | // this is not allowed here @@ -6,14 +6,14 @@ : ^^^^ 7 | // arguments is not allowed here `---- - x "use cache" functions can not use `arguments`. + 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 can not use `this`. + x "use cache" functions cannot use `this`. | ,-[input.js:12:1] 11 | // this is not allowed here @@ -21,7 +21,7 @@ : ^^^^ 13 | // arguments is not allowed here `---- - x "use cache" functions can not use `arguments`. + x "use cache" functions cannot use `arguments`. | ,-[input.js:14:1] 13 | // arguments is not allowed here @@ -29,7 +29,7 @@ : ^^^^^^^^^ 15 | } `---- - x Server Actions can not use `this`. + x Server Actions cannot use `this`. | ,-[input.js:33:1] 32 | // this is not allowed here @@ -37,7 +37,7 @@ : ^^^^ 34 | // arguments is not allowed here `---- - x Server Actions can not use `arguments`. + x Server Actions cannot use `arguments`. | ,-[input.js:35:1] 34 | // arguments is not allowed here From f4d0aaa02457ebb0b029bcd5ebdb038c36a659f9 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Sat, 23 Nov 2024 22:08:14 +0100 Subject: [PATCH 5/5] Use `replace` to store old values and set new ones Co-authored-by: Janka Uryga --- .../src/transforms/server_actions.rs | 66 ++++++++----------- 1 file changed, 26 insertions(+), 40 deletions(-) diff --git a/crates/next-custom-transforms/src/transforms/server_actions.rs b/crates/next-custom-transforms/src/transforms/server_actions.rs index 56a97c396f560..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; @@ -910,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) { @@ -952,16 +948,12 @@ impl VisitMut for ServerActions { // 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; @@ -1089,8 +1081,7 @@ impl VisitMut for ServerActions { } fn visit_mut_fn_decl(&mut self, f: &mut FnDecl) { - let old_this_status = self.this_status.clone(); - self.this_status = ThisStatus::Allowed; + 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 @@ -1124,14 +1115,11 @@ impl VisitMut for ServerActions { { // 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); @@ -1236,8 +1224,7 @@ impl VisitMut for ServerActions { self.arrow_or_fn_expr_ident = Some(ident_name.clone().into()); } - let old_this_status = self.this_status.clone(); - self.this_status = ThisStatus::Allowed; + 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); @@ -1275,9 +1262,9 @@ impl VisitMut for ServerActions { 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 + // 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); @@ -1291,8 +1278,7 @@ impl VisitMut for ServerActions { } 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; }