Differences in `ReturnStatement` evaluations

Open
#39 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
35/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
javascript
Domain
tooling

Research direction

Start with the ReturnStatement evaluation path and compare how BinaryExpression and MemberExpression arguments are handled. Reproduce both map callbacks and the evaluate('x["b"]', {x: {a: 1}}) example, then verify that statically knowable member access does not incorrectly reject the callback while genuine null access still behaves correctly.

Written by the indexing model from the issue text.

Description

Why does the following work:

[1, 2].map(function(x) { return x + 1 })

But not the following:

[{a: 1}, {a: 2}].map(function(x) { return x["a"] })

Since .map() does not seem to be an issue, and member expressions of course aren’t, I’m not sure why this fails. The actual error is TypeError: Cannot read properties of null (reading 'a')

The inner function ASTs are respectively:

ReturnStatement {
  type: 'ReturnStatement',
  argument: BinaryExpression {
    type: 'BinaryExpression',
    operator: '+',
    left: Identifier { type: 'Identifier', name: 'x' },
    right: Literal { type: 'Literal', value: 1, raw: '1' }
  }
}

And:

ReturnStatement {
  type: 'ReturnStatement',
  argument: ComputedMemberExpression {
    type: 'MemberExpression',
    computed: true,
    object: Identifier { type: 'Identifier', name: 'x' },
    property: Literal { type: 'Literal', value: 'a', raw: '"a"' }
  }
}

Statically evaluating a MemberExpression is not harder than a BinaryExpression, is it?

After looking at the code, it seems to me that in both cases the x statement is statically evaluated to null. In the BinaryExpression code, the binary evaluation returns null + 1 which is 1, and you then allow and execute the function.

However since null["a"] raises an error, the function is not allowed and executed. However x["a"] is knowable statically (it is either the property or undefined if it’s absent − if you call evaluate('x["b"]', {x: {a: 1}}), you get undefined, not an error), so the function should execute in this case as well.

There is a simple workaround which is to return (x || {a: ""})["a"] instead of x["a"], this avoids throwing an error by ensuring there is a fallback value to x.a if x is undefined.

Dominant language
JavaScript
Stars
177
Forks
27
No merged PRs in 30d

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.

More from browserify/static-eval

All issues in browserify/static-eval

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.