Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maybe Style/StringConcatenation should be conservative by default #11757

Open
jaynetics opened this issue Apr 4, 2023 · 6 comments
Open

Maybe Style/StringConcatenation should be conservative by default #11757

jaynetics opened this issue Apr 4, 2023 · 6 comments

Comments

@jaynetics
Copy link
Contributor

jaynetics commented Apr 4, 2023

as per https://docs.rubocop.org/rubocop/cops_style.html#stylestringconcatenation, aggressive mode is currently the default.

this is not ideal because it triggers false positives and people have to research it and change the mode.

  • we hit a case with URI#+, which returns a URI and prevents double slashes
    • e.g. uri + '/some_path' always returns a valid URI
  • Pathname#+ behaves similarly and thus yields similar false positives
  • custom implementations of #+ are also affected
@koic
Copy link
Member

koic commented Apr 4, 2023

This configuration was introduced at #9153 with aggressive as the default. There are #9144 arguments, but I think I can accept this proposal to default to conservative. This is because I prefer to be as safe as possible, but others may think differently, like some of #9144.
@rubocop/rubocop-core What do you think about it?

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 4, 2023

I still stand by my original opinion. That cop is next to useless with the conservative config. Likely we'll just have it disabled by default in RuboCop 2.0, as one of the main goals is to streamline the default configuration (profile) and have be more conservative in general.

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 4, 2023

See also #10726

@noraj
Copy link

noraj commented Feb 26, 2024

rubocop -A broke my code when it's supposed to be safe.

A minimal example of how this breaks filepath:

# frozen_string_literal: true

require 'pathname'

FILENAME = 'mime.types'

pn = Pathname.new('/etc')
fp = pn + 'nginx' + FILENAME

puts fp # /etc/nginx/mime.types
➜ rubocop -A pathname.rb
Inspecting 1 file
C

Offenses:

pathname.rb:8:6: C: [Corrected] Style/StringConcatenation: Prefer string interpolation to string concatenation.
fp = pn + 'nginx' + FILENAME
     ^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected, 1 offense corrected
# frozen_string_literal: true

require 'pathname'

FILENAME = 'mime.types'

pn = Pathname.new('/etc')
fp = "#{pn}nginx#{FILENAME}"

puts fp # /etcnginxmime.types

@bquorning
Copy link
Contributor

rubocop -A broke my code when it's supposed to be safe.

-A isn’t safe:

❯ rubocop --help | grep \\-A
    -A, --autocorrect-all            Autocorrect offenses (safe and unsafe).

@noraj
Copy link

noraj commented Feb 26, 2024

Right, I should have used -a

image

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

No branches or pull requests

5 participants