If the arguments of a method call span more than one line, only allow one argument per line

[a-lavis]

I wonder what people would think of extending Method Arguments Alignment (or perhaps creating a new guideline) to include the following rule: If the arguments of a method call span more than one line, only allow one argument per line.

Here are some code examples based on the Method Arguments Alignment guideline I linked above:

# bad
def send_mail(source)
  Mailer.deliver(to: 'bob@example.com',  from: 'us@example.com', subject: 'Important message',
                 body: source.text)
end

# bad (normal indent)
def send_mail(source)
  Mailer.deliver(
    to: 'bob@example.com',  from: 'us@example.com',  subject: 'Important message',
    body: source.text
  )
end

# good
def send_mail(source)
  Mailer.deliver(to: 'bob@example.com',
                 from: 'us@example.com',
                 subject: 'Important message',
                 body: source.text)
end

# good (normal indent)
def send_mail(source)
  Mailer.deliver(
    to: 'bob@example.com',
    from: 'us@example.com',
    subject: 'Important message',
    body: source.text
  )
end

what do you think?

Pros:

1. IMO, it looks better.
2. Makes diffs easier to read when you're only changing specific arguments of a method call.
3. If turned into an autocorrectable cop/configuration option in RuboCop, it could improve the results of formatting with the Layout/LineLength cop. Right now when a method call extends past the line length character limit, oftentimes it will cut off the last argument and put it on the next line, with all the other arguments still on the original line, which looks awkward, especially with DSLs.

Cons:

1. Might be considered overly restrictive.
2. If you have a method which takes a ton of arguments (probably too many), then it might be nice to not have the method call take up too many lines. (that said, in this case, I would still prefer each argument on its own line to make diffs easier to read)

---

Sidenote - We could also add corresponding rules for Array and Hash alignment, but I wanted to start with this one since I wasn't 100% sure about those.

---

I was originally planning on opening an issue in https://github.com/rubocop/rubocop, but after looking through some of the feature request issue there it seems like this is the better repository to start in. Let me know if it would be better to raise this issue as a new cop idea in RuboCop instead.

Also, even if we decide that this isn't the right fit for the ruby style guide, we could still maybe add a configuration option to Layout/ArgumentAlignment which is disabled by default, or a new cop which is disabled by default, which allows users to enforce this this.

[pirj]

What if those are positional arguments?

What is the problem with grouping semantically related arguments (eg from, to, cc, bcc; or lat, lon; height, weight; page, per_page) on the same line?

Does it cause you trouble reading the code if there’s more than one on each line?

[a-lavis]

What if those are positional arguments?

I still think that has the same issues as keyword arguments, for example:

# bad (normal indent)
def send_mail(source)
  Mailer.deliver(
    'bob@example.com',  'us@example.com',  'Important message',
    source.text
  )
end

# good (normal indent)
def send_mail(source)
  Mailer.deliver(
    'bob@example.com',
    'us@example.com',
    'Important message',
    source.text
  )
end

What is the problem with grouping semantically related arguments (eg from, to, cc, bcc; or lat, lon; height, weight; page, per_page) on the same line?

Ah yeah, that's a fair point, didn't think about semantic grouping. Personally I don't think I would do this, but I could see the argument for doing it yeah!

Does it cause you trouble reading the code if there’s more than one on each line?

Personally, yes it does.

One example of why is diffs - if a method call has a lot of arguments, its easier to see how those arguments have been changed in a diff if they're on separate lines.

For example, see this diff, where the line length limit is set to 80 characters:

diff --git i/example.rb w/example.rb
index d1867a6..16c1044 100644
--- i/example.rb
+++ w/example.rb
@@ -1,15 +1,17 @@
 # bad
 class Comment < ApplicationRecord
-  has_many :replies, -> { unscope(where: :deleted_at) }, class_name: 'Comment',
-           foreign_key: :parent_id, dependent: :destroy, inverse_of: :parent
+  has_many :replies, -> { unscope(where: :deleted_at).where(draft: false) },
+           class_name: 'Comment', foreign_key: :parent_id,
+           dependent: :restrict_with_error, inverse_of: :parent, validate: false
 end

 # good
 class Comment < ApplicationRecord
   has_many :replies,
-           -> { unscope(where: :deleted_at) },
+           -> { unscope(where: :deleted_at).where(draft: false) },
            class_name: 'Comment',
            foreign_key: :parent_id,
-           dependent: :destroy,
-           inverse_of: :parent
+           dependent: :restrict_with_error,
+           inverse_of: :parent,
+           validate: false
 end

in the bad example, the whole thing is highlighted - in the good example, only the changed lines are highlighted.