Skip to content
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.

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

Proposal: "function-default-params" scope #34

Open
mysticatea opened this issue Nov 9, 2017 · 6 comments
Open

Proposal: "function-default-params" scope #34

mysticatea opened this issue Nov 9, 2017 · 6 comments

Comments

@mysticatea
Copy link
Member

mysticatea commented Nov 9, 2017

From #33, eslint/eslint#9335.

As the step 27.a of 9.2.12 FunctionDeclarationInstantiation( func, argumentsList ), eslint-scope should make new scope for the function body only if the parameters has any default value.

It's similar to function-expression-name scope which separates the variable of function expression names from function bodies. So I propose new function-default-params scope which separates the references in default parameters from function bodies.

  1. If the current function has one or more default parameters in traversing, the referencer defines a function-default-params scope instead of function scope.
    • Note: the function-default-params scope has implicit arguments variable except if the function is arrow function expression.
    • Note: the function-default-params scope is the child scope of function-expression-name scope if the function is function expression.
  2. The referencer evaluates the parameters.
  3. The referencer defines function scope.
  4. The referencer copies the all variables from the function-default-params scope to the function scope.
    • Note: those copies handle redeclaration such as function f(a = 1) { let a } correctly.
    • Note: at least, no-shadow rule has to handle those copies as special.
    • Note: I think we should use shallow copy here because Variable#defs and Variable#references should be shared with both scopes but Reference#resolved property cannot have multiple variable objects. For example, in function f(a, b = a) { g(a) } case, the two references of a should be aggregated into one Variable#references array.
  5. The referencer evaluates the function body.
  6. The referencer closes both function-default-params and function scopes at the end of the function.
@not-an-aardvark
Copy link
Member

Is this still something we want to do?

@shannon
Copy link

shannon commented Apr 24, 2019

I'm having a similar issue as estools/escope#130 without this.

@mysticatea
Copy link
Member Author

Oh, I had forgotten this issue. Yes, I hope to do. I added the memorandum of 7.

@shannon eslint-scope package has fixed in #33.

@kaicataldo
Copy link
Member

@mysticatea Can we close this issue if it's fixed in #33?

@not-an-aardvark
Copy link
Member

I don't think this issue has been fixed. If I'm understanding correctly, #33 fixed estools/escope#130, which is a separate issue.

@mysticatea
Copy link
Member Author

@kaicataldo No. The #33 was a superficial fix because I avoided breaking changes. And this is a follow‐up issue to make the correct scope structure.

But probably I should write an RFC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants