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

Create yield-star-spacing rule #4115

Closed
FredyC opened this Issue Oct 11, 2015 · 12 comments

Comments

Projects
None yet
8 participants
@FredyC
Copy link

FredyC commented Oct 11, 2015

My settings: "space-unary-ops": [1, { "words": true, "nonwords": false }]

Fails on the code: yield *iterateChildren(tgChildRow); with message Unary word operator "yield*" must be followed by whitespace.

Version 1.6.0

@eslintbot eslintbot added the triage label Oct 11, 2015

@eslintbot

This comment has been minimized.

Copy link

eslintbot commented Oct 11, 2015

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.

@nzakas nzakas added bug rule accepted and removed triage labels Oct 12, 2015

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Oct 12, 2015

Looks like a bug. We'll take a look.

@lo1tuma

This comment has been minimized.

Copy link
Member

lo1tuma commented Oct 12, 2015

AFAIK this is the expected behavior. The operator in the example is yield * and there is no space after it.

@FredyC

This comment has been minimized.

Copy link
Author

FredyC commented Oct 12, 2015

@lo1tuma You might be right, but that would be kinda sad if it would force me a different code styling. I would have to disable whole rule just because of one space ? :) The yield is sort of special and there is multiple options how to write it. Perhaps it could go to separate rule and remove it from this one?

@IanVS

This comment has been minimized.

Copy link
Member

IanVS commented Oct 12, 2015

Maybe something similar to generator-star-spacing?

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Oct 13, 2015

@lo1tuma it's expected based on the implementation but I'm not sure it's logically expected. From a developer perspective, yield is the operator and * is a second operator. I think some adjustment to this rule is the best option.

@michaelficarra

This comment has been minimized.

Copy link
Member

michaelficarra commented Oct 13, 2015

I guess some might consider it a word operator and some might consider it a non-word operator. I don't think it's that they perceive them as two separate operators.

@aparajita

This comment has been minimized.

Copy link
Contributor

aparajita commented Oct 13, 2015

From a formatting perspective, isn't this exactly analogous to generator-star-spacing? Even though technically yield* may be a unary operator, the user wants to maintain a consistent style between generator star spacing and yield star spacing. Thus it seems that yield* should be removed from this rule and a separate yield-star-spacing rule should be created.

@platinumazure

This comment has been minimized.

Copy link
Member

platinumazure commented Oct 13, 2015

I'm 👍 to the comment left by @aparajita. Seems to me like yield * is a unique unary operator due to actually being two tokens, and we would do better to handle it specially akin to generator-star-spacing.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Oct 14, 2015

I can buy that argument.

Since this would be a breaking change, we'll need to hold off and do it for 2.0.0.

@nzakas nzakas added enhancement breaking and removed bug labels Oct 14, 2015

@nzakas nzakas changed the title space-unary-ops forbids to use 'yield *' Create yield-star-spacing rule Oct 14, 2015

@nzakas nzakas added this to the v2.0.0 milestone Oct 14, 2015

@nzakas nzakas referenced this issue Oct 14, 2015

Closed

Roadmap: 2.0.0 #3561

11 of 11 tasks complete
@aparajita

This comment has been minimized.

Copy link
Contributor

aparajita commented Oct 14, 2015

It doesn't necessarily need to wait until 2.0. Adding an "ignoreYieldStar" property to the space-unary-ops rule + new yield-star-spacing rule would allow what we want without breaking anything.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Oct 14, 2015

I'm really not a fan of creating options where rules can overlap, as that tends to confuse people. I'd much rather wait and do it the right way.

bryanrsmith added a commit to bryanrsmith/eslint that referenced this issue Nov 20, 2015

Breaking: Implement yield-star-spacing rule (fixes eslint#4115)
This change removes checking of yield* operators from the space-unary-ops rule, and adds a new yield-star-spacing rule to check spacing around the * in yield* expressions.

@nzakas nzakas closed this in 290e0f7 Dec 1, 2015

nzakas added a commit that referenced this issue Dec 1, 2015

Merge pull request #4486 from bryanrsmith/issue-4115
Breaking: Implement yield-star-spacing rule (fixes #4115)

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.