Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for displaying function signature information #27034

Closed
bwilkerson opened this issue Aug 8, 2016 · 14 comments
Closed

Add support for displaying function signature information #27034

bwilkerson opened this issue Aug 8, 2016 · 14 comments
Labels
analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@bwilkerson
Copy link
Member

bwilkerson commented Aug 8, 2016

Add a new request, something like the following:

analysis.getSignature
request: {
  "id": String
  "method": "analysis.getSignature"
  "params": {
    "file": FilePath
    "offset": int
  }
}

response: {
  "id": String
  "error": optional RequestError
  "result": {
    "name": String
    "dartdoc": String
    "parameters": List<ParameterInfo>
  }
}

Return the signature information associated with the given location in the given file. If the signature information for the given file has not yet been computed, or the most recently computed signature information for the given file is out of date, then the response for this request will be delayed until it has been computed. If the content of the file changes after this request was received but before a response could be sent, then an error of type CONTENT_MODIFIED will be generated.

If a request is made for a file which does not exist, or which is not currently subject to analysis (e.g. because it is not associated with any analysis root specified to analysis.setAnalysisRoots), an error of type GET_SIGNATURE_INVALID_FILE will be generated.

If the location given is not inside the argument list for a function (including method and constructor) invocation, then an error of type GET_SIGNATURE_INVALID_OFFSET will be generated. If the location is inside an argument list but the function is not defined or cannot be determined (such as a method invocation where the target has type 'dynamic') then an error of type GET_SIGNATURE_UNKNOWN_FUNCTION will be generated.

Parameters

file ( FilePath )
The file in which signature information is being requested.

offset ( int )
The location for which signature information is being requested.

Returns

name ( String )
The name of the function being invoked at the given offset.

dartdoc ( String )
The dartdoc associated with the function being invoked. Other than the removal of the comment delimiters, including leading asterisks in the case of a block comment, the dartdoc is unprocessed markdown. This data is omitted if there is no referenced element, or if the element has no dartdoc.

parameters ( List )
A list of information about each of the parameters of the function being invoked.

Where ParameterInfo contains the name, type and kind (required, optional or named) of the parameter.

@bwilkerson bwilkerson added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-server P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug labels Aug 8, 2016
@DanTup
Copy link
Collaborator

DanTup commented Aug 8, 2016

This is almost perfect! Missing only one thing for me - the parameter index for where the cursor currently is. In Code this will be underlined (and presumably if it existed, it'd display the docs for that param, though for Dart that might not apply unless we have to put optional/name status there):

Current parameter is underlined

@DanTup
Copy link
Collaborator

DanTup commented Dec 15, 2016

@bwilkerson

In analyzer-discuss you said

Do let us know if it turns out to be too hard to implement it using another request, and we'll see whether we can increase the priority.

I did spend some time trying to make this work using getHover, but being able to figure out the correct location to send the request seemed complicated without a good AST. I was originally going to just walk back to the previous ( but obviously that failed quite quickly:

myFunc((1 * 2), (fr|ed));

I was also going to have a go at implementing this (by copying getHover) but I spent several days trying to find a way to make changes to the analysis server and build on my machine but failed (seemed like it needed an entire build of the SDK and it quickly seemed like more work than I really had time for, and many of the analyzer tests in the SDK wouldn't pass for me on Windows).

So, if there's any possibility of someone looking at this; it would be greatly appreciated. It's one of my biggest annoyances with Dart Code (and @sethladd noticed it missing pretty quick upon first trying out Dart Code!).

Alternatively; if you can think of a decent way I can get the correct position to send a getHover request from the data already available, I'm happy to give it another shot!

@sethladd
Copy link
Contributor

I'd imagine this would be for more than Visual Code? Would all IDEs be able to utilize this?

@DanTup
Copy link
Collaborator

DanTup commented Dec 17, 2016

@sethladd I would've thought so, but nobody else appears to have asked for it! @devoncarew said atom doesn't have it so I'm surprised nobody has asked - maybe things work differently there, like you can still see the method you completed? I'm not sure (never used Atom).

There is an IDE that has this (IntelliJ IDEA) but that parses Dart itself so is able to figure this out more easily.

@Porl91
Copy link

Porl91 commented Feb 16, 2017

@sethladd Are there any plans to add support for this? I've been running into the same issue recently.

@sethladd
Copy link
Contributor

Sorry, I'm not sure what the plans are.

@DanTup
Copy link
Collaborator

DanTup commented Jun 16, 2017

@bwilkerson @scheglov How complicated do you think this would be for an outsider (me!) to implement if I can get the analyzer code building/running tests locally? I think it'd be a big improvement for Dart Code and I hope I'll be done with Flutter integration in the next few weeks so maybe can find some time.

@devoncarew
Copy link
Member

@DanTup, if it helps, there are some setup instructions here for working on the analysis server: https://github.com/dart-lang/sdk/blob/master/pkg/analysis_server/CONTRIBUTING.md.

@DanTup
Copy link
Collaborator

DanTup commented Jun 18, 2017

That file makes it look so easy; but I'm sure I had real issues last time! =)

I'll take a look once this debugging stuff is sorted; though I was mostly after an idea of how hard it'd be to make the changes (assuming I had the environment working). I presume mostly it's like the getHover request, except there will need to be something to walk up from the current cursor position to the nearest method call first?

@bwilkerson
Copy link
Member Author

If the existing API isn't sufficient (which is the conclusion I believe we came to), then we would need to add a new request to the API and implement it. The first step would be to come to an agreement on what the API should be, then you add it to the tool/spec/spec_input.html doc and run the generator (generate_files) to generate a lot of the boilerplate. (That might not work as-is on Windows.) The implementation can probably be mostly copied from getHover, at least as far as locating where the user is in the AST and what information is being requested. It would probably be a small number of days worth of work, spread over a couple of weeks.

Missing only one thing for me - the parameter index for where the cursor currently is.

We could easily add an index field to the result, but I'm not sure how it's suppose to be populated when there are named parameters involved (because the order at the call site doesn't have to match the order at the declaration site). For example, given a declaration of the form

void method(a, {b, c, d})

and a call site of

o.method(1, c: 2, ^)

(where '^' is the cursor position), should it assume that the next argument is b or d?

@DanTup
Copy link
Collaborator

DanTup commented Jun 19, 2017

We could easily add an index field to the result, but I'm not sure how it's suppose to be populated when there are named parameters involved

o.method(1, c: 2, ^)

should it assume that the next argument is b or d?

IMO either is acceptable; I don't think the user would have any real expectation there (maybe d would be more useful if we think the user would likely fill things in alphabetically but I don't think anyone would consider b to be wrong). Obviously if the user types a name then the index returned should match up to that:

// index should match d, whatever its position in the argument list returned
o.method(1, c: 2, d: ^)

@bwilkerson
Copy link
Member Author

I don't know whether VSCode would support it, but another possibility would be to return a list of "valid next parameters" and highlight both b and d. I think the point of this interface is to help users understand what to type next, and if there are multiple valid choices (as there are when dealing with named parameters), perhaps they should all be presented.

@DanTup
Copy link
Collaborator

DanTup commented Jun 19, 2017

That would be good, though Code doesn't support it. I think it's a good idea, and I can just take the "first" one for Code as the best I can support (and since named args aren't unique to Dart, maybe I'll raise a feature request for Code if/when we have it).

I guess the list could also be filtered based on the typed text:

void method(a, {b, c1, c2}) {}

// set only c2 as the active index, b is filtered out because of the already-typed c
o.method(1, c1: 2, c^)

DanTup added a commit to DanTup/sdk that referenced this issue Aug 6, 2017
DanTup added a commit to DanTup/sdk that referenced this issue Aug 6, 2017
DanTup added a commit to DanTup/sdk that referenced this issue Aug 6, 2017
DanTup added a commit to DanTup/sdk that referenced this issue Aug 12, 2017
DanTup added a commit to DanTup/sdk that referenced this issue Aug 12, 2017
DanTup added a commit to DanTup/sdk that referenced this issue Aug 12, 2017
DanTup added a commit to DanTup/sdk that referenced this issue Feb 12, 2018
DanTup added a commit to DanTup/sdk that referenced this issue Feb 12, 2018
DanTup added a commit to DanTup/sdk that referenced this issue Feb 12, 2018
dart-bot pushed a commit that referenced this issue Jul 25, 2018
…nctions

Bug: #27034
Change-Id: I8d3f1a9c9a824b4b80f9cfa0370a439fa897b226
Reviewed-on: https://dart-review.googlesource.com/64689
Commit-Queue: Danny Tuppeny <dantup@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
dart-bot pushed a commit that referenced this issue Jul 25, 2018
Bug: #27034
Change-Id: Ie47f76d4197dbcdec0fcfdada7914fdf986731ec
Reviewed-on: https://dart-review.googlesource.com/66540
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Danny Tuppeny <dantup@google.com>
dart-bot pushed a commit that referenced this issue Jul 25, 2018
…rIndex

Bug: #27034
Change-Id: I3409ab544b79a7f64d6d41e5d091ca20418f248a
Reviewed-on: https://dart-review.googlesource.com/66562
Commit-Queue: Danny Tuppeny <dantup@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
@DanTup
Copy link
Collaborator

DanTup commented Jul 30, 2018

The server now supports getSignature as spec'd above. Dart Code vNext will include support for it (though it'll only work if you're running with an SDK that includes the support for it). 🎉

It doesn't currently highlight which parameter you're on, but I may come back to that in the future if we can think of a way to make it work nicely (maybe by stealing ideas from other languages that allow named args in any order :-))

@DanTup DanTup closed this as completed Jul 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants