Improve highlighting for strong mode error

Improve highlighting for strong mode error

R=rnystrom@google.com

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

[Brian Wilkerson, 2016-03-28 16:59:30]

brianwilkerson@google.com changed reviewers:
+ rnystrom@google.com

[Brian Wilkerson, 2016-03-28 16:59:31]

We are currently highlighting the whole member, which is a bit excessive IMO.
This changes it so we're only highlighting the name of the member.

[Bob Nystrom, 2016-03-28 19:59:40]

Assuming the ".name" additions are the only substantive changes, LGTM!

https://codereview.chromium.org/1834213002/diff/1/pkg/analyzer/lib/src/task/s...
File pkg/analyzer/lib/src/task/strong/checker.dart (right):

https://codereview.chromium.org/1834213002/diff/1/pkg/analyzer/lib/src/task/s...
pkg/analyzer/lib/src/task/strong/checker.dart:640: /// Records a [DownCast] of
[expr] from [from] to [to], if there is one.
Are there changes to this, or did it just move? Is it possible to separate
patches that reorder members from ones that change contents? It's hard to find
the actual diffs in this.

https://codereview.chromium.org/1834213002/diff/1/pkg/analyzer/lib/src/task/s...
pkg/analyzer/lib/src/task/strong/checker.dart:1089: }
Maybe leave a comment or assert that the only remaining type is
ConstructorDeclaration and we don't need to do anything with it?

[Brian Wilkerson, 2016-03-28 20:34:33]

https://codereview.chromium.org/1834213002/diff/1/pkg/analyzer/lib/src/task/s...
File pkg/analyzer/lib/src/task/strong/checker.dart (right):

https://codereview.chromium.org/1834213002/diff/1/pkg/analyzer/lib/src/task/s...
pkg/analyzer/lib/src/task/strong/checker.dart:640: /// Records a [DownCast] of
[expr] from [from] to [to], if there is one.
> Are there changes to this, or did it just move?

No, there are no changes. I was only expecting changes to
_checkIndividualOverridesFromClass. The two strong-mode changes were
unintentional.

> Is it possible to separate patches that reorder members from ones that change
contents? It's hard to find
> the actual diffs in this.

I sympathize. The only way I know of to separate such patches is to figure out
which files changed, create a new branch, sort and format the files on the new
branch, then rebase the original changes onto the new branch. Maybe we should
have a test that formats and sorts every file and fails if the before and after
are not the same; that would prevent the skew that causes this kind of noise.
(It somewhat ironic, given that one of the reasons often cited for sorting and
formatting is to prevent this kind of noise.)

https://codereview.chromium.org/1834213002/diff/1/pkg/analyzer/lib/src/task/s...
pkg/analyzer/lib/src/task/strong/checker.dart:1089: }
> Maybe leave a comment or assert that the only remaining type is
> ConstructorDeclaration and we don't need to do anything with it?

I'll do that.

[Brian Wilkerson, 2016-03-28 20:35:47]

Description was changed from

==========
Improve highlighting for strong mode error
==========

to

==========
Improve highlighting for strong mode error

R=rnystrom@google.com

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

[Brian Wilkerson, 2016-03-28 20:35:48]

Committed patchset #1 (id:1) manually as
ee105910bef362c4f9f1e29d7cb3818810dabfbd (presubmit successful).

[Bob Nystrom, 2016-03-28 20:53:28]

On 2016/03/28 20:34:33, Brian Wilkerson wrote:
>
https://codereview.chromium.org/1834213002/diff/1/pkg/analyzer/lib/src/task/s...
> File pkg/analyzer/lib/src/task/strong/checker.dart (right):
> 
>
https://codereview.chromium.org/1834213002/diff/1/pkg/analyzer/lib/src/task/s...
> pkg/analyzer/lib/src/task/strong/checker.dart:640: /// Records a [DownCast] of
> [expr] from [from] to [to], if there is one.
> > Are there changes to this, or did it just move?
> 
> No, there are no changes. I was only expecting changes to
> _checkIndividualOverridesFromClass. The two strong-mode changes were
> unintentional.
> 
> > Is it possible to separate patches that reorder members from ones that
change
> contents? It's hard to find
> > the actual diffs in this.
> 
> I sympathize. The only way I know of to separate such patches is to figure out
> which files changed, create a new branch, sort and format the files on the new
> branch, then rebase the original changes onto the new branch.

Oof, that's a pain.

> Maybe we should
> have a test that formats and sorts every file and fails if the before and
after
> are not the same; that would prevent the skew that causes this kind of noise.
> (It somewhat ironic, given that one of the reasons often cited for sorting and
> formatting is to prevent this kind of noise.)

Yeah. I didn't realize until very recently that some (most? all?) people on the
analyzer team sort all declarations alphabetically. If we're going to have
anyone do this, we need to have everyone do it.

– bob