test: ensure that a function requiring conversion depends on converter

This test turned out to be a very discerning one. Adding it uncovered
  numerous bugs, large and small, in the TypeDispatcher. The most major
  one was that a Map distinguishes keys by strict equality, and hence
  since every lookup for a cached behavior was using a newly-generated array
  of types, the cache was never being hit. So it looked like methods were
  being updated as dependencies changed, when what was really happening
  was that the behavior was simply being regenerated from scratch on every
  call, which would not be performant as the prototype scaled.
  This bug is now fixed (by switching to a third-party ArrayKeyedMap), along
  with many smaller bugs too numerous to list.

  It should now be feasible to go through Pocomath and add all of the functions
  that depend on numbers and booleans only.

package.json5

@@ -18,4 +18,7 @@
   devDependencies: {
     mocha: '^11.1.0',
   },
+  dependencies: {
+    'array-keyed-map': '^2.1.3',
+  },
 }

pnpm-lock.yaml

@@ -7,6 +7,10 @@ settings:
 importers:
 
   .:
+    dependencies:
+      array-keyed-map:
+        specifier: ^2.1.3
+        version: 2.1.3
     devDependencies:
       mocha:
         specifier: ^11.1.0
@@ -49,6 +53,9 @@ packages:
   argparse@2.0.1:
     resolution: {integrity: sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==}
 
+  array-keyed-map@2.1.3:
+    resolution: {integrity: sha512-JIUwuFakO+jHjxyp4YgSiKXSZeC0U+R1jR94bXWBcVlFRBycqXlb+kH9JHxBGcxnVuSqx5bnn0Qz9xtSeKOjiA==}
+
   balanced-match@1.0.2:
     resolution: {integrity: sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==}
 
@@ -398,6 +405,8 @@ snapshots:
 
   argparse@2.0.1: {}
 
+  array-keyed-map@2.1.3: {}
+
   balanced-match@1.0.2: {}
 
   binary-extensions@2.3.0: {}

src/core/Type.js

@@ -1,3 +1,5 @@
+import {onType} from './helpers.js'
+
 const typeObject = {} // have to make sure there is only one
 
 export const types = () => typeObject
@@ -5,7 +7,8 @@ export const types = () => typeObject
 export class Type {
    constructor(f, options = {}) {
       this.test = f
-      this.from = new Map(options.from ?? [])
+      this.from = options.from ?? onType() // empty Implementations if no ...
+      // ... conversions specified
    }
    toString() {
       return this.name || `[Type ${this.test}]`

src/core/TypeDispatcher.js

@@ -1,11 +1,10 @@
-import {typeOf, Type, bootstrapTypes} from './Type.js'
+import ArrayKeyedMap from 'array-keyed-map'
 
 import {
    Implementations, isPlainFunction, isPlainObject, onType
 } from './helpers.js'
-import {
+import {bootstrapTypes, Returns, typeOf, Type} from './Type.js'
-   alwaysMatches, matched, needsCollection, Any, Multiple
+import {matched, needsCollection, Passthru} from './TypePatterns.js'
-} from './TypePatterns.js'
 
 export class TypeDispatcher {
    constructor(...specs) {
@@ -37,25 +36,39 @@ export class TypeDispatcher {
             continue
          }
          if (typeof val === 'function') {
-            val = onType(Multiple(Any), val)
+            val = onType(Passthru, val)
          }
          if (val instanceof Implementations) {
             if (!(key in this)) {
-               // need to set up the item
+               // Need to "bootstrap" the item:
+               // We initially define it with a temporary getter, only
+               // because we don't know whether ultimately it will produce
+               // a function or a non-callable value, and the "permanent"
+               // getter we want depends on which it turns out to be. This
+               // situation means it's not supported to replace a key
+               // corresponding to a method, after it's been fetched once,
+               // with a definition that produces a non-callable value;
+               // the TypeDispatcher will go on returning a function, and
+               // even if you call that function, it will throw an error
+               // when it tries to call the non-callable value.
+               // Conversely, if you try to replace a key corresponding
+               // to a non-callable value, after it's been fetched once,
+               // with a function, it will work, but it will not be able to
+               // perform type dispatch on that function.
                Object.defineProperty(this, key, {
                   enumerable: true,
                   configurable: true,
                   get: () => {
                      let tryValue
-                     let tryTypes = []
                      try {
                         tryValue = this.resolve(key, [])
                      } catch {
-                        // Has no value for the empty type list, so
+                        // Has no value for the empty type list, so therefore
-                        // find a type list it will have a value for
+                        // it must be a method, as there is no way to supply
-                        tryTypes =
+                        // any types for a non-function value. Hence, we can
-                           this._implementations[key][0][0].sampleTypes()
+                        // just make tryValue any plain function, since it is
-                        tryValue = this.resolve(key, tryTypes)
+                        // never actually used, just its type analyzed.
+                        tryValue = () => undefined
                      }
                      // Redefine the property according to what sort of
                      // entity it is:
@@ -74,7 +87,7 @@ export class TypeDispatcher {
                         })
                         return standard
                      }
-                     if (tryTypes.length) tryValue = undefined
+
                      if (typeof tryValue === 'object') {
                         if (!('resolve' in tryValue)) {
                            tryValue.resolve = types => this.resolve(key, types)
@@ -92,6 +105,7 @@ export class TypeDispatcher {
                         })
                         return get()
                      }
+
                      Object.defineProperty(this, key, {
                         enumerable: true,
                         configurable: true,
@@ -100,13 +114,16 @@ export class TypeDispatcher {
                      return tryValue
                   }
                })
+
+               // Finally, initialize the other data for this key:
                this._implementations[key] = []
-               this._behaviors[key] = new Map()
+               this._behaviors[key] = new ArrayKeyedMap()
-               this._dependencies[key] = new Map()
+               this._dependencies[key] = new ArrayKeyedMap()
             }
+
             // Now add all of the patterns of this implementation:
             for (const [pattern, result] of val.patterns) {
-               if (alwaysMatches(pattern)) {
+               if (pattern === Passthru) {
                   if (key in this._fallbacks) this._disengageFallback(key)
                   this._fallbacks[key] = result
                } else {
@@ -128,7 +145,7 @@ export class TypeDispatcher {
          }
 
          // install value as a catchall value
-         this.merge({[key]: onType(Multiple(Any), val)})
+         this.merge({[key]: onType(Passthru, val)})
       }
    }
 
@@ -151,7 +168,10 @@ export class TypeDispatcher {
          } else {
             const from = state.pos++
             if ('actual' in elt) { // incorporate conversion
-               const convert = elt.matched.from.get(elt.actual)(this)
+               let convert = elt.convertor
+               if (!convert.returns) { // it's a factory that produces convert
+                  convert = convert(this, elt.actual)
+               }
                extractors.push(args => convert(args[from]))
             } else extractors.push(args => args[from])
          }
@@ -167,14 +187,17 @@ export class TypeDispatcher {
       if (this.resolve._genDepsOf?.length) this._addToDeps(key, types)
 
       const behave = this._behaviors[key]
+      // Return the cached resolution if it's there
       if (behave.has(types)) return behave.get(types)
+
+      // Otherwise, perform the resolution and cache the result
       const imps = this._implementations[key]
       let needItem = true
       let item
-      let pattern
       let template
       if (imps.length) {
          for (const options of [{}, {convert: true}]) {
+            let pattern
             for ([pattern, item] of imps) {
                let finalIndex
                ;[finalIndex, template] = pattern.match(types, options)
@@ -189,50 +212,59 @@ export class TypeDispatcher {
       if (needItem && key in this._fallbacks) {
          needItem = false
          item = this._fallbacks[key]
-         template = [types]
+         template = types
       }
       if (needItem) {
          throw new TypeError(`no matching definition of '${key}' on '${types}'`)
       }
-      if (!isPlainFunction(item) || 'returns' in item) {
+      // If this key is producing a non-function value, we're done
+      if (!isPlainFunction(item)) {
          behave.set(types, item)
          return item
       }
-      // item is a Factory. We have to use it to build the behavior
+
+      // item is a function, either a direct behavior or
+      // a factory. We have to use it to build the final behavior
       // First set up to record dependencies
       if (!('_genDepsOf' in this.resolve)) {
          this.resolve._genDepsOf = []
       }
-      this.resolve._genDepsOf.push([key, types])
+
       let theBehavior = () => undefined
-      try {
+      this.resolve._genDepsOf.push([key, types]) // Important: make sure
-         theBehavior = item(
+      // not to return without popping _genDepsOf
-            DependencyRecorder(this, [], this), matched(template))
+      if (!('returns' in item)) {
-      } catch {
+         // looks like a factory
-         // Oops, didn't work as a factory, so guess we were wrong.
-         // Just make it the direct value for this key on these types:
-         behave.set(types, item)
-         return item
-      } finally {
-         this.resolve._genDepsOf.pop()
-      }
-      if (typeof theBehavior === 'function'
-          && theBehavior.length
-          && needsCollection(template)
-      ) {
-         // have to wrap the behavior to collect the actual arguments
-         // in the way corresponding to the template. That may generate
-         // more dependencies:
-         this.resolve._genDepsOf.push([key, types])
          try {
-            const collectFunction = this._generateCollectFunction(template)
+            theBehavior = item(
-            theBehavior = (...args) => theBehavior(...collectFunction(args))
+               DependencyRecorder(this, [], this), matched(template))
-         } finally {
+         } catch {
-            this.resolve._genDepsOf.pop()
+            // Oops, didn't work as a factory, so guess we were wrong.
+            // Just make it the direct value for this key on these types:
+            theBehavior = item
+         }
+      } else theBehavior = item
+
+      let finalBehavior = theBehavior
+      if (typeof theBehavior === 'function') {
+         const returning = theBehavior.returns
+         if (!returning) {
+            throw new TypeError(
+               `No return type specified for ${key} on ${types}`)
+         }
+         if (theBehavior.length && needsCollection(template)) {
+            // have to wrap the behavior to collect the actual arguments
+            // in the way corresponding to the template. Generating that
+            // argument transformer may generate more dependencies.
+            const morph = this._generateCollectFunction(template)
+            finalBehavior =
+               Returns(returning, (...args) => theBehavior(...morph(args)))
          }
       }
-      behave.set(types, theBehavior)
+
-      return theBehavior
+      this.resolve._genDepsOf.pop()  // OK, now it's safe to return
+      behave.set(types, finalBehavior)
+      return finalBehavior
    }
 
    // Method called to invalidate a set of behaviors

src/core/TypePatterns.js

@@ -20,8 +20,13 @@ class MatchTypePattern extends TypePattern {
       const actual = typeSequence[position]
       if (position < typeSequence.length) {
          if (actual === this.type) return [position + 1, actual]
-         if (options.convert && this.type.from.has(actual)) {
+         if (options.convert) {
-            return [position + 1, {actual, matched: this.type}]
+            for (const [pattern, convertor] of this.type.from.patterns) {
+               const [pos] = pattern.match([actual])
+               if (pos === 1) {
+                  return [position + 1, {actual, convertor, matched: this.type}]
+               }
+            }
          }
       }
       return [-1, Undefined]
@@ -131,8 +136,18 @@ class MultiPattern extends TypePattern {
 }
 
 export const Multiple = item => new MultiPattern(item)
-export const alwaysMatches =
+
-   pat => pat instanceof MultiPattern && pat.pattern instanceof AnyPattern
+// 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 {
+   match(typeSequence, options={}) {
+      const position = options.position ?? 0
+      return [typeSequence.length, typeSequence.slice(position)]
+   }
+   sampleTypes() {return []}
+}
+
+export const Passthru = new PassthruPattern()
 
 // returns the template just of matched types, dropping any actual types
 export const matched = (template) => {

src/core/__test__/TypeDispatcher.spec.js

@@ -1,7 +1,8 @@
 import assert from 'assert'
 import {TypeDispatcher} from '../TypeDispatcher.js'
-import * as numbers from '#number/all.js'
+import * as booleans from '#boolean/all.js'
 import * as generics from '#generic/all.js'
+import * as numbers from '#number/all.js'
 import {onType} from "#core/helpers.js"
 import {Any} from "#core/TypePatterns.js"
 import {Returns} from "#core/Type.js"
@@ -47,4 +48,15 @@ describe('TypeDispatcher', () => {
       gnmath.merge({multiply: plain((a,b) => Math.floor(a) * b)})
       assert.strictEqual(gnmath.square(-2.5), 7.5)
    })
+   it('detects dependencies on conversion operations', () => {
+      const bgn = new TypeDispatcher(booleans, generics, numbers)
+      const {BooleanT, NumberT} = bgn.types
+      assert(!bgn._behaviors.negate.has([BooleanT]))
+      assert.strictEqual(bgn.negate(true), -1)
+      assert(bgn._behaviors.negate.has([BooleanT]))
+      const deps = bgn._dependencies.negate
+      bgn.merge({number: onType([BooleanT], Returns(NumberT, b => b ? 2 : 0))})
+      assert(!bgn._behaviors.negate.has([BooleanT]))
+      assert.strictEqual(bgn.negate(true), -2)
+   })
 })

src/core/__test__/TypePatterns.spec.js

@@ -1,5 +1,7 @@
 import assert from 'assert'
-import {pattern, Any, Multiple, Optional} from '../TypePatterns.js'
+import {
+   pattern, Any, Multiple, Optional, needsCollection
+} from '../TypePatterns.js'
 import {Undefined, TypeOfTypes} from '../Type.js'
 
 describe('Type patterns', () => {
@@ -93,4 +95,10 @@ describe('Type patterns', () => {
          whyNot.sampleTypes(), [Undefined, TypeOfTypes])
       assert(whyNot.equal(whyNot))
    })
+   it('determines whether a template needs a collection function', () => {
+      assert(!needsCollection([Undefined]))
+      assert(needsCollection([Undefined, [Undefined, Undefined]]))
+      assert(needsCollection(
+         [Undefined, {actual: Undefined, matched: TypeOfTypes}]))
+   })
 })

src/core/__test__/helpers.spec.js

@@ -9,8 +9,8 @@ describe('Core helpers', () => {
    it('defines what Implementations are', () => {
       const imps = onType(Undefined, 7, [TypeOfTypes, Undefined], -3)
       assert(imps instanceof Implementations)
-      assert(imps.patterns instanceof Map)
+      assert(imps.patterns instanceof Array)
-      assert(imps.patterns.keys().every(k => k instanceof TypePattern))
+      assert(imps.patterns.every(([k]) => k instanceof TypePattern))
    })
 
    it('detects plain objects', () => {

src/core/helpers.js

@@ -2,12 +2,12 @@ import {pattern} from './TypePatterns.js'
 
 export class Implementations {
    constructor(imps) {
-      this.patterns = new Map()
+      this.patterns = []
       this._add(imps)
    }
    _add(imps) {
       for (let i = 0; i < imps.length; ++i) {
-         this.patterns.set(pattern(imps[i]), imps[++i])
+         this.patterns.push([pattern(imps[i]), imps[++i]])
       }
    }
    also(...imps) {

src/number/NumberT.js

@@ -1,6 +1,7 @@
 import {Type} from '#core/Type.js'
+import {onType} from '#core/helpers.js'
 import {BooleanT} from '#boolean/BooleanT.js'
 
 export const NumberT = new Type(n => typeof n === 'number', {
-    from: [[BooleanT, math => math.number.resolve([BooleanT])]],
+    from: onType(BooleanT, math => math.number.resolve([BooleanT])),
 })

src/number/__test__/NumberT.spec.js

@@ -12,7 +12,14 @@ describe('NumberT Type', () => {
    })
 
    it('can convert from BooleanT to NumberT', () => {
-      const cnvBtoN = NumberT.from.get(BooleanT)(math)
+      const convertImps = NumberT.from
+      let cnvBtoN
+      for (const [pattern, convFactory] of convertImps.patterns) {
+         if (pattern.match([BooleanT])) {
+            cnvBtoN = convFactory(math)
+            break
+         }
+      }
       assert.strictEqual(cnvBtoN(true), 1)
       assert.strictEqual(cnvBtoN(false), 0)
    })