Strong mode inferred types should be surfaced in the IDE

[leafpetersen]

Working through adding strong mode types to some larger code bases, I'm very frequently finding myself needing to figure out what types are being inferred for surrounding code in order to understand errors. I can work around this by figuring out what types things should be and annotating them that way, but this is pointless work (strong mode is already doing this), and leaves the code over-annotated.

I think it would provide significant value to users if they could see the inferred types in the IDE, either as a hover, or via a command (like go-to definition).

[zoechi]

+1

That would be great. I added lot of types because the analyzer wasn't always able to properly infer in the past. Now that this has improved a lot, it would be great to get some support to find where they can be removed again. I think it would especially be interesting to see where type annotations deviate from inferred types.

Some indication where the type annotation is narrower than it needed to be would be nice. No idea if this is feasible.

[eernstg]

Good idea! (And more controversially, maybe: How about quickfixes to put them into the code, to add a type annotation or to modify an existing one? ;-)

[bwilkerson]

We already have a quick assist to add a type annotation, but not one to update an existing annotation.

[eernstg]

OK, thanks! (I know I should have checked more carefully, long time ago, but I just discovered that I could use that little light bulb for something ;-)

On Tue, Mar 8, 2016 at 3:16 PM, Brian Wilkerson notifications@github.com wrote:

We already have a quick assist to add a type annotation, but not one to update an existing annotation.

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/sdk/issues/25945#issuecomment-193799300.

Erik Ernst - Google Danmark ApS Skt Petri Passage 5, 2 sal, 1165 København K, Denmark CVR no. 28866984

[leafpetersen]

We should also look at the integration of the quick fixes with strong mode. My guess is that the quick fix will offer the analyzer's propagated type - but probably it would be better to offer the strong mode inferred type when in strong mode.

[bwilkerson]

My guess is that the quick fix will offer the analyzer's propagated type - but probably it would be better to offer the strong mode inferred type when in strong mode.

Yes, it will use the "best" type, so that will be the propagated type when it's available. That's primarily because it's in situations where there was no type information that it was useful to add annotations. If users could see what was inferred they might not feel the need to add annotations.

I have a little app I wrote a while back and haven't run for a while that performs a strong mode analysis and compares the static and propagated types. If I'm remembering correctly, there weren't very many places where they differed. It would be interesting to see what the current state of affairs is.

[leafpetersen]

One piece of this that I think isn't currently supported would be to surface the inferred type parameters to generic classes and methods (as information, and potentially as a quick fix as well).

[bwilkerson]

@jwren @devoncarew In case you haven't seen this thread, it would be good to have some IDE design perspective.

I think it would provide significant value to users if they could see the inferred types in the IDE, either as a hover, or via a command (like go-to definition).

Getting back to the original request, hover text would be easy to enhance. I'm somewhat surprised that it isn't already showing the static type information when it's been inferred.

[devoncarew]

+1 to surfacing the into in a tooltip. Are strong mode inferred types different from propagated types? We do show those in atom and dartpad currently (https://dartpad.dartlang.org/8411c19e9c505c4d16d4).

[bwilkerson]

Are strong mode inferred types different from propagated types?

They are computed differently in a few cases, but my hypothesis is that the number of times they differ in practice is becoming increasingly small. My hope is to be able to get rid of propagated types, but we need to understand the impact on code completion, etc. before we can do so.

[leafpetersen]

We haven't spent the time to sort out what to do with propagated types in strong mode. The analyzer's type propagation will use the strong mode types when running in strong mode. I believe though that it is often the case that the propagated type only gets set if the static type is worse - so I think that implies that if strong mode infers a good type, the propagated type field won't duplicate it, and so you won't be picking it up if you're just looking at the propagated type. I think there is a best type getter which will return the better of the two - if you're using that you should get most of it (though we should check that the definition of "best" is appropriate in strong mode).

Strong mode also does additional inference besides just local variable inference that would be nice to surface: e.g. type parameters to generic classes and methods, and in some cases type annotations for unannotated methods.

I think it would also be useful to be able to surface the inferred/calculated type of expressions. Especially in future related code, I'm often seeing large chains of method calls. It would be very useful to be able to highlight an expression and see what type the analyzer thinks it has.

[jwren]

I think that you already have access to this content from IntelliJ as well. On Linux the keyboard command is Ctrl+Q:

[leafpetersen]

@jwren That's nice - didn't know about it! But that's not really what I'm asking for. That just gives the documented type for identifiers, which I can already get fairly easily via "go to definition" (though this shortcut is much better, and I will make use of it). It doesn't give me the type of expressions, and even for identifiers it doesn't give me the instantiated types. For example:

var l = <int>[3];
l.map((x) => "hello").map((x) => 3);

Ctrl+Q on ether of the calls to map just gives me map(f(E e) -> Iterable which doesn't help me at all (even ignoring the fact that this doesn't account for the currently strong mode only generic type). What I want to be able to do is click on the map identifier and see how it has been instantiated, and/or highlight the whole map invocation and see what the inferred return type is. I'd also like to be able to highlight the body of the lambda ("hello") and find out what type the analyzer thinks it has (since this can help me understand why it is inferring the type of the map expression), and similarly highlight the lambda argument and see what type has been inferred for it.

[jwren]

What I was trying to show was that the static and propagated type are displayed from the analyzer via the server, and in the example, the static and propagated types are different (navigation can only ever take you to one.) Since the content is provided by the server, additional content (whatever you and the analyzer folks want) can be put in, and the IDE will display it.

One idea that we have had for a while to display in hover is a stack trace like path for each of the static and propagated types so that users could find out how some type has been computed.

One point I will make here is that the hover API, has an input of an offset, not a range, meaning your idea of selecting a range of characters wouldn't be supported by the current server API (also, there is no current UI in IntelliJ for it either.)

[leafpetersen]

@jwren Interesting - I can't reproduce the screen you are showing actually, now that I look at it more closely. When I hit Ctrl+Q I never see anything showing static or propagated type. Ctrl+Q in my linux box is bound to "Quick Documentation" which always just shows the declaration - never anything to do with the analyzer type. What is Ctrl+Q bound to for you? I can't find anything by poking around menus.

[jwren]

To repro what I did:

 @override
  int get hashCode {
    var result = 17;
    result = 37 * result + aspect1;
    result = 37 * result + aspect2;
    return result;
  }

Put the cursor in the last result, call out to get Quick Documentation (View > Quick Documentation).

[leafpetersen]

Nope. I get a popup with the following text:

analyzer.src.generated.static_type_analyzer  
StaticTypeAnalyzer 
var result

Intellj IDEA 15.0.2

[alexander-doroshko]

@leafpetersen Intellj IDEA 15.0.2 doesn't use information from Analysis Server for "Quick Documentation", you need to use IntelliJ IDEA 16 EAP + corresponding Dart plugin version or WebStorm 12 EAP.

[leafpetersen]

Updating to 16 EAP I see the inferred static types for local variables and parameters (which is great!), but I still just see the documentation signature for methods and such (which is where it would be super useful to see the instantiated type).

@jwren when will Intellij 16 be available to internal users?

[leafpetersen]

It looks the analysis server types are used in 15.0.4 as well.

[alexander-doroshko]

Cc @scheglov

leafpetersen wrote:

I see the inferred static types for local variables and parameters (which is great!), but I still just see the documentation signature for methods and such (which is where it would be super useful to see the instantiated type).

[scheglov]

There is a couple problems with hovers now.

main() {
  var l = <int>[3];
  l.map((x) => "hello").map((x) => 3);
}

1. @alexander-doroshko When I request hover on any of the "map" identifiers, DAS is actually invoked with a location in core/iterable.dart, no in my test.dart. So, we don't even know the invocation and its properties in DAS.
2. @bwilkerson @leafpetersen By some reason the static type of map in l.map((x) => "hello") is <T>((int) → T) → Iterable<T>. I would expect it to be ((int) → String) → Iterable<String>.

[bwilkerson]

@scheglov Is this with or without strong mode?

[scheglov]

@bwilkerson This is with strong mode.

[leafpetersen]

I replied to this, and then must have closed the tab before submitting. The second issue that @scheglov identified is most likely the staticType/staticInvoke type distinction that @jmesserly added to handle generic method instantiation. On a method invocation o.map, the staticType field on map will be the un-instantiated type, but the staticInvokeType will be the instantiated type. The latter is what we would like to surface to the user.

[alexander-doroshko]

@scheglov @leafpetersen @jwren I think I've fixed #1 so in the next version you'll see propagated type in Quick Doc (see screenshot). Is there anything else that should be improved at IDE side?

[leafpetersen]

Awesome. We should verify that this works in strong mode, but I see no reason it wouldn't.

@jwren - I think the fix we discussed to make sure that the full instantiated type (including the generic method instantiation) shows up in strong mode (using the .staticInvoke type) is on our side, right?

When I was in PDX last week we discussed ways of improving the IDE feedback for strong mode type inference. I'd still love to be able to highlight an expression and see what type the analyzer thinks it has, but I understood that to be harder to do given the available APIs.

We also discussed finding some way to surface the inferred type arguments to generic constructors and invocations of generic methods (directly, rather than indirectly via the resulting type of the methods). Not sure if we came up with anything to act on in the short term though.

[scheglov]

https://codereview.chromium.org/1814893002

[scheglov]

@leafpetersen Is this what you would like to see?

[leafpetersen]

That's great, thanks! Seeing the instantiated static type is super useful for trying to understand errors that come from inferred types.

I think it would be slightly better if the Signature: field showed the binder for T to make it clear that T is a generic method parameter and not something from the surrounding context.

Signature: Iterable.map<T>((int) -> T f) -> Iterable<T>

[escamoteur]

I think this support now or do I miss something?

[leafpetersen]

@bwilkerson @devoncarew Closing unless there's anything you think we should still track here.