Add Layout/LineContinuationSpacing cop #10717

bquorning · 2022-06-16T14:52:46Z

Fixes the 1st issue mentioned in #6420.

Style/LineEndConcatenation recommends that we should use \ instead of + or << to concatenate strings across multiple lines. But I am still seeing wildly inconsistent formatting of these multiline strings. This new Layout/LineContinuationSpacing cop checks that there is consistently one space before a \ at the end of a line. (Can be configured to zero spaces)

# Bad
'this text is too '\
'long'

# Bad
'this text is too '  \
'long'

# Good
'this text is too ' \
'long'

Before submitting the PR make sure the following are checked:

The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
Wrote good commit messages.
Commit message starts with [Fix #issue-number] (if the related issue exists).
Feature branch is up-to-date with master (if not - rebase it).
Squashed related commits together.
Added tests.
Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

bquorning · 2022-06-16T17:36:27Z

lib/rubocop/cop/style/line_continuation_spacing.rb

+          corrector.replace(range, correction)
+        end
+
+        def string_literal_ranges(ast)


~~Inspired~~ Copied from https://github.com/rubocop/rubocop/blob/master/lib/rubocop/cop/layout/indentation_style.rb

bbatsov · 2022-06-17T06:01:03Z

I love this cop as the inconsistent spacing has been bugging me for a while. One small question, though - won't it be better to just make this some number config?

If we stick to the styles I'd suggest naming them "single_space" and "no_space" as "zero" and "one" are kind of weird for style names. :-)

bquorning · 2022-06-17T07:04:35Z

I also thought about the number config, but then thought that I don’t want to write specs for how it works if someone chooses e.g. 20 spaces 😆 I think really only 0 and 1 are valid values.

I personally would hate to use 0, but I have seen it used in the wild and added it up front instead of waiting for the inevitable feature request.

We have a lot of cops with SupportedStyles: [space, no_space], so I wonder if I should adopt that naming instead of the suggested single_space/no_space?

bquorning · 2022-06-17T07:06:00Z

I would actually like feedback on two specific things:

Please help me find a better cop name than LineContinuationSpacing.
Does this cop belong in the Style, Lint, or Layout department?

bbatsov · 2022-06-17T07:43:47Z

Please help me find a better cop name than LineContinuationSpacing.

I think the name if fine. I thought we might have just added this to the other cop, but I think in this case it's better to have 2 of them.

Does this cop belong in the Style, Lint, or Layout department?

I think it should be a Layout cop.

When a line is too long, you can make it continue onto the next line by ending the first line with a backslash. This new cop checks how many spaces are in front of that backslash. By default it will allow only exactly one space, but the cop can be configured to instead allow exactly zero spaces.

bbatsov · 2022-06-19T14:04:07Z

Looks good to me!

Follow up rubocop/rubocop#10717 and suppresses the following offenses: ```console Offenses: lib/rubocop/cop/rails/enum_hash.rb:23:80: C: [Corrected] Layout/LineContinuationSpacing: Use one space in front of backslash. MSG = 'Enum defined as an array found in `%<enum>s` enum declaration. '\ ^ lib/rubocop/cop/rails/order_by_id.rb:26:58: C: [Corrected] Layout/LineContinuationSpacing: Use one space in front of backslash. MSG = 'Do not use the `id` column for ordering. '\ ^ spec/rubocop/cop/rails/schema_comment_spec.rb:39:77: C: [Corrected] Layout/LineContinuationSpacing: Use one space in front of backslash. it 'does not register an offense when `add_column` has `comment` option'\ ^ spec/rubocop/cop/rails/schema_comment_spec.rb:108:76: C: [Corrected] Layout/LineContinuationSpacing: Use one space in front of backslash. it 'does not register an offense when `t.integer` has `comment` option'\ ^ ```

Follow up rubocop/rubocop#10717, rubocop/rubocop#10715, and suppresses the following offenses: ```console Offenses: lib/rubocop/cop/performance/chain_array_allocation.rb:50:69: C: [Correctable] Layout/LineContinuationSpacing: Use one space in front of backslash. MSG = 'Use unchained `%<method>s` and `%<second_method>s!` '\ ^ lib/rubocop/cop/performance/chain_array_allocation.rb:51:78: C: [Correctable] Layout/LineContinuationSpacing: Use one space in front of backslash. '(followed by `return array` if required) instead of chaining '\ ^ lib/rubocop/cop/performance/collection_literal_in_loop.rb:35:70: C: [Correctable] Layout/LineContinuationSpacing: Use one space in front of backslash. MSG = 'Avoid immutable %<literal_class>s literals in loops. '\ ^ lib/rubocop/cop/performance/inefficient_hash_search.rb:61:56: C: [Correctable] Layout/LineContinuationSpacing: Use one space in front of backslash. "#{autocorrect_hash_expression(node)}."\ ^ lib/rubocop/cop/performance/inefficient_hash_search.rb:71:59: C: [Correctable] Layout/LineContinuationSpacing: Use one space in front of backslash. "Use `##{autocorrect_method(node)}` instead of "\ ^ spec/rubocop/cop/performance/constant_regexp_spec.rb:61:84: C: [Correctable] Layout/LineContinuationSpacing: Use one space in front of backslash. it 'does not register an offense when regexp contains interpolated constant and '\ ^ spec/rubocop/cop/performance/redundant_match_spec.rb:89:7: C: Layout/LineContinuationLeadingSpace: Move leading spaces to the end of previous line. ' surrounding method' do ^ 115 files inspected, 7 offenses detected, 6 offenses autocorrectable RuboCop failed! ```

ngouy · 2022-06-28T09:36:05Z

Not sure if it should be here or in the issue
I'm posting here because the issue gather 4 differents things

First of all thanks for this. It will help gain clarity in 99% of the cases.

Though: It may be a bad practice, and if it is I guess it is what it is
But maybe it's just a different POV

TLTR: on my end I like to have spaces at the beginning of the lines that continue a previous non-finished sentence:

# This is
#  a multi line
#  comment so I keep 2 spaces until my sentence is finished.
# But this sentence is a new one so I start it with 1 space instead of 2.

"And same" \
" goes for strings." \
"I like to leave an extra" \
" space at the beginning of my line if I'm in the middle" \
" of a sentence."

IDK if we could consider an option for this use-case or again, if it's just me / a bad practice

bquorning · 2022-06-28T09:41:29Z

Hi @ngouy, I think your message was meant for #10715 where Layout/LineContinuationLeadingSpace was added. The two cops are quite similar, yet different :-)

We did talk about configurability in the PR, so please have a look.

ngouy · 2022-06-28T10:15:09Z

Fair, I understand. I get that it would've significantly increase the complexity of the implementation
I can just disable it until further notice, that's fine too. That's also how rubocop works ;)

Thanks again @bquorning (and sorry for wrong thread 😂 , I tried but somehow managed to miss the right one)

bquorning · 2022-06-28T11:45:53Z

@ngouy See #10762 😅

ngouy · 2022-06-28T20:53:14Z

lol

Follow up rubocop/rubocop#10717, rubocop/rubocop#10715, and suppresses the following offenses: ```console Offenses: lib/rubocop/cop/performance/chain_array_allocation.rb:50:69: C: [Correctable] Layout/LineContinuationSpacing: Use one space in front of backslash. MSG = 'Use unchained `%<method>s` and `%<second_method>s!` '\ ^ lib/rubocop/cop/performance/chain_array_allocation.rb:51:78: C: [Correctable] Layout/LineContinuationSpacing: Use one space in front of backslash. '(followed by `return array` if required) instead of chaining '\ ^ lib/rubocop/cop/performance/collection_literal_in_loop.rb:35:70: C: [Correctable] Layout/LineContinuationSpacing: Use one space in front of backslash. MSG = 'Avoid immutable %<literal_class>s literals in loops. '\ ^ lib/rubocop/cop/performance/inefficient_hash_search.rb:61:56: C: [Correctable] Layout/LineContinuationSpacing: Use one space in front of backslash. "#{autocorrect_hash_expression(node)}."\ ^ lib/rubocop/cop/performance/inefficient_hash_search.rb:71:59: C: [Correctable] Layout/LineContinuationSpacing: Use one space in front of backslash. "Use `##{autocorrect_method(node)}` instead of "\ ^ spec/rubocop/cop/performance/constant_regexp_spec.rb:61:84: C: [Correctable] Layout/LineContinuationSpacing: Use one space in front of backslash. it 'does not register an offense when regexp contains interpolated constant and '\ ^ spec/rubocop/cop/performance/redundant_match_spec.rb:89:7: C: Layout/LineContinuationLeadingSpace: Move leading spaces to the end of previous line. ' surrounding method' do ^ 115 files inspected, 7 offenses detected, 6 offenses autocorrectable RuboCop failed! ```

bquorning mentioned this pull request Jun 16, 2022

Improved syntax checking around line continuations #6420

Closed

bquorning force-pushed the add-line-continuation-spacing-cop branch 4 times, most recently from 1e0aa74 to cfc3dd5 Compare June 16, 2022 17:28

bquorning marked this pull request as ready for review June 16, 2022 17:33

bquorning commented Jun 16, 2022

View reviewed changes

bquorning force-pushed the add-line-continuation-spacing-cop branch 2 times, most recently from 3657121 to d3d8413 Compare June 17, 2022 05:52

bquorning force-pushed the add-line-continuation-spacing-cop branch from b9348e2 to 73762d0 Compare June 17, 2022 07:21

bquorning added 2 commits June 17, 2022 09:55

Fix Layout/LineContinuationSpacing offenses

27c4e06

bquorning force-pushed the add-line-continuation-spacing-cop branch from 73762d0 to 27c4e06 Compare June 17, 2022 07:56

bquorning requested a review from bbatsov June 17, 2022 07:56

bquorning changed the title ~~Add LineContinuationSpacing cop~~ Add Layout/LineContinuationSpacing cop Jun 17, 2022

bbatsov approved these changes Jun 19, 2022

View reviewed changes

bbatsov merged commit 736e8d6 into rubocop:master Jun 19, 2022

bquorning deleted the add-line-continuation-spacing-cop branch June 19, 2022 14:08

bquorning mentioned this pull request Jun 28, 2022

Make Layout/LineContinuationLeadingSpace configurable #10762

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Layout/LineContinuationSpacing cop #10717

Add Layout/LineContinuationSpacing cop #10717

bquorning commented Jun 16, 2022 •

edited

bquorning Jun 16, 2022

bbatsov commented Jun 17, 2022

bquorning commented Jun 17, 2022

bquorning commented Jun 17, 2022

bbatsov commented Jun 17, 2022

bbatsov commented Jun 19, 2022

ngouy commented Jun 28, 2022 •

edited

bquorning commented Jun 28, 2022 •

edited

ngouy commented Jun 28, 2022

bquorning commented Jun 28, 2022

ngouy commented Jun 28, 2022

Add Layout/LineContinuationSpacing cop #10717

Add Layout/LineContinuationSpacing cop #10717

Conversation

bquorning commented Jun 16, 2022 • edited

bquorning Jun 16, 2022

Choose a reason for hiding this comment

bbatsov commented Jun 17, 2022

bquorning commented Jun 17, 2022

bquorning commented Jun 17, 2022

bbatsov commented Jun 17, 2022

bbatsov commented Jun 19, 2022

ngouy commented Jun 28, 2022 • edited

bquorning commented Jun 28, 2022 • edited

ngouy commented Jun 28, 2022

bquorning commented Jun 28, 2022

ngouy commented Jun 28, 2022

bquorning commented Jun 16, 2022 •

edited

ngouy commented Jun 28, 2022 •

edited

bquorning commented Jun 28, 2022 •

edited