Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is it an error to pass a bad option to a function? #738

Open
catamorphism opened this issue Mar 19, 2024 · 5 comments
Open

Is it an error to pass a bad option to a function? #738

catamorphism opened this issue Mar 19, 2024 · 5 comments
Labels
errors Issues related to the errors section of the spec LDML46 Items that must be first for post-tech preview (LDML46)

Comments

@catamorphism
Copy link
Collaborator

I was looking at the following test case in https://github.com/unicode-org/message-format-wg/blob/main/test/test-functions.json :

{
      "src": ".input {$foo :number minimumFractionDigits=foo} {{bar {$foo}}}",
      "params": { "foo": 4.2 },
      "exp": "bar {$foo}",
      "errors": [{ "type": "bad-option" }]
    },

This means that the message ".input {$foo :number minimumFractionDigits=foo} {{bar {$foo}}}" is supposed to trigger an error because the string "foo" can't be parsed as an integer.

However, there is no error in errors.md that corresponds to "bad-option" in the test description. Should there be one? It's not really covered by "invalid expression error" (or at least I don't think so); it's definitely not covered by "Operand Mismatch Error" since the description of that error only refers to the function's operand and not its options.

If this is really an error, I think there should be a separate "Option Mismatch Error" to go with "Operand Mismatch Error".

See #706 for the general issue to create a list of error names that can be used in spec tests. I thought this should be a separate issue since the registry spec doesn't say much about error handling for bad option values.

@catamorphism catamorphism added errors Issues related to the errors section of the spec LDML46 Items that must be first for post-tech preview (LDML46) labels Mar 19, 2024
catamorphism added a commit to catamorphism/message-format-wg that referenced this issue Mar 19, 2024
This error has nothing corresponding to it in the spec.
The tests can be re-added once
unicode-org#738
is resolved.
@aphillips
Copy link
Member

aphillips commented Mar 19, 2024

It's not really covered by "invalid expression error"

Why not? The description of that error is:

An Invalid Expression error occurs when a message includes an expression whose implementation-defined internal requirements produce an error during function resolution or when a function returns a value (such as null) that the implementation does not support.

The value of options is an implementation-defined internal requirement...

@catamorphism
Copy link
Collaborator Author

It's not really covered by "invalid expression error"

Why not? The description of that error is:

An Invalid Expression error occurs when a message includes an expression whose implementation-defined internal requirements produce an error during function resolution or when a function returns a value (such as null) that the implementation does not support.

In the message, the value of the option isn't an expression. Though I guess you could argue that the expression as a whole, including the options, is invalid if one of the options is invalid.

Still, since the test suite does use a separate error name for this case, I thought it was worth discussing.

@aphillips
Copy link
Member

I thought it was worth discussing.

Totally worth the discussion.

@eemeli
Copy link
Collaborator

eemeli commented Mar 19, 2024

This could be either a Resolution Error or a Formatting Error:

Formatting Errors occur during the formatting of a resolved value, for example when encountering a value with an unsupported type or an internally inconsistent set of options.

@catamorphism
Copy link
Collaborator Author

catamorphism commented Apr 29, 2024

We agreed in today's call that this issue will be blocked on an issue that @mihnita will open about error handling in general (the question of how to represent an error together with partial output / a fallback, given that this is hard to do idiomatically in ICU and perhaps elsewhere.)

@aphillips aphillips removed the Agenda+ label May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
errors Issues related to the errors section of the spec LDML46 Items that must be first for post-tech preview (LDML46)
Projects
None yet
Development

No branches or pull requests

3 participants