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

Style/MultilineHashBraceLayout still flagging single lines #3256

Closed
coding-red-panda opened this issue Jun 28, 2016 · 6 comments
Closed

Style/MultilineHashBraceLayout still flagging single lines #3256

coding-red-panda opened this issue Jun 28, 2016 · 6 comments

Comments

@coding-red-panda
Copy link

Hello,

I previously reported the issue with the Multiline cops flagging single line statements as false positive.
This is still happening with the Style/MultilineHashBraceLayout Cop.


Expected behavior

Do not flag single line statements as offense when using the EnforcedStyle: new_line option.

Actual behavior

Single lines are flagged as violation.

Steps to reproduce the problem

  1. Enable the Cop Style/MultilineHashBraceLayout
  2. use the configuration: EnforcedStyle: new_line
  3. run rubocop.

The following code for example get's flagged:

expect(subject.cache_dir).to eq cache_dir

let(:cache_dir) { Rails.root.join('tmp/uploads') }

VCR.configure do |c|
  c.filter_sensitive_data('<BVDWEBPASSWORD>') { settings.bureau_van_dijk.web_service_password }
end

RuboCop version

Include the output of rubocop -V:

$ bundle exec rubocop -V
0.40.0 (using Parser 2.3.1.2, running on ruby 2.2.5 x86_64-darwin15)
@Drenmi
Copy link
Collaborator

Drenmi commented Jul 3, 2016

@NekoNova: I can not reproduce this using 0.41.1. Can you confirm it has been fixed on that version?

@coding-red-panda
Copy link
Author

I cannot run that version due other bug with the. Rubyversion file
On 3 Jul 2016 08:46, "Ted Johansson" notifications@github.com wrote:

@NekoNova https://github.com/NekoNova: I can not reproduce this using
0.41.1. Can you confirm it has been fixed on that version?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3256 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAsWhUTHfNU8OYTn7X2uiMKTNVFbn3DLks5qR1q7gaJpZM4JAEQY
.

@coding-red-panda
Copy link
Author

I've upgraded Rubocop to 0.41.2, but I'm still running into issues with this Cop.

Right now I's throwing me the following error:

Closing method call brace must be on the line after the last argument.
          { ...
          ^

The offending code is the following:

def generate_some_objects(options = {})
  before :each do
     create_one_object(
        MyObject.new,
        {
            option_1: 'a',
            option_2: 'b',
            option_3: 'c'
         }.merge(options)
      )
  end
end

This code should be accepted when using the new_line configuration option.

@coding-red-panda
Copy link
Author

The following snippet in our specs is not accepted either:

    it 'should create the process' do
      expect(expected_builder).to(
        receive(:new).with(
          command_name: :test,
          user_id: 1,
          customer_id: 1,
          parent_process_id: 1,
          command_attributes: { attribute_for_command1: 2, attribute_for_command2: 3 }
        ).and_return(builder)
      )
      expect(builder).to receive(:build).and_return(process)
      expect(subject.execute(options)).to eq process
    end
spec/support/background_example_groups.rb:54:9: C: Closing method call brace must be on the line after the last argument.
        receive(:new).with( ...
        ^^^^^^^^^^^^^^^^^^^

@jonas054
Copy link
Collaborator

First of all it's the Style/MultilineMethodCallBraceLayout you're talking about.

I think its behavior, as far as detecting offenses is concerned, is consistent with the description in config/default.yml

Style/MultilineMethodCallBraceLayout:
  EnforcedStyle: symmetrical
  SupportedStyles:
    # symmetrical: closing brace is positioned in same way as opening brace
    # new_line: closing brace is always on a new line
    # same_line: closing brace is always on the same line as last argument
    - symmetrical
    - new_line
    - same_line

Note: "closing brace is always on a new line".

The problem is that the cop highlights the wrong thing. Since all the messages talk about what's wrong with the closing bracket (or brace as this cop calls it), this is what we should highlight.
Then we get reporting like this:

/tmp/3256.rb:17:22: C: Closing method call brace must be on the line after the last argument.
      }.merge(options)
                     ^
/tmp/3256.rb:30:25: C: Closing method call brace must be on the line after the last argument.
    ).and_return(builder)
                        ^

I'll submit a PR soon.

@coding-red-panda
Copy link
Author

that makes more sense.

jonas054 added a commit to jonas054/rubocop that referenced this issue Jul 22, 2016
… cops

The closing brace is what the cops are reporting, so that's what
we should highlight. It's easy to misunderstand the reports otherwise.
Neodelf pushed a commit to Neodelf/rubocop that referenced this issue Oct 15, 2016
… cops (rubocop#3337)

The closing brace is what the cops are reporting, so that's what
we should highlight. It's easy to misunderstand the reports otherwise.
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

3 participants