Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.ruby > #7363

Re: Static analysis – Unused method return value and method!() return values

Show all headers | View raw

Mon, 15 May 2017 08:19:03 +0200, /Robert Klemme/:
> On 14.05.2017 14:38, Stanimir Stamenkov wrote:
> 
>> I've tried RuboCop and RubyCritic but they don't seem to flag the given
>> usages as problems.  Is there another tool to help with this, or is
>> there specific configuration to any of the two mentioned which would
>> trigger the intended warnings?
> 
> This is hard to do correct as it requires knowledge about how a 
> method works.  At best, this would only be a rough guideline.  It 
> starts by reliably detecting the class of the receiver.

I'm kinda disappointed in the capabilities of these tools.  Most 
reports they produce are about suggested cosmetic formatting, and 
few others which are all guidelines after all.  For example, RuboCop 
suggests class variables (vs. class instance variables) should be 
avoided, which doesn't mean they can't have legitimate usage.  I've 
seen an experienced Ruby zealot blindly replacing such occurrence 
causing quite substantial bug in the behavior.

---

The examples I've given could be interpreted like:

   some_hash[:a_key].strip.gsub!(/^0+/, '')

If not defined explicitly elsewhere `strip` is a String instance 
method, and it returns a _new_ string.  I think a linter could 
produce quite certain warning the `strip` result is not assigned, 
and chained operations on it are mostly no-op (if that's not the 
last statement of a method).  Also the `gsub!` on a temporary string 
is not likely having the desired effect.

Then, if similar statement/expression is used as a return value of a 
method/block:

   some_collection.map do |foo|
     foo[:bar].strip.gsub!(/^0+/, '')
   end

`gsub!` is a String instance method which has the peculiarity of 
returning false, instead of the original string when no substitution 
takes place.  This should also be flagged as a warning, as it is 
mostly likely a mistake.  It might not be a mistake if there's an 
alternative provided:

     foo[:bar].strip.gsub!(/^0+/, '') || foo[:bar].strip

Of course this suggests the linter maintains some class/method 
table, and has some knowledge of the core Ruby library.  I'll see if 
I could pursue for such an enhancement with the RuboCop maintainers.

-- 
Stanimir

Back to comp.lang.ruby | Previous | Next — Previous in thread | Next in thread | Find similar

Thread

Static analysis – Unused method return value and method!() return values Stanimir Stamenkov <s7an10@netscape.net> - 2017-05-14 15:38 +0300
  Re: Static analysis – Unused method return value and method!() return values Robert Klemme <shortcutter@googlemail.com> - 2017-05-15 08:19 +0200
    Re: Static analysis – Unused method return value and method!() return values Stanimir Stamenkov <s7an10@netscape.net> - 2017-07-15 18:14 +0300
      Re: Static analysis – Unused method return value and method!() return values Robert Klemme <shortcutter@googlemail.com> - 2017-07-15 18:29 +0200
        Re: Static analysis – Unused method return value and method!() return values Stanimir Stamenkov <s7an10@netscape.net> - 2017-07-15 23:20 +0300

csiph-web