`Style/StringConcatenation` should be set to `Mode: conservative`

[FLeinzi]

The Cop Style/StringConcatenation leads to errors when the left side is e.g. a Pathname:

file_name = full_path + 'some_file.rb'

will be changed into

file_name = "#{full_path}some_file.rb"

in aggressive mode. In this case full_path was a Pathname which adds a / in concatenation. The "corrected" version has no slash inside, so one has to correct it by hand again.

The conservative mode will allow the concatenation. So I would vote to change it in the gem. https://docs.rubocop.org/rubocop/1.23/cops_style.html#stylestringconcatenation

:+1: Change to "Conservative" :-1: Keep "Aggressive"

[denzelem]

The cop Style/StringConcatenation has no safe autocorrect: https://github.com/makandra/makandra-rubocop/blob/0dc07af4829b9326e2809a39d15ee0a4752cb5f1/config/default.yml#L4629

Generally spoken, every change made with rubocop --auto-correct-all (short -A) has to be checked manually as it is expected to produce false corrections under certain circumstances.

The conservative mode would work for this case, but I think it's not a default we want to have:

# bad
'Hello' + user.name

# good
"Hello #{user.name}"
user.name + '!!'

@FLeinzi Imho changing this cop just because the unsafe autocorrect might produce an incorrect result looks strange. Can't you change your implementation to something like filename = fullpath.join(some_file.rb)? This would also improve the readability of the code.

[FLeinzi]

Imho changing this cop just because the unsafe autocorrect might produce an incorrect result looks strange. Can't you change your implementation to something like filename = fullpath.join(some_file.rb)? This would also improve the readability of the code.

Yes, of course it's possible to adapt the implementation. I just thought that the aggressive mode might be a bit dangerous. I always check the corrections bit by bit manually.

I'll try to make my implementation less ambiguous.

[denzelem]

Thanks @FLeinzi, so we can close this issue or do you want to have more feedback from other people regarding this topic?

[FLeinzi]

No, I think that's enough here.

I will be super attentive with this cop with Pathnames and content_tags.

This is also an offense for that cop and should NEVER EVER be changed via rubocop --auto-correct-all

content_tag(:div) do
  content_tag(:span, 'Some text') + '  ' + 'with some other text'
end