diff --git a/api-test.c b/api-test.c index ef38a7f98..6f6428b41 100644 --- a/api-test.c +++ b/api-test.c @@ -928,6 +928,88 @@ static void new_errors(void) JS_FreeRuntime(rt); } +// Constructing something that is not a constructor reports "not a +// constructor", never "not a function"; the latter is what *calling* a +// non-callable reports. +static void construct_not_a_constructor(void) +{ + JSValue not_objects[6]; + JSValue obj, exc, ret; + JSClassID class_id; + const char *s; + size_t i; + + // a class without a .call handler; an object of that class carrying the + // constructor bit is a constructor as far as the object header goes, but + // there is nothing to call + JSClassDef def = (JSClassDef){ + .class_name = "NoCall", + }; + JSRuntime *rt = new_runtime(); + class_id = 0; + JS_NewClassID(rt, &class_id); + assert(JS_NewClass(rt, class_id, &def) == 0); + JSContext *ctx = JS_NewContext(rt); + + obj = JS_NewObjectClass(ctx, class_id); + assert(JS_IsObject(obj)); + assert(JS_SetConstructorBit(ctx, obj, true)); + assert(JS_IsConstructor(ctx, obj)); + + ret = JS_CallConstructor(ctx, obj, 0, NULL); + assert(JS_IsException(ret)); + JS_FreeValue(ctx, ret); + exc = JS_GetException(ctx); + s = JS_ToCString(ctx, exc); + assert(s); + assert(!strcmp(s, "TypeError: not a constructor")); + JS_FreeCString(ctx, s); + JS_FreeValue(ctx, exc); + + // the same object reached through the interpreter's `new` + JSValue global = JS_GetGlobalObject(ctx); + JS_SetPropertyStr(ctx, global, "nocall", obj); // takes ownership + JS_FreeValue(ctx, global); + ret = eval(ctx, "try { new nocall() } catch (e) { `${e}` }"); + assert(!JS_IsException(ret)); + s = JS_ToCString(ctx, ret); + assert(s); + assert(!strcmp(s, "TypeError: not a constructor")); + JS_FreeCString(ctx, s); + JS_FreeValue(ctx, ret); + + // and constructing a non-object + not_objects[0] = JS_UNDEFINED; + not_objects[1] = JS_NULL; + not_objects[2] = JS_TRUE; + not_objects[3] = JS_FALSE; + not_objects[4] = JS_NewInt32(ctx, 42); + not_objects[5] = JS_NewFloat64(ctx, 1.5); + for (i = 0; i < countof(not_objects); i++) { + ret = JS_CallConstructor(ctx, not_objects[i], 0, NULL); + assert(JS_IsException(ret)); + JS_FreeValue(ctx, ret); + exc = JS_GetException(ctx); + s = JS_ToCString(ctx, exc); + assert(s); + assert(!strcmp(s, "TypeError: not a constructor")); + JS_FreeCString(ctx, s); + JS_FreeValue(ctx, exc); + } + + // calling a non-callable is still "not a function" + ret = eval(ctx, "try { undefined() } catch (e) { `${e}` }"); + assert(!JS_IsException(ret)); + s = JS_ToCString(ctx, ret); + assert(s); + assert(!strcmp(s, "TypeError: not a function")); + JS_FreeCString(ctx, s); + JS_FreeValue(ctx, ret); + + JS_FreeContext(ctx); + JS_FreeRuntime(rt); +} + static void backtrace_oom_callsite_array(void) { static const char setup_code[] = @@ -1840,6 +1922,7 @@ int main(void) promise_hook(); dump_memory_usage(); new_errors(); + construct_not_a_constructor(); backtrace_oom_current_exception(); backtrace_oom_callsite_array(); proxy_own_keys_huge_length(); diff --git a/quickjs.c b/quickjs.c index bbac00c33..5fe5befdd 100644 --- a/quickjs.c +++ b/quickjs.c @@ -21071,17 +21071,15 @@ static JSValue JS_CallConstructorInternal(JSContext *ctx, return JS_EXCEPTION; flags |= JS_CALL_FLAG_CONSTRUCTOR; if (unlikely(JS_VALUE_GET_TAG(func_obj) != JS_TAG_OBJECT)) - goto not_a_function; + return JS_ThrowTypeErrorNotAConstructor(ctx, func_obj); p = JS_VALUE_GET_OBJ(func_obj); if (unlikely(!p->is_constructor)) return JS_ThrowTypeErrorNotAConstructor(ctx, func_obj); if (unlikely(p->class_id != JS_CLASS_BYTECODE_FUNCTION)) { JSClassCall *call_func; call_func = ctx->rt->class_array[p->class_id].call; - if (!call_func) { - not_a_function: - return JS_ThrowTypeErrorNotAFunction(ctx); - } + if (!call_func) + return JS_ThrowTypeErrorNotAConstructor(ctx, func_obj); return call_func(ctx, func_obj, new_target, argc, argv, flags); } diff --git a/tests/new-not-a-constructor.js b/tests/new-not-a-constructor.js new file mode 100644 index 000000000..51cc742bc --- /dev/null +++ b/tests/new-not-a-constructor.js @@ -0,0 +1,193 @@ +import { assert } from "./assert.js"; + +/* `new x` where x is not an object is a "not a constructor" TypeError, the + same as `new x` on an object that is not a constructor. It used to be + reported as "not a function", which is what a *call* of a non-callable + reports; the two are distinct errors. */ + +function message(fn) { + try { + fn(); + } catch (e) { + assert(e instanceof TypeError, true); + return e.message; + } + return ""; +} + +/* every primitive type, both as `new x` and `new x(...)` */ +{ + const notObjects = [ + ["undefined", undefined], + ["null", null], + ["boolean", true], + ["number", 1], + ["double", 1.5], + ["string", "x"], + ["symbol", Symbol.iterator], + ["bigint", 1n], + ]; + + for (const [what, v] of notObjects) { + assert(message(() => new v), "not a constructor", what); + assert(message(() => new v()), "not a constructor", `${what} ()`); + assert(message(() => new v(1, 2, 3)), "not a constructor", + `${what} (args)`); + assert(message(() => Reflect.construct(v, [])), "not a constructor", + `${what} Reflect.construct`); + assert(message(() => Reflect.construct(Object, [], v)), + "not a constructor", `${what} newTarget`); + } + + /* a missing property and an undefined variable reach the same path */ + const o = {}; + assert(message(() => new o.missing()), "not a constructor"); + assert(message(() => new o.missing), "not a constructor"); +} + +/* the arguments are still evaluated before the check, as for any call */ +{ + let evaluated = 0; + const v = undefined; + assert(message(() => new v(evaluated++)), "not a constructor"); + assert(evaluated, 1); +} + +/* objects that are not constructors keep the same message */ +{ + assert(message(() => new {}()), "not a constructor"); + assert(message(() => new Math.max()), "not a constructor"); + assert(message(() => new Symbol()), "not a constructor"); + assert(message(() => new BigInt(1)), "not a constructor"); + assert(message(() => new (() => {})()), "not a constructor"); + assert(message(() => new (async function() {})()), "not a constructor"); + assert(message(() => new (new Proxy({}, {}))()), "not a constructor"); + assert(message(() => new (Reflect.construct)()), "not a constructor"); +} + +/* a named bytecode function still names itself in the message */ +{ + function* g() {} + assert(message(() => new g()), "g is not a constructor"); + assert(message(() => Reflect.construct(g, [])), "g is not a constructor"); +} + +/* `extends null` makes super() a construct of a non-object */ +{ + class D extends null { + constructor() { + super(); + } + } + assert(message(() => new D()), "not a constructor"); +} + +/* calling a non-callable is a *different* error and must keep its message */ +{ + const v = undefined; + assert(message(() => v()), "not a function"); + assert(message(() => (1)()), "not a function"); + assert(message(() => "s"()), "not a function"); + assert(message(() => ({})()), "not a function"); + assert(message(() => Reflect.apply(undefined, null, [])), + "not a function"); + + const o = {}; + assert(message(() => o.missing()), "not a function"); +} + +/* the function-shaped things that are still not constructors */ +{ + function* gen() {} + async function* agen() {} + const obj = { + method() {}, + *genMethod() {}, + async asyncMethod() {}, + get accessor() { return 1; }, + }; + class C { + method() {} + static staticMethod() {} + get accessor() { return 1; } + } + const accessor = Object.getOwnPropertyDescriptor(obj, "accessor").get; + + const cases = [ + ["generator", gen], + ["async generator", agen], + ["method shorthand", obj.method], + ["generator method", obj.genMethod], + ["async method", obj.asyncMethod], + ["getter", accessor], + ["class method", C.prototype.method], + ["static class method", C.staticMethod], + ["bound arrow", (() => {}).bind(null)], + ["bound method", obj.method.bind(null)], + ["proxy of a method", new Proxy(obj.method, {})], + ["proxy of an arrow", new Proxy(() => {}, {})], + ]; + for (const [what, v] of cases) { + assert(typeof v, "function", what); + /* a named function names itself, so only the tail is fixed */ + assert(message(() => new v()).endsWith("not a constructor"), true, what); + assert(message(() => Reflect.construct(v, [])).endsWith( + "not a constructor"), true, `${what} via Reflect.construct`); + assert(message(() => Reflect.construct(Object, [], v)).endsWith( + "not a constructor"), true, `${what} as new.target`); + } + + /* ... and the ones that are */ + for (const [what, v] of [["class", C], ["function", function() {}], + ["bound function", (function() {}).bind(null)], + ["proxy of a class", new Proxy(C, {})]]) { + const r = new v(); + assert(typeof r, "object", what); + } +} + +/* a revoked proxy reports a revoked proxy, not a missing constructor */ +{ + const { proxy, revoke } = Proxy.revocable(function() {}, {}); + assert(new proxy() instanceof Object, true); + revoke(); + let caught = null; + try { + new proxy(); + } catch (e) { + caught = e; + } + assert(caught instanceof TypeError, true); + + const bad = Proxy.revocable({}, {}); + bad.revoke(); + let caught2 = null; + try { + new bad.proxy(); + } catch (e) { + caught2 = e; + } + assert(caught2 instanceof TypeError, true); +} + +/* super() in a derived class whose parent is not a constructor */ +{ + const notCtor = () => {}; + class D extends Object { + constructor() { + super(); + } + } + assert(new D() instanceof D, true); + + let caught = null; + try { + const E = class extends Object { constructor() { super(); } }; + Object.setPrototypeOf(E, notCtor); + new E(); + } catch (e) { + caught = e; + } + assert(caught instanceof TypeError, true); + assert(caught.message.includes("not a constructor"), true); +}