Skip to content
This repository has been archived by the owner on Jan 19, 2019. It is now read-only.

Parameter properties information not being capture in ESTree parameter node #143

Closed
weirdpattern opened this issue Jan 14, 2017 · 3 comments
Labels

Comments

@weirdpattern
Copy link
Contributor

What version of TypeScript are you using?
2.0.10 and 2.1.5

What version of typescript-eslint-parser are you using?
1.0.2

What code were you trying to parse?

class Foo {
    constructor(public name : string) {}
}
class Foo {
    constructor(protected name : string) {}
}
class Foo {
    constructor(private name : string) {}
}
class Foo {
    constructor(readonly name : string) {}
}
class Foo {
    constructor(public readonly name : string) {}
}
class Foo {
    constructor(protected readonly name : string) {}
}
class Foo {
    constructor(private readonly name : string) {}
}

What did you expect to happen?
Parameter properties should be parsed correctly and modifiers should be part of the final ESTree parameter node.

What happened?
The modifiers property of the TS Node is being ignored by the AST Converter. The final ESTree contains the parameters, but without the accessibility modifiers and/or readonly modifiers.

Sources: Parameter properties

@weirdpattern
Copy link
Contributor Author

weirdpattern commented Jan 14, 2017

I'm currently working on this. So far I can think of two possible solutions:

  1. Add the isReadonly and accessibility properties to the current output node (convertedParam, in the image below).
  2. Wrap the current output (convertedParam, in the image below) in a new TSParameterProperty node.

This is the current code
constructor

As parameter properties can only be declare in constructors, I think this is the best place to do the change.

What do you think @nzakas? Adding properties or new node?

@soda0289
Copy link
Member

I think it should be a new node, TsParameterProperty makes sense. The ESTree already allows params to contain different kinds of pattern nodes such as: AssigmentPattern, ObjectPattern, RestElement, and so on. A new node would allow for more accurate rules that could align better with Class property rules, which might not make sense with method parameter rules.

@weirdpattern
Copy link
Contributor Author

I agree, I will work on this tomorrow

weirdpattern added a commit to weirdpattern/typescript-eslint-parser that referenced this issue Feb 22, 2017
weirdpattern added a commit to weirdpattern/typescript-eslint-parser that referenced this issue Feb 22, 2017
weirdpattern added a commit to weirdpattern/typescript-eslint-parser that referenced this issue Feb 27, 2017
weirdpattern added a commit to weirdpattern/typescript-eslint-parser that referenced this issue Feb 27, 2017
weirdpattern added a commit to weirdpattern/typescript-eslint-parser that referenced this issue Feb 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants