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

Validate use of $0 only if it's "simple" #70

Closed
chriseidhof opened this issue May 29, 2015 · 7 comments · Fixed by #5451
Closed

Validate use of $0 only if it's "simple" #70

chriseidhof opened this issue May 29, 2015 · 7 comments · Fixed by #5451

Comments

@chriseidhof
Copy link
Contributor

e.g. if it's the first thing that's switched on, or if the closure is really simple (not many tokens).

@jpsim
Copy link
Collaborator

jpsim commented May 29, 2015

👍

@segiddins
Copy link
Contributor

Maybe only allow for single-line closures by default?

@chriseidhof
Copy link
Contributor Author

Yeah... I often do this pattern:

blablaFunc {
    switch $0 {
        // multiple cases
    }
}

But maybe that's very specific. I guess the important thing is that the $0 is very close to the corresponding opening brace. But having them only in single-line closures is a good first step.

@keith
Copy link
Collaborator

keith commented Aug 20, 2015

I've implemented a rule for this, but it's pretty simple. It just validates that all $0s have a { before them and a } after on the same line. This does mean that it doesn't catch all cases, something like:

foo.map {
     bar.something { $0 }
}

Wouldn't fail, but we feel like most of those cases will be caught by our line length as well. I can submit this upstream if the leniency is alright.

@jpsim
Copy link
Collaborator

jpsim commented Nov 18, 2015

@keith could you make a PR with your single-line rule? I can't guarantee we'd merge it right away, but it would be a good starting point, at least to stir a conversation.

@keith
Copy link
Collaborator

keith commented Nov 18, 2015

Yep, I'll do that soon.

@keith
Copy link
Collaborator

keith commented Nov 18, 2015

Submitted: #218

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants