Require legacy form of type parameters in factories and report errors if a factory class does not...

Require legacy form of type parameters in factories and report errors if a factory class does not have a required parametrized type (fixes crashes in VM). Fix frog code, however, there are frogs problem with --compileall:

leg/util/link_implementation.dart:38:12: error: wrong number of arguments, expected 0 but found 2
    return new Link<T>(element, this);
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
Committed: https://code.google.com/p/dart/source/detail?r=1464

[regis, 2011-11-11 01:11:06]

LGTM

http://codereview.chromium.org/8515024/diff/2002/frog/leg/util/link_implement...
File frog/leg/util/link_implementation.dart (right):

http://codereview.chromium.org/8515024/diff/2002/frog/leg/util/link_implement...
frog/leg/util/link_implementation.dart:5: class LinkFactory<T> {
Can you please add:

// TODO(regis): Move type parameter <T> from the class declaration to the
// factory declarations after the new factory syntax is implemented.
// See issues 226, 5257789, 5408808.

[srdjan, 2011-11-11 01:18:11]

http://codereview.chromium.org/8515024/diff/2002/frog/leg/util/link_implement...
File frog/leg/util/link_implementation.dart (right):

http://codereview.chromium.org/8515024/diff/2002/frog/leg/util/link_implement...
frog/leg/util/link_implementation.dart:5: class LinkFactory<T> {
On 2011/11/11 01:11:07, regis wrote:
> Can you please add:
> 
> // TODO(regis): Move type parameter <T> from the class declaration to the
> // factory declarations after the new factory syntax is implemented.
> // See issues 226, 5257789, 5408808.

With pleasure.

[ngeoffray, 2011-11-11 10:09:44]

http://codereview.chromium.org/8515024/diff/7006/tests/language/src/NonParame...
File tests/language/src/NonParameterizedFactoryTest.dart (right):

http://codereview.chromium.org/8515024/diff/7006/tests/language/src/NonParame...
tests/language/src/NonParameterizedFactoryTest.dart:10: // A factory must be
parametrized. Use legacy form.
parameterized.

I don't understand the comment. Why 'must' it be? What's the legacy form?

[jimhug, 2011-11-11 14:48:29]

lgtm

But I'm adding Peter as a reviewer since I honestly don't understand the current
state of generics and factories in dart - and the code in frog that you need to
change is his.

[ahe, 2011-11-11 14:54:30]

Unfortunately, the change to the test is not correct. I'm having a headache
today so I won't be available to chat tonight about this, but we can chat on
Monday if you like.

http://codereview.chromium.org/8515024/diff/7006/tests/language/src/NonParame...
File tests/language/src/NonParameterizedFactoryTest.dart (right):

http://codereview.chromium.org/8515024/diff/7006/tests/language/src/NonParame...
tests/language/src/NonParameterizedFactoryTest.dart:10: // A factory must be
parametrized. Use legacy form.
On 2011/11/11 10:09:44, ngeoffray wrote:
> parameterized.
> 
> I don't understand the comment. Why 'must' it be? What's the legacy form?

A factory *class* do not have to be parameterized.

http://codereview.chromium.org/8515024/diff/7006/tests/language/src/NonParame...
tests/language/src/NonParameterizedFactoryTest.dart:11: class Factory<T> {
Remove the type variable.

http://codereview.chromium.org/8515024/diff/7006/tests/language/src/NonParame...
tests/language/src/NonParameterizedFactoryTest.dart:12: factory Interface() {
This should be:

factory Interface<T>() {

http://codereview.chromium.org/8515024/diff/7006/tests/language/src/NonParame...
tests/language/src/NonParameterizedFactoryTest.dart:16: factory
Interface.withArg(value) {
This should be:

factory Interface<T>.withArg(value) {

[srdjan, 2011-11-11 16:31:04]

On 2011/11/11 10:09:44, ngeoffray wrote:
>
http://codereview.chromium.org/8515024/diff/7006/tests/language/src/NonParame...
> File tests/language/src/NonParameterizedFactoryTest.dart (right):
> 
>
http://codereview.chromium.org/8515024/diff/7006/tests/language/src/NonParame...
> tests/language/src/NonParameterizedFactoryTest.dart:10: // A factory must be
> parametrized. Use legacy form.
> parameterized.
> 
> I don't understand the comment. Why 'must' it be? What's the legacy form?

See issue & bugs 226, 5257789, 5408808

Legacy form is that we specify the type argument in the factory class instead of
in the factory method (it must be somewhere).

[srdjan, 2011-11-11 16:33:39]

http://codereview.chromium.org/8515024/diff/7006/tests/language/src/NonParame...
File tests/language/src/NonParameterizedFactoryTest.dart (right):

http://codereview.chromium.org/8515024/diff/7006/tests/language/src/NonParame...
tests/language/src/NonParameterizedFactoryTest.dart:11: class Factory<T> {
On 2011/11/11 14:54:30, ahe wrote:
> Remove the type variable.

Done.

http://codereview.chromium.org/8515024/diff/7006/tests/language/src/NonParame...
tests/language/src/NonParameterizedFactoryTest.dart:12: factory Interface() {
On 2011/11/11 14:54:30, ahe wrote:
> This should be:
> 
> factory Interface<T>() {

Done.

http://codereview.chromium.org/8515024/diff/7006/tests/language/src/NonParame...
tests/language/src/NonParameterizedFactoryTest.dart:16: factory
Interface.withArg(value) {
On 2011/11/11 14:54:30, ahe wrote:
> This should be:
> 
> factory Interface<T>.withArg(value) {

Done.

http://codereview.chromium.org/8515024/diff/7006/tests/language/src/NonParame...
tests/language/src/NonParameterizedFactoryTest.dart:19: }
Marked the test failing with bugs mentioned.

[srdjan, 2011-11-11 16:56:01]

http://codereview.chromium.org/8515024/diff/14001/frog/leg/util/link_implemen...
File frog/leg/util/link_implementation.dart (right):

http://codereview.chromium.org/8515024/diff/14001/frog/leg/util/link_implemen...
frog/leg/util/link_implementation.dart:5: class LinkFactory {
Note the change: Instead of putting the type argument in the wrong place, I am
eliminating the type argument until it is properly implemented.