Add option to only enforce map/block correction of Perfomance/Sum

[dgollahon]

Is your feature request related to a problem? Please describe.

I have found the Performance/Sum cop useful in several cases but the false positive rate is too high for me because there are various cases in my codebase(s) that use reduce(:+) on objects where sum is not applicable. In cases where I'm already using #sum, however, there were several opportunities to use the block form which is simpler and faster. I would like to be able to have these detected without enabling the entire cop.

Describe the solution you'd like

An option that just enforces using the block form of #sum when it can be detected:

# bad
[1, 2, 3].map { |elem| elem ** 2 }.sum
[1, 2, 3].collect(&:count).sum(10)

# good
[1, 2, 3].sum { |elem| elem ** 2 }
[1, 2, 3].sum(10, &:count)

Describe alternatives you've considered

This may be too niche of an option but I think it would be helpful since I will likely disable this cop otherwise because the false positive rate for me is too high. There are also a couple of cases where I might be able to use the init parameter to #sum but there are cases I have where I do not prefer writing the code that way as it is not necessary or helpful AFAICT.

Additional context

N/A

[leoarnold]

that use reduce(:+) on objects where sum is not applicable.

@dgollahon Could you please give a code example for this.

[dgollahon]

There are three general cases i've run across:

• Concatenating arrays
• Merging sets
• Custom classes where I have defined +

Here is a quick example:

[[1, 2].to_set, [1,3].to_set].reduce(:+) # This gets flagged by Performance/Sum # => #<Set: {1, 2, 3}>
# But I can't do...
[[1, 2].to_set, [1,3].to_set].sum # => TypeError: Set can't be coerced into Integer

I could do:

[[1, 2].to_set, [1,3].to_set].sum(Set.new)

of course, but I don't find that to be more readable and I would expect it to be less performant (though I have not benchmarked it).

Essentially, I only care about the "obvious" cases where we can have a higher confidence that it actually makes sense to use sum so I don't have as much rubocop:disable noise from these. It's not a huge deal, I just prefer lower false positive rates to more potential cases of this being relevant.

[leoarnold]

It turns out that the documentation of said cop actually discusses this case:

rubocop-performance/lib/rubocop/cop/performance/sum.rb

Lines 11 to 28 in b59c260

	# Its auto-correction is marked as safe by default (`SafeAutoCorrect: true`)
	# to prevent `TypeError` in auto-correced code when initial value is not
	# specified as shown below:
	#
	# [source,ruby]
	# ----
	# ['a', 'b'].sum # => (String can't be coerced into Integer)
	# ----
	#
	# Therefore if initial value is not specified, unsafe auto-corrected will not occur.
	#
	# If you always want to enable auto-correction, you can set `SafeAutoCorrect: false`.
	#
	# [source,yaml]
	# ----
	# Performance/Sum:
	# SafeAutoCorrect: false
	# ----

So, the intention of the author is to set SafeAutoCorrect: false (which currently seems broken, but that's a different issue). But then again, this also would not solve the issue at hand, as Rubocop would not correct, but still complain.

Nevertheless, I have a different solution to offer: The issues arise because the method :+ is used in a non-arithmetic way:

• In your case, you are forming the union of two sets.
• In the case of the documentation, we are concatenating strings.

Therefore, the Ruby way ™️ to solve this is "more telling aliases":

# offenses
['a', 'b'].reduce(:+)
[[1, 2].to_set, [1,3].to_set].reduce(:+)

# no offense
['a', 'b'].reduce(:concat)
[[1, 2].to_set, [1,3].to_set].reduce(:union)

[dgollahon]

I did notice that documentation today, although I don't recall it discussing it when I opened this issue. I wonder if it was there then too?

Sure, that's definitely an option and I think those example cases are a bit more readable--that said it still feels wrong/noisy for this cop to be complaining and I would personally prefer it to be more conservative.

I also still have a few custom classes where + does feel like the most readable/idiomatic way to express the operation, but I still get complaints from the cop. I don't feel like + should be exclusively arithmetic or is truly more the Ruby Way, so to speak.

That said, this is not a huge issue, it's just an annoyance. I think that it should be viewed as a false positive regardless of whether or not there is a neater way to express what I want to do though.

[leoarnold]

Well, I think you do have a point there. To sum it up:

When #reduce or #inject is used with :+, but without an initial value, we cannot be sure (from syntax alone) that #sum is an equivalent. This is because #sum assumes an initial value of zero (Integer) when no initial value is provided, whereas #reduce or #inject do not make assumptions:

> [].reduce(:+)
# => nil
> ['a'].reduce(:+)
# => "a"

> [].sum
# => 0
> ['a'].sum
# => in `+': String can't be coerced into Integer (TypeError)

And we found:

• Bug: This unsafe correction is performed in safe-autocorrect mode. The documentation talks about setting SafeAutoCorrect in your RuboCop config, but this does not seem to be supported by the cop.
• Feature Suggestion: Add an option to Performance/Sum to ignore (i.e. not register as offense) when #reduce / #inject is used without initial value. This option should be disabled by default.

[dgollahon]

Yup, good summary!

[leoarnold]

@dgollahon Documentation will be extended in #270. Option OnlySumOrWithInitialValue will ship with v1.13 (See #271)