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

Design document for variable mutability and namespacing #469

Merged
merged 15 commits into from
Oct 9, 2023

Conversation

aphillips
Copy link
Member

Per today's call (2023-09-04) here is the start of the design doc to address the need to decide how to handle:

  • variable mutability
  • local vs. external variable "masking"

This is based in large part on the discussion in #310 coupled with some discussion elsewhere. I incorporated commentary liberally.

The design presented here is for limited mutability (only via a separate keyword) and for separate namespaces.

Let the games begin.

@aphillips aphillips added syntax Issues related with MF Syntax design Design principles, decisions Action-Item specification labels Sep 4, 2023
@aphillips aphillips self-assigned this Sep 4, 2023
Copy link
Collaborator

@eemeli eemeli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A first pass.

_What properties does the solution have to manifest to enable the use-cases above?_

- Be able to re-annotate variables without having to rename them in the message body
- Allow static analysis to detect mistakes when referencing an undefined local variable
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Allow static analysis to detect mistakes when referencing an undefined local variable

I don't think this should be considered as a requirement. At best it's a nice-to-have feature improving static analysis when a single translated message needs to be considered in isolation, and not with respect to its form in the source locale.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you prefer a use case for this? This specific list was taken from @stam's comment in #310 near the end (after I had written a longer and more detailed set of requirements). The "static analysis" requirement is kind of "boiling down" the question of whether people will make mistakes with overwriting.


```abnf
variable = local_var / external_var
local_var = "@_" name
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, no. We categorically do not need any two-character prefixes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See later discussion. This is horrifying while we agree on something...

> ```
> let @_local = {$external :transform}
> modify @_local = {@_local :modification with=options}
> modify $external = {$external :transform adding=annotation}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks an awful lot like we're doing pass-by-reference for formatting function arguments and we're modifying the $external value in the calling context. Is that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It intentionally looks like it is modifying the value. In practice, it (probably) does not have write access to the external value and is only masking it for the duration of this message. But the idea is that the value is mutable only if you are consciously mutating it.

Note that we could disallow modify to externals, that is, you have to either let or modify a local variable in order to annotate an external var.

Comment on lines 177 to 180
- `@@foo`
- `@foo@`
- `@!foo`
- `@ONLY_UPPER_ASCII_SNAKE`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are all pretty ugly, but I presume that's intentional. Do we not want users to use local variables? Because that's what we're saying by making them ugly and clumsy.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a beautiful way to make locale variables and not have them be confused with external variables "of the same name"?

An alternative not presented is the "rope merchant" option: do nothing (ugly or otherwise) and let people adopt whatever convention prevents mistakes, e.g. $foo and #foo are separate values (external vs. local).

Note: I just recalled that we wanted to use @ for annotation, so will change that to # globally.

Comment on lines +206 to +209
{"arg1": "10000"}
...
let $arg1 = {42}
{This always says {$arg1} == 42}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposed design has this same flaw:

let @_arg1 = {42}
{This always says {@_arg1} == 42}

Anyone not working with MF2 daily will easily forget when looking at that message which prefix is for external and which for internal ones, leading to the exact same failure mode.

Is this the only argument against this alternative?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposed design has the matching example in the next block, in which I call out the similarity error. But it isn't "the same flaw". In this case, the declaration completely blocks the original value.

This may not be super terrible. Not having separate namespaces is conceptually simpler and the ability to just annotate via declaration is much easier than doing multiple assignments and teaching developers/translators/etc. about local vs. external vars. Maybe modify (however we choose to spell it in the end) solves the "accidental overwrite" problem and is enough?

```
{ "arg1": "37"}
...
let $arg1 = {|42| :number maxFractionDigits=2} // error
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not throwing an error for this should be considered a requirement.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this option, this is an error, because one is attempting to overwrite $arg1 with a new declaration. Presumably if we chose this option, it would be to ensure that this was an error.

I happen to agree that there is a use case that needs to be accounted for here (I have made the argument elsewhere many times about declarations not knowing what all the values are externally)

aphillips and others added 2 commits September 4, 2023 14:54
- change `@` to `#` because we want to use `@` for annotations such as `@locale`
- Provide text that considers not making ugly local variables
- Provide use cases for static analysis
- Call out the perfidy of the author in stealing ill-baked requirements
@eemeli
Copy link
Collaborator

eemeli commented Sep 6, 2023

Here's an alternative proposal with immutable variables that I think could work. I think someone else may have previously proposed something like this, but I can't find that source right away.

Let us introduce a new keyword input that allows for the annotation of external variables. It works like this:

input {$count :number}
input {$date :datetime dateStyle=long}
match {$count}
when 1 {You received one message on {$date}}
when * {You received {$count} messages on {$date}}

This effectively replaces the current "hack" of saying let $foo = {$foo ...}, and provides a way to explicitly declare that a variable is non-local: input {$bar} doesn't require an annotation.

At the syntax/data model level, input or let declarations would not be required for all selectors and placeholders, but a user-configured validator could of course be stricter.

In the ABNF the change would look like this:

message = [s] *(input-annotation [s]) *(declaration [s]) body [s]

input-annotation = input [s] "{" [s] operand [s annotation] [s] "}"
input = %x69.6E.70.75.74  ; "input"

The expression rule can't be used here directly because the operand is required.

With this in place, I would support prohibiting re-assignment/shadowing, as long as a local let declaration would take precedence and not cause an error if a similarly named external variable is passed to the formatter without a corresponding input declaration in the message.

The use case of chaining operations on a variable with a single name is not supported here, and the $foo1, $foo2 $foo3 sorts of names would be required for that.

One observation I do have is that if our variables are immutable, then we should be able to reorder local variable declarations exactly like we allow for when variants to be reordered. We do still need to prohibit referential loops, but that doesn't require the declarations to be in any specific order.

@aphillips
Copy link
Member Author

Here's an alternative proposal with immutable variables that I think could work. I think someone else may have previously proposed something like this, but I can't find that source right away.

I think this is an interesting proposal. I am not sure that I'm excited about it, although it does solve annotation of external values.

One observation I do have is that if our variables are immutable, then we should be able to reorder local variable declarations exactly like we allow for when variants to be reordered. We do still need to prohibit referential loops, but that doesn't require the declarations to be in any specific order.

This would bring back shadowing concerns, though, wouldn't it?

For inputs a=1;b=2;c=3 what do these print:

let $c = {$b}
let $b = {$a}
{a={$a} b={$b} c={$c}}

let $b = {$a}
let $c = {$b}
{a={$a} b={$b} c={$c}}

@stasm
Copy link
Collaborator

stasm commented Sep 9, 2023

Here's an alternative proposal with immutable variables that I think could work. I think someone else may have previously proposed something like this, but I can't find that source right away.

We briefly discussed defining input variables in #403 (comment). The, in #310 (comment) @aphillips suggested the modify keyword which would avoid the need of reassignment.

I quite like the idea of input {$count :number} because it cleverly address one of the most common existing use-cases for local variables (decorating input variables) without introducing local variables.

Earlier in the thread, @eemeli commented on proposed syntax for local variables:

These are all pretty ugly, but I presume that's intentional. Do we not want users to use local variables? Because that's what we're saying by making them ugly and clumsy.

Maybe we actually don't want local variables to be common, especially if there's a convenient and dedicated solution to annotate input variables (like the input ... syntax). What are the use-cases for local variables that are not transformed input variables? I can think of a few:

  • Transforming input variables in a way that prevents access to the original value. Most formatters can be implemented in a "lazy" way that makes it possible to access the raw original value. However, I can imagine some formatters don't, although it can be argued that the following example is overly complex and contrived.

    let $firstUser = {$guests :list.take count=1}
    let $tailCount = {$guests :list.length offset=1}
    
    {{$firstUser} and ${tailCount} other people like this.}
    
  • Factoring out literals with their formatting options to avoid repetition across multiple variants. A synthetic example:

    let $epoch = {0 :datetime weekday=long}
    match {$count}
    when 1 {{$epoch} one.}
    when * {{$epoch} many.}
    

    I note, however, that perhaps we should consider how badly we want to avoid repetition in such cases. After all, top-level variants do cause repetition of translations anyways, by design.

  • Factoring out markup to avoid repetition across multiple variants.

    let $openBold = {bold +html opt=val}
    match {$count}
    when 1 {{$openBold}strong{bold -html}}
    when * {{$openBold}strong{bold -html}}
    

    I think this is underspecified in the current design of open/close elements (Exploration: sigils inside expressions #448), and I'm not sure if it should be allowed. Same as above, perhaps repetition isn't actually that bad? We're OK with repeating text content in all variants — why not markup?

@eemeli
Copy link
Collaborator

eemeli commented Sep 9, 2023

One observation I do have is that if our variables are immutable, then we should be able to reorder local variable declarations exactly like we allow for when variants to be reordered. We do still need to prohibit referential loops, but that doesn't require the declarations to be in any specific order.

This would bring back shadowing concerns, though, wouldn't it?

Not really, because each $var would get looked up by a well defined method:

  1. If there's an input statement for $var, find the corresponding var value in the inputs and apply the annotation to it.
  2. Else, if there's a let $var statement, use its value.
  3. Else, find the corresponding var value in the inputs.
  4. Else, complain with Unresolved Variable.

So if the value gets determined in step 2, the corresponding input is ignored.

For inputs a=1;b=2;c=3 what do these print:

let $c = {$b}
let $b = {$a}
{a={$a} b={$b} c={$c}}

let $b = {$a}
let $c = {$b}
{a={$a} b={$b} c={$c}}

With logic as defined above, both of those would end up formatting as a=1 b=1 c=1.

@aphillips
Copy link
Member Author

@eemeli noted:

Not really, because each $var would get looked up by a well defined method:

I think I'm still worried about the conceptual simplicity element of this. For example, would this be technically allowed?

input {$a :whatever}
let $a = {$b}
...

I put the reordered examples because people might make mistakes based on thinking of the lines as being executed in order.

@stasm noted:

I quite like the idea of input {$count :number} because it cleverly address one of the most common existing use-cases for local variables (decorating input variables) without introducing local variables

I wasn't keen on input previously because the way it was originally suggested was to require it before using the variable (that is, variables had to be declared before use). This seems onerous when all I really want is {Hello {$user}}. Our syntax should be easy to use (at least in the common cases). But I agree with you that it helps give us an out for modifying values without allocating local variables.

Stepping back: does everyone agree with the matrix of options in the document? Are there other options we want to consider? Comparing the options and weighting our reasons behind those choices seems useful

Comment on lines 177 to 181
The order of declarations does not matter,
to allow for e.g. `input` annotations to refer to `local` variables.

References to later declarations are not allowed,
so this is considered an error:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This confuses me. If the order doesn't matter then forward definitions should work, but then you say that they don't (so the order would matter). I think the first paragraph might be a holdover from when you had input followed by local (the new ABNF allows intermixing: intentional?)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We ended up determining that forward-referencing should not be allowed, because if we do allow it, someone not aware of that fact looking at the example message could easily presume that the first $bar reference is to an external variable that's shadowed by the local $bar declaration.

exploration/variable-mutability.md Outdated Show resolved Hide resolved
aphillips and others added 2 commits October 4, 2023 13:28
Specifically the one about forward references
Copy link
Collaborator

@eemeli eemeli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're clearly already presuming this to be our syntax in our further work, so it'd be good to note this as "Accepted".

exploration/variable-mutability.md Show resolved Hide resolved
exploration/variable-mutability.md Outdated Show resolved Hide resolved
exploration/variable-mutability.md Outdated Show resolved Hide resolved
@aphillips aphillips merged commit 0fc21d0 into main Oct 9, 2023
3 checks passed
@aphillips aphillips deleted the aphillips-variable-mutability-design branch October 9, 2023 18:09
aphillips added a commit that referenced this pull request Oct 11, 2023
* Create notes-2023-10-02.md (#486)

* Design document for variable mutability and namespacing (#469)

* Design document for variable mutability and namespacing

* style: Apply Prettier

* Partly address #299

* style: Apply Prettier

* Address comments, fix sigil choice

- change `@` to `#` because we want to use `@` for annotations such as `@locale`
- Provide text that considers not making ugly local variables
- Provide use cases for static analysis
- Call out the perfidy of the author in stealing ill-baked requirements

* style: Apply Prettier

* Add @eemelie's `input` proposal as an option considered

* Update exploration/variable-mutability.md

Co-authored-by: Eemeli Aro <[email protected]>

* Add new proposed design

* Update exploration/variable-mutability.md

Co-authored-by: Addison Phillips <[email protected]>

* Address @eemeli's comments

Specifically the one about forward references

* style: Apply Prettier

* Update exploration/variable-mutability.md

Co-authored-by: Eemeli Aro <[email protected]>

* Update exploration/variable-mutability.md

Co-authored-by: Eemeli Aro <[email protected]>

* Update exploration/variable-mutability.md

Co-authored-by: Eemeli Aro <[email protected]>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Eemeli Aro <[email protected]>
Co-authored-by: Eemeli Aro <[email protected]>

* Create notes-2023-10-09.md

* Update notes-2023-10-09.md

* Remove the Prettier push action (#491)

Remove the Prettier lint action

* Remove numbers from the existing design proposals (#490)

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Eemeli Aro <[email protected]>
Co-authored-by: Eemeli Aro <[email protected]>
Co-authored-by: Stanisław Małolepszy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design principles, decisions specification syntax Issues related with MF Syntax
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants