Hover information over export default

[basarat]

If you have a file with the following contents:

export default function(a: string, b: number) {
  return Promise.resolve(a + b);
}

Then there is no way to see function signature through hovering. Basically, because it has no name? It would be nice to get the hover information over the export or default keyword :-)

Original request : TypeStrong/atom-typescript#251

[mhegazy]

why not function? that would help with function expressions as well..

[basarat]

Cool. It was just a copy paste. "function" would be best ❤️

[tinganho]

I began on working on this. Shouldn't it be all reserved words related to a declaration?

[DanielRosenwasser]

@tinganho I'm not entirely sure; if we do this, then anonymous class expressions certainly should get the same behavior for class.

[tinganho]

@DanielRosenwasser with the exception of declarations lists with a length bigger than 1.

/*1*/var a: string = 1, b: string; // doens't get hover info on marker 1

[DanielRosenwasser]

So we should clarify what we want to see; if we're just going to show the word function, then this sounds very unhelpful.

If, however, we're going to show type information for a contextually typed function expression, that might be very useful.

For instance:

declare f(g: (x: number) => number);

f(/**/function (x) {
});

Quick info at that position should show something like

(local function) (x: number): number;

I'm not really big on how it looks, but it'd be worth discussing a bit.

Similarly, for the class keyword, I think it depends on whether or not a constructor function can be contextually typed.

[tinganho]

@DanielRosenwasser just another clarification. Should the export default etc. keywords have the same quick info as the related declaration?

Quick info at that position should show something like

Don't you think it should be:

function (x: number): number;

because the current non-anonymous is:

function A(x: number): number;

[DanielRosenwasser]

because the current non-anonymous is:

function A(x: number): number;

It depends on what you mean by non-anonymous:

I think the latter is inconsistent and something we should probably fix.

[tinganho]

@DanielRosenwasser I have practically finished this issue but before I send a PR. There are some weird namings currently in the language service.

/// <reference path='fourslash.ts' />

//// interface IFunction{
////     (g: (a: string, b: string) => void): any;
//// }
////
//// declare var f: IFunction;
////
//// f(/*contextualType*/function(a, b) { });
////
//// /*namedFunction*/function functionName1() {}
////
//// /*exportAnonymousFunction*/export /*defaultAnonymousFunction*/default /*anonymousFunction*/function(a: string, b: string) {}

goTo.marker("contextualType");
verify.quickInfoIs("(local function) __function(a: string, b: string): void");

goTo.marker("namedFunction");
verify.quickInfoIs("function functionName1(): void");

goTo.marker("exportAnonymousFunction");
verify.quickInfoIs("function default(a: string, b: string): void");

goTo.marker("defaultAnonymousFunction");
verify.quickInfoIs("function default(a: string, b: string): void");

goTo.marker("anonymousFunction");
verify.quickInfoIs("function default(a: string, b: string): void");

[DanielRosenwasser]

I need someone to wear their end-user hat and help decide what is best to show in these cases (@RyanCavanaugh @danquirk). Clearly we should not be showing the symbol name for an anonymous function. I think that default is not correct either.

We keep running into situations where we resolve the symbol as default in the language service whereas what we really want is the local symbol. @ahejlsberg what's the best way to go about fixing this?

[CyrusNajmabadi]

Seeing a dunder name (i.e. __function) is never going to be appropriate for the user. We should present the item to the user, as they wrote it. i.e. as either function(...) or default function(...).

Also, as an aside, i see no purpose in labeling something a local function.

[tinganho]

function(...) or default function(...)

I vote for just function(...). I see the default as a modifier to the export keyword not the function.

How about just keeping it simple?

function Name(...) {}
function(...) {}

[DanielRosenwasser]

How about just keeping it simple?

function Name(...) {}
function(...) {}

👍