oxc-project / oxc-project/backlog

Support reading `Ancestor` siblings in a `Vec` in `Traverse` visitors

Open
#138 2 comments 0 reactions 1 assignee View on GitHub

@overlookmotel is already working on this.

Since Sep 11, 2024.

Dominant language
No language data
Stars
7
Forks
0
PR merge metrics
No merged PRs in 30d

Description

Broken out from oxc-project/backlog#128:

We've hit a case where the following APIs in Traverse visitors would be useful:

  • Read siblings in a Vec.
  • Get which index current node is in a Vec.

Minifier (#4725) needs to check when visiting a BooleanExpression whether it is true in:

Object.defineProperty(exports, 'Foo', {
  enumerable: true,
  get: function() { return Foo_1.Foo; }
});

Currently this is implemented by checking when entering a CallExpression whether it matches this pattern and setting a flag if so. It would be preferable to do this check by reading up AST in enter_boolean_literal instead.

fn enter_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
    self.compress_boolean(expr, ctx);
}

fn compress_boolean(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) -> bool {
    let Expression::BooleanLiteral(lit) = expr else { return false; };
    if Self::bool_is_in_define_export(lit, ctx) { return false; }

    let num = self.ast.expression_numeric_literal(
        SPAN,
        if lit.value { 0.0 } else { 1.0 },
        if lit.value { "0" } else { "1" },
        NumberBase::Decimal,
    );
    *expr = self.ast.expression_unary(SPAN, UnaryOperator::LogicalNot, num);
    true
}

/// Check if `BooleanLiteral` is `true` in:
/// ```js
/// Object.defineProperty(exports, 'Foo', {
///   enumerable: true,
///   get: function() { return Foo_1.Foo; }
/// });
/// ```
fn bool_is_in_define_export(lit: &BooleanLiteral, ctx: &mut TraverseCtx<'a>) -> bool {
    // We are only interested in `true`
    if !lit.value { return false; }
    // We are only interested in this construct at top level
    if ctx.ancestors_depth() != 5 { return false; } // TODO: Is 5 correct?

    // Check `true` is in an object property `enumerable: true`
    let Ancestor::ObjectPropertyValue(prop) = ctx.parent() else { return false; };
    let PropertyKey::StaticIdentifier(key) = prop.key() else { return false; };
    if key.name != "enumerable" { return false; }

    // Check that object is param of a call expression.
    // Skip over checking parent's parent - it must be an `ObjectExpression`.
    let object_parent = ctx.ancestor(3).unwrap();
    let Ancestor::CallExpressionArguments(call_expr) = object_parent else { return false; };

    // We want to check that:
    // 1. First arg is `exports`.
    // 2. Object is 3rd arg.
    // But we can't because `Ancestor` does not give us access to `arguments`.
    // TODO(@overlookmotel): Make this possible.

    // Check callee is `Object.defineProperty`.
    // Use tighter check than `call_expr.callee.is_specific_member_access("Object", "defineProperty")`
    // because we're looking for `Object.defineProperty` specifically, not e.g. `Object['defineProperty']`.
    if let Expression::StaticMemberExpression(callee) = call_expr.callee() {
        if let Expression::Identifier(id) = &callee.object {
            if id.name == "Object" && callee.property.name == "defineProperty" {
                return true;
            }
        }
    }
    false
}

This isn't currently possible as cannot access call_expr.arguments. It would be safe to access other elements in arguments except for the current one, but this is not supported currently by Traverse's API.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.