Issue 1251. Check for getter existence.

Issue 1251. Check for getter existence.
http://code.google.com/p/dart/issues/detail?id=1251


R=zundel@google.com
BUG=
TEST=

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

[zundel, 2012-01-27 20:40:46]

https://chromiumcodereview.appspot.com/9296016/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java (right):

https://chromiumcodereview.appspot.com/9296016/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:1305: FieldElement
fieldElement = (FieldElement) element;
I didn't think about this before, but what happens for the opposite case, where
a getter, but no setter is defined and the property is on the LHS of an
assignment?  Is this visitor going to run on the LHS of the assignment and
trigger a bogus error?

class SetOnly {
  var _a;
  set a(arg) => _a = arg;
}

var instance = new SetOnly();
instance.a = 1;

[scheglov, 2012-01-30 15:09:27]

https://chromiumcodereview.appspot.com/9296016/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java (right):

https://chromiumcodereview.appspot.com/9296016/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:1305: FieldElement
fieldElement = (FieldElement) element;
On 2012/01/27 20:40:46, zundel wrote:
> I didn't think about this before, but what happens for the opposite case,
where
> a getter, but no setter is defined and the property is on the LHS of an
> assignment?  Is this visitor going to run on the LHS of the assignment and
> trigger a bogus error?
> 
> class SetOnly {
>   var _a;
>   set a(arg) => _a = arg;
> }
> 
> var instance = new SetOnly();
> instance.a = 1;

1. I've added check for left/right side of assignment.

2. I've added same check/warning for properties without setter.

[zundel, 2012-01-30 16:17:03]

https://chromiumcodereview.appspot.com/9296016/diff/6001/compiler/java/com/go...
File compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java (right):

https://chromiumcodereview.appspot.com/9296016/diff/6001/compiler/java/com/go...
compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:1334: if
(node.getParent() instanceof DartBinaryExpression) {
I don't think this test is going to catch all cases:

new Foo().bar().baz = "Hello"

in this case, you've got several nodes between the field and the binary
operation that contains the "DartNewExpression" as its 'arg1'.  You may need a
helper function to walk up the chain of parents until you find a binary
expression and then test if its an assignment.

[scheglov, 2012-01-30 17:28:19]

https://chromiumcodereview.appspot.com/9296016/diff/6001/compiler/java/com/go...
File compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java (right):

https://chromiumcodereview.appspot.com/9296016/diff/6001/compiler/java/com/go...
compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:1334: if
(node.getParent() instanceof DartBinaryExpression) {
On 2012/01/30 16:17:03, zundel wrote:
> I don't think this test is going to catch all cases:
> 
> new Foo().bar().baz = "Hello"
> 
> in this case, you've got several nodes between the field and the binary
> operation that contains the "DartNewExpression" as its 'arg1'.  You may need a
> helper function to walk up the chain of parents until you find a binary
> expression and then test if its an assignment.

Actually this works.
You will have AST like this:
DartPropertyAccess(DartMethodInvocation(new Foo().bar()), "baz") = "Hello";
i.e. LHS in assignment is actual DartPropertyAccess which has invocation as
qualifier, but we don't care about this.

And if PropertyAccess is not the last operation before assignment, then its
parent is not BinaryExpression assignment and we will report problem.

[zundel, 2012-01-30 17:35:08]

lgtm