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

False positive for the Layout/EmptyLinesAroundArguments cop #5224

Closed
kyrylo opened this issue Dec 12, 2017 · 10 comments
Closed

False positive for the Layout/EmptyLinesAroundArguments cop #5224

kyrylo opened this issue Dec 12, 2017 · 10 comments

Comments

@kyrylo
Copy link

kyrylo commented Dec 12, 2017

Use this file and run latest rubocop against it:
https://github.com/airbrake/airbrake/blob/v7.1.0/lib/airbrake/capistrano/capistrano2.rb


Expected behavior

No offence of the Layout/EmptyLinesAroundArguments cop

Actual behavior

lib/airbrake/capistrano/capistrano2.rb:4:1: C: Layout/EmptyLinesAroundArguments: Empty line detected around arguments.
    # rubocop:disable Metrics/AbcSize
^
lib/airbrake/capistrano/capistrano2.rb:7:1: C: Layout/EmptyLinesAroundArguments: Empty line detected around arguments.
        after 'deploy', 'airbrake:deploy'
^

The offending lines are actually L18 & L21. I believe it's wrong to force me to format my strings how I want.

RuboCop version

$ rubocop -V
0.52.0 (using Parser 2.4.0.2, running on ruby 2.4.2 x86_64-darwin16)
@jaredbeck
Copy link
Contributor

jaredbeck commented Dec 12, 2017

Kyrylo's example actually involves arguments, but I'm seeing a false positive that does not involve arguments.

module A
  module B # <- offense on this line: "Empty line detected around arguments."
    # .. 100 lines later ..
    def c
      d.
        e(f).
  
        # removing the blank line above fixes offense
        g(h)
    end
  end
end

@ValenciaMgmt
Copy link

ValenciaMgmt commented Dec 12, 2017

Here's a short bit of nonsense code that still gets the false positive. Removing any remaining part makes the warning go away.

(1..10).map do |number| # Rubocop complains about this line
  next if number.even?

  number
end.compact

@uasi
Copy link

uasi commented Dec 13, 2017

Another false positive here.

A = [
  1,
  # comment
  2
]

B = A + [
  3,

  4
]
$ rubocop -V
0.52.0 (using Parser 2.4.0.2, running on ruby 2.4.1 x86_64-darwin16)

$ rubocop --only Layout/EmptyLinesAroundArguments input.rb
[snip]
input.rb:3:1: C: Layout/EmptyLinesAroundArguments: Empty line detected around arguments.
  # comment
^

Weirdly, deleting the assignment to B or the blank line after 3, or the A + portion fixes offense.

I tried #5231 but it didn't help.

@jodosha
Copy link

jodosha commented Dec 13, 2017

We've got a lot of random false positive. Sometimes it's a class definition, sometimes it's a module, or an end or indentation space. Here's a few of them:

class InvoiceRetryWorker
^
module Dnsimple
^
  # Some code comment
^
    def call(notification)
^
RSpec.describe "..." do
^
      rescue ActiveRecord::ActiveRecordError => e
^
  let(:counter) {
^
      end
^

@printercu
Copy link

It seems like location is reported incorrectly:

Rollbar.configure do |config|
  # Without configuration, Rollbar is enabled in all environments.
  # To disable in specific environments, set config.enabled=false.

  config.enabled = Rails.env.in?(%w[production staging])

  config.exception_level_filters.merge!(
    'ActionController::Other' => 'ignore',
    'ActionController::BadRequest' => 'ignore',

    'ActionController::InvalidCrossOriginRequest' => 'ignore',
    'ActionController::OtherCSRF' => 'ignore'
  )
end

=>

config/initializers/rollbar.rb:4:1: C: Layout/EmptyLinesAroundArguments: Empty line detected around arguments.

@eamonn-webster
Copy link

I would agree with @printercu, I had the same issue, it would appear that it is the simply reporting the wrong location. It seems to give the offset within the block rather than the file.

@garettarrowood
Copy link
Contributor

@eamonn-webster & @printercu - You are absolutely right. I'm able to confirm with a variety of examples, and I see/understand the problem in the code. Working on a solution now.

@revolter
Copy link
Contributor

I think I'm getting a false positive too for:

sh('script.py', error_callback: lambda { |result|
    error = JSON.parse(result, symbolize_names: true)
    error_text = error[:error]

    UI.user_error!("Error: #{error_text}")
})

at line 4 using RuboCop 0.52.1.

@garettarrowood
Copy link
Contributor

@revolter - You are correct. The final fixes for this cop are unreleased. You can test it against rubocop master if you'd like, or hold off on enabling the cop until the next version. This scenario should now be accounted for though.

@revolter
Copy link
Contributor

Ok, thanks for the quick update and confirmation!

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

9 participants