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

New rule: consistent-return #221

Closed
LinusU opened this issue Aug 4, 2015 · 3 comments

Comments

@LinusU
Copy link
Member

commented Aug 4, 2015

Suggesting that we include the rule consistent-return


This rule is aimed at ensuring all return statements either specify a value or don't specify a value.

The following patterns are considered warnings:

function doSomething(condition) {

    if (condition) {
        return true;
    } else {
        return;
    }
}

function doSomething(condition) {

    if (condition) {
        return;
    } else {
        return true;
    }
}

The following patterns are considered okay and do not cause warnings:

function doSomething(condition) {

    if (condition) {
        return true;
    } else {
        return false;
    }
}

I found the rule while comparing which rules xo has versus standard here: #216 (comment)

@feross

This comment has been minimized.

Copy link
Member

commented Aug 4, 2015

On the surface, this seems like a good rule. But it's actually not that well implemented, and it gives a ton of false positives. 23 repos in the test suite will fail if you enable this rule.

standard has to be pragmatic -- we can't enable flaky rules!

Of particular note, the popular return cb(err) pattern confuses the rule quite a bit:

      rs.pipe(through.obj(function (data, enc, cb) {
        if (data.value.toString()[0] === '{') return cb()
        if (verify) {
          verify(u, val.message, val.signature, function (_, valid) {
            m.valid = !!valid
            processor(m, cb)
          })
          return
        }
     })

You can play around with potential rule changes by modifying eslint-config-standard and running the standard tests with npm test.

@feross feross closed this Aug 4, 2015

@LinusU

This comment has been minimized.

Copy link
Member Author

commented Aug 4, 2015

Ahh, that makes very much sense.

I'll try to play around with the rules and see if there is anything that we can learn from xo and others :)

@feross

This comment has been minimized.

Copy link
Member

commented Aug 4, 2015

Great idea, thanks!
On Tue, Aug 4, 2015 at 3:49 PM Linus Unnebäck notifications@github.com
wrote:

Ahh, that makes very much sense.

I'll try to play around with the rules and see if there is anything that
we can learn from xo and others :)


Reply to this email directly or view it on GitHub
#221 (comment).

@lock lock bot locked as resolved and limited conversation to collaborators May 11, 2018

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