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

Indent for an expression spanning multiple lines #1646

Closed
elyzion opened this issue Feb 10, 2015 · 16 comments
Closed

Indent for an expression spanning multiple lines #1646

elyzion opened this issue Feb 10, 2015 · 16 comments

Comments

@elyzion
Copy link

elyzion commented Feb 10, 2015

Rubocop complains about the indentation on the .new and .permit_method_call lines in the code snippet below. Aligning the calls with the some_instance line resolves the complaints, but in this case it should be calculating the indent from the SomeService line. I am using version 0.29.0.

I also included my .rubocop.yml below. Is this a Rubocop limitation, or is it something I can resolve with my configuration? (I tried the indent and align configuration options to no avail.)

some_instance.member.attributes =
  SomeService
    .new(@class_member)
    .permit_method_call(params[:some_key])
AlignParameters:
  EnforcedStyle: with_fixed_indentation

AsciiComments:
  Enabled: false

ClassLength:
  Max: 500

CyclomaticComplexity:
  Max: 10

LineLength:
  Max: 140

MethodLength:
  Max: 20

StringLiterals:
  EnforcedStyle: double_quotes
@mvz
Copy link
Contributor

mvz commented Mar 31, 2015

If I'm not mistaken, it's not the AlignParameters cop that complains here, but instead MultilineOperationIndentation.

@elyzion
Copy link
Author

elyzion commented Jun 14, 2015

Sorry for the long break in communications. Using the following .rubocop.yml still results in the complaints about indentation.

AbcSize:
  Max: 32

AlignParameters:
  EnforcedStyle: with_fixed_indentation

AsciiComments:
  Enabled: false

ClassLength:
  Max: 500

CyclomaticComplexity:
  Max: 10

LineLength:
  Max: 140

MethodLength:
  Max: 32

MultilineOperationIndentation:
  EnforcedStyle: indented

StringLiterals:
  EnforcedStyle: double_quotes

I included the complaints below for reference, I hope this is just a configuration issue on our side!

app/services/redacted_service.rb:34:9: C: Use 2 (not 4) spaces for indenting an expression spanning multiple lines.
        .new(@class_member)
        ^^^^
app/services/redacted_service.rb:35:9: C: Use 2 (not 4) spaces for indenting an expression spanning multiple lines.
        .permit_method_call(params[:some_key])
        ^^^^^^^^^^^^^^^^^^^

@jonas054
Copy link
Collaborator

No, you can't solve this with configuration, unless you want to disable the cop. Looking at this issue and at #1633, it seems to me that multiline method call chains should be checked by a separate cop, as people have different expectations and preferences for expressions like the one above compared to things like

some_instance.member.attributes =
  some_value +
  new_class_member +
  params[:some_key]

@alexdowad
Copy link
Contributor

Is this solved with @jonas054's new MultilineMethodCallIndentation cop?

@jonas054
Copy link
Collaborator

Not exactly. The current master supports the following styles. With

Style/MultilineMethodCallIndentation:
  EnforcedStyle: indented

RuboCop accepts

some_instance.member.attributes = SomeService
  .new(@class_member)
  .permit_method_call(params[:some_key])

and

some_instance.member.attributes =
  SomeService
  .new(@class_member)
  .permit_method_call(params[:some_key])

With

Style/MultilineMethodCallIndentation:
  EnforcedStyle: aligned

it accepts

some_instance.member.attributes = SomeService
                                  .new(@class_member)
                                  .permit_method_call(params[:some_key])

and

some_instance.member.attributes = SomeService.new(@class_member)
                                             .permit_method_call(params[:some_key])

and

some_instance.member.attributes =
  SomeService
  .new(@class_member)
  .permit_method_call(params[:some_key])

@alexdowad
Copy link
Contributor

@jonas054 OK, thanks. It seems like the style requested here is quite reasonable. Do you have any ideas what a 3rd style for MultilineMethodCallIndentation could be called?

@jonas054
Copy link
Collaborator

jonas054 commented Jan 1, 2016

Here's my understanding of what is requested.

The style is a variation on the indented style, and it differs from the current indented style when the expression is a binary operation, or assignment. In this new style, the base for the indentation is the right hand side of the binary operation rather than the whole expression (or the left hand side).

If my understanding is correct, it follows that the following indentation would be enforced, for example.

some_instance.member.attributes == SomeService
                                     .new(@class_member)
                                     .permit_method_call(params[:some_key])

The name that springs to mind is indented_rhs, but you might be able to come up with a better suggestion, @alexdowad.

@alexdowad
Copy link
Contributor

I don't think this style has anything specifically to do with assignments. If the selector in a dotted method call is pushed to a following line, then you follow the chain of dotted method calls back to its root, and indent from the receiver of the first call in the chain.

So the example you gave is right. But so are these:

if Something
     .call1
     .call2

while Something
        .call1
        .call2

class A < Something
            .call

@alexdowad
Copy link
Contributor

@elyzion, does that seem to describe what you are looking for?

@maia
Copy link

maia commented Jan 7, 2016

Looking forward to a style that allows:

@something = Someclass
                 .where(kind: :hello)
                 .includes(:other_class)
                 .order(created_at: :desc)

(using the 4 char standard continued indentation from Rubymine)

@alexdowad
Copy link
Contributor

Could we call this style offset (from the receiver)? relative (to the receiver)? Or give it a long and wordy name which describes it more precisely?

@jonas054
Copy link
Collaborator

I'd go for long and wordy. How about indented_from_receiver, or even indented_relative_to_receiver?

@nicolas-fricke
Copy link

nicolas-fricke commented May 11, 2016

Has there been any progress on this issue? In our projects we're now disabling this cop as we're normally writing our code indented_relative_to_receiver (I like the name!).

What we'd like to have is a cop which enforces the following code style:

my_variable = MyClass
                .method_1(123)
                .method_2

If no one is working on this, I might have a look in how to make this possible myself.

@aried3r
Copy link

aried3r commented Jun 14, 2016

I'm not sure I'm reading the code correctly, but shouldn't it almost handle this case already?
I found this code in the cop but can't really make sense of it, because it seems that line either goes unused or ignored (hence this issue).

@nicolas-fricke I'd love to help out, we're also disabling this cop because of this same issue.

@jonas054
Copy link
Collaborator

Yes, almost being the operative word. What you see there is alignment where the right hand side of the expression (b and .c) are aligned with each other without any extra indentation. As I've said earlier in this discussion, we need another style an addition to aligned and indented. It's doable but not trivial to implement.

jonas054 added a commit to jonas054/rubocop that referenced this issue Jun 19, 2016
…ver style

Add the style indented_relative_to_receiver in
Style/MultilineMethodCallIndentation for indentation of
method calls so that each new line of a call chain is
indented one step more than the ultimate receiver.
@nicolas-fricke
Copy link

Thanks @jonas054 for taking care of this – looking forward to upgrading rubocop to the next version and then reenabling this new cop :)

Neodelf pushed a commit to Neodelf/rubocop that referenced this issue Oct 15, 2016
…ver style (rubocop#3233)

Add the style indented_relative_to_receiver in
Style/MultilineMethodCallIndentation for indentation of
method calls so that each new line of a call chain is
indented one step more than the ultimate receiver.
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

7 participants