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

Groups > comp.lang.ruby > #7365

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

Show all headers | View raw

On 15.07.2017 17:14, Stanimir Stamenkov wrote:
> 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.

You need to be aware of what is possible at all.  Your expectations seem 
unrealistic high to me.

>  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.

But they should really be avoided.  I have yet to see a problem that is 
better solved with class variables than with instance variables of the 
class (or even different means).

>  I've seen an experienced 
> Ruby zealot blindly replacing such occurrence causing quite substantial 
> bug in the behavior.

Any code change can cause bugs (and especially when done with little 
involvement of the conscious mind).  That is the simple truth.

> 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, 

The difficulties start by "not explicitly defined elsewhere".  The 
method could be in a piece of code that is generated at runtime, could 
be in C etc.

> 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.

You can say that because you probably have enough knowledge of the 
context.  But the fact that something is called "some_hash" does not 
guarantee at all that the variable points to a Hash instance.  And it 
goes on.  Ruby is so dynamic that you can only know at runtime what 
types are in play etc.  Of course, you could produce warnings like you 
suggest but then you'd have to wade through a huge pile of false 
positives.  And at the same time some real issues probably go undetected.

> 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,

No, nil.

> 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:

... or if the programmer knows that there will always be at least one 
substitution.

>      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.

I wish you luck!

Kind regards

	robert

-- 
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/

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