[GR-59866] Add Module#set_temporary_name method

PullRequest: truffleruby/4455

CHANGELOG.md

@@ -59,6 +59,7 @@ Compatibility:
 * Emit warning when `Kernel#format` called with excessive arguments (@andrykonchin).
 * Fix `Integer#ceil` when self is 0 (@andrykonchin).
 * Fix `Module#remove_const` and emit warning when constant is deprecated (@andrykonchin).
+* Add `Module#set_temporary_name` (#3681, @andrykonchin).
 
 Performance:
 

spec/ruby/core/module/fixtures/set_temporary_name.rb

@@ -0,0 +1,4 @@
+module ModuleSpecs
+  module SetTemporaryNameSpec
+  end
+end

spec/ruby/core/module/set_temporary_name_spec.rb

@@ -1,4 +1,5 @@
 require_relative '../../spec_helper'
+require_relative 'fixtures/set_temporary_name'
 
 ruby_version_is "3.3" do
   describe "Module#set_temporary_name" do
@@ -13,13 +14,34 @@
       m.name.should be_nil
     end
 
+    it "returns self" do
+      m = Module.new
+      m.set_temporary_name("fake_name").should.equal? m
+    end
+
     it "can assign a temporary name which is not a valid constant path" do
       m = Module.new
-      m.set_temporary_name("a::B")
-      m.name.should == "a::B"
+
+      m.set_temporary_name("name")
+      m.name.should == "name"
 
       m.set_temporary_name("Template['foo.rb']")
       m.name.should == "Template['foo.rb']"
+
+      m.set_temporary_name("a::B")
+      m.name.should == "a::B"
+
+      m.set_temporary_name("A::b")
+      m.name.should == "A::b"
+
+      m.set_temporary_name("A::B::")
+      m.name.should == "A::B::"
+
+      m.set_temporary_name("A::::B")
+      m.name.should == "A::::B"
+
+      m.set_temporary_name("A=")
+      m.name.should == "A="
     end
 
     it "can't assign empty string as name" do
@@ -43,7 +65,7 @@
       -> { Object.set_temporary_name("fake_name") }.should raise_error(RuntimeError, "can't change permanent name")
     end
 
-    it "can assign a temporary name to a nested module" do
+    it "can assign a temporary name to a module nested into an anonymous module" do
       m = Module.new
       module m::N; end
       m::N.name.should =~ /\A#<Module:0x\h+>::N\z/
@@ -55,6 +77,17 @@ module m::N; end
       m::N.name.should be_nil
     end
 
+    it "discards a temporary name when an outer anonymous module gets a permanent name" do
+      m = Module.new
+      module m::N; end
+
+      m::N.set_temporary_name("fake_name")
+      m::N.name.should == "fake_name"
+
+      ModuleSpecs::SetTemporaryNameSpec::M = m
+      m::N.name.should == "ModuleSpecs::SetTemporaryNameSpec::M::N"
+    end
+
     it "can update the name when assigned to a constant" do
       m = Module.new
       m::N = Module.new
@@ -64,5 +97,45 @@ module m::N; end
       m::M = m::N
       m::M.name.should =~ /\A#<Module:0x\h+>::M\z/m
     end
+
+    it "can reassign a temporary name repeatedly" do
+      m = Module.new
+
+      m.set_temporary_name("fake_name")
+      m.name.should == "fake_name"
+
+      m.set_temporary_name("fake_name_2")
+      m.name.should == "fake_name_2"
+    end
+
+    it "does not affect a name of a module nested into an anonymous module with a temporary name" do
+      m = Module.new
+      m::N = Module.new
+      m::N.name.should =~ /\A#<Module:0x\h+>::N\z/
+
+      m.set_temporary_name("foo")
+      m::N.name.should =~ /\A#<Module:0x\h+>::N\z/
+    end
+
+    it "keeps temporary name when assigned in an anonymous module" do
+      outer = Module.new
+      m = Module.new
+      m.set_temporary_name "m"
+      m.name.should == "m"
+      outer::M = m
+      m.name.should == "m"
+      m.inspect.should == "m"
+    end
+
+    it "keeps temporary name when assigned in an anonymous module and nested before" do
+      outer = Module.new
+      m = Module.new
+      outer::A = m
+      m.set_temporary_name "m"
+      m.name.should == "m"
+      outer::M = m
+      m.name.should == "m"
+      m.inspect.should == "m"
+    end
   end
 end

spec/tags/core/module/set_temporary_name_tags.txt

@@ -1,8 +1,2 @@
-fails:Module#set_temporary_name can assign a temporary name
-fails:Module#set_temporary_name can assign a temporary name which is not a valid constant path
-fails:Module#set_temporary_name can't assign empty string as name
-fails:Module#set_temporary_name can't assign a constant name as a temporary name
-fails:Module#set_temporary_name can't assign a constant path as a temporary name
-fails:Module#set_temporary_name can't assign name to permanent module
-fails:Module#set_temporary_name can assign a temporary name to a nested module
 fails:Module#set_temporary_name can update the name when assigned to a constant
+fails:Module#set_temporary_name does not affect a name of a module nested into an anonymous module with a temporary name

src/main/java/org/truffleruby/core/module/ModuleFields.java

@@ -90,15 +90,39 @@ public static void debugModuleChain(RubyModule module) {
 
     private final PrependMarker start;
 
+    /**
+     * <pre>
+     * Naming modules is complicated, we try to summarize the transitions here.
+     * fully named (simple): module M; end -> module with givenBaseName="M" hasFullName=true
+     * fully named (nested): module N; module M; end; end -> module with name="N::M" hasFullName=true
+     * anonymous: Module.new -> #name is nil, #inspect is #<Module:0x...>
+     * nested-anonymous: m = Module.new; module m::N; end -> givenBaseName="N" hasFullName=false hasPartialName=true #name is "#<Module:0x...>::N"
+     *
+     * Transitions:
+     * * anonymous to nested-anonymous, by constant assignment
+     * * anonymous/nested-anonymous to full, by constant assignment (for anonymous), or lexical parent gets fully named (for nested-anonymous)
+     * * temporary to full, by constant assignment or lexical parent gets fully named
+     * * anonymous/nested-anonymous to temporary, by set_temporary_name("...") or set_temporary_name(nil)
+     * * temporary to temporary, by set_temporary_name("...") or set_temporary_name(nil)
+     * They form a lattice/total order, as it's never possible to go back from a transition
+     *
+     * Note there is no temporary to anonymous/nested-anonymous, it just stays temporary in that case.
+     * </pre>
+     */
+
     private final RubyModule lexicalParent;
-    /** Module (or class) own explicitly specified name */
+    /** Module (or class) own explicitly specified name: either `class Foo` or `module Foo` or a core module/class */
     public final String givenBaseName;
-
     /** Means whether a fully qualified name (with parent lexical scope names) is already set */
     private boolean hasFullName = false;
-    /** Either fully qualified name or lazily evaluated anonymous name */
+    /** Either fully qualified name or lazily evaluated anonymous name, or temporary name */
     private String name = null;
+    /** Same as {@link #name} except null if {@link #hasPartialName()} */
     private ImmutableRubyString rubyStringName;
+    /** A temporary name assigned to an anonymous module */
+    private String temporaryName = null;
+    /** Whether a temporary name is assigned. Is needed to distinguish a case when nil value is assigned. */
+    private boolean isTemporaryNameAssigned = false;
 
     /** Whether this is a refinement module (R), created by #refine */
     private boolean isRefinement = false;
@@ -180,7 +204,7 @@ private void nameModule(
                 nameChildrenModules(context);
             } else {
                 // Our lexicalParent is also an anonymous module
-                // and will name us when it gets named via updateAnonymousChildrenModules(),
+                // and will name us when it gets named via nameChildrenModules(),
                 // or we'll compute an anonymous name on #getName() if needed
             }
         }
@@ -717,11 +741,12 @@ public boolean undefineConstantIfStillAutoload(RubyConstant autoloadConstant) {
     }
 
     public String getName() {
-        final String name = this.name;
+        // Lazily compute the anonymous name because it is expensive
         if (name == null) {
-            // Lazily compute the anonymous name because it is expensive
-            return getAnonymousName();
+            recomputeAnonymousOrTemporaryName();
         }
+
+        assert name != null;
         return name;
     }
 
@@ -737,25 +762,48 @@ public String getSimpleName() {
     }
 
     @TruffleBoundary
-    private String getAnonymousName() {
-        final String anonymousName = createAnonymousName();
-        setName(anonymousName);
-        return anonymousName;
+    private void recomputeAnonymousOrTemporaryName() {
+        if (!isAnonymousOrTemporary()) {
+            return;
+        }
+
+        final String newName;
+        if (temporaryName != null) {
+            newName = temporaryName;
+        } else {
+            newName = createFullyQualifiedAnonymousName();
+        }
+
+        setName(newName);
     }
 
     public void setFullName(String name) {
         assert name != null;
         hasFullName = true;
         setName(name);
+        resetTemporaryName();
     }
 
     private void setName(String name) {
         this.name = name;
         if (hasPartialName()) {
             this.rubyStringName = language.getFrozenStringLiteral(TStringUtils.utf8TString(name), Encodings.UTF_8);
+        } else {
+            this.rubyStringName = null;
         }
     }
 
+    public void setTemporaryName(String name) {
+        this.temporaryName = name;
+        this.isTemporaryNameAssigned = true;
+        recomputeAnonymousOrTemporaryName();
+    }
+
+    private void resetTemporaryName() {
+        temporaryName = null;
+        isTemporaryNameAssigned = false;
+    }
+
     public Object getRubyStringName() {
         if (hasPartialName()) {
             if (rubyStringName == null) {
@@ -769,7 +817,7 @@ public Object getRubyStringName() {
     }
 
     @TruffleBoundary
-    private String createAnonymousName() {
+    private String createFullyQualifiedAnonymousName() {
         if (givenBaseName != null) {
             if (lexicalParent == getObjectClass()) {
                 return givenBaseName;
@@ -804,11 +852,15 @@ public boolean hasFullName() {
         return hasFullName;
     }
 
+    /** Whether Module#name Ruby method returns a value other than nil */
     public boolean hasPartialName() {
+        if (isTemporaryNameAssigned) {
+            return temporaryName != null;
+        }
         return hasFullName() || givenBaseName != null;
     }
 
-    public boolean isAnonymous() {
+    public boolean isAnonymousOrTemporary() {
         return !this.hasFullName;
     }
 

src/main/java/org/truffleruby/core/module/ModuleNodes.java

@@ -645,7 +645,7 @@ public abstract static class IsAnonymousNode extends PrimitiveArrayArgumentsNode
 
         @Specialization
         boolean isAnonymous(RubyModule module) {
-            return module.fields.isAnonymous();
+            return module.fields.isAnonymousOrTemporary();
         }
 
     }
@@ -2296,6 +2296,48 @@ private RubyModule newRefinementModule(RubyModule namespace, RubyModule moduleTo
 
     }
 
+    @CoreMethod(names = "set_temporary_name", required = 1)
+    public abstract static class SetTemporaryNameNode extends CoreMethodArrayArgumentsNode {
+
+        @TruffleBoundary
+        @Specialization
+        RubyModule setTemporaryName(RubyModule self, Object name,
+                @Cached ToJavaStringNode toJavaStringNode) {
+            if (name == nil) {
+                self.fields.setTemporaryName(null);
+                return self;
+            }
+
+            final String string = toJavaStringNode.execute(this, name);
+            validateName(string, self);
+            self.fields.setTemporaryName(string);
+
+            return self;
+        }
+
+        private void validateName(String name, RubyModule self) {
+            if (name.isEmpty()) {
+                throw new RaiseException(
+                        getContext(this),
+                        coreExceptions(this).argumentError("empty class/module name", this));
+            }
+
+            if (self.fields.hasFullName()) {
+                throw new RaiseException(
+                        getContext(this),
+                        coreExceptions(this).runtimeError("can't change permanent name", this));
+            }
+
+            if (Identifiers.isValidConstantPath(name)) {
+                throw new RaiseException(
+                        getContext(this),
+                        coreExceptions(this).argumentError(
+                                "the temporary name must not be a constant path to avoid confusion", this));
+            }
+        }
+
+    }
+
     @GenerateUncached
     @CoreMethod(names = "using", required = 1, visibility = Visibility.PRIVATE, alwaysInlined = true)
     public abstract static class ModuleUsingNode extends UsingNode {

src/main/java/org/truffleruby/parser/Identifiers.java

@@ -44,6 +44,29 @@ public static boolean isValidConstantName(String id) {
         return isConstantFirstCodePoint(first) && isNameString(id, Character.charCount(first));
     }
 
+    /** Check if String is a valid constant path, e.g. A::B or ::A */
+    @TruffleBoundary
+    public static boolean isValidConstantPath(String id) {
+        boolean isConstantPath = true;
+
+        final String[] pathSections = id.split("::", -1);
+        for (int i = 0; i < pathSections.length; i++) {
+            String section = pathSections[i];
+
+            // is a constant path prefixed with '::' (e.g. '::A::B')?
+            if (section.isEmpty() && i == 0) {
+                continue;
+            }
+
+            if (!isValidConstantName(section)) {
+                isConstantPath = false;
+                break;
+            }
+        }
+
+        return isConstantPath;
+    }
+
     @TruffleBoundary
     public static boolean isValidLocalVariableName(String id) {
         return isValidIdentifier(id, 0);