rubocop-thread_safety icon indicating copy to clipboard operation
rubocop-thread_safety copied to clipboard

Consider disallowing ivars in methods of any module

Open biinari opened this issue 4 years ago • 1 comments

As an extension to the suggestion in #4, perhaps we should flag up ivars used in methods of any module.

This would unfortunately add some false positives as modules may be included in classes, making instance methods and hence would be using instance variables rather than class instance variables.

They could be individually checked by the user and ignored with a rubocop:disable comment. Or perhaps this should be an option to the InstanceVariableInClassMethod, which may be set depending on how prevalent extending modules is in the user's codebase.

Concerns I would like to catch would be:

module Mod
  def some_method(something)
    @params = something
  end
end

module ExtMod
  extend Mod
end

class ExtClass
  extend Mod
end

ExtMod.some_method(something) # sets a class instance variable on ExtMod
ExtClass.some_method(something) # sets a class instance variable on ExtClass

False positives that would be difficult / impossible to rule out in a static analysis:

module Mod
  def some_method(something)
    @params = something
  end
end

class Includer
  include Mod
end

instance = Includer.new
instance.some_method(something) # sets an instance variable on instance

biinari avatar Jun 22 '20 16:06 biinari

Makes perfect sense, but the false positives concern me. I suspect we'd get a bunch of those.

mikegee avatar Jun 22 '20 20:06 mikegee