Provide the range of the symbol to be renamed during rename refactors

[DanTup]

Currently if I send a rename for an import like:

import "dart:async" as a;

The range I get back is the whole line. The VS Code API is being improved to allow us to handle more renames, but they won't allow us to provide the text to pre-populate the rename box directly (effectively the oldName property), only a range for where it appears in the document (I've tried and tried to change their minds with no luck).

Is it possible to have this added (in the case where the name doesn't exist - for example if the above didn't include an alias) can return an empty range (I'd put it before the semi-colon, but it doesn't really matter if it's empty so the box is populated blank)?

@bwilkerson

[bwilkerson]

When you say "send a rename" do you mean send an edit.getRefactoring request? And when you say "The range I get back" do mean the offset and length inside the response's feedback object? If so, then I would have expected the offset to be the offset of a and the length to be 1.

It isn't clear from the documentation what the offset and length are when there is no prefix, but I would at least expect the length to be 0.

If that's not what you're seeing, then it sounds like a bug.

[DanTup]

Sorry, I should've included more info. Yes, I mean exactly what you said. Here's a sample...

Given the file:

import "dart:async" as b;

main() async {
  await new b.Future.delayed(const Duration(seconds: 1));
}

If I send a rename on the word import and provide x as the new name, the full line is included in the range (whereas I hope just to get the b). It seems that it actually behaves correctly if my cursor was on the b.

The problem with this is that the rename box would be populate with the entire line, when it should only contain b. Code's propose API won't let me provide oldName directly, so I need the range that represents it.

As a workaround, I may reject renames where feedback.oldName.length does not match feedback.length.

[09:11:26]: Spawning /Users/dantup/Dev/dart-sdk/dart-2.0.0-dev.40.0/bin/dart with args ["/Users/dantup/Dev/dart-sdk/dart-2.0.0-dev.40.0/bin/snapshots/analysis_server.dart.snapshot","--client-id=Dart-Code.dart-code","--client-version=2.11.1"]
[09:11:26]: ==> {"id":"3","method":"analysis.updateContent","params":{"files":{"/Users/dantup/Desktop/Dart Sample/bin/main.dart":{"content":"import \"dart:async\" as b;\n\nmain() async {\n  await new b.Future.delayed(const Duration(seconds: 1));\n}\n","type":"add"}}}}
[09:11:31]: ==> {"id":"11","method":"edit.getRefactoring","params":{"file":"/Users/dantup/Desktop/Dart Sample/bin/main.dart","kind":"RENAME","length":6,"offset":0,"options":{"newName":"x"},"validateOnly":false}}
[09:11:31]: <== {"id":"11","result":{"initialProblems":[],"optionsProblems":[],"finalProblems":[],"feedback":{"offset":0,"length":25,"elementKindName":"import directive","oldName":"b"},"change":{"message":"Rename Import Prefix 'b' to 'x'","edits":[{"file":"/Users/dantup/Desktop/Dart Sample/bin/main.dart","fileStamp":0,"edits":[{"offset":54,"length":2,"replacement":"x."},{"offset":23,"length":1,"replacement":"x"}]}],"linkedEditGroups":[]}}}

[DanTup]

Note: I'm sending an offset of 0 and a length of 6 for the rename because Code is expanding to the nearest word (this is part of the wonky behave they're fixing). Probably as part of the changes I will just sent the offset and always a length of 0 and that should work?

[DanTup]

Actually, as a better workaround (that won't fail if the lengths just match up by fluke) is for me to read the content from the document at the offset/length provided and see if it matches oldName. If it does, I'll go ahead, and otherwise I'll pretend the location is invalid for a rename (this will mean you can't press F2 in import, but it'll magically fix itself once this is fixed).

[bwilkerson]

@scheglov Is the selection of the whole import directive intentional?

[DanTup]

I can see some logic to it if it might be considered weird if the returned range doesn't include the offset you sent; but it ties my hands here. VS Code really doesn't want to allow extensions to supply the text directly and although I could search inside the provided range for the text from oldName, that could be problematic if there are multiple occurrences (eg. import "dart:async" as a;) and the editor decides to highlight the range I give it. Having a reliable range that covers the symbol to be renamed would be best.

[scheglov]

No, using the range of the whole import directive it not intentional.
I will fix it to use the range of the import prefix.

Not 100% sure what to do when there is no import prefix, probably use offset: -1, length: 0.
Any actual offset that we would use would be a lie.

[DanTup]

I can handle -1/0 but I think it might be nicer to provide a valid range to avoid the chance of someone not handling it and then having the editor crash on an invalid value. I'd personally pick the empty range before the semicolon (since that's where the edit would be).

[scheglov]

https://dart-review.googlesource.com/c/sdk/+/48682