Inconsistent reporting for FeatureEnvy

[klobuczek]

The following code

class << self
  def normalize_path(scope, path)
    path_desc = new(scope)
    path_desc.increment(path.shift) while path_desc.next? && path.present?
    path_desc if path.empty?
  end
end

raises JsonapiExt::Scoping::Include::PathDescriptor#normalize_path refers to 'path' more than self (maybe move it to another class?)

however, if I rewrite it to:

def self.normalize_path(scope, path)
  path_desc = new(scope)
  path_desc.increment(path.shift) while path_desc.next? && path.present?
  path_desc if path.empty?
end

it does not.

[mvz]

I've looked into this and it's super easy to make this consistent. The change doesn't even break any of our tests, which shows that we completely missed this 😆.

[pardaloupa]

This problem seems to still occur when dealing with private_class_method used within a module.

module Admins
  class << self
    private
      attr_reader: users
  end

  module_function
  ...
  
  def show_active_users
   ...
   active = active_users
   ...
  end

  private_class_method
    
  def users
    @users ||=[]
  end
  
  def active_users
    active =
      users
        .select { |user| user.deleted? == false }
    active.map { |user| user.visible = true
   
    Workspace.fetch_users(active) unless active.empty?
  end
end

refers to 'user' more than self (maybe move it to another class?)

Goes away if I use self.

[mvz]

@pardaloupa AIUI, private_class_method doesn't work without arguments.

[pardaloupa]

Wow, I never realized that... Ruby never complained.

Is this is how you are supposed to use it?

module Admins
  class << self
    private
      attr_reader: users
  end

  module_function
  ...
  
  def show_active_users
   ...
   active = active_users
   ...
  end
    
  def users
    @users ||=[]
  end
  
  def active_users
    active =
      users
        .select { |user| user.deleted? == false }
    active.map { |user| user.visible = true
   
    Workspace.fetch_users(active) unless active.empty?
  end
  private_class_method :active_users
end

If that is right, reek still complains 😞

Edit: all the examples I have seen seem to be using self then passing it over to private_class_method. Maybe is actually required?

[pardaloupa]

I am really confused on how to use module_function with private class methods.

This comment clears things up by using extending self / private combo instead of module_function and private_class_method but goes against the ruby style guideline... Not sure if it is a reek problem now... Any suggestions would be highly appreciated.

[mvz]

If that is right, reek still complains

Ok, now that is a valid bug 😆.

[pardaloupa]

Great thanks for confirming it @mvz then I guess I will keep using module_function and private_class_method and the reek issues will vanish once a bug fix is pushed. Let me know if you need me to open a new issue as this is hard to track 👍

Edit: Same issue occurs with module_function methods

[mvz]

@pardaloupa yes if you could open a new issue with your example code that would be great!

[pardaloupa]

@mvz done #1397 👍