Check for keys in term definition outside that expected #214
Conversation
…, `@id`, `@language`, `@reverse`, and `@type`. This also sets up for additional keywords in 1.1.
|
This, or something similar, should probably be part of 0.5.x, as it was a hole in the 1.0 spec that we did not check for extra keywords in a term definition. That said, it perhaps should be something short of an error, or check for some weaker mode. It could also constrain itself to only look for things that start with |
lib/context.js
Outdated
| @@ -442,6 +460,10 @@ api.createTermDefinition = (activeCtx, localCtx, term, defined) => { | |||
| mapping['@language'] = language; | |||
| } | |||
|
|
|||
| if('@next' in value) { | |||
|
|
|||
| } | |||
There was a problem hiding this comment.
Should have been removed, and should have been @nest for a different branch.
|
Is this covered in 1.1 spec and/or the test suite? Not sure what the compatibility story is for such changes. I'm fine adding more checks though at some point we'll want to profile the code and put such things behind flags if needed so there can be a fast mode for known correct data. |
|
1.0 did not require such checks, which is why we needed to add |
|
Won't this always run in both 1.0 and 1.1 modes? Just worried about the edge case of 1.0 input with bogus keys that used to be processed but now fails. |
|
It's true that an old processor would process invalid term definition and container keys and not report an error, but this should be considered a bug, IMHO, even though the 1.0 spec did not call it out. It's a feature that an updated processor, in 1.0 mode, will find such problems, and it enhances forward compatibility; if we had done this, we wouldn't need There are a couple of other cases where the new spec causes a processor running in 1.0 mode to be have differently, for example, compact IRI selection of appropriate terms to use for prefixes. Your call; my processor performs this test in 1.0, and I've not seen any reported issues. If you don't do this, and someone runs through the playground using new features, but without being in 1.1 mode, they could be confused about why output isn't as expected, which would be a worse problem then people using bad contexts now that report an error. An alternative, if you have the capability, would be to make it a softer error/warning, and allow a |
|
I'm fine with better checks going forward. Are there test suite tests for this? Would be good to have them so at least all implementations do the same thing. I don't think we have a warning system yet. The compactionMap/expansionMap callback-like api options are close though. But in cases like this probably better to just bail for problems. |
|
There is one test on the use of Similarly, there are not, and should be 1.0 tests to make sure |
@container,@id,@language,@reverse, and@type. This also sets up for additional keywords in 1.1.