jruby · Sep 18, 2015 · Sep 18, 2015 · Sep 18, 2015
Showing with 65 additions and 25 deletions.

+19 −22 core/src/main/java/org/jruby/RubyModule.java

+46 −3 test/jruby/test_module.rb
diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
@@ -128,7 +128,6 @@ public class RubyModule extends RubyObject {
 
     private static final boolean DEBUG = false;
     protected static final String ERR_INSECURE_SET_CONSTANT  = "Insecure: can't modify constant";
-    protected static final String ERR_FROZEN_CONST_TYPE = "class/module ";
 
     public static final ObjectAllocator MODULE_ALLOCATOR = new ObjectAllocator() {
         @Override
@@ -3239,10 +3238,7 @@ public IRubyObject private_constant(ThreadContext context, IRubyObject rubyName)
     @JRubyMethod(required = 1, rest = true)
     public IRubyObject private_constant(ThreadContext context, IRubyObject[] rubyNames) {
         for (IRubyObject rubyName : rubyNames) {
-            String name = validateConstant(rubyName);
-
-            setConstantVisibility(context, name, true);
-            invalidateConstantCache(name);
+            private_constant(context, rubyName);
         }
         return this;
     }
@@ -3259,9 +3255,7 @@ public IRubyObject public_constant(ThreadContext context, IRubyObject rubyName)
     @JRubyMethod(required = 1, rest = true)
     public IRubyObject public_constant(ThreadContext context, IRubyObject[] rubyNames) {
         for (IRubyObject rubyName : rubyNames) {
-            String name = validateConstant(rubyName);
-            setConstantVisibility(context, name, false);
-            invalidateConstantCache(name);
+            public_constant(context, rubyName);
         }
         return this;
     }
@@ -3295,7 +3289,7 @@ private void setConstantVisibility(ThreadContext context, String name, boolean h
             throw context.runtime.newNameError("constant " + getName() + "::" + name + " not defined", name);
         }
 
-        getConstantMapForWrite().put(name, new ConstantEntry(entry.value, hidden));
+        storeConstant(name, entry.value, hidden);
     }
 
     //
@@ -3882,7 +3876,6 @@ public List<String> getClassVariableNameList() {
     }
 
     protected static final String ERR_INSECURE_SET_CLASS_VAR = "Insecure: can't modify class variable";
-    protected static final String ERR_FROZEN_CVAR_TYPE = "class/module ";
 
     protected final String validateClassVariable(String name) {
         if (IdUtil.isValidClassVariableName(name)) {
@@ -3892,17 +3885,7 @@ protected final String validateClassVariable(String name) {
     }
 
     protected final void ensureClassVariablesSettable() {
-        Ruby runtime = getRuntime();
-
-        if (!isFrozen()) {
-            return;
-        }
-
-        if (this instanceof RubyModule) {
-            throw runtime.newFrozenError(ERR_FROZEN_CONST_TYPE);
-        } else {
-            throw runtime.newFrozenError("");
-        }
+        checkAndRaiseIfFrozen();
     }
 
     //
@@ -4030,7 +4013,21 @@ protected final String validateConstant(String name, IRubyObject errorName) {
     }
 
     protected final void ensureConstantsSettable() {
-        if (isFrozen()) throw getRuntime().newFrozenError(ERR_FROZEN_CONST_TYPE);
+        checkAndRaiseIfFrozen();
+    }
+
+    private void checkAndRaiseIfFrozen() throws RaiseException {
+        if ( isFrozen() ) {
+            if (this instanceof RubyClass) {
+                if (getBaseName() == null) { // anonymous
+                    // MRI 2.2.2 does get ugly ... as it skips this logic :
+                    // RuntimeError: can't modify frozen #<Class:#<Class:0x0000000095a920>>
+                    throw getRuntime().newFrozenError(getName());
+                }
+                throw getRuntime().newFrozenError("#<Class:" + getName() + '>');
+            }
+            throw getRuntime().newFrozenError("Module");
+        }
     }
 
     protected boolean constantTableContains(String name) {

diff --git a/test/jruby/test_module.rb b/test/jruby/test_module.rb
@@ -1,5 +1,48 @@
-def test_prepend_features_type_error
-  assert_raise(TypeError) do
-    Module.new.instance_eval { prepend_features(1) }
+require 'test/unit'
+require 'test/jruby/test_helper'
+
+class ModuleTest < Test::Unit::TestCase
+  # include TestHelper
+
+  def test_prepend_features_type_error
+    assert_raise(TypeError) do
+      Module.new.instance_eval { prepend_features(1) }
+    end
   end
+
+  module M
+    C = 'public';  public_constant  :C
+    D = 'private'; private_constant :D
+  end
+  M.freeze
+
+  def test_frozen_raises_on_constant_visibility_change
+    assert_raises { M.public_constant :D }
+    begin
+      M.private_constant :C
+    rescue => e
+      assert_match /can\'t modify frozen /, e.message
+    else fail
+    end
+    assert_equal [ :C ], M.constants
+  end
+
+  class C; end
+  C.freeze
+
+  def test_frozen_error_message # matching MRI 2.2 messages
+    begin
+      M.const_set :SOME, 42
+    rescue => e
+      assert_equal "can't modify frozen Module", e.message
+    else fail
+    end
+    begin
+      C.class_variable_set :@@some, 42
+    rescue => e
+      assert_equal "can't modify frozen #<Class:ModuleTest::C>", e.message
+    else fail
+    end
+  end
+
 end