Linter: Empty statements lint should not count for/while loops

[matanlurey]

For example, this is entirely kosher (you could be iterating through something):

while (something.meetsCondition());

[alexeieleusis]

I think in this case having the ignore annotation is better, it conveys maintainers that it is intentional.

Unless something is in a third party library, I think it should have a getInUsableCondition() method or similar, I would expect this kind of occurrences be pretty rare.

[matanlurey]

I don't agree, you shouldn't have to ignore valid code that contains a valid common pattern.

Almost every parser-type library will do something like this.

[zoechi]

you shouldn't have to ignore valid code

In this case the // ignore: ... feature would be useless

valid common pattern.

Also unawaited_futures are quite common but it still causes a linter warning.

I guess this requires some generic discussion how much possible missed bugs weight against the inconvenience of suppressing hints for valid code.

[matanlurey]

I strongly disagree with unawaited_futures even existing (and I won't allow it on any of my projects or official dart-lang ones) - there is the difference between "suggest possible bugs for me to review" and "force me to ignore totally valid code".

[zoechi]

I don't have a strong opinion on ether side. I just wish for some consistency.
Perhaps splitting it into two rules for sure and possible bugs?

[alexeieleusis]

I think that's the idea behind having lints and hints, hints cannot be
opted out and lints are opt in.

Alexei Diaz | Software Engineer | [email protected] | [phone]

On Wed, Jan 25, 2017 at 10:36 PM, Günter Zöchbauer <[email protected]

wrote:

I don't have a strong opinion on ether side. I just wish for some
consistency.
Perhaps splitting it into two rules for sure and possible bugs?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/dart-lang/linter/issues/383#issuecomment-275317818,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABLWk6tPgdPV9wuUomWL321VPf39zYVsks5rWD78gaJpZM4Ls055
.

[matanlurey]

At least as defined by Google, lints are something you warn about every time you see them. If that's the case maybe we need something like a "hint-lint" that you can completely disable (I'd never like to see them).

[bwilkerson]

Thank you, but we don't need more categories of diagnostics. :-)

Hints are for cases that are clearly bugs in code but that the spec doesn't define to be an error or warning. Hints must have zero false positives.

Lints are for cases that are more stylistic in nature, where some people will want to follow a rule and others won't. Lints should have zero false positives, but in some rare cases where it's too expensive to remove the false positives and they are extremely rare we might choose to allow them. In those cases it is possible to locally disable the lints where they would generate a false positive. (Hence the motivation for that feature.)

As @zoechi suggested, if we have a lint that some people want to follow in some cases but not others, we should seriously consider splitting the rule. People who want both rules to be in effect can enable both lints.

[matanlurey]

I'd be a fan of splitting the lint

[pq]

FWIW: if splitting the lint means more folks get value from it, I'm all for it.

We've strayed a bit from the original reported issue though. Should we take this to a fresh enhancement request?

As for @matanlurey's observation that this triggers a lint:

while (something.meetsCondition());

I think that's pretty common for this lint as implemented elsewhere. An escape hatch might be to notice if there's a comment in the body.

while (something.meetsCondition()) {
  // iterate, really
}

(No lint here.)

This is, for example, what eslint does (not that this is an exemplar necessarily!)