Fix some warnings in pub and pkg packages.

Fix some warnings in pub and pkg packages.

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

[nweiz, 2013-04-11 22:58:22]

Gram is for unittest, Alan for intl, Bob for safe_http_server and pub.

[Bob Nystrom, 2013-04-15 23:15:17]

LGTM.

[gram, 2013-04-15 23:25:55]

https://codereview.chromium.org/13817008/diff/1/pkg/unittest/lib/unittest.dart
File pkg/unittest/lib/unittest.dart (right):

https://codereview.chromium.org/13817008/diff/1/pkg/unittest/lib/unittest.dar...
pkg/unittest/lib/unittest.dart:208: final List<TestCase> testCases = _testCases;
If the unmodifiable stuff (which KevMoo added) is bad, then rather than the
above it would probably be better to just make testCases a getter for
_testCases, which is what it was originally.

[nweiz, 2013-04-15 23:41:15]

https://codereview.chromium.org/13817008/diff/1/pkg/unittest/lib/unittest.dart
File pkg/unittest/lib/unittest.dart (right):

https://codereview.chromium.org/13817008/diff/1/pkg/unittest/lib/unittest.dar...
pkg/unittest/lib/unittest.dart:208: final List<TestCase> testCases = _testCases;
On 2013/04/15 23:25:55, gram wrote:
> If the unmodifiable stuff (which KevMoo added) is bad, then rather than the
> above it would probably be better to just make testCases a getter for
> _testCases, which is what it was originally. 

I've just made it a property, since it's final either way. I've also added a
TODO to present an unmodifiable view when that's possible.

[Alan Knight, 2013-04-18 19:42:24]

https://codereview.chromium.org/13817008/diff/6001/pkg/intl/example/basic/mes...
File pkg/intl/example/basic/messages_de.dart (right):

https://codereview.chromium.org/13817008/diff/6001/pkg/intl/example/basic/mes...
pkg/intl/example/basic/messages_de.dart:20: bool localeExists(localeName) =>
null;
In normal usage these will be generated files, so just adding the method here
probably doesn't help. Looking at this more closely, I think that the abstract
method in the superclass is in fact unused, and can be removed.

https://codereview.chromium.org/13817008/diff/6001/pkg/intl/lib/number_format...
File pkg/intl/lib/number_format.dart (right):

https://codereview.chromium.org/13817008/diff/6001/pkg/intl/lib/number_format...
pkg/intl/lib/number_format.dart:732: // TODO(nweiz): remove this when issue 3780
is fixed.
I wasn't getting any warnings from this in the editor, where do they show up? I
find it annoying to have to do this level of boilerplate, but if it's necessary,
fine.

[nweiz, 2013-04-18 19:59:23]

https://codereview.chromium.org/13817008/diff/6001/pkg/intl/example/basic/mes...
File pkg/intl/example/basic/messages_de.dart (right):

https://codereview.chromium.org/13817008/diff/6001/pkg/intl/example/basic/mes...
pkg/intl/example/basic/messages_de.dart:20: bool localeExists(localeName) =>
null;
On 2013/04/18 19:42:25, Alan Knight wrote:
> In normal usage these will be generated files, so just adding the method here
> probably doesn't help. Looking at this more closely, I think that the abstract
> method in the superclass is in fact unused, and can be removed.

Done.

https://codereview.chromium.org/13817008/diff/6001/pkg/intl/lib/number_format...
File pkg/intl/lib/number_format.dart (right):

https://codereview.chromium.org/13817008/diff/6001/pkg/intl/lib/number_format...
pkg/intl/lib/number_format.dart:732: // TODO(nweiz): remove this when issue 3780
is fixed.
On 2013/04/18 19:42:25, Alan Knight wrote:
> I wasn't getting any warnings from this in the editor, where do they show up?
I
> find it annoying to have to do this level of boilerplate, but if it's
necessary,
> fine.

They showed up when running the analyzer. It's weird that they didn't show up in
the editor; the code is definitely broken. You can't use for/each with an
iterator.

[Alan Knight, 2013-04-18 20:07:18]

LGTM given the additional deletion below.

https://codereview.chromium.org/13817008/diff/6001/pkg/intl/example/basic/mes...
File pkg/intl/example/basic/messages_de.dart (right):

https://codereview.chromium.org/13817008/diff/6001/pkg/intl/example/basic/mes...
pkg/intl/example/basic/messages_de.dart:20: bool localeExists(localeName) =>
null;
On 2013/04/18 19:59:23, nweiz wrote:
> On 2013/04/18 19:42:25, Alan Knight wrote:
> > In normal usage these will be generated files, so just adding the method
here
> > probably doesn't help. Looking at this more closely, I think that the
abstract
> > method in the superclass is in fact unused, and can be removed.
> 
> Done.

Then they also should be deleted from this and the _de file.

https://codereview.chromium.org/13817008/diff/6001/pkg/intl/lib/number_format...
File pkg/intl/lib/number_format.dart (right):

https://codereview.chromium.org/13817008/diff/6001/pkg/intl/lib/number_format...
pkg/intl/lib/number_format.dart:732: // TODO(nweiz): remove this when issue 3780
is fixed.
On 2013/04/18 19:59:23, nweiz wrote:
> On 2013/04/18 19:42:25, Alan Knight wrote:
> > I wasn't getting any warnings from this in the editor, where do they show
up?
> I
> > find it annoying to have to do this level of boilerplate, but if it's
> necessary,
> > fine.
> 
> They showed up when running the analyzer. It's weird that they didn't show up
in
> the editor; the code is definitely broken. You can't use for/each with an
> iterator.

The code runs fine because the iterator also implements get iterator => this;

but fine, it's probably clearer to do it the long way.

[nweiz, 2013-04-18 20:24:54]

Committed patchset #3 manually as r21701 (presubmit successful).