change type of Variant Key Mismatch errors

[macchiati]

The Variant Key Mismatch errors are miscategorized in Variant Key Mismatch errors as being "Data Model errors". They are clearly syntax errors, and do not require any knowledge of the data model or "semantics" to detect. Instead, they are syntactic errors, and can be detected in parsing.

I think there is a bit of confusion because they are not detectible by the BNF. But that should not be the criterion. There are many instances in many specifications of syntax errors that are not detectible by a BNF processor, such as Well-formedness constraint in https://www.w3.org/TR/xml/#sec-logical-struct and elsewhere.

[stasm]

Agreed that this can be detected during parsing. This looks like a naming issue to me. Both errors in the "data model" category are about issues that can be discovered when building the AST. I guess that's what the "data model" is referring to -- the AST. Perhaps calling them "abstract syntax errors" or "syntax validation errors" would work better?

[eemeli]

Here's what I replied on this previously in #320 (comment), which may have been missed among all the other comments on that PR:

---

The intent in differentiating this as a "data model" rather than "syntax" error is that it allows us to apply the same error check independently of the syntax used by the message. Here's what I said about this earlier in #320 (comment):

Let's say that there's a FooFormat9000 that's exactly like MF1, except that it doesn't require an "other" case for its selectors and defaults to an empty string instead. Now, if a FooFormat9000 message is parsed into an MF2 data model for use with an MF2 runtime, the code doing so might not add the expected default variant to the data model. When the MF2 runtime discovers this, it needs to emit a [Data Model] Error rather than a Syntax Error, because it doesn't know anything about FooFormat9000 where this syntax is valid.

An MF2 syntax parser would be expected to emit the data model errors during its work, as this is included in the spec (line 176):

Syntax and Data Model errors must be emitted as soon as possible.

This separation also matches the differentiation between "well-formed" and "valid" messages in the syntax spec.

[macchiati]

That theoretic concern is, I believe, swamped by the misleading categorization of the error. It is in fact, following the spec, an error in the syntax.

Now, if you were to bolt on a very different syntax onto MF2.0 (which we certainly want to allow), it might be that that (and perhaps other errors, like missing match expressions) errors get caught at different points. However, errors should be reported in as sensible a way as possible to the user.

[aphillips]

I agree that this is a syntax error. These errors can be detected by the parser without any knowledge of the meaning of the parsed tokens (and the resulting message is not well-formed). Data Model errors in this area would include things such as type mismatches or unknown function types (e.g. using one of the reserved sigils that is undefined by the implementation).

[eemeli]

Okay, let's say we change it so that Variant Key Mismatch is a Syntax Error rather than a Data Model Error.

Now, given that our first deliverable is "A formal definition of the canonical data model for representing localizable dynamic message strings", I think we need to consider what happens if such a representation of a message is constructed by some external system, and then fed into an MF2 runtime.

Given this situation, if the message does not have a matching number of selectors and keys, when do we emit what error, or is this undefined behaviour?

[macchiati]

Of course. If at any level of a pipeline the well-formedness of structures passed in by higher modules is not guaranteed, it is perfectly fine (and best practice) to emit an error for ill-formed structures, etc.

…

On Thu, Apr 13, 2023 at 8:19 AM Eemeli Aro ***@***.***> wrote: Okay, let's say we change it so that Variant Key Mismatch is a Syntax Error rather than a Data Model Error. Now, given that our first deliverable is "A formal definition of the canonical data model for representing localizable dynamic message strings", I think we need to consider what happens if such a representation of a message is constructed by some external system, and then fed into an MF2 runtime. Given this situation, if the message does not have a matching number of selectors and keys, when do we emit what error, or is this undefined behaviour? — Reply to this email directly, view it on GitHub <#367 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACJLEMGIMAOUKKNOAWWEHTLXBAKR3ANCNFSM6AAAAAAVYNYB5U> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[eemeli]

Before the change proposed in this issue, that "error for ill-formed structures" would be a "Variant Key Mismatch" Data Model Error. So if we ought to still emit such an error, is the ask here to have two different "Variant Key Mismatch" errors, or something else?

[macchiati]

I don't see any problem with the same error "id".

…

On Thu, Apr 13, 2023 at 9:08 AM Eemeli Aro ***@***.***> wrote: Before the change proposed in this issue, that "error for ill-formed structures" would be a "Variant Key Mismatch" Data Model Error. So if we ought to still emit such an error, is the ask here to have two different "Variant Key Mismatch" errors, or something else? — Reply to this email directly, view it on GitHub <#367 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACJLEMHY4T2FKFDBVL2JRTTXBAQH7ANCNFSM6AAAAAAVYNYB5U> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[aphillips]

I have changed my opinion here. I agree that it is an error in the syntax of the message to not have the number of keys match the number of selectors. However, the spec defines a "data model error":

Data Model errors occur when a message is invalid due to violating one of the semantic requirements on its structure

Matching the keys to the selectors is, in fact, a semantic requirement, since ABNF cannot define the match. Thus I agree with @eemeli that we should keep it as-is in the spec. Note that the categorization in the spec doesn't matter (is not normative in any meaningful way).

Marking as resolve-candidate. Please comment if you disagree with me.