-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
The Semantic Rule for { } VS do/end #162
Comments
👍 We use the semantic rule in our internal style guide and I'd like to see it used more. |
Really interesting. However, I feel a little awkward about |
@bbatsov thoughts? |
Generally I dislike semantic rules, because they cannot be enforced automatically. If the code style cannot be automatically enforced it's quite likely it will be violated. At one point I supported the semantic |
do...end could aways be put on multiple lines even if you have a single line statement. I like the semantic rule. |
hmm, and it could be enforced that if you're chaining a method call to the block you should be using brackets |
I agree with @bbatsov, in my experience a rule that is not automatically enforced is not truly a rule, it's more like a guideline because it will not be followed consistently. Also, I feel like style rules should preferably be discoverable by reading well-written code. This is where most styles start, after all, adopted from large bodies of code or commonly-read books, consistently written in a single way. I don't believe that the semantic rule for blocks is sufficiently discoverable by this test. Without previous knowledge of the rule, looking at a large body of code ... would I assume that there was a pattern to the choice of |
Just a couple of comments. (1) The rule could certainly be automatically enforceable via analysis of the source of the called function. And even if the source is not available, the behavior of the method of Enumerable is well know and could be encoded into an enforcer (e.g. "each" never uses the return value, "map" always does). This would cover the majority of the block uses. (2) Style rules that only reflect what is easily discoverable by a trivial examination of the source code seem to me to be the least valuable rules. If the style is not conveying additional information, why bother? E.g. the one VS multiline rule tells me nothing about the code that isn't already painfully obvious from the layout of the code. The semantic rule gives the reader additional information about the intent of the code. And that's what I find valuable. Just some thoughts. Thanks. |
@lee-dohm Has it occurred to you that this thing is called style guide 😜 ? Other than that, I just can agree with
👍 |
It's been a long while, but I want to pick up Jim's torch here and run with it a bit. Namely:
There's probably an easy win here that can cover a majority of the cases quite easily. and
and to push back on @bbatsov's comment:
So, you're planning to phase out I think it can also be argued that those who use the Weirich method generally don't use |
I wasn't too specific - I actually like semantic rules, but I dislike enforcing them. Some rules (e.g. semantic
They won't be phased out and I'd be happy to add support for Jim's style to RuboCop. But I'm wary of suggesting its usage in this document.
Good point. |
* rubocop#1600 * rubocop/ruby-style-guide#162 * http://devblog.avdi.org/2011/07/26/the-procedurefunction-block-convention-in-ruby/ Add a configuration option to the Style/Blocks cop to permit two styles: * multiline (the current default); * weirich (the semantic rule as described in the links above). With Weirich style enabled, this allows multi-line blocks with braces if the block is considered "functional". The current implementation checks whether the return value of a block is used to classify it as "functional". It performs the following checks: 1. Is the return value of the block being assigned? 2. Is the return value of the block sent a message? 3. Is the return value of the block the last thing in its scope? This should cover the following Weirich-style use cases: # 1 foo = map { |x| x * 2 } # 2 map { |x| x * 2 }.inspect # 3 block do foo map { |x| x * 2 } end # 3 puts map { |x| x * 2 } Add offenses if the return value of a block is used but do...end is used instead of the intention-revealing {...}. Conversely, if the return value of a block is not used, add an offense if {...} is used instead of do...end. Add a configurable whitelist of methods that use the return value of their block without being obvious to the caller. This permits use cases such as let or subject in RSpec. As DSLs often use do...end (e.g. RSpec.describe), do not add an offense if a block uses do...end even though it could potentially be the return value of its outer scope, e.g. RSpec.describe Foo do it 'blah' do # ... end end
* rubocop#1600 * rubocop/ruby-style-guide#162 * http://devblog.avdi.org/2011/07/26/the-procedurefunction-block-convention-in-ruby/ Add a configuration option to the Style/Blocks cop to permit two styles: * multiline (the current default); * weirich (the semantic rule as described in the links above). With Weirich style enabled, this allows multi-line blocks with braces if the block is considered "functional". The current implementation checks whether the return value of a block is used to classify it as "functional". It performs the following checks: 1. Is the return value of the block being assigned? 2. Is the return value of the block sent a message? 3. Is the return value of the block the last thing in its scope? This should cover the following Weirich-style use cases: # 1 foo = map { |x| x * 2 } # 2 map { |x| x * 2 }.inspect # 3 block do foo map { |x| x * 2 } end # 3 puts map { |x| x * 2 } Add offenses if the return value of a block is used but do...end is used instead of the intention-revealing {...}. Conversely, if the return value of a block is not used, add an offense if {...} is used instead of do...end. Add a configurable whitelist of methods that use the return value of their block without being obvious to the caller. This permits use cases such as let or subject in RSpec. As DSLs often use do...end (e.g. RSpec.describe), do not add an offense if a block uses do...end even though it could potentially be the return value of its outer scope, e.g. RSpec.describe Foo do it 'blah' do # ... end end
* rubocop#1600 * rubocop/ruby-style-guide#162 * http://devblog.avdi.org/2011/07/26/the-procedurefunction-block-convention-in-ruby/ Add a configuration option to the Style/Blocks cop to permit two styles: * multiline (the current default); * weirich (the semantic rule as described in the links above). With Weirich style enabled, this allows multi-line blocks with braces if the block is considered "functional". The current implementation checks whether the return value of a block is used to classify it as "functional". It performs the following checks: 1. Is the return value of the block being assigned? 2. Is the return value of the block sent a message? 3. Is the return value of the block the last thing in its scope? This should cover the following Weirich-style use cases: # 1 foo = map { |x| x * 2 } # 2 map { |x| x * 2 }.inspect # 3 block do foo map { |x| x * 2 } end # 3 puts map { |x| x * 2 } Add offenses if the return value of a block is used but do...end is used instead of the intention-revealing {...}. Conversely, if the return value of a block is not used, add an offense if {...} is used instead of do...end. Add a configurable whitelist of methods that use the return value of their block without being obvious to the caller. This permits use cases such as let or subject in RSpec. As DSLs often use do...end (e.g. RSpec.describe), do not add an offense if a block uses do...end even though it could potentially be the return value of its outer scope, e.g. RSpec.describe Foo do it 'blah' do # ... end end
* rubocop#1600 * rubocop/ruby-style-guide#162 * http://devblog.avdi.org/2011/07/26/the-procedurefunction-block-convention-in-ruby/ Add a configuration option to the Style/Blocks cop to permit two styles: * multiline (the current default); * weirich (the semantic rule as described in the links above). With Weirich style enabled, this allows multi-line blocks with braces if the block is considered "functional". The current implementation checks whether the return value of a block is used to classify it as "functional". It performs the following checks: 1. Is the return value of the block being assigned? 2. Is the return value of the block sent a message? 3. Is the return value of the block the last thing in its scope? This should cover the following Weirich-style use cases: # 1 foo = map { |x| x * 2 } # 2 map { |x| x * 2 }.inspect # 3 block do foo map { |x| x * 2 } end # 3 puts map { |x| x * 2 } Add offenses if the return value of a block is used but do...end is used instead of the intention-revealing {...}. Conversely, if the return value of a block is not used, add an offense if {...} is used instead of do...end. Add a configurable whitelist of methods that use the return value of their block without being obvious to the caller. This permits use cases such as let or subject in RSpec. The same whitelist can be used to permit methods that don't use the return value of their block despite being used in assignment such as tap or Active Record's create and new. As DSLs often use do...end (e.g. RSpec.describe), do not add an offense if a block uses do...end even though it could potentially be the return value of its outer scope, e.g. RSpec.describe Foo do it 'blah' do # ... end end
* rubocop#1600 * rubocop/ruby-style-guide#162 * http://devblog.avdi.org/2011/07/26/the-procedurefunction-block-convention-in-ruby/ Add a configuration option to the Style/Blocks cop to permit two styles: * multiline (the current default); * weirich (the semantic rule as described in the links above). With Weirich style enabled, this allows multi-line blocks with braces if the block is considered "functional". The current implementation checks whether the return value of a block is used to classify it as "functional". It performs the following checks: 1. Is the return value of the block being assigned? 2. Is the return value of the block sent a message? 3. Is the return value of the block the last thing in its scope? This should cover the following Weirich-style use cases: # 1 foo = map { |x| x * 2 } # 2 map { |x| x * 2 }.inspect # 3 block do foo map { |x| x * 2 } end # 3 puts map { |x| x * 2 } Add offenses if the return value of a block is used but do...end is used instead of the intention-revealing {...}. Conversely, if the return value of a block is not used, add an offense if {...} is used instead of do...end. Add a configurable whitelist of methods that use the return value of their block without being obvious to the caller. This permits use cases such as let or subject in RSpec. The same whitelist can be used to permit methods that don't use the return value of their block despite being used in assignment such as tap or Active Record's create and new. As DSLs often use do...end (e.g. RSpec.describe), do not add an offense if a block uses do...end even though it could potentially be the return value of its outer scope, e.g. RSpec.describe Foo do it 'blah' do # ... end end
* rubocop#1600 * rubocop/ruby-style-guide#162 * http://devblog.avdi.org/2011/07/26/the-procedurefunction-block-convention-in-ruby/ Add a configuration option to the `Style/Blocks` cop to permit two styles: * `multiline` (the current default); * `weirich` (the semantic rule as described in the links above). With Weirich style enabled, this allows multi-line blocks with braces if the block is considered "functional". The current implementation checks whether the return value of a block is used to classify it as "functional". It performs the following checks: 1. Is the return value of the block being assigned? 2. Is the return value of the block sent a message? 3. Is the return value of the block the last thing in its scope? This should cover the following Weirich-style use cases: # 1 foo = map { |x| x * 2 } # 2 map { |x| x * 2 }.inspect # 3 block do foo map { |x| x * 2 } end # 3 puts map { |x| x * 2 } Add offenses if the return value of a block is used but `do`...`end` is used instead of the intention-revealing `{`...`}`. Conversely, if the return value of a block is not used, add an offense if `{`...`}` is used instead of `do`...`end`. As there are some methods that are functional or procedural but cannot be categorised as such from their usage alone, add configurable lists to permit methods such as `RSpec::Core::ExampleGroup.let` which is functional but appears procedural and `tap` which is procedural but appears functional. For methods which can be both functional and procedural (such as `lambda`) and cannot be categorised by usage, add a configurable list of ignored methods. As DSLs often use `do`...`end` (e.g. `RSpec.describe`), do not add an offense if a block uses `do`...`end` even though it could potentially be the return value of its outer scope, e.g. RSpec.describe Foo do it 'blah' do # ... end end
* rubocop#1600 * rubocop/ruby-style-guide#162 * http://devblog.avdi.org/2011/07/26/the-procedurefunction-block-convention-in-ruby/ Add a configuration option to the `Style/Blocks` cop to permit two styles: * `multiline` (the current default); * `weirich` (the semantic rule as described in the links above). With Weirich style enabled, this allows multi-line blocks with braces if the block is considered "functional". The current implementation checks whether the return value of a block is used to classify it as "functional". It performs the following checks: 1. Is the return value of the block being assigned? 2. Is the return value of the block sent a message? 3. Is the return value of the block the last thing in its scope? This should cover the following Weirich-style use cases: # 1 foo = map { |x| x * 2 } # 2 map { |x| x * 2 }.inspect # 3 block do foo map { |x| x * 2 } end # 3 puts map { |x| x * 2 } Add offenses if the return value of a block is used but `do`...`end` is used instead of the intention-revealing `{`...`}`. Conversely, if the return value of a block is not used, add an offense if `{`...`}` is used instead of `do`...`end`. As there are some methods that are functional or procedural but cannot be categorised as such from their usage alone, add configurable lists to permit methods such as `RSpec::Core::ExampleGroup.let` which is functional but appears procedural and `tap` which is procedural but appears functional. For methods which can be both functional and procedural (such as `lambda`) and cannot be categorised by usage, add a configurable list of ignored methods. As DSLs often use `do`...`end` (e.g. `RSpec.describe`), do not add an offense if a block uses `do`...`end` even though it could potentially be the return value of its outer scope, e.g. RSpec.describe Foo do it 'blah' do # ... end end
* rubocop#1600 * rubocop/ruby-style-guide#162 * http://devblog.avdi.org/2011/07/26/the-procedurefunction-block-convention-in-ruby/ Add a configuration option to the `Style/Blocks` cop to permit two styles: * `multiline` (the current default); * `weirich` (the semantic rule as described in the links above). With Weirich style enabled, this allows multi-line blocks with braces if the block is considered "functional". The current implementation checks whether the return value of a block is used to classify it as "functional". It performs the following checks: 1. Is the return value of the block being assigned? 2. Is the return value of the block sent a message? 3. Is the return value of the block the last thing in its scope? This should cover the following Weirich-style use cases: # 1 foo = map { |x| x * 2 } # 2 map { |x| x * 2 }.inspect # 3 block do foo map { |x| x * 2 } end # 3 puts map { |x| x * 2 } Add offenses if the return value of a block is used but `do`...`end` is used instead of the intention-revealing `{`...`}`. Conversely, if the return value of a block is not used, add an offense if `{`...`}` is used instead of `do`...`end`. As there are some methods that are functional or procedural but cannot be categorised as such from their usage alone, add configurable lists to permit methods such as `RSpec::Core::ExampleGroup.let` which is functional but appears procedural and `tap` which is procedural but appears functional. For methods which can be both functional and procedural (such as `lambda`) and cannot be categorised by usage, add a configurable list of ignored methods. As DSLs often use `do`...`end` (e.g. `RSpec.describe`), do not add an offense if a block uses `do`...`end` even though it could potentially be the return value of its outer scope, e.g. RSpec.describe Foo do it 'blah' do # ... end end
* rubocop#1600 * rubocop/ruby-style-guide#162 * http://devblog.avdi.org/2011/07/26/the-procedurefunction-block-convention-in-ruby/ Add a configuration option to the `Style/Blocks` cop to permit two styles: * `multiline` (the current default); * `weirich` (the semantic rule as described in the links above). With Weirich style enabled, this allows multi-line blocks with braces if the block is considered "functional". The current implementation checks whether the return value of a block is used to classify it as "functional". It performs the following checks: 1. Is the return value of the block being assigned? 2. Is the return value of the block sent a message? 3. Is the return value of the block the last thing in its scope? This should cover the following Weirich-style use cases: # 1 foo = map { |x| x * 2 } # 2 map { |x| x * 2 }.inspect # 3 block do foo map { |x| x * 2 } end # 3 puts map { |x| x * 2 } Add offenses if the return value of a block is used but `do`...`end` is used instead of the intention-revealing `{`...`}`. Conversely, if the return value of a block is not used, add an offense if `{`...`}` is used instead of `do`...`end`. As there are some methods that are functional or procedural but cannot be categorised as such from their usage alone, add configurable lists to permit methods such as `RSpec::Core::ExampleGroup.let` which is functional but appears procedural and `tap` which is procedural but appears functional. For methods which can be both functional and procedural (such as `lambda`) and cannot be categorised by usage, add a configurable list of ignored methods. As DSLs often use `do`...`end` (e.g. `RSpec.describe`), do not add an offense if a block uses `do`...`end` even though it could potentially be the return value of its outer scope, e.g. RSpec.describe Foo do it 'blah' do # ... end end
* rubocop#1600 * rubocop/ruby-style-guide#162 * http://devblog.avdi.org/2011/07/26/the-procedurefunction-block-convention-in-ruby/ Add a configuration option to the `Style/Blocks` cop to permit two styles: * `line_count_based` (the current default); * `semantic` (the semantic rule as described in the links above). With semantic style enabled, this allows multi-line blocks with braces if the block is considered "functional". The current implementation checks whether the return value of a block is used to classify it as "functional". It performs the following checks: 1. Is the return value of the block being assigned? 2. Is the return value of the block sent a message? 3. Is the return value of the block the last thing in its scope? This should cover the following semantic style use cases: # 1 foo = map { |x| x * 2 } # 2 map { |x| x * 2 }.inspect # 3 block do foo map { |x| x * 2 } end # 3 puts map { |x| x * 2 } Add offenses if the return value of a block is used but `do`...`end` is used instead of the intention-revealing `{`...`}`. Conversely, if the return value of a block is not used, add an offense if `{`...`}` is used instead of `do`...`end`. As there are some methods that are functional or procedural but cannot be categorised as such from their usage alone, add configurable lists to permit methods such as `RSpec::Core::ExampleGroup.let` which is functional but appears procedural and `tap` which is procedural but appears functional. For methods which can be both functional and procedural (such as `lambda`) and cannot be categorised by usage, add a configurable list of ignored methods. As DSLs often use `do`...`end` (e.g. `RSpec.describe`), do not add an offense if a block uses `do`...`end` even though it could potentially be the return value of its outer scope, e.g. RSpec.describe Foo do it 'blah' do # ... end end
Style/BlockDelimiters: Enabled: true EnforcedStyle: semantic See, eg: rubocop/ruby-style-guide#162 http://www.virtuouscode.com/2011/07/26/the-procedurefunction-block-convention-in-ruby/ Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>
The default option for Style/BlockDelimiters, `line_count_based`, simply counts the number of lines in the block to determine whether to use braces or do/end. These leads to superficial visual consistency, but actually contradicts the semantic rule, which says that the choice of braces vs do/end should depend on whether the block's purpose is functional or procedural, i.e. whether it returns a value or has side effects. When possible we should be using static analysis to enforce conventions around a programmer's intent, which is more critical information than how many lines a piece of code has. The `semantic` option, used here, is implemented with a relatively simple check to see if the return value of the block is assigned, receives a method call, or is the last thing inside of an outer scope that will thus return it. General consensus on the PR is that, although you can probably find some edge-cases that will lead to counterintuitive rule interpretations, this is a good-enough implementation. Implementing PR: rubocop/rubocop#1731 Jim Weirich's initial request for the feature (RIP): rubocop/ruby-style-guide#162
The default option for Style/BlockDelimiters, `line_count_based`, simply counts the number of lines in the block to determine whether to use braces or do/end. These leads to superficial visual consistency, but actually contradicts the semantic rule, which says that the choice of braces vs do/end should depend on whether the block's purpose is functional or procedural, i.e. whether it returns a value or has side effects. When possible we should be using static analysis to enforce conventions around a programmer's intent, which is more critical information than how many lines a piece of code has. The `semantic` option, used here, is implemented with a relatively simple check to see if the return value of the block is assigned, receives a method call, or is the last thing inside of an outer scope that will thus return it. General consensus on the PR is that, although you can probably find some edge-cases that will lead to counterintuitive rule interpretations, this is a good-enough implementation. Implementing PR: rubocop/rubocop#1731 Jim Weirich's initial request for the feature (RIP): rubocop/ruby-style-guide#162
The style guide suggests using {...} for single line and do/end for multiple line blocks.
Is it worth mentioning the semantic rule for choosing {} VS do/end as another possibility? The semantic rule says use {} for blocks where the primary purpose of the block is to return a value, use do/end for blocks where the primary purpose of the block is to execute side effects.
Although a minority style choice, the semantic rule does have a significant following:
The text was updated successfully, but these errors were encountered: