Implement named arguments anywhere feature

[chloestefantsova]

This issue is for implementing dart-lang/language#1072 and referencing it in the related CLs.

The language flag here is named-arguments-anywhere, and the github project for this feature is https://github.com/orgs/dart-lang/projects/33.

Spec and test tasks

• write the spec for the named arguments anywhere feature language#1948
• language tests for named arguments anywhere #47576
• co19 tests Add tests for "Named arguments anywhere" feature co19#1228

Implementation tasks

• CFE implementation: fcd684e
• Analyzer implementation: 82dc2cf
• syntax highlighters implementation (https://github.com/dart-lang/dart-syntax-highlight/, https://github.com/highlightjs/highlight.js, https://github.com/codemirror/CodeMirror/); assumed no-op
• DDC implementation (assumed no-op)
• dart2js implementation (assumed no-op)
• VM implementation (assumed no-op)
• build systems implementation (assumed no-op)
• web debugging support (assumed no-op)
• core library updates for named arguments anywhere #47577
• VM service protocol support for the named-arguments-anywhere feature #47816
• dartdoc implementation for for named arguments anywhere dartdoc#2850
• IDE tooling (analysis server) implementation for named arguments anywhere #47578
• dart_style implementation for named arguments anywhere dart_style#1072
• Cider implementation for for named arguments anywhere #47580
• Kythe implementation for named arguments anywhere #47579
• ☂️ linter implementation for named arguments anywhere #58562
• Angular compiler implementation for named arguments anywhere  #47591
• IntelliJ implementation for named arguments anywhere #47592
• documentation for named arguments anywhere site-www#3653
• tests validated as passing (tracking via a google sheet)

[chloestefantsova]

The implementation in the CFE is landed as fcd684e.

[mit-mit]

@chloestefantsova what experiment flag is this behind?

[chloestefantsova]

@chloestefantsova what experiment flag is this behind?

At the moment, it's behind an internal CFE flag --enable-unscheduled-experiments. Once the actual experiment name is determined, it will be updated.

[eernstg]

Perhaps --enable-experiment=named-arguments-anywhere?

[chloestefantsova]

Perhaps --enable-experiment=named-arguments-anywhere?

I like the name :) I'll add the flag.

[eernstg]

@devoncarew, @srawlins, @chloestefantsova, I have the impression that this feature would be well placed in the language evolution pipeline at some point in Q1. Do you agree, and do you have a specific milestone in mind?

[srawlins]

Q1 sounds great to me.

[devoncarew]

I'm going to start creating sub-issues here to scope the work.

[devoncarew]

@sigmundch - I assume the DDC and dart2js implementations are no-ops here. Can you confirm that (or ask someone else to)? Thanks!

[devoncarew]

@a-siva / @mkustermann - can you confirm that the VM implementation for this is a no-op? Otherwise we'll create a tracking bug for the VM work.

[sigmundch]

Agree, I believe nothing is needed for DDC and dart2js directly. /cc @nshahan @fishythefish

The only open question is whether there is work needed for the web debugger integration, which may entail some work in DDC. At a glance, I would be surprised if we needed anything special for that either, but it's good to double check. /cc @nshahan @annagrin

[mkustermann]

@a-siva / @mkustermann - can you confirm that the VM implementation for this is a no-op? Otherwise we'll create a tracking bug for the VM work.

It seems this is purely a use-site only feature, so one wouldn't need to worry about observing mixed positional and named parameters on declaration site with dart:mirrors.

If the CFE desugars use-sites to existing Kernel AST constructs with the correct evaluation order (@chloestefantsova guess this is true?), then there is indeed no work for the VM team here (pending @bkonyi's answer on vm-service, since that includes debugging support).

[devoncarew]

@jakemac53 - do we expect any implementation work for this wrt build systems? If so we'll file an issue to track.

[jakemac53]

Nothing major that jumps to mind other than general language versioning support which we already have in place.

Internally language versioning is weird (we don't expose actual versions to users), so some amount of work will have to happen to allow users to enable this (similar to how null safety is enabled internally).

EDIT: One other potential area of work would be around the analyzer apis - in general builders don't operate on method bodies though and shouldn't see these types of invocations, but some do go to the AST, and could be affected. Not anything the dart team owns though afaik.

[annagrin]

The only open question is whether there is work needed for the web debugger integration, which may entail some work in DDC. At a glance, I would be surprised if we needed anything special for that either, but it's good to double check.

We would need to at least add tests for newly allowed expressions to evaluate correctly. The debugging support depends on whether there are any changes in the vm service API (@bkonyi) or significant changes to support the compilation in DDC (@nshahan) but most likely nothing needed to be done for the debugger.

[devoncarew]

@bkonyi - can you speak to Anna's question above wrt any service protocol changes to support this feature?

[bkonyi]

Sorry for the delay. No, I don't think there's anything special we'd need to change from the service's perspective.

[devoncarew]

Thanks!

[devoncarew]

I'll keep a task for the vm service protocol to cover Anna's comment above - 'we would need to at least add tests for newly allowed expressions to evaluate correctly'. It sounds like no protocol / api changes however, so nothing that would leak through to other clients.

@nshahan - for Anna's other question - The debugging support depends ... or significant changes to support the compilation in DDC; I'm guessing that there would be no such?

[nshahan]

@devoncarew My assumption was that there would be little to no implementation on the DDC side. I will take a look at the feature tests that are failing to get an idea of what will be required. I'll report back if I find any significant issues.

[nshahan]

It looks like the failing tests in DDC are all related to redirecting constructors that apply named arguments anywhere in the redirection. We are not correctly creating the temporary variables created by the CFE lowering. I'll investigate a fix but it doesn't look like it will be very big.

See #47831

[rakudrama]

Update: dart2js now passing.

dart2js has an existing bug (#47047) which is tickled by the new tests. I'll fix it (WIP https://dart-review.googlesource.com/c/sdk/+/223381)

[munificent]

I have this implemented in dart_style now, but not yet released and rolled into the Dart SDK. I'll roll it in once I get enhanced enums done too.

[chloestefantsova]

@devoncarew Can this issue be closed now, given the unchecked boxes in the top posting?

[devoncarew]

Yes, I think we can close (and track any remaining work in separate issues).

@leafpetersen @mit-mit - I lost track of this issue when I switched away from TPM style work. Not sure what the new project management process for language features looks like going forward.