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

Flow type triggers space-infix-ops error #5211

Closed
danez opened this issue Feb 11, 2016 · 8 comments
Closed

Flow type triggers space-infix-ops error #5211

danez opened this issue Feb 11, 2016 · 8 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint good first issue Good for people who haven't worked on ESLint before rule Relates to ESLint's core rules

Comments

@danez
Copy link

danez commented Feb 11, 2016

I know that eslint is not supporting flow types, but it seems that the space-infix-ops rule is assuming that between left and right side of an ExpressionPattern, there is only on Punctation which in the case of flow type is not true.
(https://github.com/eslint/eslint/blob/master/lib/rules/space-infix-ops.js#L32)

example:

(foo: string = 'test') => {};

triggers:

 1:5  error  Infix operators must be spaced  space-infix-ops

What would be a proper way to fix this? I think this is something in the rule itself should be accounted for and it should ignore anything which belongs to left or right side instead of treating it as "something between left and right".

refs babel/babel-eslint#226

eslint version: 1.10.3

@eslintbot
Copy link

@danez Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Feb 11, 2016
@ilyavolodin ilyavolodin added rule Relates to ESLint's core rules evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion enhancement This change enhances an existing feature of ESLint and removed triage An ESLint team member will look at this issue soon labels Feb 11, 2016
@ilyavolodin
Copy link
Member

This rule was create for JavaScript and it rightly assumes that there shouldn't be anything between left and right sides of an ExpressionPattern, since that's the case in JavaScript. I don't think we want to start adding exceptions for Flow into core rules. I think your best bet is to create a similar rule in eslint-plugin-babel that would account for Flow's deviation from standards.

@nzakas
Copy link
Member

nzakas commented Feb 12, 2016

I've been trying to get some standardization on where type annotations go in the tree, and I think I'm close. If it can be hammered down, we can add an exception.

@gajus
Copy link
Contributor

gajus commented Mar 14, 2016

This is being tracked on babel/babel-eslint#226 too.

@nzakas nzakas added good first issue Good for people who haven't worked on ESLint before accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Apr 21, 2016
@nzakas
Copy link
Member

nzakas commented Apr 21, 2016

There's some standardization now. The Identifier node now has a typeAnnotation property that indicates a type annotation is present (it's null or undefined if there's no type annotation). Using that, we should be able to make this rule skip the : that's part of the type annotation.

@danez
Copy link
Author

danez commented Apr 24, 2016

I would work on creating a PR for this now, but I'm not sure how to test it, as espree doesn't support any typeannotation when parsing. Is there a way to workaround this? Or should i create the test in babel-eslint or include babel-eslint as devDependency?

@nzakas
Copy link
Member

nzakas commented Apr 25, 2016

I think if you include babel-eslint as a dev dependency then you should be able to pass that parser in a test like you would parserOptions.

@danez
Copy link
Author

danez commented Jul 22, 2016

Thank you so much.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint good first issue Good for people who haven't worked on ESLint before rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants