[Dot Shorthands] DevTools support

[kallentu]

This issue tracks the work needed to update dev tools to support dot shorthands.

• Property editor changes (enum selection stays shorthand or long-form depending on what user wrote)

Feel free to update if there's more work.

cc. @elliette @kenzieschmoll

[bwilkerson]

Property editor changes ...

The property editor changes are actually in the analysis_server package.

[elliette]

Just tested the Property Editor with dot-shorthands enabled. I think the only work here is to make sure that if a user is already using dot shorthands for an enum, we don't then change it to the full enum representation when they edit that property. (See gif below)

I believe we just need to modify the _computeValueCode method (here) in the Analysis Server to return the dot shorthand version of the enum.

[kallentu]

Given that this is only for enum dot shorthands, we would probably need to check that we have a DotShorthandPropertyAccess AST node (with the isDotShorthand mixin flag being true, but this should always be the case with single enum dot shorthands).

Then we'd make sure we're keeping the shorthand representation once we know we're working with a shorthand.

[elliette]

FYI @DanTup - this seems like an easy little change that I'm going to try to make to get more familiar with Property Editor API implementation, but will let you know if I run into difficulties!

[DanTup]

@elliette sgtm!

I believe we just need to modify the _computeValueCode method

I think you'll also need to modify the code that returns the set of strings to the property editor before the edit:

sdk/pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_editable_arguments.dart

Lines 203 to 205 in 37b06b4

	} else if (parameter.type case InterfaceType(:EnumElement element)) {
	type = 'enum';
	options = getQualifiedEnumConstantNames(element);

Both of them use getQualifiedEnumConstantNames which currently returns the fully qualified name. Perhaps the validation in _computeValueCode could be updated to allow both qualified and shorthand names, then only the code above needs to change behaviour to use/not use dot shorthands.

There's probably also a question of whether to use shorthand or not when there is no current value (or if we support changing it, a variable or some other indirect reference to an enum value).

[elliette]

I think you'll also need to modify the code that returns the set of strings to the property editor before the edit:

Currently the Property Editor client strips the enum namespace to only display the values in the dropdown (essentially displayed them with dot shorthand syntax), so I think as long as the Analysis Server always sends the full enums to the client regardless of whether dot shorthands are used or not we should be good.

[bwilkerson]

Given that this is only for enum dot shorthands ...

Is it (only for enums)? What do we display if the user has used dot shorthands for something other than an enum? (I'm assuming that can happen.)

There's probably also a question of whether to use shorthand or not when there is no current value ...

I think we should encourage the use of dot shorthands by using them where possible when adding a new argument that wasn't there before. Gated, of course, on whether dot shorthands are supported in the effective language version for the file.

I think as long as the Analysis Server always sends the full enums to the client regardless of whether dot shorthands are used or not we should be good.

The property editor can't check the effective language version for the file, so it has no idea whether using dot shorthands is appropriate. I think the server has to provide this information. Is it going to be a problem if the property editor doesn't get the fully qualified name?

[elliette]

Is it (only for enums)? What do we display if the user has used dot shorthands for something other than an enum? (I'm assuming that can happen.)

Since the only other property types we display are strings, doubles/ints and booleans, I don't think we need to handle dot-shorthand syntax for anything other than enums.

I think we should encourage the use of dot shorthands by using them where possible when adding a new argument that wasn't there before. Gated, of course, on whether dot shorthands are supported in the effective language version for the file.

Ah that's a good point! Will do.

The property editor can't check the effective language version for the file, so it has no idea whether using dot shorthands is appropriate. I think the server has to provide this information. Is it going to be a problem if the property editor doesn't get the fully qualified name?

Yes, it will be a problem (or at least it will changes to the Property Editor client) if it receives anything other than the fully qualified name. That's why I think the best approach is for the Analysis Server to always send and receive the fully qualified name to the Property Editor client, but then send an edit to the Editor client for either the fully qualified name or the dot-shorthand syntax depending on the effective language version of the file.

[bwilkerson]

Since the only other property types we display are strings, doubles/ints and booleans, I don't think we need to handle dot-shorthand syntax for anything other than enums.

What if the value for a double-valued parameter is a dot-shorthand using constant like .nan? I'd hope that we display such constants similar to the way we display enum constants (that is, by removing the class name double).

That's why I think the best approach is for the Analysis Server to always send and receive the fully qualified name to the Property Editor client, but then send an edit to the Editor client for either the fully qualified name or the dot-shorthand syntax depending on the effective language version of the file.

I think that would work.

[DanTup]

I think the best approach is for the Analysis Server to always send and receive the fully qualified name to the Property Editor client, but then send an edit to the Editor client for either the fully qualified name or the dot-shorthand syntax depending on the effective language version of the file.

Ah yes, that makes sense to me - I misunderstood what you meant before :-)