Validation errors for ph libs

[Richard-Seaman]

Hi.

First off, thanks for all of the work that has gone into this brilliant library.

I have been using Haystack for a number of years now (mostly via Skyspark) but am only really starting to explore the power of defs.

I’m toying around with adding some custom libs and normalising them into a namespace. It’s straightforward to do this using the existing HNormalizer.spec.ts and simply including any additional custom libs here:

haystack-core/spec/core/HNormalizer.spec.ts

Lines 97 to 102 in 29425db

	libs = [
	await readLib('ph'),
	await readLib('phScience'),
	await readLib('phIoT'),
	lib,
	]

I wanted to validate my custom libs. I found the relevant existing unit tests here:
https://github.com/j2inn/haystack-core/blob/29425dbf2e489f6b13f2b59f15605c985bec1bf2/spec/core/HNormalizer.spec.ts#L600C13-L600C65

I added the following unit test:

it('ph libs have no validation errors', async function (): Promise<void> {
	await normalizer.normalize()
	expect(logger.error).not.toHaveBeenCalled()
})

I was surprised to see that errors were being generated from the standard ph libraries:

  ● HNormalizer › #normalize() › validate › verifies tag values match the def's declared types › ph libs have no validation errors

    expect(jest.fn()).not.toHaveBeenCalled()

    Expected number of calls: 0
    Received number of calls: 12

    1: "Tag 'geoState.enum' should be 'dict'", "tagIsNotKind", {"defName": "geoState", "kind": "dict", "tag": "enum"}
    2: "Tag 'geoCountry.enum' should be 'dict'", "tagIsNotKind", {"defName": "geoCountry", "kind": "dict", "tag": "enum"}
    3: "Tag 'kind.enum' should be 'dict'", "tagIsNotKind", {"defName": "kind", "kind": "dict", "tag": "enum"}

The first three examples above all relate to the enum tag. Its def is as follows:

def:^enum
doc:
 Defines an eumeration of string key names.  The range may also be applied
 to format a Bool ordered as "false,true".  The string value of this tag
 may be formatted in one of four ways:
  - dict of dicts keyed by name and Dict values for meta such as 'doc' tag
  - comma separated names on one line
  - names separated by a "\n" newline character
  - markdown unordered list formatted as a series of "- name: description\n"
 
 Enum names *should* follow valid tag naming rules (start with lowercase
 ASCII letter and contain only ASCII letters and numbers).  However in
 cases where mapping directly to external enumerations enum names can
 contain arbitrary characters such as space.
is:[^str,^dict]
lib:^lib:ph
tagOn:[^def,^point]

I think the handling of multiple inheritance (is:[^str,^dict]) within verifyValuesMatchDefTypes may be the source of the problem:

haystack-core/src/core/HNormalizer.ts

Lines 1028 to 1054 in 29425db

	private verifyValuesMatchDefTypes(
	tagName: string,
	tagDefNode: DefTreeNode,
	tagValue: HVal,
	defName: string
	): void {
	for (const kind of Object.values(Kind)) {
	if (tagDefNode.extends(kind) && !valueIsKind(tagValue, kind)) {
	// If the value does not match then test to see if this
	// is an accumulated value (i.e. a list of the values).
	const accumuatedOk =
	tagDefNode.def.has('accumulate') &&
	valueIsKind<HList>(tagValue, Kind.List) &&
	(tagValue as HList)
	.toArray()
	.every((val) => val?.isKind(kind))
	if (!accumuatedOk) {
	this.error('tagIsNotKind', {
	defName: defName,
	tag: tagName,
	kind,
	})
	}
	}
	}
	}

Should the desired behaviour be that the tag's value matches one of the kinds which it extends, rather than every one?
If this is the case, I'd be happy to raise a PR if you're open to contributions.

Note that it also seems to impact some of the existing tests as I think they rely on there being no existing validation errors. For example, the following test still passes:

it('logs an error for a bool value that is a string type', async function (): Promise<void> {
	// addDefs(
	// 	HDict.make({
	// 		def: HSymbol.make('foo'),
	// 		is: HSymbol.make(Kind.Str),
	// 	}),
	// 	HDict.make({
	// 		def: HSymbol.make('foo2'),
	// 		is: HSymbol.make(Kind.Str),
	// 		foo: HBool.make(true),
	// 	})
	// )

	await normalizer.normalize()
	expect(logger.error).toHaveBeenCalled()
})