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

Relax anti-label lint rules. #1835

Closed
jannotti opened this Issue Mar 15, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@jannotti
Copy link

commented Mar 15, 2017

Description

Lint rules complain about this:

outer:
for (const row of rows) {
  for (const elt of row) {
    if (elt === 0)
     break outer;
    // compute on elt
  }
}

which is the most succinct way to stop processing the current row and all following rows, once a zero is encountered anywhere.

They complain in two ways, first no-restricted-syntax complains about any label of any kind. Then no-labels complains twice, first about the label again, and second about the break statement with a label.

Expected behavior

No complaints. I'd probably be ok with a complaint about this:

label: {
   var a = f();
   if (something())
     break label;
   //etc
}

as that's weird (but apparently legal). However labels are needed on looping constructs for exactly the purpose outlined here.

Actual behavior

Three lint complaints.

@Timer Timer added this to the 0.9.6 milestone Mar 17, 2017

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Mar 22, 2017

I believe we agreed to go forward with this.
Can you please submit a PR removing the rule(s)? Thanks!

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Mar 31, 2017

@jannotti if you can submit a PR for this, let me know!

@shaunwallace

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2017

@Timer @jannotti I can pick this up otherwise.

@jannotti

This comment has been minimized.

Copy link
Author

commented Apr 3, 2017

@shaunwallace That would be great. I'm sure it's a one line fix, but I haven't even looked at CRA's source yet.

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Apr 3, 2017

All yours @shaunwallace.

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Apr 15, 2017

@shaunwallace Are you still interested in pursuing this? 😄

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.