Show static types in tooltips

[DanTup]

If I hover over "object" inside this if block it says Object object, but the type here is known to be Instance (and the set method is declared on Instance).

[DanTup]

We're using the elementDescription for display here, though the result does have the correct type in staticType:

{
	"id": "36",
	"result": {
		"hovers": [
			{
				"offset": 6559,
				"length": 6,
				"elementDescription": "Object object",
				"elementKind": "local variable",
				"isDeprecated": false,
				"staticType": "Instance"
			}
		]
	}
}

@bwilkerson Is there value in showing Object to the user here? I could add the staticType in addition but it might be weird if the tooltip shows both Object and Instance. However, I can't (nicely) remove Object because of it being embedded in elementDescription.

[bwilkerson]

It certainly makes sense to tell the user that the static type is "Instance" here, but there might be some value in also telling them how it was declared. Maybe it's largely a question of presentation. I don't know what the current hover text looks like, but perhaps something like this would make sense:

The static type is Instance.
It was declared as Object object.

(I say "like this" because I don't think the above is either attractive or readable.)

What I'd really like us to be able to display is the information to explain the difference in these two types (something like "because of the 'is' check on line lineNumber").

But it's possible that we ought to make the declared type and the element name be two different fields for ease of composition. (We have yet to find the sweet spot between making it flexible by having lots of fields and reducing the work on the client side by pre-formatting the output.)

[DanTup]

We have yet to find the sweet spot between making it flexible by having lots of fields and reducing the work on the client side by pre-formatting the output

Yeah, this comes up now and I was reminded of it when I saw this.

The tooltip for this currently looks kinda sparse, because it's a local variable with no docs or anything:

What I'd really like us to be able to display is the information to explain the difference in these two types (something like "because of the 'is' check on line lineNumber").

Yeah, that'd be neat. I'd love to show it as something like this:

I think putting the static type first makes sense since it's probably the one the user cares most about here, but with a note about the declared type and why it's different.

Looking in our hover code, I noticed we have something called the propagatedType:

if (propagatedType) displayString += `propagated type: ${propagatedType.trim()}`;

I'm not sure I remember every seeing that rendered, so not sure what it is (the server docs just say it's the propagated type, but I'm none the wiser). For now, I'm gonna stick the static type on the end in parens:

Object object (Instance)

And I'm moving propagated type to the same (since where it's currently rendered the label is fixed-with and looks wonky if I force it to show). Hopefully both won't appear at the same time as that'll be a little confusing!

[DanTup]

Ugh, the static type is provided even when it matches the start of elementDescription which means I end up with String a (String) on a normal string. In order to show the static type (and not look redundant), I'd need to know whether it's different, and I don't think I can do that reliably (I could check it starts with the static type then a space, but I think that might not be reliable since there could be spaces in function types?).

I think this may need a server change before we can really improve it.

[bwilkerson]

... I noticed we have something called the propagatedType ...

As of Dart 2, there shouldn't be any propagated type information (I need to deprecate the field). The propagated type was an early, analyzer-only, attempt to have better type information than the spec allowed for at that time. Now that we have type inference, we no longer compute a propagated type.

Ugh, the static type is provided even when it matches the start of elementDescription ...

I can think of three ways to resolve that problem, two of which I'd already mentioned. First, we could split the elementDescription into a declaredType and name pair. Second, we could provide a single description field that combined all of the information for clients so that they don't need to compose it for themselves. Third, we could omit the staticType field when there's no useful information.

We need to think about cases where there is no declared type (such as var x = ...), cases like the one above where the declared and static type are significantly different (Object vs Instance), and cases where the static type is just a refinement (List vs List<String>). And, of course, we shouldn't forget the cases where the variable isn't declared at all.

[DanTup]

As of Dart 2, there shouldn't be any propagated type information (I need to deprecate the field). The propagated type was an early, analyzer-only, attempt to have better type information than the spec allowed for at that time.

Ah, makes sense (and why I haven't seen it lately).

I can think of three ways to resolve that problem, two of which I'd already mentioned.

I don't have any strong preferences on this. Having separate fields is more flexible, but I also like that giving us formatted strings gives a consistency across tools (and also I think people working on the server have a much better understanding of Dart than I, so are likely to format things better than me mashing a bunch of fields together). Any implementation that allows the user to see this extra info - which is a really neat feature of Dart (though it slightly bugs me that it doesn't work if in invert the condition - not sure if that's a bug?) - would be great. I can open an SDK bug if you think it should be tracked over there.

[bwilkerson]

... though it slightly bugs me that it doesn't work if in invert the condition - not sure if that's a bug?

You and a lot of people :-). There was an effort started to enhance the flow of type information through code structures, but it keeps not being at the top of the language team's list. I keep hoping we can enhance this part of the type system some day.

I can open an SDK bug if you think it should be tracked over there.

Probably a good idea. I don't know when we'll have the resources to fix it, and I'd rather we didn't forget about it.