Remove UriReferencedElement with its uri/uriOffset/uriEnd properties.

Remove UriReferencedElement with its uri/uriOffset/uriEnd properties.

The 'Move File' refactoring does not work, so I comment out most
of its code.

R=brianwilkerson@google.com, paulberry@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/bdd69ccd6c330e4755a447964a1f1021be034f75

[Paul Berry, 2017-07-18 21:01:06]

Clarifying question: when you say "The 'Move File' refactoring does not work, so
I comment out most
of its code.", do you mean:

(a) the 'Move File' refactoring has been broken for some time, so rather than
try to modify it to work with the new changes, you've commented it out?

or (b) this change causes the 'Move File' refactoring to stop working, and it's
not practical to restore it's functionality right now?

If (a), do you have any idea how long it's been broken?  Is there a bug report?

If (b), can you elaborate on why it's not practical to fix it right now, and
when we'll be able to fix it?

Depending on the answers to these questions, maybe we should consider either
removing the "Move File" refactoring or fixing it prior to this CL.

https://codereview.chromium.org/2982993003/diff/1/pkg/analysis_server/lib/src...
File pkg/analysis_server/lib/src/services/refactoring/move_file.dart (right):

https://codereview.chromium.org/2982993003/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/services/refactoring/move_file.dart:64: //    //
move file
Can you add a TODO comment explaining why the code is commented out?  I don't
want people to have do use "git blame" to figure out what's going on.

[scheglov, 2017-07-18 21:04:42]

On 2017/07/18 21:01:06, Paul Berry wrote:
> Clarifying question: when you say "The 'Move File' refactoring does not work,
so
> I comment out most
> of its code.", do you mean:
> 
> (a) the 'Move File' refactoring has been broken for some time, so rather than
> try to modify it to work with the new changes, you've commented it out?
> 
> or (b) this change causes the 'Move File' refactoring to stop working, and
it's
> not practical to restore it's functionality right now?
> 
> If (a), do you have any idea how long it's been broken?  Is there a bug
report?
> 
> If (b), can you elaborate on why it's not practical to fix it right now, and
> when we'll be able to fix it?
> 
> Depending on the answers to these questions, maybe we should consider either
> removing the "Move File" refactoring or fixing it prior to this CL.
> 
>
https://codereview.chromium.org/2982993003/diff/1/pkg/analysis_server/lib/src...
> File pkg/analysis_server/lib/src/services/refactoring/move_file.dart (right):
> 
>
https://codereview.chromium.org/2982993003/diff/1/pkg/analysis_server/lib/src...
> pkg/analysis_server/lib/src/services/refactoring/move_file.dart:64: //    //
> move file
> Can you add a TODO comment explaining why the code is commented out?  I don't
> want people to have do use "git blame" to figure out what's going on.

It is (a) - the refactoring has been broken since moving to Analysis Driver.
IDEA does not use it, so there is little incentive to support it.
I will remove it first.

[Paul Berry, 2017-07-18 21:05:06]

On 2017/07/18 21:04:42, scheglov wrote:
> On 2017/07/18 21:01:06, Paul Berry wrote:
> > Clarifying question: when you say "The 'Move File' refactoring does not
work,
> so
> > I comment out most
> > of its code.", do you mean:
> > 
> > (a) the 'Move File' refactoring has been broken for some time, so rather
than
> > try to modify it to work with the new changes, you've commented it out?
> > 
> > or (b) this change causes the 'Move File' refactoring to stop working, and
> it's
> > not practical to restore it's functionality right now?
> > 
> > If (a), do you have any idea how long it's been broken?  Is there a bug
> report?
> > 
> > If (b), can you elaborate on why it's not practical to fix it right now, and
> > when we'll be able to fix it?
> > 
> > Depending on the answers to these questions, maybe we should consider either
> > removing the "Move File" refactoring or fixing it prior to this CL.
> > 
> >
>
https://codereview.chromium.org/2982993003/diff/1/pkg/analysis_server/lib/src...
> > File pkg/analysis_server/lib/src/services/refactoring/move_file.dart
(right):
> > 
> >
>
https://codereview.chromium.org/2982993003/diff/1/pkg/analysis_server/lib/src...
> > pkg/analysis_server/lib/src/services/refactoring/move_file.dart:64: //    //
> > move file
> > Can you add a TODO comment explaining why the code is commented out?  I
don't
> > want people to have do use "git blame" to figure out what's going on.
> 
> It is (a) - the refactoring has been broken since moving to Analysis Driver.
> IDEA does not use it, so there is little incentive to support it.
> I will remove it first.

sgtm, thanks

[scheglov, 2017-07-19 02:23:10]

PTAL

I removed the "Move File" refactoring and updated this CL.

[Paul Berry, 2017-07-19 16:26:05]

lgtm

[scheglov, 2017-07-19 16:31:09]

Description was changed from

==========
Remove UriReferencedElement with its uri/uriOffset/uriEnd properties.

The 'Move File' refactoring does not work, so I comment out most
of its code.

R=brianwilkerson@google.com, paulberry@google.com
BUG=
==========

to

==========
Remove UriReferencedElement with its uri/uriOffset/uriEnd properties.

The 'Move File' refactoring does not work, so I comment out most
of its code.

R=brianwilkerson@google.com, paulberry@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/bdd69ccd6c330e4755a447964a1f1021be034f75
==========

[scheglov, 2017-07-19 16:31:10]

Committed patchset #2 (id:20001) manually as
bdd69ccd6c330e4755a447964a1f1021be034f75 (presubmit successful).

[Brian Wilkerson, 2017-07-19 22:30:00]

> It is (a) - the refactoring has been broken since moving to Analysis
> Driver. IDEA does not use it, so there is little incentive to support it.

Does this change make it significantly more difficult to support the
refactoring? I ask because I think IntelliJ *should* use it in order to provide
a reasonable level of Dart support, so I don't want us to get into a situation
where it's prohibitively expensive to add the support back in.

[scheglov, 2017-07-19 23:59:44]

On 2017/07/19 22:30:00, Brian Wilkerson wrote:
> > It is (a) - the refactoring has been broken since moving to Analysis
> > Driver. IDEA does not use it, so there is little incentive to support it.
> 
> Does this change make it significantly more difficult to support the
> refactoring? I ask because I think IntelliJ *should* use it in order to
provide
> a reasonable level of Dart support, so I don't want us to get into a situation
> where it's prohibitively expensive to add the support back in.

  No, it does not.

The model of thinking I tried to apply recently is something what Paul suggested
- if we have to pay a reasonable price for some rarely used operation, like
search or refactoring, we will pay. Even if in theory it would be possible to
make it faster, but the cost is additional APIs or work which should be done.

  The "Move File" refactoring is very rare, so we can make it not very fast. The
worst case is that we fully resolve defining unit of every library that imports
the file being moved. If this happens to be a problem (and I don't think it
will), we can include additional information into index - list of (uriString,
offset, length) for every URI directive in the unit. This information is very
cheap both to create and access (when analysis is done, it is always in the byte
store).