Fix for 16078- clarifications on the StaticWarningCode.FUNCTION_WITHOUT_CALL warning

Fix for first half of 16078- clarifications on the StaticWarningCode.FUNCTION_WITHOUT_CALL warning

R=brianwilkerson@google.com

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

[Brian Wilkerson, 2014-04-10 22:26:15]

https://codereview.chromium.org/231473002/diff/60001/editor/tools/plugins/com...
File
editor/tools/plugins/com.google.dart.engine_test/src/com/google/dart/engine/resolver/NonErrorResolverTest.java
(right):

https://codereview.chromium.org/231473002/diff/60001/editor/tools/plugins/com...
editor/tools/plugins/com.google.dart.engine_test/src/com/google/dart/engine/resolver/NonErrorResolverTest.java:1173:
public void test_functionWithoutCall_withNoSuchMethod() throws Exception {
I don't understand what the important difference is between these two methods
and hence why both are needed. Is it the addition of @proxy, or the addition of
a call site?

What about the case where the class doesn't implement Function? Should that be
an error?

[jwren, 2014-04-11 21:14:35]

https://codereview.chromium.org/231473002/diff/60001/editor/tools/plugins/com...
File
editor/tools/plugins/com.google.dart.engine_test/src/com/google/dart/engine/resolver/NonErrorResolverTest.java
(right):

https://codereview.chromium.org/231473002/diff/60001/editor/tools/plugins/com...
editor/tools/plugins/com.google.dart.engine_test/src/com/google/dart/engine/resolver/NonErrorResolverTest.java:1173:
public void test_functionWithoutCall_withNoSuchMethod() throws Exception {
The other non-error tests cover the cases where a class implementing Function
have a call() method declaration.  The change to ErrorVerifier (and the first
half of the issue) says that if noSuchMethod is declared, then we also don't
produce the warning, hence this 

On 2014/04/10 22:26:16, Brian Wilkerson wrote:
> I don't understand what the important difference is between these two methods
> and hence why both are needed. Is it the addition of @proxy, or the addition
of
> a call site?
> 
> What about the case where the class doesn't implement Function? Should that be
> an error?

No.  Adding a test case for completeness.  The spec only refers to classes that
implement Function.

[Brian Wilkerson, 2014-04-11 21:36:18]

LGTM, but if the two test methods are not both needed I'd delete one.

https://codereview.chromium.org/231473002/diff/60001/editor/tools/plugins/com...
File
editor/tools/plugins/com.google.dart.engine_test/src/com/google/dart/engine/resolver/NonErrorResolverTest.java
(right):

https://codereview.chromium.org/231473002/diff/60001/editor/tools/plugins/com...
editor/tools/plugins/com.google.dart.engine_test/src/com/google/dart/engine/resolver/NonErrorResolverTest.java:1173:
public void test_functionWithoutCall_withNoSuchMethod() throws Exception {
> The other non-error tests cover the cases where a class implementing Function
> have a call() method declaration.

Sorry if I'm being dense, but I'm still not understanding. Both of the two new
methods in this class have a test case in which the class does not implement the
'call' method. In the first method, the class is annotated with @proxy and there
is a call site. The second case has neither. I don't understand why either of
those changes is interesting (in other words, it looks to me like both methods
are testing the same code path).

[jwren, 2014-04-11 22:48:42]

https://codereview.chromium.org/231473002/diff/60001/editor/tools/plugins/com...
File
editor/tools/plugins/com.google.dart.engine_test/src/com/google/dart/engine/resolver/NonErrorResolverTest.java
(right):

https://codereview.chromium.org/231473002/diff/60001/editor/tools/plugins/com...
editor/tools/plugins/com.google.dart.engine_test/src/com/google/dart/engine/resolver/NonErrorResolverTest.java:1173:
public void test_functionWithoutCall_withNoSuchMethod() throws Exception {
I understand your confusion now, by two methods I thought you were talking about
this method (test_functionWithoutCall_withNoSuchMethod) and
test_functionWithoutCall.

The failing test above is for the non-error test case for a *different* error
code: fail_invocationOfNonFunction_proxyOnFunctionClass.  It happens to
implement noSuchMethod, but it could have implemented call, it does so that
FUNCTION_WITHOUT_CALL (this error code) isn't generated.



On 2014/04/11 21:36:18, Brian Wilkerson wrote:
> > The other non-error tests cover the cases where a class implementing
Function
> > have a call() method declaration.
> 
> Sorry if I'm being dense, but I'm still not understanding. Both of the two new
> methods in this class have a test case in which the class does not implement
the
> 'call' method. In the first method, the class is annotated with @proxy and
there
> is a call site. The second case has neither. I don't understand why either of
> those changes is interesting (in other words, it looks to me like both methods
> are testing the same code path).

[Brian Wilkerson, 2014-04-11 23:49:05]

LGTM

[jwren, 2014-04-12 00:35:44]

Committed patchset #5 manually as r35002 (presubmit successful).