My Beef with RuboCop(rubypigeon.com)
rubypigeon.com
My Beef with RuboCop
https://www.rubypigeon.com/posts/my-beef-with-rubocop/
91 comments
> I've hit similar issues to those mentioned in the article and it absolutely infuriates me when I have to jump through hoops (or worse, reduce the readability of my code) to push well thought out code that communicates intent, only to fall foul of a robot that doesn't understand nuance.
I don't use Ruby but I faced similar issues with Rust's clippy and rustfmt. But I still think it's better than the alternative.
Every time I'm tempted to override one of these links/formats because I feel like my way is better I like to remind myself that it's going to set a precedent for the other devs on the project, and it's only a matter of time before one of us does something that the others don't like and it's only a matter of time before we return to pointless arguing about coding style and inconsistent code formatting.
>Consistency is merely a tool, the goal should be readability/maintainability.
That's the thing though: consistency is somewhat objective (and enforceable), readability and maintainability not so much.
TFA is a good example of this: they argue that explicitly returning "nil" is more readable and better communicates intent but I suspect (once again, not being a Ruby coder myself) that other experienced Rubyists could respond that everybody knows that a method finishing without an explicit value ends up returning nil, and that adding explicit code to do is is just clutter and distracts from the important things.
I don't use Ruby but I faced similar issues with Rust's clippy and rustfmt. But I still think it's better than the alternative.
Every time I'm tempted to override one of these links/formats because I feel like my way is better I like to remind myself that it's going to set a precedent for the other devs on the project, and it's only a matter of time before one of us does something that the others don't like and it's only a matter of time before we return to pointless arguing about coding style and inconsistent code formatting.
>Consistency is merely a tool, the goal should be readability/maintainability.
That's the thing though: consistency is somewhat objective (and enforceable), readability and maintainability not so much.
TFA is a good example of this: they argue that explicitly returning "nil" is more readable and better communicates intent but I suspect (once again, not being a Ruby coder myself) that other experienced Rubyists could respond that everybody knows that a method finishing without an explicit value ends up returning nil, and that adding explicit code to do is is just clutter and distracts from the important things.
> I don't use Ruby but I faced similar issues with Rust's clippy and rustfmt. But I still think it's better than the alternative.
I also use Go, and haven't had issues with the autoformatter there, but I believe that's mostly because there is little to no room for ambiguity around intent. You tell the compiler you want to return a Bool, and if you don't it's a compile time error. There's no "I dunno, maybe nil was an intended return value".
> Every time I'm tempted to override one of these links/formats because I feel like my way is better I like to remind myself that it's going to set a precedent for the other devs on the project
I think this misses the point somewhat. This isn't about changing the defaults of Rubocop, it's about not having some of those defaults in the first place for the places in Ruby where it's possible to express intent in a way that it's not possible to check with a linter.
> That's the thing though: consistency is somewhat objective (and enforceable), readability and maintainability not so much.
I'm still not saying that people should not use linters, just because you can lint something doesn't mean you have to lint everything.
> TFA is a good example of this: they argue that explicitly returning "nil" is more readable and better communicates intent but I suspect (once again, not being a Ruby coder myself) that other experienced Rubyists could respond that everybody knows that a method finishing without an explicit value ends up returning nil, and that adding explicit code to do is is just clutter and distracts from the important things.
Oh, absolutely, most Ruby programmers do know this, but it's not really about the code specifically I think, it's about the contract. Take this code:
We could fix this with (amongst other things):
1. If we are changing this method we need to respect the return
2. If we are consuming this method we know the return is deliberate
This unfortunately violates Rubocop's redundant return detection and will be rejected (as will other forms).
I also use Go, and haven't had issues with the autoformatter there, but I believe that's mostly because there is little to no room for ambiguity around intent. You tell the compiler you want to return a Bool, and if you don't it's a compile time error. There's no "I dunno, maybe nil was an intended return value".
> Every time I'm tempted to override one of these links/formats because I feel like my way is better I like to remind myself that it's going to set a precedent for the other devs on the project
I think this misses the point somewhat. This isn't about changing the defaults of Rubocop, it's about not having some of those defaults in the first place for the places in Ruby where it's possible to express intent in a way that it's not possible to check with a linter.
> That's the thing though: consistency is somewhat objective (and enforceable), readability and maintainability not so much.
I'm still not saying that people should not use linters, just because you can lint something doesn't mean you have to lint everything.
> TFA is a good example of this: they argue that explicitly returning "nil" is more readable and better communicates intent but I suspect (once again, not being a Ruby coder myself) that other experienced Rubyists could respond that everybody knows that a method finishing without an explicit value ends up returning nil, and that adding explicit code to do is is just clutter and distracts from the important things.
Oh, absolutely, most Ruby programmers do know this, but it's not really about the code specifically I think, it's about the contract. Take this code:
def foo
@some_instance_var = 123
end
> a = foo
=> 123
> a
=> 123
def foo
@some_instance_var = 123
@some_other_instance_var = 456
end
We could fix this with (amongst other things):
def foo
return @some_instance_var = 123
end
1. If we are changing this method we need to respect the return
2. If we are consuming this method we know the return is deliberate
This unfortunately violates Rubocop's redundant return detection and will be rejected (as will other forms).
> It looks like returning a is merely something that happens to happen.
This is...an odd view to me. With the exception of the "?" convention for usable-as-boolean query methods (which might not be actual booleans but you shouldn't rely on anything other than the boolean sense of the return value) and methods which are specifically documented otherwise, in Ruby every return value is and should be treated as intentional, not something that "happens to happen", and explicit return is used to avoid convoluted control structures when no better way to do so exists.
This is...an odd view to me. With the exception of the "?" convention for usable-as-boolean query methods (which might not be actual booleans but you shouldn't rely on anything other than the boolean sense of the return value) and methods which are specifically documented otherwise, in Ruby every return value is and should be treated as intentional, not something that "happens to happen", and explicit return is used to avoid convoluted control structures when no better way to do so exists.
> Take this code:
> [...]
> It turns out that:
> [...]
> but you've not actually said that the return value is intended, or a side effect.
Aren't the Ruby semantics clear about what happens in both of these cases (return values of methods and assignments) and hence it was the coder's choice to leave that as the return value?
Aren't the Ruby semantics clear about what happens in both of these cases (return values of methods and assignments) and hence it was the coder's choice to leave that as the return value?
Wether the programmer chose to do something is a large part of the issue here.
An explicit return says "I chose to do this". An implicit return in these examples says "There may or may not have been a choice made" and that's the problem. If you rely on the assumption that a choice was made, you add risk.
An explicit return says "I chose to do this". An implicit return in these examples says "There may or may not have been a choice made" and that's the problem. If you rely on the assumption that a choice was made, you add risk.
I fully agree with your example. I have disabled the return rule, and I am enforcing that we add an explicit return in our codebase.
Ruby is a tool that I use the way I want, not the other way round
Ruby is a tool that I use the way I want, not the other way round
> consistency is somewhat objective (and enforceable), readability and maintainability not so much.
The worst things people do in collaborative work or management are always explained by "well, X is objective and easy to measure, so it's our target".
The worst things people do in collaborative work or management are always explained by "well, X is objective and easy to measure, so it's our target".
Whenever you disagree with Rubocop and want it to shut up:
rubocop -a --auto-gen-config --auto-gen-only-exclude
Your just added exceptions for the leftovers that didn't pass the initial lint. But obv use this only after you cleaned up anything that should be. -a by itself autofixes anything it knows how to.
rubocop -a --auto-gen-config --auto-gen-only-exclude
Your just added exceptions for the leftovers that didn't pass the initial lint. But obv use this only after you cleaned up anything that should be. -a by itself autofixes anything it knows how to.
Useful, thanks.
The point of linters is that most of us can imagine up more interesting problems to deal with. So we welcome the consistency and focus on something we care more about.
> The point of linters is that most of us can imagine up more interesting problems to deal with.
I wasn't saying that linters were a problem, I was saying that the over application of linters was a problem.
In general, overly broad application of a tool is a problem, wether in programming or not. Linting is a tool, it is possible to over apply it, and I argue that this is regularly done with Rubocop.
> So we welcome the consistency and focus on something we care more about.
What is the goal of consistency?
I wasn't saying that linters were a problem, I was saying that the over application of linters was a problem.
In general, overly broad application of a tool is a problem, wether in programming or not. Linting is a tool, it is possible to over apply it, and I argue that this is regularly done with Rubocop.
> So we welcome the consistency and focus on something we care more about.
What is the goal of consistency?
>What is the goal of consistency?
i don't know anything about linters or rubocop, but i do think you are wrong on this narrow point.
consistency is good in and of itself, it is not merely a symptom of your true goal of maintainability or readability as you mention in op.
id prefer someone/something that is consistently wrong to someone/something that is more often right, but randomly wrong. consistency implies methodology that can be changed and adapted, as opposes to simple mistakes that have to be assiduously monitored for.
you can train a person into a different habit and refactoring is much easier than bug hunting.
i don't know anything about linters or rubocop, but i do think you are wrong on this narrow point.
consistency is good in and of itself, it is not merely a symptom of your true goal of maintainability or readability as you mention in op.
id prefer someone/something that is consistently wrong to someone/something that is more often right, but randomly wrong. consistency implies methodology that can be changed and adapted, as opposes to simple mistakes that have to be assiduously monitored for.
you can train a person into a different habit and refactoring is much easier than bug hunting.
I feel the discussion about consistency is moot because, in my experience, linters often have a different idea about consistency than humans.
Let's say I have the following three methods:
Let's say I have the following three methods:
def foo
if broken?
:error
else
@x
end
end
def bar
if broken?
5
else
@y
end
end
def qux
if broken?
nil
else
@z
end
end
The thing is, for probably a good majority of Rubocops rules, the consistency has nothing to do with correctness, it's consistency for the sake of consistency.
Whether you use ' or " for strings, or the %d shorthand or fully formed arrays in Ruby has nothing to do with the correctness of your code.
When these are trivial, it's not such a big deal - perhaps a bit of friction for developers, but not in a way that slows them down significantly. Rubocop does occasionally get into style rules that can change what your code is trying to communicate though, and those can often have negative effects.
Whether you use ' or " for strings, or the %d shorthand or fully formed arrays in Ruby has nothing to do with the correctness of your code.
When these are trivial, it's not such a big deal - perhaps a bit of friction for developers, but not in a way that slows them down significantly. Rubocop does occasionally get into style rules that can change what your code is trying to communicate though, and those can often have negative effects.
> consistency is good in and of itself
It can't be, because consistency is only a concept with no intrinsic value. The value is based on the relationship between the concept and the application.
You can say "consistency in code is good" or "consistency in code is bad", but then you've made an argument and you need back that up with reason.
> id prefer someone/something that is consistently wrong to someone/something that is more often right, but randomly wrong.
Just to be clear, we're not talking about bugs here, we're talking about style where there's, sometimes subjectively, "better" or "worse" more often than "right" or "wrong".
> consistency implies methodology that can be changed and adapted, as opposes to simple mistakes that have to be assiduously monitored for.
Consistency doesn't imply anything of the sort, but if you want to apply consistency to a problem with the goal of being able to change every instance of something you're not really solving a problem, unless your actual goal is consistency, which I argue isn't actually a goal at all.
It can't be, because consistency is only a concept with no intrinsic value. The value is based on the relationship between the concept and the application.
You can say "consistency in code is good" or "consistency in code is bad", but then you've made an argument and you need back that up with reason.
> id prefer someone/something that is consistently wrong to someone/something that is more often right, but randomly wrong.
Just to be clear, we're not talking about bugs here, we're talking about style where there's, sometimes subjectively, "better" or "worse" more often than "right" or "wrong".
> consistency implies methodology that can be changed and adapted, as opposes to simple mistakes that have to be assiduously monitored for.
Consistency doesn't imply anything of the sort, but if you want to apply consistency to a problem with the goal of being able to change every instance of something you're not really solving a problem, unless your actual goal is consistency, which I argue isn't actually a goal at all.
I think that can be a dangerous mindset. Code readability isn't a trifling concern that you shouldn't care about in favour of more interesting things, it should be your primary concern as a developer. In almost every circumstance, I'll prefer readable code to something that's more clever, or even more performant.
Rubocop and other linting tools do aid in readability somewhat, but I agree that a lot of the time they can either focus on trivial concerns that don't really make a large impact on readability (things like which quote characters to use), and in some cases actively harm readability (the examples from this article).
Personally, my feeling on tools like Rubocop is that where they break down is often more important than the areas where they help (high level structure and communicating intent), and often the benefits of consistency in syntax are frankly pretty marginal, but I can get people having different opinions on this. There's certainly Rubocop rules I enable and get value from, but I do have quite a lot turned off and don't really notice much impact from it.
Rubocop and other linting tools do aid in readability somewhat, but I agree that a lot of the time they can either focus on trivial concerns that don't really make a large impact on readability (things like which quote characters to use), and in some cases actively harm readability (the examples from this article).
Personally, my feeling on tools like Rubocop is that where they break down is often more important than the areas where they help (high level structure and communicating intent), and often the benefits of consistency in syntax are frankly pretty marginal, but I can get people having different opinions on this. There's certainly Rubocop rules I enable and get value from, but I do have quite a lot turned off and don't really notice much impact from it.
The problem is Ruby itself.
Implicit return, whether you like it or not, is frankly a weird feature, and is sure to rub some people the wrong way.
Implicit return, whether you like it or not, is frankly a weird feature, and is sure to rub some people the wrong way.
We use rubocop heavily at GitLab. I'm generally happy with it, and we have a largely sane set of linters. I think when your codebase gets to a certain size and you have a lot of engineers working on it, having some sane defaults is very beneficial and helps standardise the codebase a bit.
Where I think rubocop really shines is in picking up _problem_ code, like potential bugs, performance issues, or errors, in advance. We also use it heavily when deprecating older modules/classes, like replacing JSON calls with our own wrapper.
It does, however, sometimes suggest something totally bonkers: https://gitlab.com/gitlab-org/gitlab/-/issues/271570, but it's fairly trivial to skip it. After all, humans should be allowed to say that the linter is wrong and either ignore it or suggest an improvement, and that's a process issue rather than a rubocop issue :)
Not everyone is going to agree with every cop you enable, but the goal is standardisation rather than appeasing everybody. If something is particularly contentious then I would suggest removing/disabling that cop.
Every few years at GitLab we do seem to get a singlequotes vs doublequotes cop argument crop up, which is why we (thankfully) still don't have any requirement around those. Team doublequotes represent!
Edit: Oh and unrelated to the content of the article, but I really like the design and colour palette of the blog.
Where I think rubocop really shines is in picking up _problem_ code, like potential bugs, performance issues, or errors, in advance. We also use it heavily when deprecating older modules/classes, like replacing JSON calls with our own wrapper.
It does, however, sometimes suggest something totally bonkers: https://gitlab.com/gitlab-org/gitlab/-/issues/271570, but it's fairly trivial to skip it. After all, humans should be allowed to say that the linter is wrong and either ignore it or suggest an improvement, and that's a process issue rather than a rubocop issue :)
Not everyone is going to agree with every cop you enable, but the goal is standardisation rather than appeasing everybody. If something is particularly contentious then I would suggest removing/disabling that cop.
Every few years at GitLab we do seem to get a singlequotes vs doublequotes cop argument crop up, which is why we (thankfully) still don't have any requirement around those. Team doublequotes represent!
Edit: Oh and unrelated to the content of the article, but I really like the design and colour palette of the blog.
> Every few years at GitLab we do seem to get a singlequotes vs doublequotes cop argument crop up, which is why we (thankfully) still don't have any requirement around those
So you randomly have single and double quotes throughout the codebase?
I don't care which one a project uses, but it seems wise to pick one. Uniform syntax is very helpful for code readability.
So you randomly have single and double quotes throughout the codebase?
I don't care which one a project uses, but it seems wise to pick one. Uniform syntax is very helpful for code readability.
Double quotes allow you to do string interpolation, which is not possible with single quotes. So I assume one faction argues that everything should be double quoted because it's does not matter if there's interpolation or not, while the other group would rather only use double quotes when necessary, otherwise use single quotes.
I'm personally part of the second group simply because my linter forced it, but at the end of the day, it's such a low impact we don't really care.
I'm personally part of the second group simply because my linter forced it, but at the end of the day, it's such a low impact we don't really care.
> would rather only use double quotes when necessary, otherwise use single quotes
So in that case, they're not used randomly according to the individual user's preference. There's a consistent rule. It's helpful for readability, too. And not sure about Rubocop in Ruby, but many linters in other languages can pick up on these different applications.
So in that case, they're not used randomly according to the individual user's preference. There's a consistent rule. It's helpful for readability, too. And not sure about Rubocop in Ruby, but many linters in other languages can pick up on these different applications.
Generally in ruby/rails, the people in favor of double quotes would use double quotes all the time.
Rubocop would have you use singles for non interpolated and double for interpolated.
I get that there is a potential to catch some kind of error where you use double quotes on a string that will interpolated something which it wouldn't if it were in single quotes. In 12 years, I've never had that happen. I have, however, had it happen dozens of times where I changed something from interpolated to not and then I have to bounce over to change the quotes. This happens a lot and is annoying.
It's also an eyesore when you have an array of messages, each neatly aligned, but some are interpolated and some aren't, so the quotes change.
In Ruby, knowing that a string has interpolation is easy enough by seeing the #{} in the middle of it. It's hard to miss, really.
The single/double quote cop which is turned on by default is also counter to the existing style of every-string-is-double when it came along.
Rubocop would have you use singles for non interpolated and double for interpolated.
I get that there is a potential to catch some kind of error where you use double quotes on a string that will interpolated something which it wouldn't if it were in single quotes. In 12 years, I've never had that happen. I have, however, had it happen dozens of times where I changed something from interpolated to not and then I have to bounce over to change the quotes. This happens a lot and is annoying.
It's also an eyesore when you have an array of messages, each neatly aligned, but some are interpolated and some aren't, so the quotes change.
In Ruby, knowing that a string has interpolation is easy enough by seeing the #{} in the middle of it. It's hard to miss, really.
The single/double quote cop which is turned on by default is also counter to the existing style of every-string-is-double when it came along.
interpolation isn't the only difference between single- and double-quoted strings. If you need control, etc., characters via backslash escapes, you must use double (or semantic equivalents like %/%Q). If you need non-eyesore strings with literal backslashes, you need single (or semantic equivalents like %q.)
{} are not required for instance variables.
irb(main):001:0> puts '1\t2',"1\t2"
1\t2
1 2
{} are not required for instance variables.
> the people in favor of double quotes would use double quotes all the time.
Fair enough. I'm not much into Ruby but if I were, that's what I would encourage. But I'd be ok with whatever house style, as long as it's consistent.
Fair enough. I'm not much into Ruby but if I were, that's what I would encourage. But I'd be ok with whatever house style, as long as it's consistent.
Rubocop does pick it up by default.
> Uniform syntax is very helpful for code readability.
I don't understand this argument. Are people really having trouble reading code because it has two different quote characters? This goes against my understanding of "readability"? What is it that is really meant by this?
I don't understand this argument. Are people really having trouble reading code because it has two different quote characters? This goes against my understanding of "readability"? What is it that is really meant by this?
I don't think anyone is trying to make the case that a single stylistic preference impacts readability, but in aggregate many diverging preferences definitely can.
Add in programming styles and a flexible language like Ruby can look like entirely different languages from one dev to the next (Ruby is a very traditional OOP capable language, or a weird but capable lisp, mix the two and readability suffers in new and exciting ways).
Add in programming styles and a flexible language like Ruby can look like entirely different languages from one dev to the next (Ruby is a very traditional OOP capable language, or a weird but capable lisp, mix the two and readability suffers in new and exciting ways).
Agreed, anything trivial enough for a linter to fix probably didn't matter in the first place.
Ah ha, yes I would say it would make sense to pick one, but only if that one is doublequotes ;)
Personally I have a few reasons why singlequotes are inferior:
- String interpolation requires doublequotes
- Using an apostrophe requires you to escape it or use doublequotes
- They're harder to type on a Japanese-layout keyboard, which I use (it's shift+7, lol), which is otherwise really efficient for Ruby
But there are arguments against doublequotes too:
- I guess they're maybe harder to type on an ANSI keyboard?
- They parse ASCII escape codes, and singlequotes don't
But in my experience I need to use interpolation and apostrophes more than I care about not parsing \r so I use doublequotes everywhere in my own codebase.
So I would just rather not force either, as people seem to feel strongly both ways and the added frustration is probably worse for productivity than having it look a bit neater :)
Honestly I think the "community" chose wrongly in trying to standardise on singlequotes-unless-you-need-interpolation, when it should probably be doublequotes-unless-you-have-a-good-reason, but it's pretty hard to change that now.
Personally I have a few reasons why singlequotes are inferior:
- String interpolation requires doublequotes
- Using an apostrophe requires you to escape it or use doublequotes
- They're harder to type on a Japanese-layout keyboard, which I use (it's shift+7, lol), which is otherwise really efficient for Ruby
But there are arguments against doublequotes too:
- I guess they're maybe harder to type on an ANSI keyboard?
- They parse ASCII escape codes, and singlequotes don't
But in my experience I need to use interpolation and apostrophes more than I care about not parsing \r so I use doublequotes everywhere in my own codebase.
So I would just rather not force either, as people seem to feel strongly both ways and the added frustration is probably worse for productivity than having it look a bit neater :)
Honestly I think the "community" chose wrongly in trying to standardise on singlequotes-unless-you-need-interpolation, when it should probably be doublequotes-unless-you-have-a-good-reason, but it's pretty hard to change that now.
> Every few years at GitLab we do seem to get a singlequotes vs doublequotes cop argument crop up
...when obviously the correct answer is to consistently use %.
/s
...when obviously the correct answer is to consistently use %.
/s
I was wondering about proposing that as a compromise. Or heredocs :D
I am not sure what is the problem with me, but for me the code with the GuardClause is easier to read.
Maybe I find it easier because I started programming with Basic and based a lot of logic on GOTO line and then moved to assembler language where again it used JMP so I represent code with nodes and edges in my mind.
Also, the original code - which the author prefers - has a positive ratio of language keywords over content (or logic) i.e extra 'end' and 'else' which does not bring too much information and makes me read it twice just to be sure I did not missed anything.
class SanitizingFilter < HTMLFilter
def render_element(name, attrs, content)
return nil unless safe?(name, attrs)
super
end
end
Maybe I find it easier because I started programming with Basic and based a lot of logic on GOTO line and then moved to assembler language where again it used JMP so I represent code with nodes and edges in my mind.
Also, the original code - which the author prefers - has a positive ratio of language keywords over content (or logic) i.e extra 'end' and 'else' which does not bring too much information and makes me read it twice just to be sure I did not missed anything.
I agree with you that I find it more readable. However, I've spent a long time living with this exact RoboCop setting so I am probably very used to looking at this style. The author mentioned scanning by indentation which is likely something that I do not employ in the same way that someone who has not lived with similar robocut settings does.
Agreed again.
Also, the longer I write code the more I find myself avoiding anything but single-line if/unless statement modifiers. And I avoid if/else in almost all scenarios.
For the first situation, most `if` branches fall into two categories: guard statements, or genuine branching logic. For the former, guard statement style greatly improves clarity and you only need the statement modifier form. For the latter, I find in almost all cases you either should be writing two methods (one for the true case, one for the false case), or the inner logic of the `if` should simply be its own method.
For `if`/`else`, I avoid them at all costs. If there's exactly two cases, odds are I probably want to separate methods (and can write a short method that chooses between the two if necessary). In almost all other cases, I'd infinitely rather prefer a `case` statement that can use tabular alignment for clarity.
Also, the longer I write code the more I find myself avoiding anything but single-line if/unless statement modifiers. And I avoid if/else in almost all scenarios.
For the first situation, most `if` branches fall into two categories: guard statements, or genuine branching logic. For the former, guard statement style greatly improves clarity and you only need the statement modifier form. For the latter, I find in almost all cases you either should be writing two methods (one for the true case, one for the false case), or the inner logic of the `if` should simply be its own method.
return nil if thing?
return true unless other_thing?
logic()
special_situation if
another_thing?
logic2()
For `if`/`else`, I avoid them at all costs. If there's exactly two cases, odds are I probably want to separate methods (and can write a short method that chooses between the two if necessary). In almost all other cases, I'd infinitely rather prefer a `case` statement that can use tabular alignment for clarity.
case foo
when bar then baz
when ham then spam
else qux
end
> I'd infinitely rather prefer a `case` statement
Generally, yes.
> that can use tabular alignment for clarity.
I hate you. I mean, not you personally, but please don't. Even when I'm using a monospaced font, that vast gulf of whitespace between the else keyword and the substance of the else clause kills readability, and since I actually now pretty consistently use proportional fonts for programming... (I mean, sure, if its an environment where you now elastic tabstops are available, tabular alignment is fine; if you're faking it with spaces, though...)
Generally, yes.
> that can use tabular alignment for clarity.
I hate you. I mean, not you personally, but please don't. Even when I'm using a monospaced font, that vast gulf of whitespace between the else keyword and the substance of the else clause kills readability, and since I actually now pretty consistently use proportional fonts for programming... (I mean, sure, if its an environment where you now elastic tabstops are available, tabular alignment is fine; if you're faking it with spaces, though...)
Strongly agree.
The point of stating an explicit nil in the article must be to reduce the chance of future changes erroneously leaving something on the stack, without realizing the implicit nil.
The early return in your version makes it much more obvious why it is written the way it is.
The point of stating an explicit nil in the article must be to reduce the chance of future changes erroneously leaving something on the stack, without realizing the implicit nil.
The early return in your version makes it much more obvious why it is written the way it is.
I dunno, I think the correct answer for the specific code is a one-line ternary:
class SanitizingFilter < HTMLFilter
def render_element(name, attrs, content)
safe?(name, attrs) ? super : nil
end
endI don't get what is the big problem. Just turn off the disturbing Cop. It's a linter not an AI-Linter to know what is expected.
I don't even think that this is RuboCop's fault but Ruby's. Because you can write said code in a lot of different ways (like shown in the post).
I don't even think that this is RuboCop's fault but Ruby's. Because you can write said code in a lot of different ways (like shown in the post).
> I don't even think that this is RuboCop's fault but Ruby's. Because you can write said code in a lot of different ways (like shown in the post).
Honestly, the problem is that rubocop was not designed by a community of experts. It started as the opinions of a singular person, one motivated to the point of writing a linter where none existed. As a result, it's heavily opinionated on trivial shit. Yes, you can configure it, but the out-of-box experience is overbearing.
I generally prefer https://relaxed.ruby.style which is ... a lot more chill.
Honestly, the problem is that rubocop was not designed by a community of experts. It started as the opinions of a singular person, one motivated to the point of writing a linter where none existed. As a result, it's heavily opinionated on trivial shit. Yes, you can configure it, but the out-of-box experience is overbearing.
I generally prefer https://relaxed.ruby.style which is ... a lot more chill.
Exactly - you don't even need to turn it off globally, just add the magic comment and then an explanation for why you are being explicit about returning nil. ezpz
The thing that you can write said code in a lot of different ways is not a fault but a feature for which Ruby is loved by many. As can be also seen from the article it lets a programmer to be neatly expressive in various different situations.
Now if you personally don't like this feature you should probably use a language which feature is that you can write a thing only in one way and no other.
Now if you personally don't like this feature you should probably use a language which feature is that you can write a thing only in one way and no other.
I personally like standardrb (https://github.com/testdouble/standard). It is a set of sane defaults that I agree with mostly.
Standard was also complaining a little but omitting the explicit else branch made it happy.
Anyways you can customize RuboCop to your likings, use standardrb that comes with a lot of carefully considered defaults and uses RuboCop under the hood or don't use a linter at all and move on with your life.
Standard was also complaining a little but omitting the explicit else branch made it happy.
Anyways you can customize RuboCop to your likings, use standardrb that comes with a lot of carefully considered defaults and uses RuboCop under the hood or don't use a linter at all and move on with your life.
Of no one will ever agree with every choice made by a linter, whether default or customized. I would rather work in a team that agrees to follow some set of conventions than than one in which everyone does things the way they prefer personally.
For the examples in the post, by all means discuss as a team and agree to exclude them.
For the examples in the post, by all means discuss as a team and agree to exclude them.
> Of no one will ever agree with every choice made by a linter, whether default or customized.
This comment comes with the the smell of the idea that there is a right and wrong way to format code, when in fact with, Ruby at least, there's a lot of nuance that can be added in the way the article describes.
There are legitimate reasons for expressing the same functionality of code in different ways to express intent, something that Rubocop unfortunately isn't able to judge.
> I would rather work in a team that agrees to follow some set of conventions than than one in which everyone does things the way they prefer personally.
These aren't the only two options, there is a middle ground, in which you use cops that solve problems that are unambiguous, such as tabs/spaces for initial indentation, which brings me to the next point:
> For the examples in the post, by all means discuss as a team and agree to exclude them.
My main problem with Rubocop (which is a great tool) is that everything is enabled by default (and this seems to be encouraged) and the tool is overused. To me Rubocop smacks of the classic programmer problem of discovering you can do something, and so applying the solution to all the places the solution can be applied. "Because we can, not because we should".
This comment comes with the the smell of the idea that there is a right and wrong way to format code, when in fact with, Ruby at least, there's a lot of nuance that can be added in the way the article describes.
There are legitimate reasons for expressing the same functionality of code in different ways to express intent, something that Rubocop unfortunately isn't able to judge.
> I would rather work in a team that agrees to follow some set of conventions than than one in which everyone does things the way they prefer personally.
These aren't the only two options, there is a middle ground, in which you use cops that solve problems that are unambiguous, such as tabs/spaces for initial indentation, which brings me to the next point:
> For the examples in the post, by all means discuss as a team and agree to exclude them.
My main problem with Rubocop (which is a great tool) is that everything is enabled by default (and this seems to be encouraged) and the tool is overused. To me Rubocop smacks of the classic programmer problem of discovering you can do something, and so applying the solution to all the places the solution can be applied. "Because we can, not because we should".
> , whether default or customized.
This comment comes with the the smell of the idea that there is a right and wrong way to format code, when in fact with, Ruby at least, there's a lot of nuance that can be added in the way the article describes.
I'm sorry you smelled that; I don't believe there is one way to do format code, but the opposite. I am saying that working in teams it's convenient to work to the team's style rather than your own particular likes and dislikes. Mainly avoid getting into code review Jenga about style preferences.
I'm sorry you smelled that; I don't believe there is one way to do format code, but the opposite. I am saying that working in teams it's convenient to work to the team's style rather than your own particular likes and dislikes. Mainly avoid getting into code review Jenga about style preferences.
I believe there are two problems at play here.
1. Inconsistency causing maintainability/readability problems.
2. The desire to avoid discussions about style within a team.
If you're talking about 1. here then what you're talking about is consistency as a tool that can be used to further the goal of maintainability/readability, but as with all tools it can be applied too broadly to problems it doesn't suit, which I would argue includes the subtlety and nuance around the intent of some code rather than the function, and so to force consistency there, and to nullify intent, harms the persuit of the end goal.
Point 2. is in my experience so rare that I'm tempted to assume it's just a post-rationalisation used to justify the pleasing sensation programmers get from making things the same. However, assuming it does exist, just in companies I've not experienced, this isn't really a goal that in my opinion is best served by a linter, once you've got to the point that you've automatically fixed all the issues that are obvious (such as lead spaces, syntax errors, security issues etc).
Everything you're left with after this point is a programmer either expressing intent, or making an unsound style choice, and this is best solved in code review. Expressing intent is valuable, especially in a language like Ruby where there is room for nuance.
If you've got two developers fighting over preferred style, bring in a senior developer to educate about this and perhaps even bring it up in the employee review. If you've got two senior developers who disagree about the style of a piece of code, then they should be expressing their opinions in a non-argumentative and suggestive style:
Senior a> "I would prefer to see this code written like this, but this is a personal preference"
Senior b> "I wrote it like this to show x intent" or "I have amended the code as you suggested"
1. Inconsistency causing maintainability/readability problems.
2. The desire to avoid discussions about style within a team.
If you're talking about 1. here then what you're talking about is consistency as a tool that can be used to further the goal of maintainability/readability, but as with all tools it can be applied too broadly to problems it doesn't suit, which I would argue includes the subtlety and nuance around the intent of some code rather than the function, and so to force consistency there, and to nullify intent, harms the persuit of the end goal.
Point 2. is in my experience so rare that I'm tempted to assume it's just a post-rationalisation used to justify the pleasing sensation programmers get from making things the same. However, assuming it does exist, just in companies I've not experienced, this isn't really a goal that in my opinion is best served by a linter, once you've got to the point that you've automatically fixed all the issues that are obvious (such as lead spaces, syntax errors, security issues etc).
Everything you're left with after this point is a programmer either expressing intent, or making an unsound style choice, and this is best solved in code review. Expressing intent is valuable, especially in a language like Ruby where there is room for nuance.
If you've got two developers fighting over preferred style, bring in a senior developer to educate about this and perhaps even bring it up in the employee review. If you've got two senior developers who disagree about the style of a piece of code, then they should be expressing their opinions in a non-argumentative and suggestive style:
Senior a> "I would prefer to see this code written like this, but this is a personal preference"
Senior b> "I wrote it like this to show x intent" or "I have amended the code as you suggested"
> Mainly avoid getting into code review Jenga about style preferences.
What about a world where style concerns aren't part of a code review? Style debates provide the superficial appearance of a code review while offering no useful review of security, architecture and intent.
I like a super basic stripped down rubocop, and instead of a focus on "style", a more considered focus on readability.
What about a world where style concerns aren't part of a code review? Style debates provide the superficial appearance of a code review while offering no useful review of security, architecture and intent.
I like a super basic stripped down rubocop, and instead of a focus on "style", a more considered focus on readability.
Honestly half the reason I argue for rubocop is to avoid having readability discussions with other devs when I don't have strong opinions. I've discovered that I am nonplussed by patterns that leave other devs downright emotional and it's just not worth the effort to discuss, especially when 99% of readability issues (eg naming) are absolutely undetectable to rubocop. Having a consistent style allows splitting styling and, erm, more fruitful readability discussions.
Anyway remember you're writing code, not poetry. The inclination to act as if you're writing the latter without any kind of cost/benefit analysis is a major reason I dislike working with ruby devs. If only I were actually writing for powerpoint slides rather than for concrete business ends!
Anyway remember you're writing code, not poetry. The inclination to act as if you're writing the latter without any kind of cost/benefit analysis is a major reason I dislike working with ruby devs. If only I were actually writing for powerpoint slides rather than for concrete business ends!
> I've discovered that I am nonplussed by patterns that leave other devs downright emotional and it's just not worth the effort to discuss
One thing that annoys me (and I'm not saying that you're doing it here) is that the people who say it's not worth arguing about are usually the ones doing the arguing. Like, if it makes no tangible difference either way, then just let people do whatever. It literally doesn't matter. But that's not acceptable to the "it's not worth discussing" people, and suddenly it becomes very important to discuss consistency. And not just any old consistency. We can't just flip a coin. No, it has to be consistent with their favourites, which are correct and therefore not worth discussing.
My personal standard is that if somebody suggests a rule and nobody really opposes it then great, enable it. If there is opposition, then it stays disabled until one side can persuade the other. If they truly care about consistency then they are welcome to change sides at any time. Attempts to browbeat the opposition should result in some private feedback from their line manager. There are exceptions for things that matter, but a lot of this stuff just doesn't.
One thing that annoys me (and I'm not saying that you're doing it here) is that the people who say it's not worth arguing about are usually the ones doing the arguing. Like, if it makes no tangible difference either way, then just let people do whatever. It literally doesn't matter. But that's not acceptable to the "it's not worth discussing" people, and suddenly it becomes very important to discuss consistency. And not just any old consistency. We can't just flip a coin. No, it has to be consistent with their favourites, which are correct and therefore not worth discussing.
My personal standard is that if somebody suggests a rule and nobody really opposes it then great, enable it. If there is opposition, then it stays disabled until one side can persuade the other. If they truly care about consistency then they are welcome to change sides at any time. Attempts to browbeat the opposition should result in some private feedback from their line manager. There are exceptions for things that matter, but a lot of this stuff just doesn't.
Good defaults are important. It is more difficult to find an agreement over changing a default than configuring a missing configuration option.
Last time I raised a formatting change with our team, I got replies like "the default settings have been agreed with the community"(a blatant lie) or "I wasted enough time in my life discussing formatting" (okay, this guy has to write C# mostly, the standard formatting is already a lost cause there anyway).
Last time I raised a formatting change with our team, I got replies like "the default settings have been agreed with the community"(a blatant lie) or "I wasted enough time in my life discussing formatting" (okay, this guy has to write C# mostly, the standard formatting is already a lost cause there anyway).
Agree that can be an issue. Often it's not worth the energy doing that. But without the linter there's a good chance you'll end up discussing the formatting in code review which doesn't really benefit anyone.
Every time Yet Another RuboCop Storm arrives, I wish that Bozhidar will eventually drop the whole thing and focus on other areas, where his work is very much appreciated by everyone — his Emacs work is unparalleled, and his work on CIDER (Clojure development environment) and nrepl is universally loved. These communities deeply appreciate the work and instead of recurring drama, provide sponsorship through OpenCollective (for CIDER) and directly to Bozhidar through Github Sponsors.
While certain RuboCop rules do cause drama, you'd be hard pressed to find anybody who doesn't appreciate the existence of Rubocop. It is absolutely ridiculous to state that discussions around specific cops and their applications are equivalent to any sort of hatred for Rubocop's creator.
I do remember some of his tweets where he seemed to be rather discouraged by the drama.
For me, guard clauses look more readable, especially that they can be easily added or eliminated, they separate main logic from edge cases, and they reduce nested clauses a lot. Stomping a foot about "branching must look like branching at all cost" reeks more of a religion than anything else.
Or maybe, if the author has a custom style, they should make their own linter rules.
Or maybe, just maybe, be hardcore and don't use linters at all. If you treat your code style as a form of art unto itself (which is not a bad thing at all IMO, depending on what you create), linters can be detrimental — they sure strip your code of a lot of its character.
Or maybe, if the author has a custom style, they should make their own linter rules.
Or maybe, just maybe, be hardcore and don't use linters at all. If you treat your code style as a form of art unto itself (which is not a bad thing at all IMO, depending on what you create), linters can be detrimental — they sure strip your code of a lot of its character.
I don't think it's a case of guard clauses being more or less readable in and of themselves. The point the authour is trying to make is that guard clauses convey a different intent from a standard branch. It's a way of communicating "this method is not relevant / necessary (or sometimes invalid if you're raising) for this set of inputs". If what the guard clause is doing is the "point" of your method, you're losing that intent from someone reading it, and conveying something else.
I don't think it's Rubocop's fault in this case - there's really no way that we can expect a linter to glean "what is this method really about". But structural issues like this probably don't make sense to use a linter for IMO.
I don't think it's Rubocop's fault in this case - there's really no way that we can expect a linter to glean "what is this method really about". But structural issues like this probably don't make sense to use a linter for IMO.
In his case, the guard clause is completely appropriate, because "super" is "the rest of the method" the guard clause is protecting. It's not a filter method, it's a selective render method.
Sure, I can see that argument. But the larger point is that that can't be universally applied to every case that Rubocop flags.
Then the author should have made their case with a more suitable example. I bet they could have found cases with something that's absolutely not a guard clause in a disguise getting converted into a guard clause proper.
Unless they couldn't, and the linter does a good job. ;-)
Unless they couldn't, and the linter does a good job. ;-)
I literally hit two today. Off the top of my head:
If you can't think of examples, then you haven't been doing much thinking.
def enhance_payload(payload)
payload[:h] = thing_h
if situation_one?
payload[:a] = thing_a
payload[:b] = thing_b
end
if situation_two?
payload[:x] = thing_x
payload[:y] = thing_y
end
end
def enhance_payload(payload)
payload[:h] = thing_h
if situation_one?
payload[:a] = thing_a
payload[:b] = thing_b
end
return unless situation_two?
payload[:x] = thing_x
payload[:y] = thing_y
end
If you can't think of examples, then you haven't been doing much thinking.
> The point the authour is trying to make is that guard clauses convey a different intent from a standard branch.
This is a matter of idiom and convention, and I disagree with the author on the specific point in Ruby. Were it Python, I would agree with it. (Well, not on "guard clauses" per se, but the significance of an implicit vs. explicit nil/None return.)
It is certainly an issue for polyglot programmers that as well as differences in syntax and behaviorally-visible semantics, language communities have different idioms that impact the communicative semantics of code (having been deeper into Ruby before getting more into Python, it took my a while to stop writing Python with Ruby conventions.)
This is a matter of idiom and convention, and I disagree with the author on the specific point in Ruby. Were it Python, I would agree with it. (Well, not on "guard clauses" per se, but the significance of an implicit vs. explicit nil/None return.)
It is certainly an issue for polyglot programmers that as well as differences in syntax and behaviorally-visible semantics, language communities have different idioms that impact the communicative semantics of code (having been deeper into Ruby before getting more into Python, it took my a while to stop writing Python with Ruby conventions.)
> The explicit nil in the original implementation implies that the method is being called for its return value
This kind of insight into the dev mind/process is interesting b/c it shows how code is actually consumed. It also reveals where the implicit logic exists, so we can consider if it should be more explicit - in this case if functions should declare return types.
This kind of insight into the dev mind/process is interesting b/c it shows how code is actually consumed. It also reveals where the implicit logic exists, so we can consider if it should be more explicit - in this case if functions should declare return types.
I completely disagree. I'm on two separate projects right now. One is a legacy mess of unnecessary if statements where none are required (in Django, but that's not the issue) and the other is one is a Rails project I took over from a reasonable developer.
Rubocop is awesome. The -A flag does the right thing and the lint fixes really do make code easier to read. Even the examples in this post make me even more convinced of Rubocop's awesomeness.
Rubocop is awesome. The -A flag does the right thing and the lint fixes really do make code easier to read. Even the examples in this post make me even more convinced of Rubocop's awesomeness.
super if safe?(name, attrs)
I worked at a place with RuboCop as part of the CI, it was a horrible experience. I spent more time fixing lint errors than actual work, and all it does is make my code less readable.
Likewise.
In our case, we were permitted to disable Rubocop rules at-will via inline comments, which made it slightly bearable, but still onerous considering how slow our CI was (45+ minutes in many cases for a test run)
My preferred method is:
1. As a matter of productivity, all coders are expected to have Rubocop enabled in their editor of choice, for the quickest possible feedback loop.
2. Use HoundCI or equivalent to highlight Rubocop complaints in Github's pull request conversation. Then they may become a matter of discussion. Obviously, this only works if the team is mature enough to avoid bikeshedding.
3. Rubocop failures do not, however, cause a hard failure during CI.
4. Haven't set this up yet, but ideally I'd configure things so that perhaps some egregious Rubocop failures (ie, tabs vs. spaces, SQL injection issues, etc) would cause CI failures. Whereas more subjective style issues would simply be flagged for developer attention/discussion.
5. Pull requests to modify the project's .rubocop.yml are always encouraged. Again, culture/maturity come into play here. I've been on teams that bikeshed this sort of thing to death. My current team does not.
In our case, we were permitted to disable Rubocop rules at-will via inline comments, which made it slightly bearable, but still onerous considering how slow our CI was (45+ minutes in many cases for a test run)
My preferred method is:
1. As a matter of productivity, all coders are expected to have Rubocop enabled in their editor of choice, for the quickest possible feedback loop.
2. Use HoundCI or equivalent to highlight Rubocop complaints in Github's pull request conversation. Then they may become a matter of discussion. Obviously, this only works if the team is mature enough to avoid bikeshedding.
3. Rubocop failures do not, however, cause a hard failure during CI.
4. Haven't set this up yet, but ideally I'd configure things so that perhaps some egregious Rubocop failures (ie, tabs vs. spaces, SQL injection issues, etc) would cause CI failures. Whereas more subjective style issues would simply be flagged for developer attention/discussion.
5. Pull requests to modify the project's .rubocop.yml are always encouraged. Again, culture/maturity come into play here. I've been on teams that bikeshed this sort of thing to death. My current team does not.
Is there not an autoformatter available? We use linting as part of our Python workflow and it works pretty well along with black. There's really not much linting work: just write your code, run black, and it's good to go.
Rubocop can autoformat the code it complains about.
Not all cops have autoformatters, though, and some formatters can cause breaking changes.
Yes, RuboCop would be awful if you're only running it as part of the CI build.
I've been using RuboCop very heavily for the last few years. I've found it very useful as a solo developer, but it also makes it much easier to work with other developers since we never have to waste time talking about style issues.
Here's all the ways I've integrated RuboCop and made it an amazing experience:
I use VS Code with the ruby-rubocop extension, and I've enabled the "Format On Save" option. (This uses Prettier for most other file formats.) This would be unbearably slow with vanilla RuboCop, but I get a huge speed boost with rubocop-daemon. (Especially with the bash wrapper script that I wrote [2].) So now every time I save a Ruby file, the file is auto-formatted instantly to correct any warnings. In case it can't automatically fix a warning, I'll still the warning right in my editor, so I can immediately fix it before moving on.
The second step is a git hook script in `.git/hooks/commit-msg`. I set this up to run RuboCop for Ruby, plus prettier and eslint for JavaScript. If RuboCop fails with a warning, the script retries with `rubocop -A` to automatically correct any errors. Then I run `git diff` to make sure everything looks good before committing the changes.
The final step is to run `rubocop` as part of my CI build, to make sure I didn't miss any warnings (or for any developers who haven't set this up on their machines.) This almost always passes, because the previous two steps usually catch everything first.
I disagree with a lot of the default cops, so I just disable lots of them. But I'm pretty happy with my current setup and RuboCop configuration.
I totally agree that linters would be awful when you have a very slow feedback loop, and you have to wait anywhere from 10 - 60 minutes before you get a notification for a failed build. But they can be really pleasant experience when the feedback loops are under 100ms and most of the warnings are automatically fixed for you. I now lean on it really heavily, and I've even started to take some shortcuts and save keystrokes, because I know how the auto-formatter will tidy up the code.
[1] https://github.com/fohte/rubocop-daemon
[2] https://github.com/fohte/rubocop-daemon#more-speed
I've been using RuboCop very heavily for the last few years. I've found it very useful as a solo developer, but it also makes it much easier to work with other developers since we never have to waste time talking about style issues.
Here's all the ways I've integrated RuboCop and made it an amazing experience:
I use VS Code with the ruby-rubocop extension, and I've enabled the "Format On Save" option. (This uses Prettier for most other file formats.) This would be unbearably slow with vanilla RuboCop, but I get a huge speed boost with rubocop-daemon. (Especially with the bash wrapper script that I wrote [2].) So now every time I save a Ruby file, the file is auto-formatted instantly to correct any warnings. In case it can't automatically fix a warning, I'll still the warning right in my editor, so I can immediately fix it before moving on.
The second step is a git hook script in `.git/hooks/commit-msg`. I set this up to run RuboCop for Ruby, plus prettier and eslint for JavaScript. If RuboCop fails with a warning, the script retries with `rubocop -A` to automatically correct any errors. Then I run `git diff` to make sure everything looks good before committing the changes.
The final step is to run `rubocop` as part of my CI build, to make sure I didn't miss any warnings (or for any developers who haven't set this up on their machines.) This almost always passes, because the previous two steps usually catch everything first.
I disagree with a lot of the default cops, so I just disable lots of them. But I'm pretty happy with my current setup and RuboCop configuration.
I totally agree that linters would be awful when you have a very slow feedback loop, and you have to wait anywhere from 10 - 60 minutes before you get a notification for a failed build. But they can be really pleasant experience when the feedback loops are under 100ms and most of the warnings are automatically fixed for you. I now lean on it really heavily, and I've even started to take some shortcuts and save keystrokes, because I know how the auto-formatter will tidy up the code.
[1] https://github.com/fohte/rubocop-daemon
[2] https://github.com/fohte/rubocop-daemon#more-speed
But a formatter can only format auto-fixable code right? I meant lints like high cyclomatic complexity, which requires me to extract out functions pointlessly.
I was an intern at that time so didn't make a fuss about changing the config.
I was an intern at that time so didn't make a fuss about changing the config.
Ah yes, that's right. I actually turn off all of the cyclomatic complexity rules or set very high limits because I find those to be really annoying. So I mainly use RuboCop as a code formatter, plus some minor linting and style rules that aren't too obtrusive. I actually disagree with the parent article, and I've grown to really appreciate guard clauses. I find them much easier to read now.
VS Code will still highlight those cyclomatic complexity errors in the editor, so at least you will be able to fix them (or turn them off) without needing to wait for a CI build to finish.
VS Code will still highlight those cyclomatic complexity errors in the editor, so at least you will be able to fix them (or turn them off) without needing to wait for a CI build to finish.
If your team as a whole agrees it's less readable you can disable these specific rules.
All the project I've seen had specific adjustments to blanket disable some rules (on the top of my head, ` > 0 -> positive?` for instance) and change de defaults of others (e.g. line length)
The goal is tailor it to your needs, unlike other linters who have a much more oppiniated and rigid approach on what is supposed to be "right"
All the project I've seen had specific adjustments to blanket disable some rules (on the top of my head, ` > 0 -> positive?` for instance) and change de defaults of others (e.g. line length)
The goal is tailor it to your needs, unlike other linters who have a much more oppiniated and rigid approach on what is supposed to be "right"
I found it annoying until I added a git hook and got used to auto correcting. Its low friction for me now, and well worth it imo.
If you run into a very large number of linting errors then you might naturally write code in an unidiomatic style, which unfortunately takes a while to adjust to.
If you run into a very large number of linting errors then you might naturally write code in an unidiomatic style, which unfortunately takes a while to adjust to.
One pass of rubocop with autofocus, commit, push
Next time just have autocorrect on Save done
Definitely just turn off the cop if you don't like it.
But let's nitpick a bit. I find it interesting that the arguments made in the article are about the intention of the programmer.
Perhaps an explicit nil does signal to me that the nil is intentional. But does that mean that all implicit nils are unimportant? For a skilled programmer like the OP, it probably does mean that! But how often do I actually know who wrote the code I'm reading? Of course I can git blame, but maybe the author left the org before I joined. What kind of programmer were they? Are implicit nils important or unimportant to them? Perhaps it's easy to argue that they should be unimportant. But in reality, I don't know what the author of a particular method was thinking while they were writing it. In fact even an explicit nil might be unimportant. Maybe somebody replaced a `raise SomethingError` line with `nil` to hot-fix an urgent bug.
I guess from my perspective as a long-time Ruby developer, all nils are important. It doesn't matter what the intention of the programmer was. Ruby is very expressive and very dynamic - anything can return nil and there are a hundred ways to do it. If your nil is intentional, write a test case for it. If you have no test suite in a Ruby project, then may God have mercy on your soul.
But let's nitpick a bit. I find it interesting that the arguments made in the article are about the intention of the programmer.
Perhaps an explicit nil does signal to me that the nil is intentional. But does that mean that all implicit nils are unimportant? For a skilled programmer like the OP, it probably does mean that! But how often do I actually know who wrote the code I'm reading? Of course I can git blame, but maybe the author left the org before I joined. What kind of programmer were they? Are implicit nils important or unimportant to them? Perhaps it's easy to argue that they should be unimportant. But in reality, I don't know what the author of a particular method was thinking while they were writing it. In fact even an explicit nil might be unimportant. Maybe somebody replaced a `raise SomethingError` line with `nil` to hot-fix an urgent bug.
I guess from my perspective as a long-time Ruby developer, all nils are important. It doesn't matter what the intention of the programmer was. Ruby is very expressive and very dynamic - anything can return nil and there are a hundred ways to do it. If your nil is intentional, write a test case for it. If you have no test suite in a Ruby project, then may God have mercy on your soul.
That's something to keep in mind, but it's not the whole picture. Just because it's not guaranteed to be true doesn't mean that it's unpredictable. Like, you can't guarantee that the file for a Rails controller exists in `app/controllers/`, but that doesn't mean that they are randomly scattered across every directory of the project. It also doesn't mean that every developer puts them in a different folder depending on their personality. Sometimes you need to read carefully to determine the exact behaviour, but you only need rules of thumb to quickly navigate around a project.
rubocop config in this scenario is objectively wrong in my opinion -- a better config would be to flag "foo if bar?" as linter violation and just never use it. (I would also forbid "unless" statements -- why the hell does anyone need unless??)
The only reasoning I need to support my opinion -- Ruby debuggers are not good enough to support mid line breakpoints -- so if you want to break only when the condition is true -- which is a very common thing if you are maintaining or debugging code you didn't write yourself -- you literally can't without changing the code or using a conditional breakpoint and duplicating the conditional inside the breakpoint definition -- this is very annoying when tracing unfamiliar code and trying to figure out what invariants are relevant to a given scenario (or missing).
In my opinion - linters need an actual concept of a user that they are trying to help -- and ideally that concept should lead to objective opinions. Linters like rubocop that are just piles of functionality tend to be breeding grounds for time wasting arguments between insufficiently experienced developers over opinions that aren't supportable from any usefully pragmatic basis.
The only reasoning I need to support my opinion -- Ruby debuggers are not good enough to support mid line breakpoints -- so if you want to break only when the condition is true -- which is a very common thing if you are maintaining or debugging code you didn't write yourself -- you literally can't without changing the code or using a conditional breakpoint and duplicating the conditional inside the breakpoint definition -- this is very annoying when tracing unfamiliar code and trying to figure out what invariants are relevant to a given scenario (or missing).
In my opinion - linters need an actual concept of a user that they are trying to help -- and ideally that concept should lead to objective opinions. Linters like rubocop that are just piles of functionality tend to be breeding grounds for time wasting arguments between insufficiently experienced developers over opinions that aren't supportable from any usefully pragmatic basis.
> The explicit nil in the original implementation implies that the method is being called for its return value — that it’s probably functional, with inconsequential side effects.
> Making the nil implicit says that the return value is not important. This method now reads like it’s implemented by avoiding side effects within super.
That's...not my understanding of idiomatic ruby conventions. The idea that a method is assumed to have no or inconsequential side effects and be called only for its return value with an explicit return, but to be called for side effects with an implicit one, is a Python idiom, not a Ruby idiom. As I understand Ruby idiom, it is basically "a method defined with `def` is assumed to have both significant side effects and a significant return value unless documented otherwise on either point, except that a method with a name ending in `?` is assumed to return a value meaningful in a boolean context and have no or inconsequential side effects." Concise function bodies with implicit nil returns are normal, including for functions called for their return value.
> Making the nil implicit says that the return value is not important. This method now reads like it’s implemented by avoiding side effects within super.
That's...not my understanding of idiomatic ruby conventions. The idea that a method is assumed to have no or inconsequential side effects and be called only for its return value with an explicit return, but to be called for side effects with an implicit one, is a Python idiom, not a Ruby idiom. As I understand Ruby idiom, it is basically "a method defined with `def` is assumed to have both significant side effects and a significant return value unless documented otherwise on either point, except that a method with a name ending in `?` is assumed to return a value meaningful in a boolean context and have no or inconsequential side effects." Concise function bodies with implicit nil returns are normal, including for functions called for their return value.
@dang: I think we have another user with some reasonable looking comments who's been incorrectly shadowbanned: https://news.ycombinator.com/threads?id=Technically
The solution here would be to disable the cop with a magic comment, write an explanation accompanying the comment and move on.
If you need an exception to Rubocop, that's available to you.
If you need an exception to Rubocop, that's available to you.
I've read a million times that the difference between Python and Ruby is that in Python there is one right way to do things, and in Ruby there are many ways to express yourself. But then in the real world, most companies betting on Ruby add RubyCop.
I don't think the spirit of that maxim really applies at the same level as linters – after all, Python has linters too.
[deleted]
It seems that a lot of people think that one of the main goals for code linting is consistency, when that shouldn't be a goal at all. Consistency is merely a tool, the goal should be readability/maintainability. Consistency is just one tool that is available to us in persuit of that goal.
I've hit similar issues to those mentioned in the article and it absolutely infuriates me when I have to jump through hoops (or worse, reduce the readability of my code) to push well thought out code that communicates intent, only to fall foul of a robot that doesn't understand nuance.
Don't get me wrong, I think Rubocop is a great tool, it's just over-used, and that's in part because everything is on by default, and this seems to be encouraged by the general community. It seems that once programmers have found a solution to something they love to apply that solution to all places that it will work, regardless of the efficacy.
My preference, and I suppose suggestion, is to let Rubocop handle things it knows about with a high degree of accuracy, such as tabs/spaces for lead indentation, security issues etc. and leave anything that requires nuance or subtlety to the programmer who wrote it, and the educational opportunity of the code review.
After all, "Consistency is the last refuge of the unimaginative" :)