From ac37d5560080137d734fc660c975c5ceb09b9002 Mon Sep 17 00:00:00 2001 From: Wang Wenzhe Date: Tue, 16 Apr 2024 09:44:29 +0800 Subject: [PATCH] feat(linter/tree-shaking): support DoWhileStatement and IfStatement (#2994) --- .../listener_map.rs | 22 ++++-- .../no_side_effects_in_initialization/mod.rs | 52 +++++++------- .../no_side_effects_in_initialization.snap | 72 +++++++++++++++++++ crates/oxc_linter/src/utils/tree_shaking.rs | 50 ++++++++++++- 4 files changed, 165 insertions(+), 31 deletions(-) diff --git a/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/listener_map.rs b/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/listener_map.rs index dbb4e70db..b5a0326ac 100644 --- a/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/listener_map.rs +++ b/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/listener_map.rs @@ -104,8 +104,8 @@ impl<'a> ListenerMap for Statement<'a> { Self::IfStatement(stmt) => { let test_result = stmt.test.get_value_and_report_effects(options); - if let Some(falsy) = test_result.get_falsy_value() { - if falsy { + if let Some(is_falsy) = test_result.get_falsy_value() { + if is_falsy { if let Some(alternate) = &stmt.alternate { alternate.report_effects(options); } @@ -119,6 +119,20 @@ impl<'a> ListenerMap for Statement<'a> { } } } + Self::DoWhileStatement(stmt) => { + if stmt + .test + .get_value_and_report_effects(options) + .get_falsy_value() + .is_some_and(|is_falsy| is_falsy) + { + return; + } + stmt.body.report_effects(options); + } + Self::DebuggerStatement(stmt) => { + options.ctx.diagnostic(NoSideEffectsDiagnostic::Debugger(stmt.span)); + } _ => {} } } @@ -435,8 +449,8 @@ impl<'a> ListenerMap for ConditionalExpression<'a> { fn get_value_and_report_effects(&self, options: &NodeListenerOptions) -> Value { let test_result = self.test.get_value_and_report_effects(options); - if let Some(falsy) = test_result.get_falsy_value() { - if falsy { + if let Some(is_falsy) = test_result.get_falsy_value() { + if is_falsy { self.alternate.get_value_and_report_effects(options) } else { self.consequent.get_value_and_report_effects(options) diff --git a/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/mod.rs b/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/mod.rs index c0a9998ac..bc3e896c5 100644 --- a/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/mod.rs +++ b/crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/mod.rs @@ -53,6 +53,10 @@ enum NoSideEffectsDiagnostic { #[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling function parameter")] #[diagnostic(severity(warning))] CallParameter(#[label] Span), + + #[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Debugger statements are side-effects")] + #[diagnostic(severity(warning))] + Debugger(#[label] Span), } /// @@ -171,12 +175,12 @@ fn test() { "const x = ()=>{}; (false ? ext : x)()", // ContinueStatement "while(true){continue}", - // // DoWhileStatement - // "do {} while(true)", - // "do {} while(ext > 0)", - // "const x = ()=>{}; do x(); while(true)", - // // EmptyStatement - // ";", + // DoWhileStatement + "do {} while(true)", + "do {} while(ext > 0)", + "const x = ()=>{}; do x(); while(true)", + // EmptyStatement + ";", // // ExportAllDeclaration // r#"export * from "import""#, // // ExportDefaultDeclaration @@ -231,9 +235,9 @@ fn test() { // // Identifier when mutated // "const x = {}; x.y = ext", // // IfStatement - // "let y;if (ext > 0) {y = 1} else {y = 2}", - // "if (false) {ext()}", - // "if (true) {} else {ext()}", + "let y;if (ext > 0) {y = 1} else {y = 2}", + "if (false) {ext()}", + "if (true) {} else {ext()}", // // ImportDeclaration // r#"import "import""#, // r#"import x from "import-default""#, @@ -442,16 +446,16 @@ fn test() { "const x = ext ? ext() : 2", "const x = ext ? 1 : ext()", "if (false ? false : true) ext()", - // // ConditionalExpression when called - // "const x = ()=>{}; (true ? ext : x)()", - // "const x = ()=>{}; (false ? x : ext)()", - // "const x = ()=>{}; (ext ? x : ext)()", - // // DebuggerStatement - // "debugger", - // // DoWhileStatement - // "do {} while(ext())", - // "do ext(); while(true)", - // "do {ext()} while(true)", + // ConditionalExpression when called + "const x = ()=>{}; (true ? ext : x)()", + "const x = ()=>{}; (false ? x : ext)()", + "const x = ()=>{}; (ext ? x : ext)()", + // DebuggerStatement + "debugger", + // DoWhileStatement + "do {} while(ext())", + "do ext(); while(true)", + "do {ext()} while(true)", // // ExportDefaultDeclaration // "export default ext()", // "export default /* tree-shaking no-side-effects-when-called */ ext", @@ -523,11 +527,11 @@ fn test() { // "var x = {}; var x = ext; x.y = 1", // "var x = {}; x = ext; x.y = 1; x.y = 1; x.y = 1", // "const x = {y:ext}; const {y} = x; y.z = 1", - // // IfStatement - // "if (ext()>0){}", - // "if (1>0){ext()}", - // "if (1<0){} else {ext()}", - // "if (ext>0){ext()} else {ext()}", + // IfStatement + "if (ext()>0){}", + "if (1>0){ext()}", + "if (1<0){} else {ext()}", + "if (ext>0){ext()} else {ext()}", // // ImportDeclaration // r#"import x from "import-default"; x()"#, // r#"import x from "import-default"; x.z = 1"#, diff --git a/crates/oxc_linter/src/snapshots/no_side_effects_in_initialization.snap b/crates/oxc_linter/src/snapshots/no_side_effects_in_initialization.snap index 824702933..47baa2446 100644 --- a/crates/oxc_linter/src/snapshots/no_side_effects_in_initialization.snap +++ b/crates/oxc_linter/src/snapshots/no_side_effects_in_initialization.snap @@ -326,6 +326,78 @@ expression: no_side_effects_in_initialization · ─── ╰──── + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext` + ╭─[no_side_effects_in_initialization.tsx:1:27] + 1 │ const x = ()=>{}; (true ? ext : x)() + · ─── + ╰──── + + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext` + ╭─[no_side_effects_in_initialization.tsx:1:32] + 1 │ const x = ()=>{}; (false ? x : ext)() + · ─── + ╰──── + + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext` + ╭─[no_side_effects_in_initialization.tsx:1:30] + 1 │ const x = ()=>{}; (ext ? x : ext)() + · ─── + ╰──── + + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Debugger statements are side-effects + ╭─[no_side_effects_in_initialization.tsx:1:1] + 1 │ debugger + · ──────── + ╰──── + + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext` + ╭─[no_side_effects_in_initialization.tsx:1:13] + 1 │ do {} while(ext()) + · ─── + ╰──── + + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext` + ╭─[no_side_effects_in_initialization.tsx:1:4] + 1 │ do ext(); while(true) + · ─── + ╰──── + + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext` + ╭─[no_side_effects_in_initialization.tsx:1:5] + 1 │ do {ext()} while(true) + · ─── + ╰──── + + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext` + ╭─[no_side_effects_in_initialization.tsx:1:5] + 1 │ if (ext()>0){} + · ─── + ╰──── + + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext` + ╭─[no_side_effects_in_initialization.tsx:1:10] + 1 │ if (1>0){ext()} + · ─── + ╰──── + + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext` + ╭─[no_side_effects_in_initialization.tsx:1:18] + 1 │ if (1<0){} else {ext()} + · ─── + ╰──── + + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext` + ╭─[no_side_effects_in_initialization.tsx:1:12] + 1 │ if (ext>0){ext()} else {ext()} + · ─── + ╰──── + + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext` + ╭─[no_side_effects_in_initialization.tsx:1:25] + 1 │ if (ext>0){ext()} else {ext()} + · ─── + ╰──── + ⚠ eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling global function `ext` ╭─[no_side_effects_in_initialization.tsx:1:15] 1 │ const x = new ext() diff --git a/crates/oxc_linter/src/utils/tree_shaking.rs b/crates/oxc_linter/src/utils/tree_shaking.rs index 958037210..0059046cb 100644 --- a/crates/oxc_linter/src/utils/tree_shaking.rs +++ b/crates/oxc_linter/src/utils/tree_shaking.rs @@ -5,7 +5,7 @@ use oxc_syntax::operator::BinaryOperator; use crate::LintContext; -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug, PartialEq)] pub enum Value { Boolean(bool), Number(f64), @@ -14,7 +14,7 @@ pub enum Value { } // We only care if it is falsy value (empty string). -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug, PartialEq)] pub enum StringValue { Empty, NonEmpty, @@ -49,7 +49,7 @@ impl Value { Value::Unknown => None, Value::Boolean(boolean) => Some(!*boolean), Value::Number(num) => Some(*num == 0.0), - Value::String(str) => Some(matches!(str, StringValue::Empty)), + Value::String(str) => Some(!matches!(str, StringValue::Empty)), } } } @@ -89,6 +89,7 @@ pub fn has_pure_notation(span: Span, ctx: &LintContext) -> bool { } /// Port from +/// pub fn calculate_binary_operation(op: BinaryOperator, left: Value, right: Value) -> Value { match op { BinaryOperator::Addition => match (left, right) { @@ -106,6 +107,49 @@ pub fn calculate_binary_operation(op: BinaryOperator, left: Value, right: Value) (Value::Number(a), Value::Number(b)) => Value::Number(a - b), _ => Value::Unknown, }, + // + #[allow(clippy::single_match)] + BinaryOperator::LessThan => match (left, right) { + // + (Value::Unknown, Value::Number(_)) | (Value::Number(_), Value::Unknown) => { + Value::Boolean(false) + } + (Value::Number(a), Value::Number(b)) => Value::Boolean(a < b), + _ => Value::Unknown, + }, _ => Value::Unknown, } } + +#[test] +fn test_calculate_binary_operation() { + use oxc_syntax::operator::BinaryOperator; + + let fun = calculate_binary_operation; + + // "+" + let op = BinaryOperator::Addition; + assert_eq!(fun(op, Value::Number(1.0), Value::Number(2.0),), Value::Number(3.0)); + assert_eq!( + fun(op, Value::String(StringValue::Empty), Value::String(StringValue::Empty)), + Value::String(StringValue::Empty) + ); + assert_eq!( + fun(op, Value::String(StringValue::Empty), Value::String(StringValue::NonEmpty)), + Value::String(StringValue::NonEmpty) + ); + + assert_eq!( + fun(op, Value::String(StringValue::NonEmpty), Value::String(StringValue::NonEmpty)), + Value::String(StringValue::NonEmpty) + ); + + // "-" + let op = BinaryOperator::Subtraction; + assert_eq!(fun(op, Value::Number(1.0), Value::Number(2.0),), Value::Number(-1.0)); + + // "<" + let op = BinaryOperator::LessThan; + assert_eq!(fun(op, Value::Number(1.0), Value::Number(2.0),), Value::Boolean(true)); + assert_eq!(fun(op, Value::Unknown, Value::Number(2.0),), Value::Boolean(false)); +}