Use Object.defineProperty for methods and props definition (#1821)

Using Object.defineProperty(obj, prop, { enumerable: false }) does not pollute core JS objects. All props are not enumerable anymore.
opal · May 10, 2018 · 34f89df · 34f89df
1 parent 3d856c4
commit 34f89df
Show file tree

Hide file tree

Showing 15 changed files with 450 additions and 248 deletions.
diff --git a/opal/corelib/array.rb b/opal/corelib/array.rb
@@ -5,7 +5,7 @@ class Array < `Array`
   include Enumerable
 
   # Mark all javascript arrays as being valid ruby arrays
-  `def.$$is_array = true`
+  `Opal.defineProperty(Array.prototype, '$$is_array', true)`
 
   %x{
     function toArraySubclass(obj, klass) {

diff --git a/opal/corelib/basic_object.rb b/opal/corelib/basic_object.rb
@@ -13,7 +13,13 @@ def eql?(other)
   alias equal? ==
 
   def __id__
-    `self.$$id || (self.$$id = Opal.uid())`
+    %x{
+      if (self.$$id != null) {
+        return self.$$id;
+      }
+      Opal.defineProperty(self, '$$id', Opal.uid());
+      return self.$$id;
+    }
   end
 
   def __send__(symbol, *args, &block)
@@ -72,7 +78,7 @@ def instance_eval(*args, &block)
       // Need to pass $$eval so that method definitions know if this is
       // being done on a class/module. Cannot be compiler driven since
       // send(:instance_eval) needs to work.
-      if (self.$$is_class || self.$$is_module) {
+      if (self.$$is_a_module) {
         self.$$eval = true;
         try {
           result = block.call(self, self);
@@ -100,7 +106,7 @@ def instance_exec(*args, &block)
 
       block.$$s = null;
 
-      if (self.$$is_class || self.$$is_module) {
+      if (self.$$is_a_module) {
         self.$$eval = true;
         try {
           result = block.apply(self, args);
@@ -128,6 +134,10 @@ def singleton_method_removed(*)
   def singleton_method_undefined(*)
   end
 
+  def class
+    `self.$$class`
+  end
+
   def method_missing(symbol, *args, &block)
     message = if `self.$inspect && !self.$inspect.$$stub`
                 "undefined method `#{symbol}' for #{inspect}:#{`self.$$class`}"

diff --git a/opal/corelib/boolean.rb b/opal/corelib/boolean.rb
@@ -1,6 +1,6 @@
 class Boolean < `Boolean`
-  `def.$$is_boolean = true`
-  `def.$$meta = #{self}`
+  `Opal.defineProperty(Boolean.prototype, '$$is_boolean', true)`
+  `Opal.defineProperty(Boolean.prototype, '$$meta', #{self})`
 
   class << self
     def allocate

diff --git a/opal/corelib/class.rb b/opal/corelib/class.rb
@@ -15,6 +15,7 @@ def self.new(superclass = Object, &block)
 
       superclass.$inherited(klass);
       Opal.module_initialize(klass, block);
+      Opal.refresh_ancestors(klass);
 
       return klass;
     }
@@ -55,7 +56,7 @@ def to_s
     %x{
       var singleton_of = self.$$singleton_of;
 
-      if (singleton_of && (singleton_of.$$is_class || singleton_of.$$is_module)) {
+      if (singleton_of && (singleton_of.$$is_a_module)) {
         return #{"#<Class:#{`singleton_of`.name}>"};
       }
       else if (singleton_of) {

diff --git a/opal/corelib/kernel.rb b/opal/corelib/kernel.rb
@@ -44,22 +44,11 @@ def method(name)
 
   def methods(all = true)
     %x{
-      var methods = [];
-
-      for (var key in self) {
-        if (key[0] == "$" && typeof(self[key]) === "function") {
-          if (all == false || all === nil) {
-            if (!Opal.hasOwnProperty.call(self, key)) {
-              continue;
-            }
-          }
-          if (self[key].$$stub === undefined) {
-            methods.push(key.substr(1));
-          }
-        }
+      if (#{Opal.truthy?(all)}) {
+        return Opal.methods(self);
+      } else {
+        return Opal.own_methods(self);
       }
-
-      return methods;
     }
   end
 
@@ -116,20 +105,23 @@ def copy_instance_variables(other)
 
   def copy_singleton_methods(other)
     %x{
-      var name;
+      var i, name, names, length;
 
       if (other.hasOwnProperty('$$meta')) {
         var other_singleton_class_proto = Opal.get_singleton_class(other).$$proto;
         var self_singleton_class_proto = Opal.get_singleton_class(self).$$proto;
+        names = Object.getOwnPropertyNames(other_singleton_class_proto);
 
-        for (name in other_singleton_class_proto) {
-          if (name.charAt(0) === '$' && other_singleton_class_proto.hasOwnProperty(name)) {
+        for (i = 0, length = names.length; i < length; i++) {
+          name = names[i];
+          if (name.charAt(0) === '$') {
             self_singleton_class_proto[name] = other_singleton_class_proto[name];
           }
         }
       }
 
-      for (name in other) {
+      for (i = 0, names = Object.getOwnPropertyNames(other), length = names.length; i < length; i++) {
+        name = names[i];
         if (name.charAt(0) === '$' && name.charAt(1) !== '$' && other.hasOwnProperty(name)) {
           self[name] = other[name];
         }

diff --git a/opal/corelib/math.rb b/opal/corelib/math.rb
@@ -154,7 +154,7 @@ def cosh(x)
 
   unless defined?(`Math.erf`)
     %x{
-      Math.erf = function(x) {
+      Opal.defineProperty(Math, 'erf', function(x) {
         var A1 =  0.254829592,
             A2 = -0.284496736,
             A3 =  1.421413741,
@@ -174,7 +174,7 @@ def cosh(x)
         var y = 1.0 - (((((A5 * t + A4) * t) + A3) * t + A2) * t + A1) * t * Math.exp(-x * x);
 
         return sign * y;
-      }
+      });
     }
   end
 
@@ -184,7 +184,7 @@ def erf(x)
 
   unless defined?(`Math.erfc`)
     %x{
-      Math.erfc = function(x) {
+      Opal.defineProperty(Math, 'erfc', function(x) {
         var z = Math.abs(x),
             t = 1.0 / (0.5 * z + 1.0);
 
@@ -207,7 +207,7 @@ def erf(x)
         else {
           return a;
         }
-      }
+      });
     }
   end
 

diff --git a/opal/corelib/module.rb b/opal/corelib/module.rb
@@ -132,17 +132,12 @@ def attr_reader(*names)
         })(ivar);
 
         // initialize the instance variable as nil
-        proto[ivar] = nil;
+        Opal.defineProperty(proto, ivar, nil);
 
         body.$$parameters = [];
         body.$$arity = 0;
 
-        if (self.$$is_singleton) {
-          proto.constructor.prototype[id] = body;
-        }
-        else {
-          Opal.defn(self, id, body);
-        }
+        Opal.defn(self, id, body);
       }
     }
 
@@ -170,14 +165,9 @@ def attr_writer(*names)
         body.$$arity = 1;
 
         // initialize the instance variable as nil
-        proto[ivar] = nil;
+        Opal.defineProperty(proto, ivar, nil);
 
-        if (self.$$is_singleton) {
-          proto.constructor.prototype[id] = body;
-        }
-        else {
-          Opal.defn(self, id, body);
-        }
+        Opal.defn(self, id, body);
       }
     }
 
@@ -418,26 +408,13 @@ def include(*mods)
 
   def included_modules
     %x{
-      var results;
+      var results = [];
 
-      var module_chain = function(klass) {
-        var included = [];
+      for (var i = 0, ancestors = self.$$ancestors, length = ancestors.length; i < length; i++) {
+        var ancestor = ancestors[i];
 
-        for (var i = 0, ii = klass.$$inc.length; i < ii; i++) {
-          var mod_or_class = klass.$$inc[i];
-          included.push(mod_or_class);
-          included = included.concat(module_chain(mod_or_class));
-        }
-
-        return included;
-      };
-
-      results = module_chain(self);
-
-      // need superclass's modules
-      if (self.$$is_class) {
-        for (var cls = self; cls; cls = cls.$$super) {
-          results = results.concat(module_chain(cls));
+        if (ancestor !== self && ancestor.$$is_module) {
+          results.push(ancestor);
         }
       }
 
@@ -478,43 +455,11 @@ def instance_method(name)
 
   def instance_methods(include_super = true)
     %x{
-      var value,
-          methods = [],
-          proto   = self.$$proto;
-
-      for (var prop in proto) {
-        if (prop.charAt(0) !== '$' || prop.charAt(1) === '$') {
-          continue;
-        }
-
-        value = proto[prop];
-
-        if (typeof(value) !== "function") {
-          continue;
-        }
-
-        if (value.$$stub) {
-          continue;
-        }
-
-        if (!self.$$is_module) {
-          if (self !== Opal.BasicObject && value === Opal.BasicObject.$$proto[prop]) {
-            continue;
-          }
-
-          if (!include_super && !proto.hasOwnProperty(prop)) {
-            continue;
-          }
-
-          if (!include_super && value.$$donated) {
-            continue;
-          }
-        }
-
-        methods.push(prop.substr(1));
+      if (#{Opal.truthy?(include_super)}) {
+        return Opal.instance_methods(self);
+      } else {
+        return Opal.own_instance_methods(self);
       }
-
-      return methods;
     }
   end
 

diff --git a/opal/corelib/number.rb b/opal/corelib/number.rb
@@ -2,7 +2,7 @@
 
 class Number < Numeric
   Opal.bridge(self, `Number`)
-  `Number.prototype.$$is_number = true`
+  `Opal.defineProperty(Number.prototype, '$$is_number', true)`
   `self.$$is_number_class = true`
 
   class << self

diff --git a/opal/corelib/proc.rb b/opal/corelib/proc.rb
@@ -1,6 +1,6 @@
 class Proc < `Function`
-  `def.$$is_proc = true`
-  `def.$$is_lambda = false`
+  `Opal.defineProperty(Function.prototype, '$$is_proc', true)`
+  `Opal.defineProperty(Function.prototype, '$$is_lambda', false)`
 
   def self.new(&block)
     unless block

diff --git a/opal/corelib/regexp.rb b/opal/corelib/regexp.rb
@@ -5,7 +5,7 @@ class Regexp < `RegExp`
   EXTENDED = 2
   MULTILINE = 4
 
-  `def.$$is_regexp = true`
+  `Opal.defineProperty(RegExp.prototype, '$$is_regexp', true)`
 
   class << self
     def allocate