Skip to content

Commit

Permalink
feat(linter/tree-shaking): support ThisExpression and NewExpression (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
mysteryven authored Apr 3, 2024
1 parent d3eb1c3 commit ce34829
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 22 deletions.
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use std::cell::RefCell;
use std::cell::{Cell, RefCell};

use oxc_ast::{
ast::{
Argument, ArrayExpressionElement, ArrowFunctionExpression, AssignmentTarget,
BindingPattern, BindingPatternKind, CallExpression, ComputedMemberExpression, Declaration,
Expression, FormalParameter, Function, IdentifierReference, MemberExpression,
ModuleDeclaration, ParenthesizedExpression, PrivateFieldExpression, Program,
SimpleAssignmentTarget, Statement, StaticMemberExpression, VariableDeclarator,
ModuleDeclaration, NewExpression, ParenthesizedExpression, PrivateFieldExpression, Program,
SimpleAssignmentTarget, Statement, StaticMemberExpression, ThisExpression,
VariableDeclarator,
},
AstKind,
};
Expand All @@ -16,14 +17,16 @@ use rustc_hash::FxHashSet;

use crate::{
ast_util::{get_declaration_of_variable, get_symbol_id_of_variable},
utils::{get_write_expr, no_effects, Value},
utils::{get_write_expr, has_pure_notation, no_effects, Value},
LintContext,
};

use super::NoSideEffectsDiagnostic;
pub struct NodeListenerOptions<'a, 'b> {
checked_mutated_nodes: RefCell<FxHashSet<SymbolId>>,
ctx: &'b LintContext<'a>,
has_valid_this: Cell<bool>,
call_with_new: Cell<bool>,
}

impl<'a, 'b> NodeListenerOptions<'a, 'b> {
Expand All @@ -34,7 +37,12 @@ impl<'a, 'b> NodeListenerOptions<'a, 'b> {

impl<'a, 'b> NodeListenerOptions<'a, 'b> {
pub fn new(ctx: &'b LintContext<'a>) -> Self {
Self { checked_mutated_nodes: RefCell::new(FxHashSet::default()), ctx }
Self {
checked_mutated_nodes: RefCell::new(FxHashSet::default()),
ctx,
has_valid_this: Cell::new(false),
call_with_new: Cell::new(false),
}
}
}

Expand Down Expand Up @@ -119,14 +127,14 @@ impl<'a> ListenerMap for AstNode<'a> {
}
}
AstKind::FormalParameter(param) => {
options.ctx.diagnostic(NoSideEffectsDiagnostic::MutationOfParameter(param.span));
options.ctx.diagnostic(NoSideEffectsDiagnostic::MutateParameter(param.span));
}
AstKind::BindingRestElement(rest) => {
let start = rest.span.start + 3;
let end = rest.span.end;
options.ctx.diagnostic(NoSideEffectsDiagnostic::MutationOfParameter(Span::new(
start, end,
)));
options
.ctx
.diagnostic(NoSideEffectsDiagnostic::MutateParameter(Span::new(start, end)));
}
_ => {}
}
Expand Down Expand Up @@ -195,6 +203,9 @@ impl<'a> ListenerMap for Expression<'a> {
Self::ParenthesizedExpression(expr) => {
expr.report_effects(options);
}
Self::NewExpression(expr) => {
expr.report_effects(options);
}
Self::ArrowFunctionExpression(_)
| Self::FunctionExpression(_)
| Self::Identifier(_)
Expand All @@ -217,9 +228,12 @@ impl<'a> ListenerMap for Expression<'a> {
Self::CallExpression(expr) => {
expr.report_effects_when_mutated(options);
}
Self::ThisExpression(expr) => {
expr.report_effects_when_mutated(options);
}
_ => {
// Default behavior
options.ctx.diagnostic(NoSideEffectsDiagnostic::Mutation(self.span()));
options.ctx.diagnostic(NoSideEffectsDiagnostic::Mutate(self.span()));
}
}
}
Expand Down Expand Up @@ -262,6 +276,27 @@ fn defined_custom_report_effects_when_called(expr: &Expression) -> bool {
)
}

impl ListenerMap for ThisExpression {
fn report_effects_when_mutated(&self, options: &NodeListenerOptions) {
if !options.has_valid_this.get() {
options.ctx.diagnostic(NoSideEffectsDiagnostic::MutateOfThis(self.span));
}
}
}

impl<'a> ListenerMap for NewExpression<'a> {
fn report_effects(&self, options: &NodeListenerOptions) {
if has_pure_notation(self.span, options.ctx) {
return;
}
self.arguments.iter().for_each(|arg| arg.report_effects(options));
let old_val = options.call_with_new.get();
options.call_with_new.set(true);
self.callee.report_effects_when_called(options);
options.call_with_new.set(old_val);
}
}

impl<'a> ListenerMap for ParenthesizedExpression<'a> {
fn report_effects(&self, options: &NodeListenerOptions) {
self.expression.report_effects(options);
Expand All @@ -283,16 +318,22 @@ impl<'a> ListenerMap for ParenthesizedExpression<'a> {
impl<'a> ListenerMap for ArrowFunctionExpression<'a> {
fn report_effects_when_called(&self, options: &NodeListenerOptions) {
self.params.items.iter().for_each(|param| param.report_effects(options));
let old_val = options.has_valid_this.get();
options.has_valid_this.set(options.call_with_new.get());
self.body.statements.iter().for_each(|stmt| stmt.report_effects(options));
options.has_valid_this.set(old_val);
}
}

impl<'a> ListenerMap for Function<'a> {
fn report_effects_when_called(&self, options: &NodeListenerOptions) {
self.params.items.iter().for_each(|param| param.report_effects(options));
let old_val = options.has_valid_this.get();
options.has_valid_this.set(options.call_with_new.get());
if let Some(body) = &self.body {
body.statements.iter().for_each(|stmt| stmt.report_effects(options));
}
options.has_valid_this.set(old_val);
}
}

Expand Down Expand Up @@ -330,7 +371,7 @@ impl<'a> ListenerMap for CallExpression<'a> {
}
}
fn report_effects_when_mutated(&self, options: &NodeListenerOptions) {
options.ctx.diagnostic(NoSideEffectsDiagnostic::MutationOfFunctionReturnValue(self.span));
options.ctx.diagnostic(NoSideEffectsDiagnostic::MutateFunctionReturnValue(self.span));
}
}

Expand Down Expand Up @@ -426,7 +467,7 @@ impl<'a> ListenerMap for IdentifierReference<'a> {
node.report_effects_when_mutated(options);
}
} else {
ctx.diagnostic(NoSideEffectsDiagnostic::MutationWithName(
ctx.diagnostic(NoSideEffectsDiagnostic::MutateWithName(
self.name.to_compact_str(),
self.span,
));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,23 @@ enum NoSideEffectsDiagnostic {

#[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of mutating")]
#[diagnostic(severity(warning))]
Mutation(#[label] Span),
Mutate(#[label] Span),

#[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of mutating `{0}`")]
#[diagnostic(severity(warning))]
MutationWithName(CompactStr, #[label] Span),
MutateWithName(CompactStr, #[label] Span),

#[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of mutating function return value")]
#[diagnostic(severity(warning))]
MutationOfFunctionReturnValue(#[label] Span),
MutateFunctionReturnValue(#[label] Span),

#[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of mutating function parameter")]
#[diagnostic(severity(warning))]
MutationOfParameter(#[label] Span),
MutateParameter(#[label] Span),

#[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of mutating unknown this value")]
#[diagnostic(severity(warning))]
MutateOfThis(#[label] Span),

#[error("eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of calling")]
#[diagnostic(severity(warning))]
Expand Down Expand Up @@ -292,9 +296,9 @@ fn test() {
// "class x {a(){}}",
// "class x {static a(){}}",
// // NewExpression
// "const x = new (function (){this.x = 1})()",
// "function x(){this.y = 1}; const z = new x()",
// "/*@__PURE__*/ new ext()",
"const x = new (function (){this.x = 1})()",
"function x(){this.y = 1}; const z = new x()",
"/*@__PURE__*/ new ext()",
// // ObjectExpression
// "const x = {y: ext}",
// r#"const x = {["y"]: ext}"#,
Expand Down Expand Up @@ -382,7 +386,7 @@ fn test() {
"ext += 1",
"ext.x = 1",
"const x = {};x[ext()] = 1",
// "this.x = 1",
"this.x = 1",
// // AssignmentPattern
// "const {x = ext()} = {}",
// "const {y: {x = ext()} = {}} = {}",
Expand Down Expand Up @@ -577,8 +581,8 @@ fn test() {
// // MethodDefinition
// "class x {static [ext()](){}}",
// // NewExpression
// "const x = new ext()",
// "new ext()",
"const x = new ext()",
"new ext()",
// // ObjectExpression
// "const x = {y: ext()}",
// r#"const x = {["y"]: ext()}"#,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@ expression: no_side_effects_in_initialization
· ───
╰────

eslint-plugin-tree-shaking(no-side-effects-in-initialization): Cannot determine side-effects of mutating unknown this value
╭─[no_side_effects_in_initialization.tsx:1:1]
1this.x = 1
· ────
╰────

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 │ (()=>{})(ext(), 1)
Expand All @@ -127,3 +133,15 @@ expression: no_side_effects_in_initialization
1 │ const x = ()=>ext; const y = x(); y.z = 1
· ───
╰────

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()
· ───
╰────

⚠ 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 │ new ext()
· ───
╰────
22 changes: 22 additions & 0 deletions crates/oxc_linter/src/utils/tree_shaking.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use oxc_ast::{ast::Expression, AstKind};
use oxc_semantic::AstNodeId;
use oxc_span::Span;

use crate::LintContext;

Expand All @@ -21,3 +22,24 @@ pub fn get_write_expr<'a, 'b>(
}

pub fn no_effects() {}

/// Comments containing @__PURE__ or #__PURE__ mark a specific function call
/// or constructor invocation as side effect free.
///
/// Such an annotation is considered valid if it directly
/// precedes a function call or constructor invocation
/// and is only separated from the callee by white-space or comments.
///
/// The only exception are parentheses that wrap a call or invocation.
///
/// <https://rollupjs.org/configuration-options/#pure>
pub fn has_pure_notation(span: Span, ctx: &LintContext) -> bool {
let Some((start, comment)) = ctx.semantic().trivias().comments_range(..span.start).next_back()
else {
return false;
};
let span = Span::new(*start, comment.end);
let raw = span.source_text(ctx.semantic().source_text());

raw.contains("@__PURE__") || raw.contains("#__PURE__")
}

0 comments on commit ce34829

Please sign in to comment.