Added a cop Style/Send. Disabled by default. #2081

syndbg · 2015-07-28T18:50:10Z

Motivation:
I was reading through The Well Ground Rubyist 2nd edition where I read in chapter 2, that it's preferred to use __send__ or public_send instead of send.

Implementation:

Added a Style/Send cop that registers offences when send is used. Instead it recommends public_send or __send__.
Added the cop in disabled.yml. I think the cop is a bit too strict to be enabled by default.
Added send tests that verify correct behavior when linting code that uses public_send, send and __send__.

bbatsov · 2015-07-29T04:29:48Z

config/disabled.yml

@@ -44,6 +44,11 @@ Style/MissingElse:
    - case
    - both

+Style/Send:
+  Description: 'Prefer __send__ or public_send to send,


It's better to qualify those - BasicObject#__send__ and Object#public_send.

bbatsov · 2015-07-29T04:35:58Z

That's a good idea and you should submit a matching rule to the community Ruby Style Guide. The implementation, however, has to be fixed, as right now you'll be registering offences for any method invocation except __send__ and public_send without receivers, which is obviously wrong.

Submitted as requested in rubocop/rubocop#2081

syndbg · 2015-07-29T17:50:53Z

config/disabled.yml

+                as `send` may overlap with existing methods.'
+  StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#prefer-public-send'
+  Enabled: false
+


Updated description and linked the relevant style guide. The rule about preferring __send__ over send will appear beneath prefer-public-send, so it's intentional that I link to prefer-public-send.

syndbg · 2015-07-29T17:56:29Z

Rebased latest master. Should be good for another review. Sorry for the troubles.

bbatsov · 2015-07-30T09:02:58Z

config/disabled.yml

@@ -44,6 +44,12 @@ Style/MissingElse:
    - case
    - both

+Style/Send:
+  Description: 'Prefer `BasicObject#__send__` or `Object#public_send` to `send`,
+                as `send` may overlap with existing methods.'


No need to split this string here.

bbatsov · 2015-08-01T08:14:25Z

@syndbg ping

syndbg · 2015-08-01T09:27:50Z

@bbatsov pong. Sorry about the delays 🎱

bbatsov · 2015-08-01T09:57:40Z

A bit more feedback:

you need some tests where the invocations have receivers
the commit message doesn't match the project's commit message guidelines

bbatsov · 2015-08-04T06:21:56Z

spec/rubocop/cop/style/send_spec.rb

+        inspect_source(cop, 'Object.send(:inspect)')
+      end
+
+      it 'registers an offense for an invocation without args' do


Btw, why are we registering an offence for send without arguments? That would clearly be a false positive. We should change this.

bbatsov · 2015-08-04T12:21:28Z

The commit message should be New copStyle/Sendchecks for the use ofsend(disabled by defaut).

bbatsov · 2015-08-04T12:22:20Z

lib/rubocop/cop/style/send.rb

+    module Style
+      # This cop checks for the use of the send method.
+      class Send < Cop
+        MSG = 'Prefer `BasicObject#__send__` or `Object#public_send` to `send`'


Object#send

The message should end with ..

syndbg · 2015-08-04T15:19:54Z

@bbatsov Regarding the commit message, isn't it the same format as the example ones in https://github.com/bbatsov/rubocop/blob/master/CONTRIBUTING.md#changelog-entry-format ?

bbatsov · 2015-08-04T15:20:48Z

This is the changelog entry format. :-)

syndbg · 2015-08-04T15:21:57Z

Alright.

bbatsov · 2015-08-04T16:08:51Z

One final remark - drop that . from the commit message's title. Titles are not sentences. That's also mentioned in our CONTRIBUTING.md.

Add a new cop Style/Send

bbatsov · 2015-08-04T16:35:12Z

👍 Thanks!

syndbg · 2015-08-04T16:36:46Z

Thanks for the patience! 🍻

Submitted as requested in rubocop/rubocop#2081

cf rubocop/rubocop#2081

Submitted as requested in rubocop/rubocop#2081

andyw8 · 2017-04-06T17:40:51Z

@syndbg I think this should have been two separate cops.

One should be to prefer __send__ over send (in case send is already defined).

The other should be to prefer public_send, when calling another object, so as to respect's the object's encapsulation, since send or __send__ will circumvent private or protected.

(I was the author of that rule in the style guide (rubocop/ruby-style-guide#360)).

Any thoughts?

Currently the Style/Send cop doesn't enforce `#public_send` however (that's what we want). It simply discourages the use of #send. See rubocop/rubocop#2081 (comment) for details. A new entry on the Code Conventions doc has been added to overcome this limitation: https://github.com/openfoodfoundation/openfoodnetwork/wiki/Code-Conventions#prefer-public_send-over-send

Devs keep using `#send` although that method does not preserve private/protected visibility. Watching after this turned out to be quite time-consuming while doing code review. Currently, the Style/Send cop doesn't enforce `#public_send` however (that's what we want). It simply discourages the use of #send. See rubocop/rubocop#2081 (comment) for details. So a new entry on the Code Conventions doc has been added to overcome this limitation: https://github.com/openfoodfoundation/openfoodnetwork/wiki/Code-Conventions#prefer-public_send-over-send

bbatsov reviewed Jul 29, 2015
View reviewed changes

syndbg added a commit to syndbg/ruby-style-guide that referenced this pull request Jul 29, 2015

Add a rule about send metaprogramming.

1d4c4ab

Submitted as requested in rubocop/rubocop#2081

syndbg mentioned this pull request Jul 29, 2015

Add a rule about send metaprogramming. rubocop/ruby-style-guide#461

Merged

syndbg force-pushed the style_cop_send branch 2 times, most recently from a0ffab0 to 195e4d9 Compare July 29, 2015 17:45

syndbg added a commit to syndbg/ruby-style-guide that referenced this pull request Jul 29, 2015

Add a rule about send metaprogramming.

42382a3

Submitted as requested in rubocop/rubocop#2081

syndbg reviewed Jul 29, 2015
View reviewed changes

syndbg force-pushed the style_cop_send branch from 195e4d9 to 9862373 Compare July 29, 2015 17:55

bbatsov reviewed Jul 30, 2015
View reviewed changes

syndbg force-pushed the style_cop_send branch from 9862373 to c770117 Compare August 1, 2015 09:27

syndbg force-pushed the style_cop_send branch from c770117 to f8e18f7 Compare August 1, 2015 09:33

syndbg force-pushed the style_cop_send branch 2 times, most recently from d0c8140 to 06b8309 Compare August 1, 2015 11:32

bbatsov reviewed Aug 4, 2015
View reviewed changes

syndbg force-pushed the style_cop_send branch from 06b8309 to 94d1ee2 Compare August 4, 2015 10:10

bbatsov reviewed Aug 4, 2015
View reviewed changes

syndbg force-pushed the style_cop_send branch from 94d1ee2 to df93d44 Compare August 4, 2015 15:18

syndbg force-pushed the style_cop_send branch from df93d44 to 05190fa Compare August 4, 2015 15:21

New cop Style/Send checks for the use of send (disabled by default)

35ef5ed

syndbg force-pushed the style_cop_send branch from 05190fa to 35ef5ed Compare August 4, 2015 16:24

bbatsov added a commit that referenced this pull request Aug 4, 2015

Merge pull request #2081 from syndbg/style_cop_send

242a761

Add a new cop Style/Send

bbatsov merged commit 242a761 into rubocop:master Aug 4, 2015

syndbg deleted the style_cop_send branch August 4, 2015 16:36

syndbg added a commit to syndbg/ruby-style-guide that referenced this pull request Aug 9, 2015

Add a rule about send metaprogramming

7d4e590

Submitted as requested in rubocop/rubocop#2081

shyouhei pushed a commit to shyouhei/ruby-style-guide that referenced this pull request Nov 11, 2015

Add a rule about send metaprogramming

f597dc7

Submitted as requested in rubocop/rubocop#2081

shyouhei added a commit to shyouhei/ruby-style-guide that referenced this pull request Nov 16, 2015

cherry-pick 7d4e590

455e13e

cf rubocop/rubocop#2081

shyouhei added a commit to shyouhei/ruby-style-guide that referenced this pull request Nov 17, 2015

cherry-pick 7d4e590

6a055b3

cf rubocop/rubocop#2081

ellimist pushed a commit to ellimist/ruby-style-guide that referenced this pull request Mar 15, 2017

Add a rule about send metaprogramming

92f848e

Submitted as requested in rubocop/rubocop#2081

sauloperez mentioned this pull request Aug 30, 2018

Support removal of enterprise logo and promo image openfoodfoundation/openfoodnetwork#2512

Merged

sauloperez mentioned this pull request Aug 30, 2018

Favor #public_send over #send using Rubocop's cop openfoodfoundation/openfoodnetwork#2618

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a cop Style/Send. Disabled by default. #2081

Added a cop Style/Send. Disabled by default. #2081

syndbg commented Jul 28, 2015

bbatsov Jul 29, 2015

bbatsov commented Jul 29, 2015

syndbg Jul 29, 2015

syndbg commented Jul 29, 2015

bbatsov Jul 30, 2015

bbatsov commented Aug 1, 2015

syndbg commented Aug 1, 2015

bbatsov commented Aug 1, 2015

bbatsov Aug 4, 2015

bbatsov commented Aug 4, 2015

bbatsov Aug 4, 2015

bbatsov Aug 4, 2015

syndbg commented Aug 4, 2015

bbatsov commented Aug 4, 2015

syndbg commented Aug 4, 2015

bbatsov commented Aug 4, 2015

bbatsov commented Aug 4, 2015

syndbg commented Aug 4, 2015

andyw8 commented Apr 6, 2017 •

edited

Loading

Added a cop Style/Send. Disabled by default. #2081

Added a cop Style/Send. Disabled by default. #2081

Conversation

syndbg commented Jul 28, 2015

bbatsov Jul 29, 2015

Choose a reason for hiding this comment

bbatsov commented Jul 29, 2015

syndbg Jul 29, 2015

Choose a reason for hiding this comment

syndbg commented Jul 29, 2015

bbatsov Jul 30, 2015

Choose a reason for hiding this comment

bbatsov commented Aug 1, 2015

syndbg commented Aug 1, 2015

bbatsov commented Aug 1, 2015

bbatsov Aug 4, 2015

Choose a reason for hiding this comment

bbatsov commented Aug 4, 2015

bbatsov Aug 4, 2015

Choose a reason for hiding this comment

bbatsov Aug 4, 2015

Choose a reason for hiding this comment

syndbg commented Aug 4, 2015

bbatsov commented Aug 4, 2015

syndbg commented Aug 4, 2015

bbatsov commented Aug 4, 2015

bbatsov commented Aug 4, 2015

syndbg commented Aug 4, 2015

andyw8 commented Apr 6, 2017 • edited Loading

andyw8 commented Apr 6, 2017 •

edited

Loading