1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2022-11-09 12:17:21 -05:00

Turn class variable warnings into exceptions

This changes the following warnings:

* warning: class variable access from toplevel
* warning: class variable @foo of D is overtaken by C

into RuntimeErrors.  Handle defined?(@@foo) at toplevel
by returning nil instead of raising an exception (the previous
behavior warned before returning nil when defined? was used).

Refactor the specs to avoid the warnings even in older versions.
The specs were checking for the warnings, but the purpose of
the related specs as evidenced from their description is to
test for behavior, not for warnings.

Fixes [Bug #14541]
This commit is contained in:
Jeremy Evans 2020-03-27 15:08:52 -07:00
parent defc0ee9d1
commit 900e83b501
Notes: git 2020-04-10 16:29:30 +09:00
9 changed files with 37 additions and 34 deletions

View file

@ -250,7 +250,9 @@ assert_equal 'ok', %q{
assert_equal 'ok', %q{
begin
12.instance_eval { @@a }
class A
12.instance_eval { @@a }
end
rescue NameError
:ok
end
@ -258,7 +260,9 @@ assert_equal 'ok', %q{
assert_equal 'ok', %q{
begin
12.instance_exec { @@a }
class A
12.instance_exec { @@a }
end
rescue NameError
:ok
end

View file

@ -64,8 +64,8 @@ tests = [
[ 'setinstancevariable', %q{ @x = true }, ],
[ 'getinstancevariable', %q{ @x = true; @x }, ],
[ 'setclassvariable', %q{ @@x = true }, ],
[ 'getclassvariable', %q{ @@x = true; @@x }, ],
[ 'setclassvariable', %q{ class A; @@x = true; end }, ],
[ 'getclassvariable', %q{ class A; @@x = true; @@x end }, ],
[ 'setconstant', %q{ X = true }, ],
[ 'setconstant', %q{ Object::X = true }, ],

View file

@ -268,8 +268,10 @@ assert_equal %q{}, %q{
defined?(@@a)
}
assert_equal %q{class variable}, %q{
@@a = 1
defined?(@@a)
class A
@@a = 1
defined?(@@a)
end
}
assert_equal %q{}, %q{
defined?($a)

View file

@ -236,7 +236,7 @@ getclassvariable
/* "class variable access from toplevel" warning can be hooked. */
// attr bool leaf = false; /* has rb_warning() */
{
val = rb_cvar_get(vm_get_cvar_base(vm_get_cref(GET_EP()), GET_CFP()), id);
val = rb_cvar_get(vm_get_cvar_base(vm_get_cref(GET_EP()), GET_CFP(), 1), id);
}
/* Set value of class variable id of klass as val. */
@ -249,7 +249,7 @@ setclassvariable
// attr bool leaf = false; /* has rb_warning() */
{
vm_ensure_not_refinement_module(GET_SELF());
rb_cvar_set(vm_get_cvar_base(vm_get_cref(GET_EP()), GET_CFP()), id, val);
rb_cvar_set(vm_get_cvar_base(vm_get_cref(GET_EP()), GET_CFP(), 1), id, val);
}
/* Get constant variable id. If klass is Qnil and allow_nil is Qtrue, constants

View file

@ -21,9 +21,7 @@ describe "NameError#name" do
it "returns a class variable name as a symbol" do
-> {
-> {
@@doesnt_exist
}.should complain(/class variable access from toplevel/)
eval("class singleton_class::A; @@doesnt_exist end", binding, __FILE__, __LINE__)
}.should raise_error(NameError) { |e| e.name.should == :@@doesnt_exist }
end

View file

@ -28,10 +28,8 @@ describe "NameError#receiver" do
it "returns the Object class when an undefined class variable is called" do
-> {
-> {
@@doesnt_exist
}.should complain(/class variable access from toplevel/)
}.should raise_error(NameError) {|e| e.receiver.should equal(Object) }
eval("class singleton_class::A; @@doesnt_exist end", binding, __FILE__, __LINE__)
}.should raise_error(NameError) {|e| e.receiver.should equal(singleton_class::A) }
end
it "returns a class when an undefined class variable is called in a subclass' namespace" do

View file

@ -275,9 +275,7 @@ describe "The defined? keyword for an expression" do
end
it "returns nil for an expression with '!' and an unset class variable" do
-> {
@result = defined?(!@@defined_specs_undefined_class_variable)
}.should complain(/class variable access from toplevel/)
@result = eval("class singleton_class::A; defined?(!@@doesnt_exist) end", binding, __FILE__, __LINE__)
@result.should be_nil
end
@ -286,9 +284,7 @@ describe "The defined? keyword for an expression" do
end
it "returns nil for an expression with 'not' and an unset class variable" do
-> {
@result = defined?(not @@defined_specs_undefined_class_variable)
}.should complain(/class variable access from toplevel/)
@result = eval("class singleton_class::A; defined?(not @@doesnt_exist) end", binding, __FILE__, __LINE__)
@result.should be_nil
end
@ -897,17 +893,21 @@ describe "The defined? keyword for a variable scoped constant" do
end
it "returns nil if the class scoped constant is not defined" do
-> {
@@defined_specs_obj = DefinedSpecs::Basic
defined?(@@defined_specs_obj::Undefined).should be_nil
}.should complain(/class variable access from toplevel/)
eval(<<-END, binding, __FILE__, __LINE__)
class singleton_class::A
@@defined_specs_obj = DefinedSpecs::Basic
defined?(@@defined_specs_obj::Undefined).should be_nil
end
END
end
it "returns 'constant' if the constant is defined in the scope of the class variable" do
-> {
@@defined_specs_obj = DefinedSpecs::Basic
defined?(@@defined_specs_obj::A).should == "constant"
}.should complain(/class variable access from toplevel/)
eval(<<-END, binding, __FILE__, __LINE__)
class singleton_class::A
@@defined_specs_obj = DefinedSpecs::Basic
defined?(@@defined_specs_obj::A).should == "constant"
end
END
end
it "returns nil if the local scoped constant is not defined" do

View file

@ -3064,7 +3064,8 @@ cvar_overtaken(VALUE front, VALUE target, ID id)
st_data_t did = (st_data_t)id;
if (RTEST(ruby_verbose) && original_module(front) != original_module(target)) {
rb_warning("class variable % "PRIsVALUE" of %"PRIsVALUE" is overtaken by %"PRIsVALUE"",
rb_raise(rb_eRuntimeError,
"class variable % "PRIsVALUE" of %"PRIsVALUE" is overtaken by %"PRIsVALUE"",
ID2SYM(id), rb_class_name(original_module(front)),
rb_class_name(original_module(target)));
}

View file

@ -989,7 +989,7 @@ vm_get_ev_const(rb_execution_context_t *ec, VALUE orig_klass, ID id, bool allow_
}
static inline VALUE
vm_get_cvar_base(const rb_cref_t *cref, rb_control_frame_t *cfp)
vm_get_cvar_base(const rb_cref_t *cref, rb_control_frame_t *cfp, int top_level_raise)
{
VALUE klass;
@ -1002,8 +1002,8 @@ vm_get_cvar_base(const rb_cref_t *cref, rb_control_frame_t *cfp)
CREF_PUSHED_BY_EVAL(cref))) {
cref = CREF_NEXT(cref);
}
if (!CREF_NEXT(cref)) {
rb_warn("class variable access from toplevel");
if (top_level_raise && !CREF_NEXT(cref)) {
rb_raise(rb_eRuntimeError, "class variable access from toplevel");
}
klass = vm_get_iclass(cfp, CREF_CLASS(cref));
@ -3567,7 +3567,7 @@ vm_defined(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, rb_num_t op_
break;
case DEFINED_CVAR: {
const rb_cref_t *cref = vm_get_cref(GET_EP());
klass = vm_get_cvar_base(cref, GET_CFP());
klass = vm_get_cvar_base(cref, GET_CFP(), 0);
if (rb_cvar_defined(klass, SYM2ID(obj))) {
expr_type = DEFINED_CVAR;
}