From 727afc3b6ab45e796e987c646d3d9511da123e77 Mon Sep 17 00:00:00 2001 From: Michael Haufe Date: Sun, 21 Aug 2022 12:03:24 -0500 Subject: [PATCH] bugfix #236 (#237) --- .vscode/launch.json | 41 +++++++--------------- CHANGELOG.md | 5 ++- package-lock.json | 30 +++++++++------- package.json | 4 +-- src/Contracted.ts | 22 ++++++------ src/lib/ClassRegistration.ts | 59 +++++++++++++++++--------------- src/lib/assertDemands.ts | 14 ++++---- src/lib/assertInvariants.ts | 10 +++--- src/tests/factoryPattern.test.ts | 49 ++++++++++++++++++++++++++ 9 files changed, 139 insertions(+), 95 deletions(-) create mode 100644 src/tests/factoryPattern.test.ts diff --git a/.vscode/launch.json b/.vscode/launch.json index 62cd269..f5f4b60 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -3,44 +3,27 @@ // Hover to view descriptions of existing attributes. // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 "version": "0.2.0", - // See: https://github.com/Microsoft/vscode-recipes/tree/master/debugging-jest-tests "configurations": [ { - "name": "Jest All", - "type": "pwa-node", + "type": "node", "request": "launch", - "runtimeArgs": [ - "run-script", - "test", - "--runInBand" - ], - "runtimeExecutable": "npm", - "console": "integratedTerminal", - "skipFiles": [ - "/**" + "name": "Jest All", + "program": "${workspaceRoot}\\node_modules\\jest\\bin\\jest.js", + "args": [ + "-i" ], - "sourceMaps": true + "internalConsoleOptions": "openOnSessionStart" }, { - "name": "Jest Current File", - "type": "pwa-node", + "type": "node", "request": "launch", - "runtimeArgs": [ - "run-script", - "test", - "--runInBand", + "name": "Jest Current File", + "program": "${workspaceRoot}\\node_modules\\jest\\bin\\jest.js", + "args": [ + "-i", "${fileBasenameNoExtension}" ], - "runtimeExecutable": "npm", - "console": "integratedTerminal", - "skipFiles": [ - "/**" - ], - "resolveSourceMapLocations": [ - "${workspaceFolder}/**" - //"!**/node_modules/**" - ], - "sourceMaps": true + "internalConsoleOptions": "openOnSessionStart", } ] } \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index a91cc18..4cafc82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,11 @@ # Changelog -## vNext +## v0.24.0 * Updated debugger settings to leverage modern VSCode +* Fixed a subtle issue with subcontracting resolution (breaking change) ([#236](https://github.com/final-hill/decorator-contracts/issues/236)) +* Updated VSCode debug settings to support the modern IDE +* Updated npm dependencies ## v0.23.1 diff --git a/package-lock.json b/package-lock.json index 07fe420..bced244 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@final-hill/decorator-contracts", - "version": "0.23.1", + "version": "0.24.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@final-hill/decorator-contracts", - "version": "0.23.1", + "version": "0.24.0", "license": "AGPL-3.0-only", "devDependencies": { "@types/jest": "^28.1.6", @@ -2322,14 +2322,20 @@ } }, "node_modules/caniuse-lite": { - "version": "1.0.30001265", - "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001265.tgz", - "integrity": "sha512-YzBnspggWV5hep1m9Z6sZVLOt7vrju8xWooFAgN6BA5qvy98qPAPb7vNUzypFaoh2pb3vlfzbDO8tB57UPGbtw==", + "version": "1.0.30001380", + "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001380.tgz", + "integrity": "sha512-OO+pPubxx16lkI7TVrbFpde8XHz66SMwstl1YWpg6uMGw56XnhYVwtPIjvX4kYpzwMwQKr4DDce394E03dQPGg==", "dev": true, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/browserslist" - } + "funding": [ + { + "type": "opencollective", + "url": "https://opencollective.com/browserslist" + }, + { + "type": "tidelift", + "url": "https://tidelift.com/funding/github/npm/caniuse-lite" + } + ] }, "node_modules/chalk": { "version": "4.1.2", @@ -8858,9 +8864,9 @@ "dev": true }, "caniuse-lite": { - "version": "1.0.30001265", - "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001265.tgz", - "integrity": "sha512-YzBnspggWV5hep1m9Z6sZVLOt7vrju8xWooFAgN6BA5qvy98qPAPb7vNUzypFaoh2pb3vlfzbDO8tB57UPGbtw==", + "version": "1.0.30001380", + "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001380.tgz", + "integrity": "sha512-OO+pPubxx16lkI7TVrbFpde8XHz66SMwstl1YWpg6uMGw56XnhYVwtPIjvX4kYpzwMwQKr4DDce394E03dQPGg==", "dev": true }, "chalk": { diff --git a/package.json b/package.json index 222424d..a124661 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@final-hill/decorator-contracts", - "version": "0.23.1", + "version": "0.24.0", "description": "Code Contracts for TypeScript and ECMAScript classes", "main": "dist/index.js", "types": "dist/index.d.ts", @@ -86,4 +86,4 @@ "files": [ "dist" ] -} +} \ No newline at end of file diff --git a/src/Contracted.ts b/src/Contracted.ts index 198b9a6..7497671 100644 --- a/src/Contracted.ts +++ b/src/Contracted.ts @@ -22,7 +22,7 @@ const isContracted = Symbol('isContracted'), */ function hasProperties(registration: ClassRegistration, obj: U): boolean { return Object.entries(Object.getOwnPropertyDescriptors(obj)) - .some(([key, desc]) => new Feature(registration,key,desc).isProperty); + .some(([key, desc]) => new Feature(registration, key, desc).isProperty); } /** @@ -40,10 +40,10 @@ function Contracted< T extends Contract = Contract, U extends ClassType = ClassType >(contract: T = new Contract() as T): ClassDecorator { - return function(Base: U & {[innerContract]?: Contract}) { + return function (Base: U & { [innerContract]?: Contract }) { assert(!Object.getOwnPropertySymbols(Base).includes(isContracted), MSG_SINGLE_CONTRACT); - if(contract[checkedMode] === false) { + if (contract[checkedMode] === false) { return Base; } @@ -62,15 +62,15 @@ function Contracted< super(...args); const Class = this.constructor as ClassType, - classRegistration = CLASS_REGISTRY.getOrCreate(Class); + classRegistration = CLASS_REGISTRY.getOrCreate(Class); - assert(!hasProperties(classRegistration,this), MSG_NO_PROPERTIES); + assert(!hasProperties(classRegistration, this), MSG_NO_PROPERTIES); - if(!classRegistration.contractsChecked) { + if (!classRegistration.contractsChecked) { let ancRegistrations = classRegistration.ancestry().reverse(); // top-down check overrides and pre-existing properties [...ancRegistrations, classRegistration].forEach(ancRegistration => { - if(!ancRegistration.contractsChecked) { + if (!ancRegistration.contractsChecked) { ancRegistration.checkOverrides(); ancRegistration.contractsChecked = true; } @@ -79,7 +79,7 @@ function Contracted< // bottom-up to closest Contracted class bind contracts ancRegistrations = takeWhile(ancRegistrations.reverse(), (cr => cr.Class !== Base)); [classRegistration, ...ancRegistrations, CLASS_REGISTRY.get(Base)!].forEach(registration => { - registration.bindContract(this[innerContract]); + registration.bindContract(InnerContracted[innerContract]); }); } @@ -91,8 +91,8 @@ function Contracted< // FIXME: dirty hack. Possibly resolved by moving to contracts as classes // The static getter is used by the construction invariant check // The instance getter is used by the feature declarations - static get [innerContract](){ return contract; } - get [innerContract](){ return contract; } + static get [innerContract]() { return contract; } + get [innerContract]() { return contract; } } const classRegistration = CLASS_REGISTRY.getOrCreate(InnerContracted); @@ -104,5 +104,5 @@ function Contracted< } as ClassDecorator; } -export {isContracted, innerContract}; +export { isContracted, innerContract }; export default Contracted; \ No newline at end of file diff --git a/src/lib/ClassRegistration.ts b/src/lib/ClassRegistration.ts index 24c2c39..30fbddf 100644 --- a/src/lib/ClassRegistration.ts +++ b/src/lib/ClassRegistration.ts @@ -25,13 +25,16 @@ function checkedFeature( registration: ClassRegistration ) { return function innerCheckedFeature(this: any, ...args: any[]) { - const {contract, Class} = registration, + const { Class } = registration; + + assert(this instanceof Class, MSG_INVALID_CONTEXT); + + const contract = Reflect.get(this, innerContract), className = Class.name; - if(!contract[checkedMode]) { - return fnOrig.apply(this,args); + if (!contract[checkedMode]) { + return fnOrig.apply(this, args); } - assert(this instanceof Class, MSG_INVALID_CONTEXT); assertInvariants(this, contract); assertDemands(this, contract, className, featureName, args); @@ -39,26 +42,26 @@ function checkedFeature( try { const old = Object.create(null); unChecked(contract, () => { - registration.features.forEach(({name, hasGetter}) => { - if(hasGetter) { - Object.defineProperty(old, name, {value: this[name]}); + registration.features.forEach(({ name, hasGetter }) => { + if (hasGetter) { + Object.defineProperty(old, name, { value: this[name] }); } }); }); const within: number = Reflect.get(contract.assertions, featureName)?.within, - // TODO:Not all environments support performance.now() as a global and conditional import is inconvenient + // TODO:Not all environments currently support performance.now() as a global and conditional import is inconvenient t0 = Date.now(); - result = fnOrig.apply(this,args); + result = fnOrig.apply(this, args); const t1 = Date.now(); - if(within) { + if (within) { assert(t1 - t0 < within, `Timing constraint violated. Constraint: ${within}ms, actual: ${t1 - t0}ms` ); } assertEnsures(this, contract, className, featureName, old, args); - } catch(error) { - const rescue = Reflect.get(contract.assertions,featureName)?.rescue; - if(rescue == null) { + } catch (error) { + const rescue = Reflect.get(contract.assertions, featureName)?.rescue; + if (rescue == null) { assertInvariants(this, contract); throw error; } @@ -71,7 +74,7 @@ function checkedFeature( result = innerCheckedFeature.call(this, ...args); }); }); - if(!hasRetried) { + if (!hasRetried) { assertInvariants(this, contract); throw error; } @@ -92,8 +95,8 @@ class ClassRegistration { const proto = this.Class.prototype; this.#features = Reflect.ownKeys(proto) - .filter(key => key != 'constructor' && key !== innerContract) - .map(key => new Feature(this, key, Object.getOwnPropertyDescriptor(proto,key)!)); + .filter(key => key != 'constructor' && key !== innerContract) + .map(key => new Feature(this, key, Object.getOwnPropertyDescriptor(proto, key)!)); } /** @@ -126,12 +129,12 @@ class ClassRegistration { * @returns {ClassRegistration[]} - The ancestor class registrations */ ancestry(): ClassRegistration[] { - if(this.ParentClass == null) { + if (this.ParentClass == null) { return []; } else { const parentRegistry = CLASS_REGISTRY.getOrCreate(this.ParentClass); - return [parentRegistry,...parentRegistry.ancestry()]; + return [parentRegistry, ...parentRegistry.ancestry()]; } } @@ -142,23 +145,23 @@ class ClassRegistration { * @returns {Feature[]} - The feature names */ ancestryFeatures(): Feature[] { - return this.ancestry().flatMap(({features}) => features); + return this.ancestry().flatMap(({ features }) => features); } bindContract>(contract: T) { this.contract = contract; - if(!contract[checkedMode]) { + if (!contract[checkedMode]) { return; } const proto = this.Class.prototype; this.features.forEach(feature => { - const {name,hasGetter, hasSetter, isMethod} = feature; + const { name, hasGetter, hasSetter, isMethod } = feature; Object.defineProperty(proto, name, { enumerable: true, - ...(hasGetter ? {get: checkedFeature(name, feature.getter!, this) } : {}), - ...(hasSetter ? {set: checkedFeature(name, feature.setter!, this) } : {}), - ...(isMethod ? {value: checkedFeature(name, feature.value, this) } : {}) + ...(hasGetter ? { get: checkedFeature(name, feature.getter!, this) } : {}), + ...(hasSetter ? { set: checkedFeature(name, feature.setter!, this) } : {}), + ...(isMethod ? { value: checkedFeature(name, feature.value, this) } : {}) }); feature.descriptor = Object.getOwnPropertyDescriptor(proto, name)!; @@ -170,10 +173,10 @@ class ClassRegistration { * @throws {AssertionError} - Throws if the verification fails */ checkOverrides(): void { - const ancestryFeatureNames = new Set(this.ancestryFeatures().map(({name}) => name)); - this.features.forEach(({name, hasOverrides}) => { + const ancestryFeatureNames = new Set(this.ancestryFeatures().map(({ name }) => name)); + this.features.forEach(({ name, hasOverrides }) => { const str = `${this.Class.name}.prototype.${String(name)}`; - assert(!hasOverrides || ancestryFeatureNames.has(name),`Unnecessary @override declaration on ${str}`); + assert(!hasOverrides || ancestryFeatureNames.has(name), `Unnecessary @override declaration on ${str}`); assert(hasOverrides || !ancestryFeatureNames.has(name), `@override decorator missing on ${str}`); }); } @@ -186,7 +189,7 @@ class ClassRegistration { * @returns {Feature | undefined} - The feature if it exists else otherwise */ findFeature(propertyKey: PropertyKey): Feature | undefined { - return this.features.find(({name}) => name === propertyKey) ?? + return this.features.find(({ name }) => name === propertyKey) ?? this.parentRegistration?.findFeature(propertyKey); } } diff --git a/src/lib/assertDemands.ts b/src/lib/assertDemands.ts index 11ec29f..6ed0df8 100644 --- a/src/lib/assertDemands.ts +++ b/src/lib/assertDemands.ts @@ -19,22 +19,22 @@ import unChecked from './unChecked'; * @param {any[]} args - The arguments of the feature to apply to the assertion * @throws {AssertionError} */ -function assertDemands(ctx: U, contract: Contract, className: string, featureName: PropertyKey, args: any[]){ +function assertDemands(ctx: U, contract: Contract, className: string, featureName: PropertyKey, args: any[]) { let result = true; const d: Demands | undefined = Reflect.get(contract.assertions, featureName)?.demands, demandsError = `demands not met on ${className}.prototype.${String(featureName)}\r\n${d}`; - if(contract[checkedMode]) { - if(d) { + if (contract[checkedMode]) { + if (d) { unChecked(contract, () => - result = d.call(ctx,ctx, ...args) + result = d.call(ctx, ctx, ...args) ); } } - if(contract[extend] && !result) { - assertDemands(ctx,contract[extend]!, className, featureName, args); + if (contract[extend] && !result) { + assertDemands(ctx, contract[extend], className, featureName, args); } else { - assert(result,demandsError); + assert(result, demandsError); } } diff --git a/src/lib/assertInvariants.ts b/src/lib/assertInvariants.ts index 3139ce7..6806fb0 100644 --- a/src/lib/assertInvariants.ts +++ b/src/lib/assertInvariants.ts @@ -16,15 +16,15 @@ import { assert, checkedMode, Contract, extend, invariant } from '../'; * @throws {AssertionError} */ function assertInvariants(ctx: U, contract: Contract) { - if(contract[checkedMode]) { - unChecked(contract,() => { + if (contract[checkedMode]) { + unChecked(contract, () => { const iv = contract[invariant]; - assert(iv.call(ctx, ctx),`Invariant violated. ${iv.toString()}`); + assert(iv.call(ctx, ctx), `Invariant violated. ${iv.toString()}`); }); } - if(contract[extend]) { - assertInvariants(ctx,contract[extend]!); + if (contract[extend]) { + assertInvariants(ctx, contract[extend]!); } } diff --git a/src/tests/factoryPattern.test.ts b/src/tests/factoryPattern.test.ts new file mode 100644 index 0000000..5f3fb29 --- /dev/null +++ b/src/tests/factoryPattern.test.ts @@ -0,0 +1,49 @@ +/*! + * @license + * Copyright (C) 2022 Final Hill LLC + * SPDX-License-Identifier: AGPL-3.0-only + * @see + */ + +import { Contract, Contracted, extend, invariant } from '../'; + +describe('Factory Pattern', () => { + const factoryContract = new Contract(); + + @Contracted(factoryContract) + class Factory { + child(value: string) { return new Child(value); } + } + + const childContract = new Contract({ + [extend]: factoryContract, + [invariant](self) { return self.value.length === 1; } + }); + + @Contracted(childContract) + class Child extends Factory { + #value: string; + + constructor(value: string) { + super(); + this.#value = value; + } + + get value() { return this.#value; } + } + + test('Construct Child', () => { + const c = new Child('a'); + + expect(c).toBeInstanceOf(Child); + expect(c.value).toBe('a'); + }); + + test('Factory Construct Child', () => { + const f = new Factory(), + c = f.child('a'); + + expect(c).toBeInstanceOf(Child); + expect(c.value).toBe('a'); + }); +}); \ No newline at end of file