Skip to content

refactor(minifier): remove clippy::unused_self allow directive by converting methods to associated functions #13029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 13, 2025
Merged

refactor(minifier): remove clippy::unused_self allow directive by converting methods to associated functions #13029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
eb40a8c
Initial plan
Copilot Aug 12, 2025
eb05d67
Start fixing unused_self issues: convert fold_object_exp and inline_t…
Copilot Aug 12, 2025
32c55e3
Continue fixing unused_self: convert 5 more methods to associated fun…
Copilot Aug 12, 2025
b0a0e77
Continue fixing unused_self: convert additional methods to associated…
Copilot Aug 12, 2025
45c6d2b
Continue fixing unused_self: convert more methods to associated funct…
Copilot Aug 12, 2025
d146c25
wip
Boshen Aug 12, 2025
4125ba5
fix
Boshen Aug 12, 2025
8fad933
fmt
Boshen Aug 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/oxc_minifier/src/peephole/fold_constants.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ impl<'a> PeepholeOptimizations {
}
}

pub fn fold_object_exp(&self, e: &mut ObjectExpression<'a>, ctx: &mut Ctx<'a, '_>) {
pub fn fold_object_exp(e: &mut ObjectExpression<'a>, ctx: &mut Ctx<'a, '_>) {
fn should_fold_spread_element<'a>(e: &Expression<'a>, ctx: &mut Ctx<'a, '_>) -> bool {
match e {
Expression::ArrayExpression(o) if o.elements.is_empty() => true,
Expand Down Expand Up @@ -719,7 +719,7 @@ impl<'a> PeepholeOptimizations {
/// Inline constant values in template literals
///
/// - `foo${1}bar${i}` => `foo1bar${i}`
pub fn inline_template_literal(&self, t: &mut TemplateLiteral<'a>, ctx: &mut Ctx<'a, '_>) {
pub fn inline_template_literal(t: &mut TemplateLiteral<'a>, ctx: &mut Ctx<'a, '_>) {
let has_expr_to_inline = t
.expressions
.iter()
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_minifier/src/peephole/inline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::ctx::Ctx;
use super::PeepholeOptimizations;

impl<'a> PeepholeOptimizations {
pub fn init_symbol_value(&self, decl: &VariableDeclarator<'a>, ctx: &mut Ctx<'a, '_>) {
pub fn init_symbol_value(decl: &VariableDeclarator<'a>, ctx: &mut Ctx<'a, '_>) {
let BindingPatternKind::BindingIdentifier(ident) = &decl.id.kind else { return };
let Some(symbol_id) = ident.symbol_id.get() else { return };
// Skip for `var` declarations, due to TDZ problems.
Expand All @@ -19,7 +19,7 @@ impl<'a> PeepholeOptimizations {
ctx.init_value(symbol_id, value);
}

pub fn inline_identifier_reference(&self, expr: &mut Expression<'a>, ctx: &mut Ctx<'a, '_>) {
pub fn inline_identifier_reference(expr: &mut Expression<'a>, ctx: &mut Ctx<'a, '_>) {
let Expression::Identifier(ident) = expr else { return };
let Some(reference_id) = ident.reference_id.get() else { return };
let Some(symbol_id) = ctx.scoping().get_reference(reference_id).symbol_id() else { return };
Expand Down
47 changes: 22 additions & 25 deletions crates/oxc_minifier/src/peephole/minimize_conditional_expression.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,19 @@ use super::PeepholeOptimizations;

impl<'a> PeepholeOptimizations {
pub fn minimize_conditional(
&self,
span: Span,
test: Expression<'a>,
consequent: Expression<'a>,
alternate: Expression<'a>,
ctx: &mut Ctx<'a, '_>,
) -> Expression<'a> {
let mut cond_expr = ctx.ast.conditional_expression(span, test, consequent, alternate);
self.try_minimize_conditional(&mut cond_expr, ctx)
Self::try_minimize_conditional(&mut cond_expr, ctx)
.unwrap_or_else(|| Expression::ConditionalExpression(ctx.ast.alloc(cond_expr)))
}

/// `MangleIfExpr`: <https://github.com/evanw/esbuild/blob/v0.24.2/internal/js_ast/js_ast_helpers.go#L2745>
pub fn try_minimize_conditional(
&self,
expr: &mut ConditionalExpression<'a>,
ctx: &mut Ctx<'a, '_>,
) -> Option<Expression<'a>> {
Expand All @@ -40,7 +38,7 @@ impl<'a> PeepholeOptimizations {
let Expression::SequenceExpression(sequence_expr) = &mut sequence else {
unreachable!()
};
let expr = self.minimize_conditional(
let expr = Self::minimize_conditional(
span,
sequence_expr.expressions.pop().unwrap(),
expr.consequent.take_in(ctx.ast),
Expand All @@ -57,16 +55,16 @@ impl<'a> PeepholeOptimizations {
let test = test_expr.argument.take_in(ctx.ast);
let consequent = expr.alternate.take_in(ctx.ast);
let alternate = expr.consequent.take_in(ctx.ast);
return Some(
self.minimize_conditional(expr.span, test, consequent, alternate, ctx),
);
return Some(Self::minimize_conditional(
expr.span, test, consequent, alternate, ctx,
));
}
}
Expression::Identifier(id) => {
// "a ? a : b" => "a || b"
if let Expression::Identifier(id2) = &expr.consequent {
if id.name == id2.name {
return Some(self.join_with_left_associative_op(
return Some(Self::join_with_left_associative_op(
expr.span,
LogicalOperator::Or,
expr.test.take_in(ctx.ast),
Expand All @@ -78,7 +76,7 @@ impl<'a> PeepholeOptimizations {
// "a ? b : a" => "a && b"
if let Expression::Identifier(id2) = &expr.alternate {
if id.name == id2.name {
return Some(self.join_with_left_associative_op(
return Some(Self::join_with_left_associative_op(
expr.span,
LogicalOperator::And,
expr.test.take_in(ctx.ast),
Expand All @@ -98,9 +96,9 @@ impl<'a> PeepholeOptimizations {
let test = expr.test.take_in(ctx.ast);
let consequent = expr.consequent.take_in(ctx.ast);
let alternate = expr.alternate.take_in(ctx.ast);
return Some(
self.minimize_conditional(expr.span, test, alternate, consequent, ctx),
);
return Some(Self::minimize_conditional(
expr.span, test, alternate, consequent, ctx,
));
}
}
_ => {}
Expand All @@ -111,7 +109,7 @@ impl<'a> PeepholeOptimizations {
if ctx.expr_eq(&consequent.alternate, &expr.alternate) {
return Some(ctx.ast.expression_conditional(
expr.span,
self.join_with_left_associative_op(
Self::join_with_left_associative_op(
expr.test.span(),
LogicalOperator::And,
expr.test.take_in(ctx.ast),
Expand All @@ -129,7 +127,7 @@ impl<'a> PeepholeOptimizations {
if ctx.expr_eq(&alternate.consequent, &expr.consequent) {
return Some(ctx.ast.expression_conditional(
expr.span,
self.join_with_left_associative_op(
Self::join_with_left_associative_op(
expr.test.span(),
LogicalOperator::Or,
expr.test.take_in(ctx.ast),
Expand All @@ -150,7 +148,7 @@ impl<'a> PeepholeOptimizations {
return Some(ctx.ast.expression_sequence(
expr.span,
ctx.ast.vec_from_array([
self.join_with_left_associative_op(
Self::join_with_left_associative_op(
expr.test.span(),
LogicalOperator::Or,
expr.test.take_in(ctx.ast),
Expand All @@ -171,7 +169,7 @@ impl<'a> PeepholeOptimizations {
return Some(ctx.ast.expression_sequence(
expr.span,
ctx.ast.vec_from_array([
self.join_with_left_associative_op(
Self::join_with_left_associative_op(
expr.test.span(),
LogicalOperator::And,
expr.test.take_in(ctx.ast),
Expand All @@ -189,7 +187,7 @@ impl<'a> PeepholeOptimizations {
if logical_expr.operator.is_or() && ctx.expr_eq(&logical_expr.right, &expr.alternate) {
return Some(ctx.ast.expression_logical(
expr.span,
self.join_with_left_associative_op(
Self::join_with_left_associative_op(
expr.test.span(),
LogicalOperator::And,
expr.test.take_in(ctx.ast),
Expand All @@ -209,7 +207,7 @@ impl<'a> PeepholeOptimizations {
{
return Some(ctx.ast.expression_logical(
expr.span,
self.join_with_left_associative_op(
Self::join_with_left_associative_op(
expr.test.span(),
LogicalOperator::Or,
expr.test.take_in(ctx.ast),
Expand Down Expand Up @@ -282,7 +280,7 @@ impl<'a> PeepholeOptimizations {
let alternate_first_arg =
alternate.arguments[0].to_expression_mut().take_in(ctx.ast);
let mut args = std::mem::replace(&mut consequent.arguments, ctx.ast.vec());
let cond_expr = self.minimize_conditional(
let cond_expr = Self::minimize_conditional(
expr.test.span(),
expr.test.take_in(ctx.ast),
consequent_first_arg,
Expand All @@ -296,7 +294,7 @@ impl<'a> PeepholeOptimizations {
}

// Not part of esbuild
if let Some(e) = self.try_merge_conditional_expression_inside(expr, ctx) {
if let Some(e) = Self::try_merge_conditional_expression_inside(expr, ctx) {
return Some(e);
}

Expand Down Expand Up @@ -385,13 +383,13 @@ impl<'a> PeepholeOptimizations {
) {
(Some(true), Some(false)) => {
let test = expr.test.take_in(ctx.ast);
let test = self.minimize_not(expr.span, test, ctx);
let test = self.minimize_not(expr.span, test, ctx);
let test = Self::minimize_not(expr.span, test, ctx);
let test = Self::minimize_not(expr.span, test, ctx);
return Some(test);
}
(Some(false), Some(true)) => {
let test = expr.test.take_in(ctx.ast);
let test = self.minimize_not(expr.span, test, ctx);
let test = Self::minimize_not(expr.span, test, ctx);
return Some(test);
}
_ => {}
Expand All @@ -417,7 +415,6 @@ impl<'a> PeepholeOptimizations {
///
/// - `x ? a = 0 : a = 1` -> `a = x ? 0 : 1`
fn try_merge_conditional_expression_inside(
&self,
expr: &mut ConditionalExpression<'a>,
ctx: &mut Ctx<'a, '_>,
) -> Option<Expression<'a>> {
Expand All @@ -442,7 +439,7 @@ impl<'a> PeepholeOptimizations {
{
return None;
}
let cond_expr = self.minimize_conditional(
let cond_expr = Self::minimize_conditional(
expr.span,
expr.test.take_in(ctx.ast),
consequent.right.take_in(ctx.ast),
Expand Down
8 changes: 3 additions & 5 deletions crates/oxc_minifier/src/peephole/minimize_conditions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ impl<'a> PeepholeOptimizations {
// associative. For example, the "+" operator is not associative for
// floating-point numbers.
pub fn join_with_left_associative_op(
&self,
span: Span,
op: LogicalOperator,
a: Expression<'a>,
Expand All @@ -34,7 +33,7 @@ impl<'a> PeepholeOptimizations {
if let Some(right) = sequence_expr.expressions.pop() {
sequence_expr
.expressions
.push(self.join_with_left_associative_op(span, op, right, b, ctx));
.push(Self::join_with_left_associative_op(span, op, right, b, ctx));
}
return Expression::SequenceExpression(sequence_expr);
}
Expand All @@ -46,7 +45,7 @@ impl<'a> PeepholeOptimizations {
if let Expression::LogicalExpression(logical_expr) = &mut b {
if logical_expr.operator == op {
let right = logical_expr.left.take_in(ctx.ast);
a = self.join_with_left_associative_op(span, op, a, right, ctx);
a = Self::join_with_left_associative_op(span, op, a, right, ctx);
b = logical_expr.right.take_in(ctx.ast);
continue;
}
Expand All @@ -56,7 +55,7 @@ impl<'a> PeepholeOptimizations {
// "a op b" => "a op b"
// "(a op b) op c" => "(a op b) op c"
let mut logic_expr = ctx.ast.expression_logical(span, a, op, b);
self.minimize_logical_expression(&mut logic_expr, ctx);
Self::minimize_logical_expression(&mut logic_expr, ctx);
logic_expr
}

Expand Down Expand Up @@ -172,7 +171,6 @@ impl<'a> PeepholeOptimizations {
///
/// This can only be done for resolved identifiers as this would avoid setting `a` when `a` is truthy.
pub fn try_compress_normal_assignment_to_combined_logical_assignment(
&self,
expr: &mut AssignmentExpression<'a>,
ctx: &mut Ctx<'a, '_>,
) {
Expand Down
38 changes: 15 additions & 23 deletions crates/oxc_minifier/src/peephole/minimize_expression_in_boolean_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,7 @@ use crate::ctx::Ctx;
use super::PeepholeOptimizations;

impl<'a> PeepholeOptimizations {
pub fn try_fold_stmt_in_boolean_context(
&self,
stmt: &mut Statement<'a>,
ctx: &mut Ctx<'a, '_>,
) {
pub fn try_fold_stmt_in_boolean_context(stmt: &mut Statement<'a>, ctx: &mut Ctx<'a, '_>) {
let expr = match stmt {
Statement::IfStatement(s) => Some(&mut s.test),
Statement::WhileStatement(s) => Some(&mut s.test),
Expand All @@ -22,25 +18,21 @@ impl<'a> PeepholeOptimizations {
};

if let Some(expr) = expr {
self.try_fold_expr_in_boolean_context(expr, ctx);
Self::try_fold_expr_in_boolean_context(expr, ctx);
}
}

/// Simplify syntax when we know it's used inside a boolean context, e.g. `if (boolean_context) {}`.
///
/// `SimplifyBooleanExpr`: <https://github.com/evanw/esbuild/blob/v0.24.2/internal/js_ast/js_ast_helpers.go#L2059>
pub fn try_fold_expr_in_boolean_context(
&self,
expr: &mut Expression<'a>,
ctx: &mut Ctx<'a, '_>,
) {
pub fn try_fold_expr_in_boolean_context(expr: &mut Expression<'a>, ctx: &mut Ctx<'a, '_>) {
match expr {
// "!!a" => "a"
Expression::UnaryExpression(u1) if u1.operator.is_not() => {
if let Expression::UnaryExpression(u2) = &mut u1.argument {
if u2.operator.is_not() {
let mut e = u2.argument.take_in(ctx.ast);
self.try_fold_expr_in_boolean_context(&mut e, ctx);
Self::try_fold_expr_in_boolean_context(&mut e, ctx);
*expr = e;
ctx.state.changed = true;
}
Expand All @@ -66,8 +58,8 @@ impl<'a> PeepholeOptimizations {
}
// "if (!!a && !!b)" => "if (a && b)"
Expression::LogicalExpression(e) if e.operator.is_and() => {
self.try_fold_expr_in_boolean_context(&mut e.left, ctx);
self.try_fold_expr_in_boolean_context(&mut e.right, ctx);
Self::try_fold_expr_in_boolean_context(&mut e.left, ctx);
Self::try_fold_expr_in_boolean_context(&mut e.right, ctx);
// "if (anything && truthyNoSideEffects)" => "if (anything)"
if e.right.get_side_free_boolean_value(ctx) == Some(true) {
*expr = e.left.take_in(ctx.ast);
Expand All @@ -76,8 +68,8 @@ impl<'a> PeepholeOptimizations {
}
// "if (!!a ||!!b)" => "if (a || b)"
Expression::LogicalExpression(e) if e.operator == LogicalOperator::Or => {
self.try_fold_expr_in_boolean_context(&mut e.left, ctx);
self.try_fold_expr_in_boolean_context(&mut e.right, ctx);
Self::try_fold_expr_in_boolean_context(&mut e.left, ctx);
Self::try_fold_expr_in_boolean_context(&mut e.right, ctx);
// "if (anything || falsyNoSideEffects)" => "if (anything)"
if e.right.get_side_free_boolean_value(ctx) == Some(false) {
*expr = e.left.take_in(ctx.ast);
Expand All @@ -86,8 +78,8 @@ impl<'a> PeepholeOptimizations {
}
Expression::ConditionalExpression(e) => {
// "if (a ? !!b : !!c)" => "if (a ? b : c)"
self.try_fold_expr_in_boolean_context(&mut e.consequent, ctx);
self.try_fold_expr_in_boolean_context(&mut e.alternate, ctx);
Self::try_fold_expr_in_boolean_context(&mut e.consequent, ctx);
Self::try_fold_expr_in_boolean_context(&mut e.alternate, ctx);
if let Some(boolean) = e.consequent.get_side_free_boolean_value(ctx) {
let right = e.alternate.take_in(ctx.ast);
let left = e.test.take_in(ctx.ast);
Expand All @@ -97,9 +89,9 @@ impl<'a> PeepholeOptimizations {
(LogicalOperator::Or, left)
} else {
// "if (anything1 ? falsyNoSideEffects : anything2)" => "if (!anything1 && anything2)"
(LogicalOperator::And, self.minimize_not(left.span(), left, ctx))
(LogicalOperator::And, Self::minimize_not(left.span(), left, ctx))
};
*expr = self.join_with_left_associative_op(span, op, left, right, ctx);
*expr = Self::join_with_left_associative_op(span, op, left, right, ctx);
ctx.state.changed = true;
return;
}
Expand All @@ -109,18 +101,18 @@ impl<'a> PeepholeOptimizations {
let span = e.span;
let (op, left) = if boolean {
// "if (anything1 ? anything2 : truthyNoSideEffects)" => "if (!anything1 || anything2)"
(LogicalOperator::Or, self.minimize_not(left.span(), left, ctx))
(LogicalOperator::Or, Self::minimize_not(left.span(), left, ctx))
} else {
// "if (anything1 ? anything2 : falsyNoSideEffects)" => "if (anything1 && anything2)"
(LogicalOperator::And, left)
};
*expr = self.join_with_left_associative_op(span, op, left, right, ctx);
*expr = Self::join_with_left_associative_op(span, op, left, right, ctx);
ctx.state.changed = true;
}
}
Expression::SequenceExpression(seq_expr) => {
if let Some(last) = seq_expr.expressions.last_mut() {
self.try_fold_expr_in_boolean_context(last, ctx);
Self::try_fold_expr_in_boolean_context(last, ctx);
}
}
_ => {}
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_minifier/src/peephole/minimize_for_statement.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use super::PeepholeOptimizations;

impl<'a> PeepholeOptimizations {
/// `mangleFor`: <https://github.com/evanw/esbuild/blob/v0.24.2/internal/js_ast/js_parser.go#L9801>
pub fn minimize_for_statement(&self, for_stmt: &mut ForStatement<'a>, ctx: &mut Ctx<'a, '_>) {
pub fn minimize_for_statement(for_stmt: &mut ForStatement<'a>, ctx: &mut Ctx<'a, '_>) {
// Get the first statement in the loop
let mut first = &for_stmt.body;
if let Statement::BlockStatement(block_stmt) = first {
Expand Down Expand Up @@ -46,7 +46,7 @@ impl<'a> PeepholeOptimizations {
Expression::UnaryExpression(unary_expr) if unary_expr.operator.is_not() => {
unary_expr.unbox().argument
}
e => self.minimize_not(e.span(), e, ctx),
e => Self::minimize_not(e.span(), e, ctx),
};

if let Some(test) = &mut for_stmt.test {
Expand Down
Loading
Loading