Annotation based dropdowns not working for static methods on built-ins.

[jdunkerley]

Both encoding parameters should have the same dropdown (correctly shown for "Hello World").

The parameters are listed correctly and the type based dropdown is shown but for the static method on the built-in the get_widget_json is not called.

[Frizi]

Looks to be a GUI issue - the widget data is not requested for static methods. If the metadata is queried manually, it is reported correctly.

[Frizi]

Dug a little deeper and discovered the root cause. The expression call is properly parsed and recognized. Span tree looks like this:

▷Text.from_bytes [13]◁Root ext_id=1b02c13f-939a-4207-aecb-3ea15227ad3a
▷Text.from_bytes [13]◁ ├─Chained ext_id=1b02c13f-939a-4207-aecb-3ea15227ad3a
▷Text.from_bytes [13]◁ │  ├─Chained ast_id=1b02c13f-939a-4207-aecb-3ea15227ad3a
▷Text.from_bytes◁      │  │  ├─Operation ast_id=595a78b6-6895-4e93-b726-1e1dcf6bec2b
▷◁                     │  │  │  ├─InsertionPoint(BeforeArgument(0))
▷Text◁                 │  │  │  ├─Argument name="self" call_id=1b02c13f-939a-4207-aecb-3ea15227ad3a ast_id=297131fe-f5d2-4233-9b8e-89bb987211c5
    ▷.◁                │  │  │  ├─Operation ast_id=7a62a262-f424-4c03-81e5-5d7f00c08807
     ▷◁                │  │  │  ├─InsertionPoint(BeforeArgument(1))
     ▷from_bytes◁      │  │  │  ├─Argument ast_id=e59b521c-0cf9-4c50-a779-18e2d8f2ffa0
               ▷◁      │  │  │  ╰─InsertionPoint(Append)
                ▷[13]◁ │  │  ╰─Argument name="bytes" call_id=1b02c13f-939a-4207-aecb-3ea15227ad3a ast_id=d1e97858-a4e1-4ca1-9277-43d8164dcbc1
                ▷[◁    │  │     ├─Token
                 ▷◁    │  │     ├─InsertionPoint(BeforeArgument(1))
                 ▷13◁  │  │     ├─Argument ast_id=527498e0-2a2e-406a-916c-66ea3d88f480
                   ▷◁  │  │     ├─InsertionPoint(Append)
                   ▷]◁ │  │     ╰─Token
                    ▷◁ │  ╰─InsertionPoint(ExpectedArgument { index: 1, named: false }) name="encoding" call_id=1b02c13f-939a-4207-aecb-3ea15227ad3a
                    ▷◁ ╰─InsertionPoint(ExpectedArgument { index: 2, named: true }) name="on_problems" call_id=1b02c13f-939a-4207-aecb-3ea15227ad3a

The call and its target is properly recognized:

CallInfoMap { call_info: {1b02c13f-939a-4207-aecb-3ea15227ad3a: CallInfo { target_id: Some(297131fe-f5d2-4233-9b8e-89bb987211c5) }} }

Then, the IDE asks for the expression info of this call ID and receives one:

{
    "expressionId": "1b02c13f-939a-4207-aecb-3ea15227ad3a",
    "type": "Standard.Base.Function.Function",
    "methodPointer": {
        "module": "Standard.Base.Data.Text.Extensions",
        "definedOnType": "Standard.Base.Data.Text.Text.type",
        "name": "from_bytes"
    },
    ...
},

The methodPointer is then used to query suggestion database. Unfortunately, no entry is found. The method is in the suggestion database, but under a different key. This is the entry that we were SUPPOSED to find:

{
  "type": "Add",
  "id": 93,
  "suggestion": {
    "type": "method",
    "module": "Standard.Base.Data.Text.Extensions",
    "name": "from_bytes",
    "selfType": "Standard.Base.Data.Text.Text",
    ...
  }
},

Note that methodPointer.definedOnType does not match suggestion.selfType. The trailing .type causes the lookup to fail.

I'm not sure if this is the IDE issue and we should treat the .type specially, or if it's the language server that is inconsistent here.

[Frizi]

It turns out in some parts of IDE we already strip that postfix, and in fact rely on it to inform us about how to interpret that method call's arguments. Removing it on the language server side would break that logic.

enso/app/gui/src/controller/graph/executed.rs

Lines 415 to 425 in 3e739a7

	// When the entry was not resolved but the `defined_on_type` has a `.type` suffix,
	// try resolving it again with the suffix stripped. This indicates that a method was
	// called on type, either because it is a static method, or because it uses qualified
	// method syntax.
	maybe_entry.or_else(|| {
	let defined_on_type = method_call.defined_on_type.strip_suffix(".type")?.to_owned();
	let method_call = MethodPointer { defined_on_type, ..method_call.clone() };
	let entry = suggestion_db.lookup_by_method_pointer(&method_call)?;
	let invocation_info = entry.invocation_info(&suggestion_db, &self.parser());
	Some(invocation_info.with_called_on_type(true))
	})

In order to fix this, we have to use the same resolution strategy for widget related method queries to the suggestion database.

[farmaazon]

There are more places to fix, unfortunately, as - apparently - only you, @Frizi, have been aware of the .type suffix so far (the snippet you posted is also written by you, according to blame). So I think no other part of IDE actually expects .type suffix.

And another problem is that Language Server seems to be not consistent in adding .type suffix, as MethodPointers and expression type updates include them, but the suggestion database does not (relying on is_static flag). The search/complete also seems unaware of .type suffix, as it does not propose static methods of given type.

Of course, we can apply a set of patches, but I think those inconsistencies should be resolved in the long run.

[Frizi]

Yeah, I remember writing that now, though It took me a while to bring that to mind at first. The .type suffix is specifically added to definedOnType when the method target is a whole type, as opposed to a value of given type. This is not unique to statics, and it influences the interpretation of passed arguments. So I don't really consider that a language-server inconsistency, but rather a defficiency of our suggestion database lookup method.

expression	static	.type suffix in definedOnType	self argument	widget data visualization target
Text.from_bytes [13]	✅	✅	none	Text
" a ".trim	❌	❌	" a "	" a "
Text.trim " a "	❌	✅	" a "	" a "
text3.trim " a "	🤷	🤷 depends on text3 type	" a " or text3	" a " or text3

The last example is actually quite important, as it would be impossible to properly resolve its widgets without having the .type suffix present in the method pointer. Notice that on the screenshot below the dropdown is in correct place in all cases, even though the two rightmost nodes use a variable method target of different type, but eventually resolve to the same method.

The issue is that this full resolution is only used for static dropdowns. Dynamic dropdowns are failing to resolve, because the method lookup is not done correctly for calls on type.

[farmaazon]

Our suggestion database reflects exactly what language server send us, it only removes unnecessary Main module names from paths.

The MethodPointer responsibility is to point to method definition. Its primary use is to tell as where is the definition of the node if we would want to enter it.

Now you see (I hope) the problem here: the definition pointer should not depend on the call syntax! This is why the field is called definedOnType, not selfArgumentType.

(The last two paragraph cc @JaroslavTulach @4e6)

In your case, I think you're taking the information from the wrong place. If you want to know what is the type of self argument, you should just read the type of the proper expression ID (in the last case on your screenshot, it could be type of text2/text3 expression in the node).

[JaroslavTulach]

The .type suffix is specifically added to definedOnType when the method target is a whole type, as opposed to a value of given type

Looking from Enso language perspective:

enso$ ./built-distribution/enso-engine-0.0.0-dev-linux-amd64/enso-0.0.0-dev/bin/enso --repl
> Text.from_bytes
>>> Text.type.from_bytes[Extensions.enso:675-2395]

e.g. the type appears in the output as well. However that is the "runtime value" - when we take a look at static source code, then there is no .type at all.

tell as where is the definition of the node if we would want to enter it.

If you are interested in "static information" - e.g. where a source code is - then we may want to modify the MethodPointer and expression type updates to avoid using .type completely. That would mimic the source structure more.

[farmaazon]

The discussion continues https://github.com/orgs/enso-org/discussions/6762

For now, we will make another place where we just check type for .type suffix`.

GitHub

Language Server API · enso-org · Discussion #6762

When I was discussion with @Frizi the issue #6715, we discovered several points about Language Server API to be discussed broader @4e6 @hubertp : What widgets needs to know The widgets need to know...