Indentation of closing parenthesis with hash argument

[dastra-mak]

I don't like the suggested indentation of a closing parenthesis when a method is called with a hash argument.

   def insert_forecast_group_row(forecast_group)
     insert_row({
       CATEGORY_COLUMN => forecast_group.name,
-    })
+    },
+               )
   end

The indentation is caused by the first argument { being on the same line as the method call. https://www.rubydoc.info/github/bbatsov/RuboCop/RuboCop/Cop/Layout/ClosingParenthesisIndentation

A very common occurence for this correction is the use of FactoryGirl factories:

  create(:moderator_contract,
    moderator: moderator,
    city_department: city_department,
- )
+       )

One solution to consider is to not use curly braces for argument hashes, e.g.:

   def insert_forecast_group_row(forecast_group)
     insert_row(
       CATEGORY_COLUMN => forecast_group.name,
     )
   end

and for factories to move the factory argument to a separate line:

  create(
    :moderator_contract,
    moderator: moderator,
    city_department: city_department,
  )

[dastra-mak]

This issue is also discussed here: https://github.com/rubocop-hq/rubocop/issues/1758

[triskweline]

Our discussion focused around the method call in your example, for which good alternatives exist.

However, I find the cop's behavior weird even when it works as intended:

# good: when x follows opening parenthesis, align parentheses
a = b * (x +
         y
        )

Hence I vote for disable.

[foobear]

Rubocop's bad indentation in this case can currently not be fixed, afaik. It stems from the method call spreading across multiple lines in this case, and Rubocop's suggested fix is definitely ugly, but any alternative will work and IMHO look better.

However, don't be fooled by the cop's intent. It will catch bad indentation like this:

def func(
  x,
  y
  )

I vote for keeping. Ideally, Rubocop will have an option that disables indenting closing parenthesis further than necessary in the future. Rubocop would then not be able to auto-correct and developer's would not be confronted with a failed attempt to fix something.

[foobear]

Bonus solution:

contract_attributes = {
  moderator: moderator,
  city_department: city_department,
}
create(:moderator_contract, contract_attributes)

IMHO more readable and flexible.

[kratob]

Also vote for disable. I consider

create(:moderator_contract,
    moderator: moderator,
    city_department: city_department,
)

the nicest solution (i don't like introducing an additional local variable either).

While it is a bit unfortunate that we have no cop checking for the indentation at all, I don't feel like misplaced closing parens are a common problem or cause for disagreement.

[denzelem]

I would like to keep this cop. The positive effect outweigh the edge cases. Using factories with the pattern above is nice. But I also saw styles like:

create(:moderator_contract, :trashed,
    moderator: moderator,
    city_department: city_department,
)

Here it's really hard to notice the :trashed trait.