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

Support props in ESLint-based rules #1100

Closed
vilchik-elena opened this issue Oct 3, 2018 · 8 comments
Closed

Support props in ESLint-based rules #1100

vilchik-elena opened this issue Oct 3, 2018 · 8 comments
Assignees

Comments

@vilchik-elena
Copy link
Contributor

No description provided.

@vilchik-elena
Copy link
Contributor Author

vilchik-elena commented Oct 3, 2018

class A {
   static foo = 42; // fails with "Unexpected token = "
}
class A {
   foo = 42; // fails with "Unexpected token = "
}

espree does not parse it, but babel-eslint does. But as we use babel-eslint only to parse flow we fail to parse this syntax.

Proposition https://github.com/tc39/proposal-class-fields

pyyding added a commit to pyyding/SonarJS that referenced this issue Dec 14, 2018
@vilchik-elena vilchik-elena changed the title Support static props in ESLint-based rules Support props in ESLint-based rules Dec 21, 2018
@simonedavico
Copy link

Wouldn't this be as simple as always using babel-eslint for parsing? Would you accept a PR?

@vilchik-elena
Copy link
Contributor Author

@simonedavico good point, but are you sure there is no such syntax supported by espree and not supported by babel?

@simonedavico
Copy link

Unfortunately I am not an expert in js/es parsers...for sure, babelsupports syntax up to stage-3, while espree does not. This is an issue at my company because we are using arrow functions as class properties, which are supported by babel but not espree. They are also shipped with hugely popular tools (as in create react app), so I think providing support is for the best.

I opened PR #1143 , would you be so kind to take a look and give some feedback?

@rstrahl
Copy link

rstrahl commented Jan 15, 2019

We're encountering the same issue in our project, and its halted usage on front-end analysis for us.
As far as I can see in the tooling implementation tracking issue reports as of this writing, espree does not support class fields. A quick check in the espree issue/PR tracker appears to support this.

Ref: tc39/proposal-class-fields#57

@gersongoulart
Copy link

gersongoulart commented Jan 16, 2019

I agree @simonedavico 's PR is all ok from the implementation point of view.

If that solution is not feasible for some reason, would this be the case of making the parse and parser options at https://github.com/SonarSource/SonarJS/blob/master/eslint-bridge/src/parser.ts#L41 configurable for the time being until the team has the confidence necessary to make necessary adjustments?

Allowing for the parser to be a configurable aspect of Sonar JS could also open up the solution for greater configurability — https://eslint.org/docs/user-guide/configuring#specifying-parser

@simonedavico
Copy link

@gersongoulart I actually wanted to make it configurable, but it would be more difficult to implement tests for each parser, since they are not feature equivalent. Of course with additional effort it could be achieved.

@slikts
Copy link

slikts commented Jan 29, 2019

It would be nice if SonarJS would be able to reuse the ESLint configuration regarding which parser to use (since SonarJS uses ESLint's Espree anyway, so there's obvious overlap) and which files to include or exclude. It would make using SonarJS more streamlined and avoid having to duplicate settings, on top of supporting proposed JS features.

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

No branches or pull requests

5 participants