dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.12k stars 1.57k forks source link

IntelliJ Generate->Implement Methods incorrect for function params #29049

Open davidmorgan opened 7 years ago

davidmorgan commented 7 years ago

Repro:

import 'dart:html';

class MockElementStream<T extends Event> implements ElementStream<T> {
}

Right click in the class and choose Generate->Implement Methods, choose all of them, press okay. Output is wrong for methods that take a function, e.g.:

  Stream<T> distinct([bool equals(bool equals(T previous, T next),
      bool equals(T previous, T next))]) {

should be

Stream<T> distinct([bool equals(T previous, T next)]) {

interestingly, if you trigger the same generation by a different path, Ctrl+Enter and choose suggestion "generate 38 missing overrides", the generated code is correct.

bwilkerson commented 7 years ago

I don't believe that implementation is coming from the analysis server.

alexander-doroshko commented 7 years ago

Yep, Generate menu is not backed by the Analysis Server. YouTrack issue is welcome.

davidmorgan commented 7 years ago

Thanks. Filed https://youtrack.jetbrains.com/issue/WEB-25934

bwilkerson commented 7 years ago

@alexander-doroshko This would be a good candidate for which to add support to the analysis server. As Dart moves forward, this is exactly the kind of thing it would be nice to have supported internally.

alexander-doroshko commented 7 years ago

@bwilkerson Cool! So, this is what we have in the Generate menu, not backed by the server: image

bwilkerson commented 7 years ago

I assume that these are context sensitive. That is, that if the cursor is with a class the generated code applies to the enclosing class (except for the last item, which I assume is the copyright at the top of the file).

If so, I think we'll want a request to get code generation suggestions that takes the file and offset and returns a list of source file edits. Or maybe a list of lists so that we can retain the separator?

alexander-doroshko commented 7 years ago

Copyright is an IntelliJ's multilingual feature, this is not something Analysis Server should care about :). Others are context-sensitive, that's true.

takes the file and offset and returns a list of source file edits

I'm afraid the task is more complex. Probably we'll have to handle each action separately, as we do with refactorings. All of them are expected to show a specific dialog to get more user input. For example, constructor generator allows to select which fields to initialize: image

This is a dialog for Override Methods: image

bwilkerson commented 7 years ago

Ah, I didn't realize how much this functionality is doing. Yes, an API similar to that for refactoring would be needed, which makes this a bigger task than I'd originally anticipated. I still think we should do it; I'll see how soon I can start putting together a proposal that we can iterate on.