Opened 2 years ago

Closed 2 months ago

#5647 closed change (rejected)

[ESLint] Allow labeled statements

Reported by: greiner Assignee:
Priority: Unknown Milestone:
Module: Unknown Keywords:
Cc: kzar, sebastian Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description

Background

When we introduced the initial version of eslint-config-eyeo (see #3692), we disabled labeled statements without providing any reason for it concluding with "I've never seen a use-case for labels [...] so I think it's OK to leave the rule on" and "it doesn't matter much" (see code review).

I'd argue, however, that by introducing such a rule we communicate that it does matter to us because we're actively prohibiting a certain practice. That is why I had to start this ticket so that I'd be allowed to consider using a labeled statement.

Therefore let's get rid of rules that we think don't matter.

What to change

Remove the rule "no-labels" from eslint-config-eyeo.

Change History (4)

comment:1 follow-up: Changed 2 years ago by sebastian

Our ESLint configuration is allowing use of labels in order to break/continue an outer loop, like we do here. We merely disallow goto-like control flow using labelled blocks:

label: {
  if (...)
    break label;
}

As well as superfluous labels:

label: for (let x of y)
{
  break label; // Same as break without label
}

Do you have any real (sane) use case where our current rules are to restrictive about labels?

comment:2 in reply to: ↑ 1 ; follow-up: Changed 2 years ago by greiner

Replying to sebastian:

Our ESLint configuration is allowing use of labels in order to break/continue an outer loop, like we do here. We merely disallow goto-like control flow using labelled blocks:

I know. The question is just why did we disallow them?

As well as superfluous labels:

This is a different rule (i.e. "no-extra-label").

Do you have any real (sane) use case where our current rules are to restrictive about labels?

Yes, that's how I ran into this issue. I considered adding an example here but I didn't want to draw attention away from the more important underlying issue which is that we shouldn't disallow things just because we couldn't come up with a use-case for them yet.

We should, of course, disallow practices that cause us problems but, as far as I'm aware of, none such have been mentioned yet in regards to labeled statements.

comment:3 in reply to: ↑ 2 Changed 2 years ago by sebastian

If a justified use case for a new practice comes up, we can easily relax our rules. But the other way around, it is difficult to make our rules stricter later on, as this will usually require changing existing code (and our code base is huge). However, it is hard to decide whether a particular practice tends to improve code or rather causes issues, before seeing real world examples (in our own context).

comment:4 Changed 2 months ago by greiner

  • Resolution set to rejected
  • Status changed from new to closed

Closing this ticket because there hasn't been any further demand for this since the creation of this ticket.

Note: See TracTickets for help on using tickets.