Project

General

Profile

Bug #10967

Is "warning: private attribute?" wrong?

Added by spastorino (Santiago Pastorino) over 2 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 2.3.0dev (2015-03-01 trunk 49796) [x86_64-darwin14]
[ruby-core:<unknown>]

Description

The following code ...

class Y
  def initialize
    @x = "ZOMG"
  end

  def print_x
    puts x
  end

  private

  attr_reader :x
end

Y.new.print_x

outputs ...

test.rb:12: warning: private attribute?

I tend to think this warning is wrong, I was surprised by https://github.com/rack/rack/pull/811 and I think this is a completely valid use case.

Also this code ...

class Y
  def initialize
    @x = "ZOMG"
  end

  def print_x
    puts x
  end

  def assign_x
    self.x = "ZOMG ZOMG"
  end

  private

  attr_accessor :x
end

y = Y.new
y.assign_x
y.print_x

Works fine with warnings also. So a private writer works ok when the receiver is self because Ruby has a special case for it, this make me think that private writers were thought to be used.

So ... am I wrong thinking that the warning should be removed or the self special case shouldn't work and be removed from Ruby code?. It doesn't make sense to me to have both things.

Associated revisions

Revision 50585
Added by zzak (Zachary Scott) about 2 years ago

* vm_method.c: Remove private attribute warning [Bug #10967]
Patch by spastorino (Santiago Pastorino) [Fixes GH-849]
https://github.com/ruby/ruby/pull/849

* test/ruby/test_module.rb: Update test for changes

Revision 50585
Added by zzak (Zachary Scott) about 2 years ago

* vm_method.c: Remove private attribute warning [Bug #10967]
Patch by spastorino (Santiago Pastorino) [Fixes GH-849]
https://github.com/ruby/ruby/pull/849

* test/ruby/test_module.rb: Update test for changes

Revision 50585
Added by zzak (Zachary Scott) about 2 years ago

* vm_method.c: Remove private attribute warning [Bug #10967]
Patch by spastorino (Santiago Pastorino) [Fixes GH-849]
https://github.com/ruby/ruby/pull/849

* test/ruby/test_module.rb: Update test for changes

History

#1 Updated by spastorino (Santiago Pastorino) over 2 years ago

One possible fix (the one removing the warning) ... https://github.com/ruby/ruby/pull/849

#2 [ruby-core:69005] Updated by nobu (Nobuyoshi Nakada) about 2 years ago

  • Description updated (diff)

It's valid in your case, but may not in others.
It doesn't sound enough reason to remove that warning to me.

If you want to suppress the warning, you can explicitly do it after the definition.

class Y
  attr_writer :x
  private :x=
end

#3 [ruby-core:69007] Updated by marcandre (Marc-Andre Lafortune) about 2 years ago

FWIW, I feel that warning should be removed. There are too many false positives, and I suspect very very few cases where that warning is of any help. Note that without the warning, it will be obvious anyways when calling that reader/writer that it fails because it is private.

#4 [ruby-core:69042] Updated by spastorino (Santiago Pastorino) about 2 years ago

There's also a discussion going on in another PR, https://github.com/ruby/ruby/pull/889

nobu (Nobuyoshi Nakada), not sure what are the invalid use cases you refer to, and also what if I need to use let's say 2 private accessors? should I do ...

  attr_accessor :a, :b

  private :a, :a=, :b, :b=

Doesn't sound great to me.

#5 [ruby-core:69186] Updated by matz (Yukihiro Matsumoto) about 2 years ago

We haven't thought of self as a receiver. Agreed to remove warnings.

Matz.

#6 Updated by zzak (Zachary Scott) about 2 years ago

  • Status changed from Open to Closed

Applied in changeset r50585.


#7 [ruby-core:69804] Updated by usa (Usaku NAKAMURA) about 2 years ago

  • Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN to 2.0.0: WONTFIX, 2.1: REQUIRED, 2.2: REQUIRED

#8 [ruby-core:69851] Updated by usa (Usaku NAKAMURA) about 2 years ago

Is this a spec change or a bug?

#9 Updated by nagachika (Tomoyuki Chikanaga) almost 2 years ago

  • Backport changed from 2.0.0: WONTFIX, 2.1: REQUIRED, 2.2: REQUIRED to 2.0.0: WONTFIX, 2.1: REQUIRED, 2.2: WONTFIX

Hmm, I don't know it's a bug fix or a spec change, but I think it's a trivial issue. I decided to not to backport to 2.2. Please notice if any objections.

#10 Updated by usa (Usaku NAKAMURA) almost 2 years ago

  • Backport changed from 2.0.0: WONTFIX, 2.1: REQUIRED, 2.2: WONTFIX to 2.0.0: WONTFIX, 2.1: WONTFIX, 2.2: WONTFIX

Ok, I follow you, chikanaga-san!

Also available in: Atom PDF