Report compile-time error if using `this` implicitly in initializers

Report compile-time error if using `this` implicitly in initializers

BUG= http://dartbug.com/21210
R=johnniwinther@google.com

Committed: https://code.google.com/p/dart/source/detail?r=45114

[Siggi Cherem (dart-lang), 2015-04-09 21:54:12]

Patchset #1 (id:1) has been deleted

[Siggi Cherem (dart-lang), 2015-04-09 22:01:31]

Patchset #1 (id:20001) has been deleted

[Siggi Cherem (dart-lang), 2015-04-09 22:04:38]

sigmund@google.com changed reviewers:
+ johnniwinther@google.com

[Johnni Winther, 2015-04-10 07:55:50]

LGTM

https://codereview.chromium.org/1078873003/diff/40001/pkg/compiler/lib/src/re...
File pkg/compiler/lib/src/resolution/members.dart (right):

https://codereview.chromium.org/1078873003/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/members.dart:2320: compiler.reportError(
From a users perspective, it might be better to have a special message that
combines the information from CANNOT_RESOLVE and NO_INSTANCE_AVAILABLE for this
situation.

I you want to do that, you could add a named argument {bool reportError: false}
to [warnAndCreateErroneousElement] (and maybe name it to
`reportAndCreateErroneousElement`) and use the `reportError` to either call
`compiler.reportWarning` or `compiler.reportError`.

[Siggi Cherem (dart-lang), 2015-04-10 18:44:18]

Patchset #2 (id:60001) has been deleted

[Siggi Cherem (dart-lang), 2015-04-10 18:46:37]

Patchset #2 (id:80001) has been deleted

[Siggi Cherem (dart-lang), 2015-04-10 18:49:03]

Patchset #2 (id:100001) has been deleted

[Siggi Cherem (dart-lang), 2015-04-10 18:53:59]

Patchset #2 (id:120001) has been deleted

[Siggi Cherem (dart-lang), 2015-04-10 18:56:26]

PTAL

BTW - I'm having trouble with rietveld right now, I uploaded a new patchset, but
I can only see it in the unified diff view. If I try the side-by-side view it
looks bad.

I've tried also creating a new CL, but it didn't work for me either :(

https://codereview.chromium.org/1078873003/diff/40001/pkg/compiler/lib/src/re...
File pkg/compiler/lib/src/resolution/members.dart (right):

https://codereview.chromium.org/1078873003/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/members.dart:2320: compiler.reportError(
On 2015/04/10 07:55:50, Johnni Winther wrote:
> From a users perspective, it might be better to have a special message that
> combines the information from CANNOT_RESOLVE and NO_INSTANCE_AVAILABLE for
this
> situation.
> 
> I you want to do that, you could add a named argument {bool reportError:
false}
> to [warnAndCreateErroneousElement] (and maybe name it to
> `reportAndCreateErroneousElement`) and use the `reportError` to either call
> `compiler.reportWarning` or `compiler.reportError`.

I have it a try. WDYT?

[Johnni Winther, 2015-04-12 11:10:45]

LGTM

https://codereview.chromium.org/1078873003/diff/140001/pkg/compiler/lib/src/r...
File pkg/compiler/lib/src/resolution/members.dart (right):

https://codereview.chromium.org/1078873003/diff/140001/pkg/compiler/lib/src/r...
pkg/compiler/lib/src/resolution/members.dart:2269: MessageKind kind, Map
arguments, {bool isError: false}) {
Nit: We would normally format this as:

ErroneousElement reportAndCreateErroneousElement(
    Node node,
    String name,
    MessageKind kind,
    Map arguments,
    {bool isError: false}) {
  ....

https://codereview.chromium.org/1078873003/diff/140001/pkg/compiler/lib/src/w...
File pkg/compiler/lib/src/warnings.dart (right):

https://codereview.chromium.org/1078873003/diff/140001/pkg/compiler/lib/src/w...
pkg/compiler/lib/src/warnings.dart:160: "instance, but instances are not
available in initializers.");
If you add [howToFix] and [examples] we will automatically test that the example
causes the correct warning/error. See CANNOT_RESOLVE_CONSTRUCTOR_FOR_IMPLICIT
for an example.

[Siggi Cherem (dart-lang), 2015-04-13 16:40:24]

thanks!

https://codereview.chromium.org/1078873003/diff/140001/pkg/compiler/lib/src/r...
File pkg/compiler/lib/src/resolution/members.dart (right):

https://codereview.chromium.org/1078873003/diff/140001/pkg/compiler/lib/src/r...
pkg/compiler/lib/src/resolution/members.dart:2269: MessageKind kind, Map
arguments, {bool isError: false}) {
On 2015/04/12 11:10:45, Johnni Winther wrote:
> Nit: We would normally format this as:
> 
> ErroneousElement reportAndCreateErroneousElement(
>     Node node,
>     String name,
>     MessageKind kind,
>     Map arguments,
>     {bool isError: false}) {
>   ....

Done.

BTW - It might be worth to start using the formatter at some point.

In the other projects were we did so, initially we found little quirks that we
would've done slightly different. For some, it was just matter of getting used
to a different style, for others we filed bugs to get the formatter fixed. But
beyond that initial phase, we really enjoy using it. It saves us time in
formatting things by hand and we have much less arguments about style in code
reviews :)

https://codereview.chromium.org/1078873003/diff/140001/pkg/compiler/lib/src/w...
File pkg/compiler/lib/src/warnings.dart (right):

https://codereview.chromium.org/1078873003/diff/140001/pkg/compiler/lib/src/w...
pkg/compiler/lib/src/warnings.dart:160: "instance, but instances are not
available in initializers.");
On 2015/04/12 11:10:45, Johnni Winther wrote:
> If you add [howToFix] and [examples] we will automatically test that the
example
> causes the correct warning/error. See CANNOT_RESOLVE_CONSTRUCTOR_FOR_IMPLICIT
> for an example. 

Nice, Done.

[Siggi Cherem (dart-lang), 2015-04-13 17:56:40]

Committed patchset #3 (id:160001) manually as 45114 (presubmit successful).