* error.c (rb_name_error_str): new function to raise NameError

  with the name string but not ID.
* object.c, proc.c, variable.c: more removal of inadvertent symbol
  creation.  [Feature #5079]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@32645 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

7 files changed, 167 insertions, 14 deletions

diff --git a/ChangeLog b/ChangeLog
index 7661cd7f78..910a248bb8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+Sun Jul 24 00:05:00 2011  Jeremy Evans  <merch-redmine@jeremyevans.net>
+
+	* error.c (rb_name_error_str): new function to raise NameError
+	  with the name string but not ID.
+
+	* object.c, proc.c, variable.c: more removal of inadvertent symbol
+	  creation.  [Feature #5079]
+
 Sat Jul 23 21:14:00 2011  Tadayoshi Funaba  <tadf@dotrb.org>
 
 	* lib/cmath.rb (cbrt): should return a real number if possible.
diff --git a/error.c b/error.c
index 42f83682ea..834eede8af 100644
--- a/error.c
+++ b/error.c
@@ -812,6 +812,21 @@ rb_name_error(ID id, const char *fmt, ...)
     rb_exc_raise(exc);
 }
 
+void
+rb_name_error_str(VALUE str, const char *fmt, ...)
+{
+    VALUE exc, argv[2];
+    va_list args;
+
+    va_start(args, fmt);
+    argv[0] = rb_vsprintf(fmt, args);
+    va_end(args);
+
+    argv[1] = str;
+    exc = rb_class_new_instance(2, argv, rb_eNameError);
+    rb_exc_raise(exc);
+}
+
 /*
  * call-seq:
  *   NameError.new(msg [, name])  -> name_error
diff --git a/include/ruby/intern.h b/include/ruby/intern.h
index 71896ce2a4..27db84e2c6 100644
--- a/include/ruby/intern.h
+++ b/include/ruby/intern.h
@@ -209,6 +209,7 @@ VALUE rb_exc_new2(VALUE, const char*);
 VALUE rb_exc_new3(VALUE, VALUE);
 PRINTF_ARGS(NORETURN(void rb_loaderror(const char*, ...)), 1, 2);
 PRINTF_ARGS(NORETURN(void rb_name_error(ID, const char*, ...)), 2, 3);
+PRINTF_ARGS(NORETURN(void rb_name_error_str(VALUE, const char*, ...)), 2, 3);
 NORETURN(void rb_invalid_str(const char*, const char*));
 PRINTF_ARGS(void rb_compile_error(const char*, int, const char*, ...), 3, 4);
 PRINTF_ARGS(void rb_compile_error_with_enc(const char*, int, void *, const char*, ...), 4, 5);
diff --git a/object.c b/object.c
index 42d9d2509b..cad41e686c 100644
--- a/object.c
+++ b/object.c
@@ -1833,8 +1833,14 @@ rb_mod_const_defined(int argc, VALUE *argv, VALUE mod)
     else {
 	rb_scan_args(argc, argv, "11", &name, &recur);
     }
-    if (!(id = rb_check_id(name)) && rb_is_const_name(name))
-	return Qfalse;
+    if (!(id = rb_check_id(name))) {
+	if (rb_is_const_name(name)) {
+	    return Qfalse;
+	}
+	else {
+	    rb_name_error_str(name, "wrong constant name %s", RSTRING_PTR(name));
+	}
+    }
     if (!rb_is_const_id(id)) {
 	rb_name_error(id, "wrong constant name %s", rb_id2name(id));
     }
@@ -1864,8 +1870,16 @@ rb_mod_const_defined(int argc, VALUE *argv, VALUE mod)
 static VALUE
 rb_obj_ivar_get(VALUE obj, VALUE iv)
 {
-    ID id = rb_to_id(iv);
+    ID id = rb_check_id(iv);
 
+    if (!id) {
+	if (rb_is_instance_name(iv)) {
+	    return Qnil;
+	}
+	else {
+	    rb_name_error_str(iv, "`%s' is not allowed as an instance variable name", RSTRING_PTR(iv));
+	}
+    }
     if (!rb_is_instance_id(id)) {
 	rb_name_error(id, "`%s' is not allowed as an instance variable name", rb_id2name(id));
     }
@@ -1926,7 +1940,14 @@ rb_obj_ivar_defined(VALUE obj, VALUE iv)
 {
     ID id = rb_check_id(iv);
 
-    if (!id && rb_is_instance_name(iv)) return Qfalse;
+    if (!id) {
+	if (rb_is_instance_name(iv)) {
+	    return Qfalse;
+	}
+	else {
+	    rb_name_error_str(iv, "`%s' is not allowed as an instance variable name", RSTRING_PTR(iv));
+	}
+    }
     if (!rb_is_instance_id(id)) {
 	rb_name_error(id, "`%s' is not allowed as an instance variable name", rb_id2name(id));
     }
@@ -1950,8 +1971,17 @@ rb_obj_ivar_defined(VALUE obj, VALUE iv)
 static VALUE
 rb_mod_cvar_get(VALUE obj, VALUE iv)
 {
-    ID id = rb_to_id(iv);
+    ID id = rb_check_id(iv);
 
+    if (!id) {
+	if (rb_is_class_name(iv)) {
+	    rb_name_error_str(iv, "uninitialized class variable %s in %s",
+			      RSTRING_PTR(iv), rb_class2name(obj));
+	}
+	else {
+	    rb_name_error_str(iv, "`%s' is not allowed as a class variable name", RSTRING_PTR(iv));
+	}
+    }
     if (!rb_is_class_id(id)) {
 	rb_name_error(id, "`%s' is not allowed as a class variable name", rb_id2name(id));
     }
@@ -2006,7 +2036,14 @@ rb_mod_cvar_defined(VALUE obj, VALUE iv)
 {
     ID id = rb_check_id(iv);
 
-    if (!id && rb_is_class_name(iv)) return Qfalse;
+    if (!id) {
+	if (rb_is_class_name(iv)) {
+	    return Qfalse;
+	}
+	else {
+	    rb_name_error_str(iv, "`%s' is not allowed as a class variable name", RSTRING_PTR(iv));
+	}
+    }
     if (!rb_is_class_id(id)) {
 	rb_name_error(id, "`%s' is not allowed as a class variable name", rb_id2name(id));
     }
diff --git a/proc.c b/proc.c
index d8077e7ef4..46296cc4c8 100644
--- a/proc.c
+++ b/proc.c
@@ -28,6 +28,7 @@ VALUE rb_cProc;
 
 static VALUE bmcall(VALUE, VALUE);
 static int method_arity(VALUE);
+static ID attached;
 
 /* Proc */
 
@@ -1139,6 +1140,29 @@ method_owner(VALUE obj)
     return data->me.klass;
 }
 
+static void
+rb_method_name_error(VALUE klass, VALUE str)
+{
+    const char *s0 = " class";
+    VALUE c = klass;
+
+    if (FL_TEST(c, FL_SINGLETON)) {
+	VALUE obj = rb_ivar_get(klass, attached);
+
+	switch (TYPE(obj)) {
+	  case T_MODULE:
+	  case T_CLASS:
+	    c = obj;
+	    s0 = "";
+	}
+    }
+    else if (RB_TYPE_P(c, T_MODULE)) {
+	s0 = " module";
+    }
+    rb_name_error_str(str, "undefined method `%s' for%s `%s'",
+		      RSTRING_PTR(str), s0, rb_class2name(c));
+}
+
 /*
  *  call-seq:
  *     obj.method(sym)    -> method
@@ -1170,7 +1194,11 @@ method_owner(VALUE obj)
 VALUE
 rb_obj_method(VALUE obj, VALUE vid)
 {
-    return mnew(CLASS_OF(obj), obj, rb_to_id(vid), rb_cMethod, FALSE);
+    ID id = rb_check_id(vid);
+    if (!id) {
+	rb_method_name_error(CLASS_OF(obj), vid);
+    }
+    return mnew(CLASS_OF(obj), obj, id, rb_cMethod, FALSE);
 }
 
 /*
@@ -1183,7 +1211,11 @@ rb_obj_method(VALUE obj, VALUE vid)
 VALUE
 rb_obj_public_method(VALUE obj, VALUE vid)
 {
-    return mnew(CLASS_OF(obj), obj, rb_to_id(vid), rb_cMethod, TRUE);
+    ID id = rb_check_id(vid);
+    if (!id) {
+	rb_method_name_error(CLASS_OF(obj), vid);
+    }
+    return mnew(CLASS_OF(obj), obj, id, rb_cMethod, TRUE);
 }
 
 /*
@@ -1220,7 +1252,11 @@ rb_obj_public_method(VALUE obj, VALUE vid)
 static VALUE
 rb_mod_instance_method(VALUE mod, VALUE vid)
 {
-    return mnew(mod, Qundef, rb_to_id(vid), rb_cUnboundMethod, FALSE);
+    ID id = rb_check_id(vid);
+    if (!id) {
+	rb_method_name_error(mod, vid);
+    }
+    return mnew(mod, Qundef, id, rb_cUnboundMethod, FALSE);
 }
 
 /*
@@ -1233,7 +1269,11 @@ rb_mod_instance_method(VALUE mod, VALUE vid)
 static VALUE
 rb_mod_public_instance_method(VALUE mod, VALUE vid)
 {
-    return mnew(mod, Qundef, rb_to_id(vid), rb_cUnboundMethod, TRUE);
+    ID id = rb_check_id(vid);
+    if (!id) {
+	rb_method_name_error(mod, vid);
+    }
+    return mnew(mod, Qundef, id, rb_cUnboundMethod, TRUE);
 }
 
 /*
@@ -1747,7 +1787,7 @@ method_inspect(VALUE method)
     rb_str_buf_cat2(str, ": ");
 
     if (FL_TEST(data->me.klass, FL_SINGLETON)) {
-	VALUE v = rb_iv_get(data->me.klass, "__attached__");
+	VALUE v = rb_ivar_get(data->me.klass, attached);
 
 	if (data->recv == Qundef) {
 	    rb_str_buf_append(str, rb_inspect(data->me.klass));
@@ -2234,5 +2274,6 @@ Init_Binding(void)
     rb_define_method(rb_cBinding, "dup", binding_dup, 0);
     rb_define_method(rb_cBinding, "eval", bind_eval, -1);
     rb_define_global_function("binding", rb_f_binding, 0);
+    attached = rb_intern("__attached__");
 }
 
diff --git a/test/ruby/test_symbol.rb b/test/ruby/test_symbol.rb
index 2a21c78540..0113504b14 100644
--- a/test/ruby/test_symbol.rb
+++ b/test/ruby/test_symbol.rb
@@ -172,4 +172,29 @@ class TestSymbol < Test::Unit::TestCase
       assert !Symbol.all_symbols.any? {|sym| sym.to_s == str}, msg
     end
   end
+
+  def test_no_inadvertent_symbol_creation2
+    feature5079 = '[ruby-core:38404]'
+    c = Class.new
+    s = "gadzoooks"
+    {:instance_variable_get => ["@#{s}1", nil],
+     :class_variable_get => ["@@#{s}1", NameError],
+     :remove_instance_variable => ["@#{s}2", NameError],
+     :remove_class_variable => ["@@#{s}2", NameError],
+     :remove_const => ["A#{s}", NameError],
+     :method => ["#{s}1", NameError],
+     :public_method => ["#{s}2", NameError],
+     :instance_method => ["#{s}3", NameError],
+     :public_instance_method => ["#{s}4", NameError],
+    }.each do |meth, arr|
+      str, ret = arr
+      msg = "#{meth}(#{str}) #{feature5079}"
+      if ret.is_a?(Class) && (ret < Exception)
+        assert_raises(ret){c.send(meth, str)}
+      else
+        assert(c.send(meth, str) == ret, msg)
+      end
+      assert !Symbol.all_symbols.any? {|sym| sym.to_s == str}, msg
+    end
+  end
 end
diff --git a/variable.c b/variable.c
index 251fb3bafb..8a34d2e49d 100644
--- a/variable.c
+++ b/variable.c
@@ -1299,7 +1299,7 @@ VALUE
 rb_obj_remove_instance_variable(VALUE obj, VALUE name)
 {
     VALUE val = Qnil;
-    const ID id = rb_to_id(name);
+    const ID id = rb_check_id(name);
     st_data_t n, v;
     struct st_table *iv_index_tbl;
     st_data_t index;
@@ -1307,6 +1307,14 @@ rb_obj_remove_instance_variable(VALUE obj, VALUE name)
     if (!OBJ_UNTRUSTED(obj) && rb_safe_level() >= 4)
 	rb_raise(rb_eSecurityError, "Insecure: can't modify instance variable");
     rb_check_frozen(obj);
+    if (!id) {
+	if (rb_is_instance_name(name)) {
+	    rb_name_error_str(name, "instance variable %s not defined", RSTRING_PTR(name));
+	}
+	else {
+	    rb_name_error_str(name, "`%s' is not allowed as an instance variable name", RSTRING_PTR(name));
+	}
+    }
     if (!rb_is_instance_id(id)) {
 	rb_name_error(id, "`%s' is not allowed as an instance variable name", rb_id2name(id));
     }
@@ -1677,8 +1685,17 @@ rb_public_const_get_at(VALUE klass, ID id)
 VALUE
 rb_mod_remove_const(VALUE mod, VALUE name)
 {
-    const ID id = rb_to_id(name);
+    const ID id = rb_check_id(name);
 
+    if (!id) {
+	if (rb_is_const_name(name)) {
+	    rb_name_error_str(name, "constant %s::%s not defined",
+			      rb_class2name(mod), RSTRING_PTR(name));
+	}
+	else {
+	    rb_name_error_str(name, "`%s' is not allowed as a constant name", RSTRING_PTR(name));
+	}
+    }
     if (!rb_is_const_id(id)) {
 	rb_name_error(id, "`%s' is not allowed as a constant name", rb_id2name(id));
     }
@@ -2189,9 +2206,18 @@ rb_mod_class_variables(VALUE obj)
 VALUE
 rb_mod_remove_cvar(VALUE mod, VALUE name)
 {
-    const ID id = rb_to_id(name);
+    const ID id = rb_check_id(name);
     st_data_t val, n = id;
 
+    if (!id) {
+	if (rb_is_class_name(name)) {
+	    rb_name_error_str(name, "class variable %s not defined for %s",
+			      RSTRING_PTR(name), rb_class2name(mod));
+	}
+	else {
+	    rb_name_error_str(name, "wrong class variable name %s", RSTRING_PTR(name));
+	}
+    }
     if (!rb_is_class_id(id)) {
 	rb_name_error(id, "wrong class variable name %s", rb_id2name(id));
     }