Allow self.Foo

[chulkilee]

omitting self in self.Foo gives uninitialized constant NameError

[coveralls]

Coverage decreased (-0.1%) when pulling 6fe373355da502b689b6d1229a6d09832125e2e8 on chulkilee:allow-self into 62a7059 on bbatsov:master.

[bbatsov]

I'm not sure I understand the nature of this problem. If the constant was defined at this point it would surely be accessible without self.

[jonas054]

@bbatsov I think it's this case. You need self. to call B, otherwise you get uninitialized constant Klass::B (NameError).

class Klass
  def A
    puts 'A'
    self.B
  end

  def B
    puts 'B'
  end
end

Klass.new.A

[coveralls]

Coverage decreased (-0.1%) when pulling b89939eb9d6417910f5af46fbbdd15446e33c33a on chulkilee:allow-self into dc7544f on bbatsov:master.

[chulkilee]

self cannot be omitted when the method name can be recognized as constant (/^A-Z/), because it just behaves differently.

Foo = 3

class Klass
    def method_missing(meth, *args, &block)
      "missing #{meth}"
    end

    def test
      self.Foo
    end

    def test_constant
      Foo
    end
end

puts Klass.new.test
puts Klass.new.test_constant

[agrimm]

You can also do Foo(), which has been allowed by RuboCop since #585

Foo = 3

class Klass
  def method_missing(meth, *args, &block)
    "missing #{meth}"
  end

  def test
    self.Foo
  end

  def test_constant
    Foo
  end

  def test_parentheses
    Foo()
  end
end

puts Klass.new.test
puts Klass.new.test_constant
puts Klass.new.test_parentheses

[chulkilee]

First, self.Foo and Foo is different, so self.Foo should not be considered as "redundant" - it's rubocop bug.

Second, as @agrimm mentioned, that self can be replaced with method call. Then should it be avoided? (This is somewhat different topic)

I think calling self.Foo sounds more like attribute than Foo() so there are some cases self.Foo is more natural. See the following example.

class Original
  def Id
    "fake id"
  end
end

class Wrapper
  def initialize(raw)
    @raw = raw
  end

  def details
    "#{self.Id}"
  end

  def method_missing(meth, *args, &block)
    @raw.send(meth)
  end
end

original = Original.new
wrapper = Wrapper.new(original)
puts wrapper.details

Wrapper#Id is more like attribute rather than method - which can be done by omitting parenthesis (which is absolutely legit in ruby)

This is the case when I need to wrap a restfoce object. restforce uses hashie to wrap Salesforce response, which includes camel case attributes (like Id, Name etc).

If we want to add rules to discourage using camel case method name, that should be another cop, not in this RedundantSelf cop.

[coveralls]

Coverage decreased (-0.07%) when pulling 5e9d697 on chulkilee:allow-self into e4b7f67 on bbatsov:master.

[bbatsov]

@jonas054 @yujinakayama I'm not sure what's the best way to handle this, since CamelCase methods are quite rare and generally used only for conversions. Maybe the best option would be to implement this as an option - AllowConstants or something like this. Furthermore, there is the (small) possibility of confusing an actual constant (for which self would truly be redundant) with a camelcase method name.

[jonas054]

Since both self.UpperCaseMethod and self.CONSTANT are rare, I think @chulkilee's solution is OK. False positives are really bad. False negatives in rare cases, we can live with.

[bbatsov]

Sounds reasonable.