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

Handle required keyword arguments by default #724

Closed
jasonmp85 opened this issue Jan 8, 2014 · 8 comments
Closed

Handle required keyword arguments by default #724

jasonmp85 opened this issue Jan 8, 2014 · 8 comments
Assignees
Labels

Comments

@jasonmp85
Copy link

It may have slipped your notice since it doesn't seem to be in the official release news item, but Ruby 2.1 allows you to omit keyword arguments' values in order to signal that those keyword arguments are required. See the "Update" section on this blog post for an example.

rubocop doesn't seem to like this. I don't know where the styleguide stands on this, but I omit the space after the colon for the required keywords. This produces two offenses using the default rubocop config…

❯❯❯ rake rubocop
Running RuboCop...
Inspecting 10 files
.C........

Offences:

lib/binding_assigner/shard.rb:18:18: C: Space missing after colon.
  def initialize(table:, nodes:)
                 ^^^^^^
lib/binding_assigner/shard.rb:18:26: C: Space missing after colon.
  def initialize(table:, nodes:)
                         ^^^^^^

10 files inspected, 2 offences detected
RuboCop failed!
❯❯❯ 

… but adding the spaces still produces one offense:

❯❯❯ rake rubocop
Running RuboCop...
Inspecting 10 files
.C........

Offences:

lib/binding_assigner/shard.rb:18:27: C: Space missing after colon.
  def initialize(table: , nodes:)
                          ^^^^^^

10 files inspected, 1 offence detected
RuboCop failed!
❯❯❯ 

I'm not sure if this is a problem in parser or rubocop, but this should probably "Just Work" out of the box. For now I'm disabling the SpaceAfterColon cop around this line, which works fine.

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 9, 2014

This is a RuboCop bug (or rather an implemented feature). Naturally there should be no space after arg:. @jonas054 would you please update this cop?

@jonas054
Copy link
Collaborator

jonas054 commented Jan 9, 2014

Yes.

@ghost ghost assigned jonas054 Jan 9, 2014
bbatsov added a commit that referenced this issue Jan 10, 2014
[Fix #724] Allow colon for required keyword argument without space.
@jasonmp85
Copy link
Author

Wow! Fast turnaround. Thanks.

@mgogov
Copy link

mgogov commented Apr 29, 2016

Hm, does this cop still support required keyword arguments?

I've got a snippet which looks like:

def configure(node, user_name:, file_action:, deployment:)
  ...
end

and I still get a Rubocop error like:

file.rb:28:31: E: unexpected token tCOMMA
(Using Ruby 2.0 parser; configure using TargetRubyVersion parameter, under AllCops)
def configure(node, user_name:, file_action:, deployment:)

I did set the TargetRubyVersion under AllCops to all possible 2.0+ Ruby versions (2.0, 2.1, 2.2, 2.3), nothing changes.

Tried with Rubocop 0.39.0 and 0.37.2, same error.

When I change the keyword arguments to have defaults:

def configure(node, user_name: nil, file_action: nil, deployment: nil)
  ...
end

the cop passes.

Any thoughts?

@andyw8
Copy link
Contributor

andyw8 commented Apr 29, 2016

I believe TargetRubyVersion is only supposed to have one version specified.

@mgogov
Copy link

mgogov commented Apr 29, 2016

@andyw8, sorry, I meant I subsequently tried it with all the possible versions, one by one as in:

AllCops:
  TargetRubyVersion: 2.0

then

AllCops:
  TargetRubyVersion: 2.1

...

@andyw8
Copy link
Contributor

andyw8 commented Apr 29, 2016

I tried your first code sample with RuboCop 0.37.2.

With TargetRubyVersion: 2.0 it failed with unexpected token tCOMMA.

With TargetRubyVersion: 2.1 or 2.2 or 2.3, it succeeded.

@mgogov
Copy link

mgogov commented Apr 29, 2016

@andyw8 indeed.

I created a very simple stripped down example repo which confirmed what you say.

My problem from above was an extract from a huge repo where I just spotted a competing nested .rubocop.yml file which was overriding the parent directory's .rubocop.yml file where I was trying to put the TargetRubyVersion: 2.1 declaration for several hours.

So, in the end, turned out a very complicated misconfiguration on my end, so apologies for the false alarm, the cop works just fine.

Thank you very much for your help 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants