Browse Source

boot.js review

Fix exotic edge case with setClassConstructor.
More `null`s instead of `nil`s.
Herbert Vojčík 7 years ago
parent
commit
1202e49a1c
3 changed files with 187 additions and 39 deletions
  1. 124 0
      src/Kernel-Tests.js
  2. 27 0
      src/Kernel-Tests.st
  3. 36 39
      support/boot.js

+ 124 - 0
src/Kernel-Tests.js

@@ -2419,6 +2419,130 @@ messageSends: ["copyClass:named:", "javascriptConstructor:", "jsConstructor", "a
 }),
 $globals.ClassTest);
 
+$core.addMethod(
+$core.method({
+selector: "testTrickySetJavaScriptConstructor",
+protocol: 'tests',
+fn: function (){
+var self=this;
+var instance;
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+return $core.withContext(function($ctx1) {
+//>>excludeEnd("ctx");
+var $2,$1,$4,$3,$6,$5,$8,$7;
+self["@theClass"]=$recv(self["@builder"])._copyClass_named_($globals.ObjectMock,"ObjectMock2");
+$recv(self["@theClass"])._javascriptConstructor_(self._trickyJsConstructor());
+$2=$recv(self["@theClass"])._superclass();
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+$ctx1.sendIdx["superclass"]=1;
+//>>excludeEnd("ctx");
+$1=$recv($2).__eq_eq($recv($globals.ObjectMock)._superclass());
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+$ctx1.sendIdx["=="]=1;
+//>>excludeEnd("ctx");
+self._assert_($1);
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+$ctx1.sendIdx["assert:"]=1;
+//>>excludeEnd("ctx");
+$4=$recv(self["@theClass"])._instanceVariableNames();
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+$ctx1.sendIdx["instanceVariableNames"]=1;
+//>>excludeEnd("ctx");
+$3=$recv($4).__eq_eq($recv($globals.ObjectMock)._instanceVariableNames());
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+$ctx1.sendIdx["=="]=2;
+//>>excludeEnd("ctx");
+self._assert_($3);
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+$ctx1.sendIdx["assert:"]=2;
+//>>excludeEnd("ctx");
+self._assert_equals_($recv(self["@theClass"])._name(),"ObjectMock2");
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+$ctx1.sendIdx["assert:equals:"]=1;
+//>>excludeEnd("ctx");
+$6=$recv(self["@theClass"])._package();
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+$ctx1.sendIdx["package"]=1;
+//>>excludeEnd("ctx");
+$5=$recv($6).__eq_eq($recv($globals.ObjectMock)._package());
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+$ctx1.sendIdx["=="]=3;
+//>>excludeEnd("ctx");
+self._assert_($5);
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+$ctx1.sendIdx["assert:"]=3;
+//>>excludeEnd("ctx");
+$8=$recv(self["@theClass"])._methodDictionary();
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+$ctx1.sendIdx["methodDictionary"]=1;
+//>>excludeEnd("ctx");
+$7=$recv($8)._keys();
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+$ctx1.sendIdx["keys"]=1;
+//>>excludeEnd("ctx");
+self._assert_equals_($7,$recv($recv($globals.ObjectMock)._methodDictionary())._keys());
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+$ctx1.sendIdx["assert:equals:"]=2;
+//>>excludeEnd("ctx");
+instance=$recv(self["@theClass"])._new();
+self._assert_($recv($recv(instance)._class()).__eq_eq(self["@theClass"]));
+self._assert_equals_($recv(instance)._value(),(4));
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+$ctx1.sendIdx["assert:equals:"]=3;
+//>>excludeEnd("ctx");
+self._shouldnt_raise_((function(){
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+return $core.withContext(function($ctx2) {
+//>>excludeEnd("ctx");
+return $recv(instance)._foo_((9));
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+}, function($ctx2) {$ctx2.fillBlock({},$ctx1,1)});
+//>>excludeEnd("ctx");
+}),$globals.Error);
+self._assert_equals_($recv(instance)._foo(),(9));
+return self;
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+}, function($ctx1) {$ctx1.fill(self,"testTrickySetJavaScriptConstructor",{instance:instance},$globals.ClassTest)});
+//>>excludeEnd("ctx");
+},
+//>>excludeStart("ide", pragmas.excludeIdeData);
+args: [],
+source: "testTrickySetJavaScriptConstructor\x0a\x09| instance |\x0a\x09theClass := builder copyClass: ObjectMock named: 'ObjectMock2'.\x0a\x09theClass javascriptConstructor: self trickyJsConstructor.\x0a\x09\x22part took from copy class test\x22\x0a\x09self assert: theClass superclass == ObjectMock superclass.\x0a\x09self assert: theClass instanceVariableNames == ObjectMock instanceVariableNames.\x0a\x09self assert: theClass name equals: 'ObjectMock2'.\x0a\x09self assert: theClass package == ObjectMock package.\x0a\x09self assert: theClass methodDictionary keys equals: ObjectMock methodDictionary keys.\x0a\x09\x22testing specific to late-wrapped class\x22\x0a\x09instance := theClass new.\x0a\x09self assert: instance class == theClass.\x0a\x09self assert: instance value equals: 4.\x0a\x09self shouldnt: [ instance foo: 9 ] raise: Error.\x0a\x09self assert: instance foo equals: 9",
+referencedClasses: ["ObjectMock", "Error"],
+//>>excludeEnd("ide");
+messageSends: ["copyClass:named:", "javascriptConstructor:", "trickyJsConstructor", "assert:", "==", "superclass", "instanceVariableNames", "assert:equals:", "name", "package", "keys", "methodDictionary", "new", "class", "value", "shouldnt:raise:", "foo:", "foo"]
+}),
+$globals.ClassTest);
+
+$core.addMethod(
+$core.method({
+selector: "trickyJsConstructor",
+protocol: 'running',
+fn: function (){
+var self=this;
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+return $core.withContext(function($ctx1) {
+//>>excludeEnd("ctx");
+
+		function Foo(){}
+		Foo.prototype.valueOf = function () {return 4;};
+		Foo.prototype._foo = function () {return "bar";};
+		return Foo;
+	;
+return self;
+//>>excludeStart("ctx", pragmas.excludeDebugContexts);
+}, function($ctx1) {$ctx1.fill(self,"trickyJsConstructor",{},$globals.ClassTest)});
+//>>excludeEnd("ctx");
+},
+//>>excludeStart("ide", pragmas.excludeIdeData);
+args: [],
+source: "trickyJsConstructor\x0a\x09<\x0a\x09\x09function Foo(){}\x0a\x09\x09Foo.prototype.valueOf = function () {return 4;};\x0a\x09\x09Foo.prototype._foo = function () {return \x22bar\x22;};\x0a\x09\x09return Foo;\x0a\x09>",
+referencedClasses: [],
+//>>excludeEnd("ide");
+messageSends: []
+}),
+$globals.ClassTest);
+
 
 
 $core.addClass('CollectionTest', $globals.TestCase, ['sampleBlock'], 'Kernel-Tests');

+ 27 - 0
src/Kernel-Tests.st

@@ -447,6 +447,15 @@ setUp
 
 tearDown
 	theClass ifNotNil: [ Smalltalk removeClass: theClass. theClass := nil ]
+!
+
+trickyJsConstructor
+	<
+		function Foo(){}
+		Foo.prototype.valueOf = function () {return 4;};
+		Foo.prototype._foo = function () {return "bar";};
+		return Foo;
+	>
 ! !
 
 !ClassTest methodsFor: 'tests'!
@@ -467,6 +476,24 @@ testSetJavaScriptConstructor
 	self assert: instance value equals: 4.
 	self shouldnt: [ instance foo: 9 ] raise: Error.
 	self assert: instance foo equals: 9
+!
+
+testTrickySetJavaScriptConstructor
+	| instance |
+	theClass := builder copyClass: ObjectMock named: 'ObjectMock2'.
+	theClass javascriptConstructor: self trickyJsConstructor.
+	"part took from copy class test"
+	self assert: theClass superclass == ObjectMock superclass.
+	self assert: theClass instanceVariableNames == ObjectMock instanceVariableNames.
+	self assert: theClass name equals: 'ObjectMock2'.
+	self assert: theClass package == ObjectMock package.
+	self assert: theClass methodDictionary keys equals: ObjectMock methodDictionary keys.
+	"testing specific to late-wrapped class"
+	instance := theClass new.
+	self assert: instance class == theClass.
+	self assert: instance value equals: 4.
+	self shouldnt: [ instance foo: 9 ] raise: Error.
+	self assert: instance foo equals: 9
 ! !
 
 TestCase subclass: #CollectionTest

+ 36 - 39
support/boot.js

@@ -223,7 +223,7 @@ define(['require', './compatibility'], function (require) {
 
         /* Method not implemented handlers */
 
-        var methods = [], methodDict = Object.create(null);
+        var methodDict = Object.create(null);
         this.selectors = [];
         this.jsSelectors = [];
 
@@ -235,12 +235,10 @@ define(['require', './compatibility'], function (require) {
             this.jsSelectors.push(jsSelector);
             method = {jsSelector: jsSelector, fn: createHandler(stSelector)};
             methodDict[stSelector] = method;
-            methods.push(method);
             manip.installMethod(method, rootAsClass);
             targetClasses.forEach(function (target) {
                 manip.installMethod(method, target);
             });
-            return method;
         };
 
         /* Dnu handler method */
@@ -286,13 +284,17 @@ define(['require', './compatibility'], function (require) {
             var myproto = klass.fn.prototype,
                 superproto = superclass.fn.prototype;
             dnu.jsSelectors.forEach(function (selector) {
-                if (!localMethodsByJsSelector[selector]) {
+                var method = localMethodsByJsSelector[selector];
+                if (!method) {
                     manip.installMethod({
                         jsSelector: selector,
                         fn: superproto[selector]
                     }, klass);
-                } else if (!myproto[selector]) {
-                    manip.installMethod(localMethodsByJsSelector[selector], klass);
+                } else if (method.fn !== myproto[selector]) {
+                    if (myproto[selector]) {
+                        console.warn("Amber forcefully rewriting method " + selector + " of " + klass.className + ".");
+                    }
+                    manip.installMethod(method, klass);
                 }
             });
         }
@@ -307,12 +309,10 @@ define(['require', './compatibility'], function (require) {
         };
     }
 
-
     function PackagesBrik(brikz, st) {
 
         var org = brikz.ensure("organize");
         var root = brikz.ensure("root");
-        var nil = root.nil;
         var SmalltalkObject = root.Object;
 
         function SmalltalkPackage() {
@@ -343,7 +343,7 @@ define(['require', './compatibility'], function (require) {
 
         st.addPackage = function (pkgName, properties) {
             if (!pkgName) {
-                return nil;
+                return null;
             }
             if (!(st.packages[pkgName])) {
                 st.packages[pkgName] = pkg({
@@ -364,7 +364,6 @@ define(['require', './compatibility'], function (require) {
         var org = brikz.ensure("organize");
         var root = brikz.ensure("root");
         var classInit = brikz.ensure("classInit");
-        var nil = root.nil;
         var rootAsClass = root.rootAsClass;
         var SmalltalkObject = root.Object;
         rootAsClass.klass = {fn: SmalltalkClass};
@@ -382,6 +381,10 @@ define(['require', './compatibility'], function (require) {
         inherits(SmalltalkClass, SmalltalkBehavior);
         inherits(SmalltalkMetaclass, SmalltalkBehavior);
 
+        SmalltalkBehavior.prototype.toString = function () {
+            return 'Smalltalk ' + this.className;
+        };
+
         SmalltalkMetaclass.prototype.meta = true;
 
         this.__init__ = function () {
@@ -440,10 +443,6 @@ define(['require', './compatibility'], function (require) {
             return that;
         }
 
-        SmalltalkBehavior.prototype.toString = function () {
-            return 'Smalltalk ' + this.className;
-        };
-
         function wireKlass(klass) {
             Object.defineProperty(klass.fn.prototype, "klass", {
                 value: klass,
@@ -470,7 +469,7 @@ define(['require', './compatibility'], function (require) {
         st.addClass = function (className, superclass, iVarNames, pkgName) {
             // While subclassing nil is allowed, it might be an error, so
             // warn about it.
-            if (typeof superclass == 'undefined' || superclass == nil) {
+            if (typeof superclass == 'undefined' || superclass && superclass.isNil) {
                 console.warn('Compiling ' + className + ' as a subclass of `nil`. A dependency might be missing.');
             }
             rawAddClass(pkgName, className, superclass, iVarNames, false, null);
@@ -483,7 +482,7 @@ define(['require', './compatibility'], function (require) {
                 throw new Error("Missing package " + pkgName);
             }
 
-            if (!superclass || superclass == nil) {
+            if (superclass == null || superclass.isNil) {
                 superclass = null;
             }
             var theClass = globals.hasOwnProperty(className) && globals[className];
@@ -572,7 +571,7 @@ define(['require', './compatibility'], function (require) {
             return classes;
         };
 
-        st.wrappedClasses = function () {
+        this.wrappedClasses = function () {
             return wrappedClasses;
         };
 
@@ -592,7 +591,7 @@ define(['require', './compatibility'], function (require) {
         var dnu = brikz.ensure("dnu");
         var SmalltalkObject = brikz.ensure("root").Object;
         brikz.ensure("selectorConversion");
-        brikz.ensure("classes");
+        var classBrik = brikz.ensure("classes");
 
         function SmalltalkMethod() {
         }
@@ -641,7 +640,7 @@ define(['require', './compatibility'], function (require) {
             propagateMethodChange(klass, method);
 
             var usedSelectors = method.messageSends,
-                targetClasses = stInit.initialized() ? st.wrappedClasses() : [];
+                targetClasses = stInit.initialized() ? classBrik.wrappedClasses() : [];
 
             dnu.make(method.selector, targetClasses);
 
@@ -850,7 +849,6 @@ define(['require', './compatibility'], function (require) {
 
         brikz.ensure("selectorConversion");
         var root = brikz.ensure("root");
-        var nil = root.nil;
         var SmalltalkObject = root.Object;
 
         function SmalltalkMethodContext(home, setup) {
@@ -934,7 +932,7 @@ define(['require', './compatibility'], function (require) {
          */
         st.seamless = function (worker) {
             return inContext(worker, function (ctx) {
-                ctx.fill(nil, "seamlessDoIt", {}, globals.UndefinedObject);
+                ctx.fill(null, "seamlessDoIt", {}, globals.UndefinedObject);
             });
         };
 
@@ -1014,7 +1012,7 @@ define(['require', './compatibility'], function (require) {
                 thisContext.init();
                 return thisContext;
             } else {
-                return nil;
+                return null;
             }
         };
     }
@@ -1079,7 +1077,7 @@ define(['require', './compatibility'], function (require) {
                 return propertyValue.apply(receiver, args || []);
             } else if (args.length > 0) {
                 receiver[propertyName] = args[0];
-                return nil;
+                return receiver;
             } else {
                 return propertyValue;
             }
@@ -1093,22 +1091,21 @@ define(['require', './compatibility'], function (require) {
 
         /* Convert a Smalltalk selector into a JS selector */
         st.st2js = function (string) {
-            var selector = '_' + string;
-            selector = selector.replace(/:/g, '_');
-            selector = selector.replace(/[\&]/g, '_and');
-            selector = selector.replace(/[\|]/g, '_or');
-            selector = selector.replace(/[+]/g, '_plus');
-            selector = selector.replace(/-/g, '_minus');
-            selector = selector.replace(/[*]/g, '_star');
-            selector = selector.replace(/[\/]/g, '_slash');
-            selector = selector.replace(/[\\]/g, '_backslash');
-            selector = selector.replace(/[\~]/g, '_tild');
-            selector = selector.replace(/>/g, '_gt');
-            selector = selector.replace(/</g, '_lt');
-            selector = selector.replace(/=/g, '_eq');
-            selector = selector.replace(/,/g, '_comma');
-            selector = selector.replace(/[@]/g, '_at');
-            return selector;
+            return '_' + string
+                    .replace(/:/g, '_')
+                    .replace(/[\&]/g, '_and')
+                    .replace(/[\|]/g, '_or')
+                    .replace(/[+]/g, '_plus')
+                    .replace(/-/g, '_minus')
+                    .replace(/[*]/g, '_star')
+                    .replace(/[\/]/g, '_slash')
+                    .replace(/[\\]/g, '_backslash')
+                    .replace(/[\~]/g, '_tild')
+                    .replace(/>/g, '_gt')
+                    .replace(/</g, '_lt')
+                    .replace(/=/g, '_eq')
+                    .replace(/,/g, '_comma')
+                    .replace(/[@]/g, '_at');
         };
 
         /* Convert a string to a valid smalltalk selector.