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

[Fix #7826] Add new Layout/SpaceAroundMethodCall cop #7857

Merged

Conversation

saurabhmaurya15
Copy link
Contributor

@saurabhmaurya15 saurabhmaurya15 commented Apr 7, 2020

This MR adds a cop to for checking dot method calls to not have spaces around them.
It works for multiple method calls in a single line or in multiple lines.

closes #7826


Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

Copy link
Contributor

@rrosenblum rrosenblum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this support auto-correction? How does this work with safe navigation?

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 7, 2020

I think this cop should be a bit broader and check for all method invocation operators - @rrosenblum already mentioned safe navigation, but there's also ::. Probably the name should be something like "SpaceAroundMethodCallOperator" or something like this.

# # good
# foo.bar
# foo.bar.buzz
# foo
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also have chaining in a suffix form.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chaining in a suffix form is a bad style. There is already a cop for that so not including it in the example.
I have added a spec for chaining in suffix form though.

module RuboCop
module Cop
module Layout
# Checks dot method calls to not have spaces around them.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd use "method call operator" terminology here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Changing it to Checks method call operators to not have spaces around them.
Keeping it plural since it can have method chains also.

@saurabhmaurya15
Copy link
Contributor Author

saurabhmaurya15 commented Apr 7, 2020

@bbatsov I have changed name to SpaceAroundMethodCallOperator. I'll add checks for safe navigation operator and :: too.

config/default.yml Outdated Show resolved Hide resolved
@bbatsov
Copy link
Collaborator

bbatsov commented Apr 10, 2020

Okay, ping me when this is ready for a review again.

@saurabhmaurya15 saurabhmaurya15 force-pushed the feature/space-around-method-call-cop branch 3 times, most recently from b27c302 to edd7c68 Compare April 11, 2020 06:58
@saurabhmaurya15
Copy link
Contributor Author

@bbatsov This is ready for review.

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 12, 2020

@saurabhmaurya15 I think you can improve the test suite a bit if you used shared examples there, as the tests for ., :: and .& are mostly the same. Apart from this - it seems to me we're pretty close to finishing the cop.

@saurabhmaurya15 saurabhmaurya15 force-pushed the feature/space-around-method-call-cop branch from edd7c68 to 3806620 Compare April 12, 2020 18:57
@saurabhmaurya15 saurabhmaurya15 force-pushed the feature/space-around-method-call-cop branch from 3806620 to f6c201e Compare April 13, 2020 07:18
@bbatsov bbatsov merged commit d4a9bf5 into rubocop:master Apr 13, 2020
@bbatsov
Copy link
Collaborator

bbatsov commented Apr 13, 2020

Thanks!

koic added a commit to koic/rubocop that referenced this pull request Apr 13, 2020
Follow rubocop#7384, rubocop#7857, and rubocop#7869.

This PR fixes the next release version with 0.82 because RuboCop 0.81
has been released. This version (0.82) is based on rubocop#7851.
@koic koic mentioned this pull request Apr 13, 2020
8 tasks
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

Successfully merging this pull request may close these issues.

Cop idea: Check no spaces between . and the method being called
3 participants