Rubocop: Rubocop "-a" changes the return type of the method

[execat]

The rewritten method on executing using "-a" puts the comma in the wrong position for the method. This might have something to do with the Style/TrailingCommaInHashLiteral rule (see below).

This is dangerous because the comma changes the return type of the method from a hash to an array because of the comma. Instead of returning type {} now the method returns type [{}, nil].

---

Expected behavior

lol.rb:

def lol
  { 
    one: 1,
    two: '',
    three: 0,
  }
end

Actual behavior

lol.rb:

def lol
  ret = { one: 1,
          two: '',
          three: 0 },
        ret
end

Steps to reproduce the problem

1. Save this as lol.rb:

def lol
  ret = { :one => 1,
          :two => '',
          :three => 0
  }
  ret
end

1. Use this rubocop.yml:

Style/Documentation:
  Enabled: false

Style/StringLiterals:
  Enabled: false

Style/ClassAndModuleChildren:
  Enabled: false

Style/FrozenStringLiteralComment:
  Enabled: false

Metrics/LineLength:
  Max: 120

Metrics/MethodLength:
  Max: 20

Metrics/ClassLength:
  Max: 200

Metrics/ModuleLength:
  Max: 200

Layout/AlignParameters:
  Enabled: false

Metrics/AbcSize:
  Enabled: false

AllCops:
  TargetRubyVersion: 2.3+
  Exclude:
    - '**/*.gemspec'
    - '**/Rakefile'

Style/TrailingCommaInArrayLiteral:
  EnforcedStyleForMultiline: comma

Style/TrailingCommaInHashLiteral:
  EnforcedStyleForMultiline: comma

Style/MutableConstant:
  Enabled: false

1. Run rubocop -a lol.rb

RuboCop version

$ rubocop -V
0.53.0 (using Parser 2.5.0.3, running on jruby 2.3.3 java)

[iridakos]

I reproduced this and I'm trying to fix it.

If anyone else is looking at this, keep in mind that this doesn't happen if you disable the Layout/MultilineHashBraceLayout. The problem could be in the combination of these two rules.

Layout/MultilineHashBraceLayout:
  Enabled: false

Update: I believe that the problem is that the TrailingCommaInHashLiteral inserts the comma after the range on the original source buffer and not at the one deriving after the MultilineHashBraceLayout autocorrection.

[garettarrowood]

@iridakos - I've come to the same conclusion, and am just now seeing your comment. I've been debugging #5932 and run into this with Layout/MultilineArrayBraceLayout. The core of the issue seems to be the violating ranges are calculated based on the original source, and not re-evaluated after the first wave of autocorrection.

So spaces are correctly removed/added for Multiline*BraceLayout based on a range from the original source code against the original source code. Then a comma is incorrectly added for TrailingCommaIn*Literal based on a range calculated from the original source code against the new altered code (that no longer needs a comma because of the ending bracket/brace).

I think the TrailingCommaIn*Literal can perhaps be made smarter to check for this. I'll give it a shot.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[stale[bot]]

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.