From fbbc2502bdcd1becaf2f2c419eafbe4535a0a686 Mon Sep 17 00:00:00 2001 From: Glen Whitney Date: Fri, 12 Dec 2025 10:21:01 +0000 Subject: [PATCH] refactor: Avoid inheritance and ban instanceof (#37) Note that instanceof is replaced by many of the nanomath classes providing a static `.holds(x)` function that tests whether `x` is an instance of the class. Currently, these `.holds()` are implemented by setting an inaccessible symbol property to be true (so I don't think there's any way to spoof it). Resolves #33. Reviewed-on: https://code.studioinfinity.org/StudioInfinity/nanomath/pulls/37 Co-authored-by: Glen Whitney Co-committed-by: Glen Whitney --- src/boolean/__test__/type.spec.js | 2 +- src/core/Implementations.js | 10 +++++ src/core/Type.js | 16 +++++--- src/core/TypeDispatcher.js | 19 +++++----- src/core/TypePatterns.js | 61 +++++++++++++++++++++---------- src/core/__test__/helpers.spec.js | 4 +- 6 files changed, 74 insertions(+), 38 deletions(-) diff --git a/src/boolean/__test__/type.spec.js b/src/boolean/__test__/type.spec.js index ad294a7..2a8c355 100644 --- a/src/boolean/__test__/type.spec.js +++ b/src/boolean/__test__/type.spec.js @@ -19,7 +19,7 @@ describe('boolean type functions', () => { }) it('converts any type to boolean', () => { for (const T in math.types) { - if (T instanceof Type) assert(boolean.resolve([T])) + if (Type.holds(T)) assert(boolean.resolve([T])) } }) }) diff --git a/src/core/Implementations.js b/src/core/Implementations.js index c53ea3e..62e3516 100644 --- a/src/core/Implementations.js +++ b/src/core/Implementations.js @@ -1,13 +1,23 @@ +const implementationsBrand = Symbol() export class Implementations { constructor(impOrImps) { if (Array.isArray(impOrImps)) { this.matchers = impOrImps } else this.matchers = [impOrImps] + this[implementationsBrand] = true } + + // Returns true if entity is an Implementations + static holds(entity) {return entity[implementationsBrand]} } +const igBrand = Symbol() export class ImplementationsGenerator { constructor(f) { this.generate = f + this[igBrand] = true } + + // Returns true if entity is an ImplementationsGenerator + static holds(entity) {return entity[igBrand]} } diff --git a/src/core/Type.js b/src/core/Type.js index 7bb7e56..42a4f3d 100644 --- a/src/core/Type.js +++ b/src/core/Type.js @@ -1,5 +1,7 @@ import ArrayKeyedMap from 'array-keyed-map' +const typeBrand = Symbol() // invisible outside this file + // Generic types are callable, so we have no choice but to extend Function export class Type extends Function { constructor(f, options = {}) { @@ -19,6 +21,7 @@ export class Type extends Function { } }) + this[typeBrand] = true this.test = f // we want property `from` to end up as an array of Matchers: this.from = options.from @@ -57,15 +60,16 @@ export class Type extends Function { return rewired } - toString() { - return this.typeName || `[Type ${this.test}]` - } + toString() {return this.typeName || `[Type ${this.test}]`} + + // Returns true if entity is a Type + static holds(entity) {return entity[typeBrand]} } export const Undefined = new Type( t => typeof t === 'undefined', {zero: undefined, one: undefined, nan: undefined}) -export const TypeOfTypes = new Type(t => t instanceof Type) +export const TypeOfTypes = new Type(t => Type.holds(t)) export const Unknown = new Type(() => true) // Danger, do not merge! Unknown._doNotMerge = true @@ -74,7 +78,7 @@ export const OneOf = (...types) => { if (!types.length) { throw new RangeError('cannot choose OneOf no types at all') } - const nonType = types.findIndex(T => !(T instanceof Type)) + const nonType = types.findIndex(T => !Type.holds(T)) if (nonType >= 0) { throw new RangeError( `OneOf can only take type arguments, not ${types[nonType]}`) @@ -106,7 +110,7 @@ export const ReturnType = f => f.returns ?? Unknown export const whichType = typs => Returns(TypeOfTypes, item => { for (const type of Object.values(typs)) { - if (!(type instanceof Type)) continue + if (!Type.holds(type)) continue if (type.test(item)) return type.refine(item, whichType(typs)) } let errorMsg = '' diff --git a/src/core/TypeDispatcher.js b/src/core/TypeDispatcher.js index 5c96fb1..cca4932 100644 --- a/src/core/TypeDispatcher.js +++ b/src/core/TypeDispatcher.js @@ -81,10 +81,10 @@ export class TypeDispatcher { // For special cases like types, config, etc, we can wrap // a function in ImplementationsGenerator to produce the thing // we should really merge: - if (val instanceof ImplementationsGenerator) val = val.generate() + if (ImplementationsGenerator.holds(val)) val = val.generate() // Now dispatch on what sort of thing we are supposed to merge: - if (val instanceof Type) { + if (Type.holds(val)) { if (val._doNotMerge) { throw new TypeError(`attempt to merge unusable type '${val}'`) } @@ -105,13 +105,13 @@ export class TypeDispatcher { // Everything else we coerce into Implementations and deal with // right here: - if (val instanceof Matcher) val = new Implementations(val) + if (Matcher.holds(val)) val = new Implementations(val) if (Array.isArray(val)) val = new Implementations(val) if (isPlainFunction(val)) { throw new RangeError( `function value for ${key} must be merged within a 'match' call`) } - if (!(val instanceof Implementations)) { + if (!Implementations.holds(val)) { val = new Implementations(match(Passthru, val)) } @@ -307,7 +307,7 @@ export class TypeDispatcher { } if (generatingDeps && typeof result === 'object' - && !(result instanceof Type) + && !Type.holds(result) ) { return DependencyRecorder(result, key, this, bhvix) } @@ -354,7 +354,7 @@ export class TypeDispatcher { behave.set(bhvix, item) if (generatingDeps && typeof item === 'object' - && !(item instanceof Type) + && !Type.holds(item) ) { return DependencyRecorder(item, key, this, bhvix) } @@ -469,7 +469,8 @@ const DependencyRecorder = (object, path, repo, bhvix) => new Proxy(object, { const result = Reflect.get(target, prop, receiver) // pass internal methods through, as well as resolve calls, // since we record dependencies within the latter: - if (prop.startsWith('_') + if (typeof prop === 'symbol' + || prop.startsWith('_') || prop === 'resolve' || (typeof result === 'function' && 'isDispatcher' in result) ) { @@ -484,7 +485,7 @@ const DependencyRecorder = (object, path, repo, bhvix) => new Proxy(object, { // dependencies on its properties (e.g. math.config.predictable) // So proxy the return value, except for types, which must maintain // strict referential identity: - if (typeof result === 'object' && !(result instanceof Type)) { + if (typeof result === 'object' && !Type.holds(result)) { return DependencyRecorder(result, newPath, repo, bhvix) } else return result } @@ -508,7 +509,7 @@ const DependencyWatcher = (object, path, ixList, repo) => new Proxy(object, { get(target, prop, receiver) { // Only thing we need to do is push the watching down const result = Reflect.get(target, prop, receiver) - if (typeof result === 'object' && !(result instanceof Type)) { + if (typeof result === 'object' && !Type.holds(result)) { const newPath = [path, prop].join('.') return DependencyWatcher(result, newPath, ixList, repo) } diff --git a/src/core/TypePatterns.js b/src/core/TypePatterns.js index 3fe6d4a..86669c8 100644 --- a/src/core/TypePatterns.js +++ b/src/core/TypePatterns.js @@ -1,20 +1,27 @@ import {ReturnType, Type, Undefined, Unknown} from './Type.js' import {isPlainFunction} from './helpers.js' +const tpBrand = Symbol() + export class TypePattern { + constructor() { + throw new Error('Cannot construct an abstract TypePattern') + } match(_typeSequence, _options={}) { throw new Error('Specific TypePatterns must implement match') } sampleTypes() { throw new Error('Specific TypePatterns must implement sampleTypes') } - equal(other) {return other.constructor === this.constructor} toString() {return 'Abstract Pattern (?!)'} + + // Returns true if entity is a TypePattern + static holds(entity) {return entity[tpBrand]} } -class MatchTypePattern extends TypePattern { +class MatchTypePattern { constructor(typeToMatch) { - super() + this[tpBrand] = true this.type = typeToMatch } match(typeSequence, options={}) { @@ -34,13 +41,15 @@ class MatchTypePattern extends TypePattern { return [-1, Undefined] } sampleTypes() {return [this.type]} - equal(other) {return super.equal(other) && this.type === other.type} + equal(other) { + return this.constructor === other.constructor && this.type === other.type + } toString() {return `Match(${this.type})`} } -class SequencePattern extends TypePattern { +class SequencePattern { constructor(itemsToMatch) { - super() + this[tpBrand] = true this.patterns = itemsToMatch.map(pattern) } match(typeSequence, options={_internal: true}) { @@ -62,16 +71,16 @@ class SequencePattern extends TypePattern { return this.patterns.map(pat => pat.sampleTypes()).flat() } equal(other) { - return super.equal(other) + return this.constructor === other.constructor && this.patterns.length === other.patterns.length && this.patterns.every((elt, ix) => elt.equal(other.patterns[ix])) } toString() {return `[${this.patterns}]`} } -class PredicatePattern extends TypePattern { +class PredicatePattern { constructor(predicate) { - super() + this[tpBrand] = true this.predicate = predicate } match(typeSequence, options={}) { @@ -85,14 +94,15 @@ class PredicatePattern extends TypePattern { throw new Error('sampleTypes() not yet implemented for PredicatePattern') } equal(other) { - return super.equal(other) && this.predicate === other.predicate + return this.constructor === other.constructor + && this.predicate === other.predicate } toString() {return `Test(${this.predicate})`} } export const pattern = patternOrSpec => { - if (patternOrSpec instanceof TypePattern) return patternOrSpec - if (patternOrSpec instanceof Type) { + if (TypePattern.holds(patternOrSpec)) return patternOrSpec + if (Type.holds(patternOrSpec)) { return new MatchTypePattern(patternOrSpec) } if (Array.isArray(patternOrSpec)) return new SequencePattern(patternOrSpec) @@ -102,7 +112,8 @@ export const pattern = patternOrSpec => { throw new TypeError(`Can't interpret '${patternOrSpec}' as a type pattern`) } -class AnyPattern extends TypePattern { +class AnyPattern { + constructor () {this[tpBrand] = true} match(typeSequence, options={}) { const position = options.position ?? 0 return position < typeSequence.length @@ -110,14 +121,15 @@ class AnyPattern extends TypePattern { : [-1, Undefined] } sampleTypes() {return [Undefined]} + equal(other) {return this.constructor === other.constructor} toString() {return 'Any'} } export const Any = new AnyPattern() -class OptionalPattern extends TypePattern { +class OptionalPattern { constructor(item) { - super() + this[tpBrand] = true this.pattern = pattern(item) } match(typeSequence, options={_internal: true}) { @@ -135,16 +147,17 @@ class OptionalPattern extends TypePattern { } sampleTypes() {return []} equal(other) { - return super.equal(other) && this.pattern.equal(other.pattern) + return this.constructor === other.constructor + && this.pattern.equal(other.pattern) } toString() {return `?${this.pattern}`} } export const Optional = item => new OptionalPattern(item) -class MultiPattern extends TypePattern { +class MultiPattern { constructor(item) { - super() + this[tpBrand] = true this.pattern = pattern(item) } match(typeSequence, options={_internal: true}) { @@ -162,7 +175,8 @@ class MultiPattern extends TypePattern { } sampleTypes() {return []} equal(other) { - return super.equal(other) && this.pattern.equal(other.pattern) + return this.constructor === other.constructor + && this.pattern.equal(other.pattern) } toString() {return `${this.pattern}*`} } @@ -171,13 +185,15 @@ export const Multiple = item => new MultiPattern(item) // Like Multiple(Any) except leaves the argument list alone; it doesn't // chunk it into a single Array of all arguments -class PassthruPattern extends TypePattern { +class PassthruPattern { + constructor () {this[tpBrand] = true} match(typeSequence, options={}) { const position = options.position ?? 0 return [typeSequence.length, typeSequence.slice(position)] } sampleTypes() {return []} toString() {return 'Passthru'} + equal(other) {return this.constructor === other.constructor} } export const Passthru = new PassthruPattern() @@ -206,11 +222,16 @@ export const needsCollection = (template) => { return 'actual' in template } +const matcherBrand = Symbol() export class Matcher { constructor(spec, facOrBehave) { this.pattern = pattern(spec) this.does = facOrBehave + this[matcherBrand] = true } + + // Returns true if entity is a matcher + static holds(entity) {return entity[matcherBrand]} } export const match = (spec, facOrBehave) => new Matcher(spec, facOrBehave) diff --git a/src/core/__test__/helpers.spec.js b/src/core/__test__/helpers.spec.js index 46576eb..10006b2 100644 --- a/src/core/__test__/helpers.spec.js +++ b/src/core/__test__/helpers.spec.js @@ -7,8 +7,8 @@ import {match, Matcher, TypePattern} from '../TypePatterns.js' describe('Core helpers', () => { it('defines what Matchers are', () => { const matcher = match([TypeOfTypes, Undefined], -3) - assert(matcher instanceof Matcher) - assert(matcher.pattern instanceof TypePattern) + assert(Matcher.holds(matcher)) + assert(TypePattern.holds(matcher.pattern)) }) it('detects plain objects', () => {