ruby-style-guide icon indicating copy to clipboard operation
ruby-style-guide copied to clipboard

Autoformat issues with v2.11.1

Open vytis opened this issue 2 years ago • 3 comments

I have updated rubocop-shopify to 2.11.1 and fixing the layout with bundle exec rubocop -A produced some weird changes.

The code before:

    def collection
      Service.not_deleted.includes(
        { vault_teams:
            [:github_team_vault_teams, :teams] },
        { app:
            [{ runtimes:
                 [:runtime_instances, :app] },
             :runtimes,
             :repo,] },
        :repo,
        :app,
        :monorepo_mobile_app,
        :bugsnag_project,
        :slack_channels,
        :vault_teams,
        :vault_users,
      ).joins(:app)
    end

The code after:

    def collection
      Service.not_deleted.includes(
        {
          vault_teams:
                      [:github_team_vault_teams, :teams],
        },
        {
          app:
                      [
                        {
                          runtimes:
                                           [:runtime_instances, :app],
                        },
                        :runtimes,
                        :repo,
                      ],
        },
        :repo,
        :app,
        :monorepo_mobile_app,
        :bugsnag_project,
        :slack_channels,
        :vault_teams,
        :vault_users,
      ).joins(:app)
    end

There are more extreme examples in this file.

It seems the problem doesn't happen if the opening bracket (or whole entry) is on the same line:

Before

    def collection
      Service.not_deleted.includes(
        { vault_teams:[:github_team_vault_teams, :teams] },
        { app:[
          { runtimes: [:runtime_instances, :app] },
             :runtimes,
             :repo,] },
        :repo,
        :app,
        :monorepo_mobile_app,
        :bugsnag_project,
        :slack_channels,
        :vault_teams,
        :vault_users,
      ).joins(:app)
    end

After

    def collection
      Service.not_deleted.includes(
        { vault_teams: [:github_team_vault_teams, :teams] },
        {
          app: [
            { runtimes: [:runtime_instances, :app] },
            :runtimes,
            :repo,
          ],
        },
        :repo,
        :app,
        :monorepo_mobile_app,
        :bugsnag_project,
        :slack_channels,
        :vault_teams,
        :vault_users,
      ).joins(:app)
    end

vytis avatar Jan 12 '23 12:01 vytis

I have a branch where I'm adding "integration tests". I'll add these so we can be sure when we've fixed it (either by tweaking config, or fixing correction upstream).

sambostock avatar Jan 12 '23 19:01 sambostock

I think the problem is with Layout/FirstHashElementLineBreak. When I disable that, the crazy indenting goes away (although it doesn't break the line, so it's still possible that it's a combination of cops).

cc. @Korri

sambostock avatar Jan 13 '23 20:01 sambostock

I think the problem is with Layout/FirstHashElementLineBreak. When I disable that, the crazy indenting goes away (although it doesn't break the line, so it's still possible that it's a combination of cops).

cc. @Korri

Yeah this is a combination of the opening tag already being on a newline originally + the new mapping + us not having a rule that forces the opening element to be on the same line not one that forces a specific indentation Hash values when it's not on the same line as the key.

Like all of the following are ok:

{
  a:
        :foo,
  b:
    :bar,
  c:
                                   :bat,
}

Korri avatar Feb 10 '23 18:02 Korri