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/MultilineWhenThen breaks some single line case statements #7635

Closed
tas50 opened this issue Jan 7, 2020 · 4 comments · Fixed by #7889
Closed

Style/MultilineWhenThen breaks some single line case statements #7635

tas50 opened this issue Jan 7, 2020 · 4 comments · Fixed by #7889
Labels

Comments

@tas50
Copy link
Contributor

tas50 commented Jan 7, 2020

Expected behavior

Style/MultilineWhenThen should detect multiline when statements with a then, but should not detect strange compact when/then statements.

Actual behavior

Given a compact case statement like this rubocop alerts Style/MultilineWhenThen:

def global_shell_init(action = nil)
  matcher = /^eval "\$\(chef shell-init bash\)"$/
  line = 'eval "$(chef shell-init bash)"'
  @global_shell_init ||= Chef::Util::FileEdit.new(bashrc_file)
  return @global_shell_init unless new_resource.global_shell_init
  case action
  when :create then @global_shell_init.insert_line_if_no_match(matcher,
                                                               line)
  when :delete then @global_shell_init.search_file_delete_line(matcher)
  end
  @global_shell_init
end
chef-dk/libraries/provider_chef_dk.rb:128:22: C: [Corrected] Style/MultilineWhenThen: Do not use then for multiline when statement.
        when :create then @global_shell_init.insert_line_if_no_match(matcher,
                     ^^^^

This will autocorrect to:

def global_shell_init(action = nil)
  matcher = /^eval "\$\(chef shell-init bash\)"$/
  line = 'eval "$(chef shell-init bash)"'
  @global_shell_init ||= Chef::Util::FileEdit.new(bashrc_file)
  return @global_shell_init unless new_resource.global_shell_init
  case action
  when :create @global_shell_init.insert_line_if_no_match(matcher,
                                                               line)
  when :delete then @global_shell_init.search_file_delete_line(matcher)
  end
  @global_shell_init
end

RuboCop version

0.79.0
@koic
Copy link
Member

koic commented Jan 8, 2020

The given code and the auto-corrected code are the same. Can you show me a reproducible given code?

@tas50
Copy link
Contributor Author

tas50 commented Jan 27, 2020

🤦‍♂ Let me find the correct example code

@tas50
Copy link
Contributor Author

tas50 commented Jan 27, 2020

Alright I've updated to show that RuboCop just nukes the first then and breaks the when.

@tagliala
Copy link
Contributor

Hi, this is still an issue with RuboCop 0.82.0

I have a working reduced test case

Original file

# frozen_string_literal: true

def bar
  []
end

def foo(arg)
  case arg
  when 'one'   then [1]
  when 'two'   then bar ||
                    [3]
  when 'three' then [4]
  end
end

puts foo('one')

Run Original File

$ ruby issue_7635.rb 
1

RuboCop output

$ rubocop 
Inspecting 1 file
C

Offenses:

issue_7635.rb:10:16: C: Style/MultilineWhenThen: Do not use then for multiline when statement.
  when 'two'   then bar ||
               ^^^^
issue_7635.rb:11:21: C: Layout/MultilineOperationIndentation: Use 2 (not 18) spaces for indenting an expression spanning multiple lines.
                    [3]
                    ^^^

1 file inspected, 2 offenses detected

RuboCop autofix (safe)

$ rubocop -a --safe
Inspecting 1 file
E

Offenses:

issue_7635.rb:10:14: E: Lint/Syntax: unexpected token tIDENTIFIER
(Using Ruby 2.4 parser; configure using TargetRubyVersion parameter, under AllCops)
  when 'two' bar ||
             ^^^
issue_7635.rb:10:16: C: [Corrected] Style/MultilineWhenThen: Do not use then for multiline when statement.
  when 'two'   then bar ||
               ^^^^
issue_7635.rb:11:21: C: [Corrected] Layout/MultilineOperationIndentation: Use 2 (not 18) spaces for indenting an expression spanning multiple lines.
                    [3]
                    ^^^

1 file inspected, 3 offenses detected, 2 offenses corrected

Fixed file

# frozen_string_literal: true

def bar
  []
end

def foo(arg)
  case arg
  when 'one'   then [1]
  when 'two' bar ||
    [3]
  when 'three' then [4]
  end
end

puts foo('one')

Run fixed file

$ ruby issue_7635.rb 
issue_7635.rb:10: syntax error, unexpected local variable or method, expecting `then' or ',' or ';' or '\n'
  when 'two' bar ||

@koic koic added the bug label Apr 18, 2020
koic added a commit to koic/rubocop that referenced this issue Apr 19, 2020
Fixes rubocop#7635

This PR fixes a false positive for `Style/MultilineWhenThen`
when `then` required for a body of `when` is used.
bbatsov pushed a commit that referenced this issue Apr 20, 2020
Fixes #7635

This PR fixes a false positive for `Style/MultilineWhenThen`
when `then` required for a body of `when` is used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants