Another case of creating from factory constructors was not taken into account in r6619

Another case of creating from factory constructors was not taken into account in r6619

http://code.google.com/p/dart/issues/detail?id=2282

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

[zundel, 2012-04-17 11:14:01]

That last change I did not properly handle classes that are abstract because
they do not implement abstract super methods.

I could not think of a way to test this with the current shared test
infrastructure, since creating such an abstract class mandates a static type
warning.  Currently, all of our checks are lenient and allow more than one
instance of an annotated static  type warning in either the standard test or
multi test case.

(but there is a Junit test for it)

[ahe, 2012-04-17 11:44:55]

LGTM

http://codereview.chromium.org/10108014/diff/1/compiler/javatests/com/google/...
File
compiler/javatests/com/google/dart/compiler/type/TypeAnalyzerCompilerTest.java
(right):

http://codereview.chromium.org/10108014/diff/1/compiler/javatests/com/google/...
compiler/javatests/com/google/dart/compiler/type/TypeAnalyzerCompilerTest.java:464:
"    return new A();",  // no error, factory constructor
Actually, this should never cause a warning. This is because the spec was
changed after I implemented the original algorithm for abstract.

This is definition from the spec:

"An abstract class is either a class that is explicitly declared with the
abstract modifier, or a class that declares at least one abstract method
(7.1.1)."

So A is not an abstract class.

However, then the idea was that we would warn about classes that have
unimplemented methods but not marked abstract. But I can't find this part in the
spec.

[scheglov, 2012-04-17 13:04:51]

LGTM

[zundel, 2012-04-17 13:49:17]

http://codereview.chromium.org/10108014/diff/1/compiler/javatests/com/google/...
File
compiler/javatests/com/google/dart/compiler/type/TypeAnalyzerCompilerTest.java
(right):

http://codereview.chromium.org/10108014/diff/1/compiler/javatests/com/google/...
compiler/javatests/com/google/dart/compiler/type/TypeAnalyzerCompilerTest.java:464:
"    return new A();",  // no error, factory constructor
Since I know the invoking factory method warning is an issue, I committed as is
and added issue http://code.google.com/p/dart/issues/detail?id=2619

This could be a 'value added' warning. But, I've seen it come out in what might
be seen as a nuisance warning in some contexts.  We need to get a story on being
able to turn individual warnings on and off.


On 2012/04/17 11:44:55, ahe wrote:
> Actually, this should never cause a warning. This is because the spec was
> changed after I implemented the original algorithm for abstract.
> 
> This is definition from the spec:
> 
> "An abstract class is either a class that is explicitly declared with the
> abstract modifier, or a class that declares at least one abstract method
> (7.1.1)."
> 
> So A is not an abstract class.
> 
> However, then the idea was that we would warn about classes that have
> unimplemented methods but not marked abstract. But I can't find this part in
the
> spec.