troessner / reek

Code smell detector for Ruby
https://github.com/troessner/reek
MIT License
4.05k stars 280 forks source link

NestedIterator should ignore _currently open_ like methods (like CSV and tempfile). #1490

Closed eloyesp closed 1 year ago

eloyesp commented 5 years ago

CSV (from standard library) uses a lot of methods that yield a csv file (that is opened and closed at the end of the block), Tempfile.create do the same:

Sample code:

CSV.generate headers: ['metric', 'value'], write_headers: true do |csv|
  metric_types.map do |id, value_type|
    csv << [id, SAMPLE_VALUE[value_type]]
  end
end

Tempfile.create('foo', '/home/temp') do |f|
   array.each do |element|
     f.puts element
  end
end

I'm not sure if I can list all the method names, but, I would use open, create and generate.

On CSV, there is also the new method that receives a block, but I'm not sure if it can be ignored (I can't think on an example where it is used as an iterator, but I'm not sure).

Currently tap is the only ignored method, what you thing about it?

troessner commented 5 years ago

Not entirely sure I follow, couldnt you just add those exceptions yourself using the ignore_iterators configuration described here?

eloyesp commented 5 years ago

@troessner Sure it can be done (it is what I did actually), but as those methods are on the stdlib and the name make it clear there is no iteration (just as the tap example) I thought it could be useful to add those method names by default.

troessner commented 5 years ago

Currently ignore_iterators works on methods alone. Whitelisting methods like create and so on would defeat the purpose of the whole smell detector since this means it would also not report on all the other cases where create is used in a non-CSV context.

I am open for pull requests that would allow us to whitelist Tempfile.create though ;)