Skip to content

Commit

Permalink
feat(linter/tree-shaking): support ConditionalExpression (#2965)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mysteryven
mysteryven authored Apr 14, 2024
1 parent da5ea41 commit 5b02ae1
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 26 deletions.
62 changes: 59 additions & 3 deletions crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/listener_map.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use oxc_ast::{
ast::{
Argument, ArrayExpressionElement, ArrowFunctionExpression, AssignmentTarget,
BinaryExpression, BindingPattern, BindingPatternKind, CallExpression, Class, ClassBody,
ClassElement, ComputedMemberExpression, Declaration, Expression, FormalParameter, Function,
IdentifierReference, MemberExpression, ModuleDeclaration, NewExpression,
ParenthesizedExpression, PrivateFieldExpression, Program, PropertyKey,
ClassElement, ComputedMemberExpression, ConditionalExpression, Declaration, Expression,
FormalParameter, Function, IdentifierReference, MemberExpression, ModuleDeclaration,
NewExpression, ParenthesizedExpression, PrivateFieldExpression, Program, PropertyKey,
SimpleAssignmentTarget, Statement, StaticMemberExpression, ThisExpression,
VariableDeclarator,
},
Expand Down Expand Up @@ -101,6 +101,24 @@ impl<'a> ListenerMap for Statement<'a> {
Self::BlockStatement(stmt) => {
stmt.body.iter().for_each(|stmt| stmt.report_effects(options));
}
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(alternate) = &stmt.alternate {
alternate.report_effects(options);
}
} else {
stmt.consequent.report_effects(options);
}
} else {
stmt.consequent.report_effects(options);
if let Some(alternate) = &stmt.alternate {
alternate.report_effects(options);
}
}
}
_ => {}
}
}
Expand Down Expand Up @@ -319,6 +337,9 @@ impl<'a> ListenerMap for Expression<'a> {
Self::ClassExpression(expr) => {
expr.report_effects(options);
}
Self::ConditionalExpression(expr) => {
expr.get_value_and_report_effects(options);
}
Self::ArrowFunctionExpression(_)
| Self::FunctionExpression(_)
| Self::Identifier(_)
Expand Down Expand Up @@ -373,6 +394,7 @@ impl<'a> ListenerMap for Expression<'a> {
Self::ClassExpression(expr) => {
expr.report_effects_when_called(options);
}
Self::ConditionalExpression(expr) => expr.report_effects_when_called(options),
_ => {
// Default behavior
options.ctx.diagnostic(NoSideEffectsDiagnostic::Call(self.span()));
Expand All @@ -385,6 +407,8 @@ impl<'a> ListenerMap for Expression<'a> {
| Self::StringLiteral(_)
| Self::NumericLiteral(_)
| Self::TemplateLiteral(_) => Value::new(self),
Self::BinaryExpression(expr) => expr.get_value_and_report_effects(options),
Self::ConditionalExpression(expr) => expr.get_value_and_report_effects(options),
_ => {
self.report_effects(options);
Value::Unknown
Expand All @@ -407,6 +431,38 @@ fn defined_custom_report_effects_when_called(expr: &Expression) -> bool {
)
}

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 {
self.alternate.get_value_and_report_effects(options)
} else {
self.consequent.get_value_and_report_effects(options)
}
} else {
self.consequent.report_effects(options);
self.alternate.report_effects(options);
test_result
}
}
fn report_effects_when_called(&self, options: &NodeListenerOptions) {
let test_result = self.test.get_value_and_report_effects(options);

if let Some(falsy) = test_result.get_falsy_value() {
if falsy {
self.alternate.report_effects_when_called(options);
} else {
self.consequent.report_effects_when_called(options);
}
} else {
self.consequent.report_effects_when_called(options);
self.alternate.report_effects_when_called(options);
}
}
}

impl<'a> ListenerMap for BinaryExpression<'a> {
fn get_value_and_report_effects(&self, options: &NodeListenerOptions) -> Value {
let left = self.left.get_value_and_report_effects(options);
Expand Down
44 changes: 22 additions & 22 deletions crates/oxc_linter/src/rules/tree_shaking/no_side_effects_in_initialization/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,19 +158,19 @@ fn test() {
"class x {y}",
"class x {y = 1}",
"class x {y = ext()}",
// // ConditionalExpression
// "const x = ext ? 1 : 2",
// "const x = true ? 1 : ext()",
// "const x = false ? ext() : 2",
// "if (true ? false : true) ext()",
// "ext ? 1 : ext.x",
// "ext ? ext.x : 1",
// ConditionalExpression
"const x = ext ? 1 : 2",
"const x = true ? 1 : ext()",
"const x = false ? ext() : 2",
"if (true ? false : true) ext()",
"ext ? 1 : ext.x",
"ext ? ext.x : 1",
// // ConditionalExpression when called
// "const x = ()=>{}, y = ()=>{};(ext ? x : y)()",
// "const x = ()=>{}; (true ? x : ext)()",
// "const x = ()=>{}; (false ? ext : x)()",
// // ContinueStatement
// "while(true){continue}",
"const x = ()=>{}, y = ()=>{};(ext ? x : y)()",
"const x = ()=>{}; (true ? x : ext)()",
"const x = ()=>{}; (false ? ext : x)()",
// ContinueStatement
"while(true){continue}",
// // DoWhileStatement
// "do {} while(true)",
// "do {} while(ext > 0)",
Expand Down Expand Up @@ -410,23 +410,23 @@ fn test() {
"try {} catch (error) {ext()}",
// TODO: check global function `ext` call when called `x()` in no strict mode
// "var x=()=>{}; try {} catch (error) {var x=ext}; x()",
// // ClassBody
// ClassBody
"class x {[ext()](){}}",
// // ClassBody when called
// ClassBody when called
"class x {constructor(){ext()}}; new x()",
"class x {constructor(){ext()}}; const y = new x()",
"class x extends ext {}; const y = new x()",
"class y {constructor(){ext()}}; class x extends y {}; const z = new x()",
"class y {constructor(){ext()}}; class x extends y {constructor(){super()}}; const z = new x()",
"class y{}; class x extends y{constructor(){super()}}; const z = new x()",
// // ClassDeclaration
// ClassDeclaration
"class x extends ext() {}",
"class x {[ext()](){}}",
// // ClassDeclaration when called
// ClassDeclaration when called
"class x {constructor(){ext()}}; new x()",
"class x {constructor(){ext()}}; const y = new x()",
"class x extends ext {}; const y = new x()",
// // ClassExpression
// ClassExpression
"const x = class extends ext() {}",
"const x = class {[ext()](){}}",
// ClassExpression when called
Expand All @@ -437,11 +437,11 @@ fn test() {
"class x {[ext()] = 1}",
// ClassProperty when called
"class x {y = ext()}; new x()",
// // ConditionalExpression
// "const x = ext() ? 1 : 2",
// "const x = ext ? ext() : 2",
// "const x = ext ? 1 : ext()",
// "if (false ? false : true) ext()",
// ConditionalExpression
"const x = ext() ? 1 : 2",
"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)()",
Expand Down
24 changes: 24 additions & 0 deletions crates/oxc_linter/src/snapshots/no_side_effects_in_initialization.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,30 @@ 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:11]
1 │ const x = ext() ? 1 : 2
· ───
╰────

⚠ 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:17]
1 │ const x = ext ? ext() : 2
· ───
╰────

⚠ 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:21]
1 │ const x = ext ? 1 : 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:27]
1 │ if (false ? false : true) 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()
Expand Down
15 changes: 14 additions & 1 deletion crates/oxc_linter/src/utils/tree_shaking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use oxc_syntax::operator::BinaryOperator;

use crate::LintContext;

#[allow(dead_code)]
#[derive(Copy, Clone)]
pub enum Value {
Boolean(bool),
Number(f64),
Expand All @@ -14,6 +14,7 @@ pub enum Value {
}

// We only care if it is falsy value (empty string).
#[derive(Copy, Clone)]
pub enum StringValue {
Empty,
NonEmpty,
Expand Down Expand Up @@ -43,6 +44,14 @@ impl Value {
_ => Value::Unknown,
}
}
pub fn get_falsy_value(&self) -> Option<bool> {
match &self {
Value::Unknown => None,
Value::Boolean(boolean) => Some(!*boolean),
Value::Number(num) => Some(*num == 0.0),
Value::String(str) => Some(matches!(str, StringValue::Empty)),
}
}
}

pub fn get_write_expr<'a, 'b>(
Expand Down Expand Up @@ -93,6 +102,10 @@ pub fn calculate_binary_operation(op: BinaryOperator, left: Value, right: Value)
}
_ => Value::Unknown,
},
BinaryOperator::Subtraction => match (left, right) {
(Value::Number(a), Value::Number(b)) => Value::Number(a - b),
_ => Value::Unknown,
},
_ => Value::Unknown,
}
}

0 comments on commit 5b02ae1

Please sign in to comment.