New analyzer snapshot.

New analyzer snapshot.

1. Fixes for converting properties for fields:
  1.1. Only if neither getter not setter are overridden. Note that we ignore the fact that it _might_ be overridden in some subclass that we don't see.
       Without this assumption almost nothing can be a field.
  2.2. Create final fields when possible.
2. Generate field formal parameters where possible.

R=brianwilkerson@google.com
BUG=

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

[Brian Wilkerson, 2013-12-10 14:57:40]

https://codereview.chromium.org/109853003/diff/1/editor/util/plugins/com.goog...
File
editor/util/plugins/com.google.dart.java2dart/src/com/google/dart/java2dart/processor/PropertySemanticProcessor.java
(right):

https://codereview.chromium.org/109853003/diff/1/editor/util/plugins/com.goog...
editor/util/plugins/com.google.dart.java2dart/src/com/google/dart/java2dart/processor/PropertySemanticProcessor.java:95:
// TODO(scheglov) should ASTNode support this?
Why do we need to do this at all? If the parent's field is set to something else
(or if the node is removed from a NodeList) then the node is no longer reachable
via the AST. If it's then used to set the child of another node (or added to a
different list) it's parent field will be re-set. Under what conditions is a
node that was removed from an AST structure still accessed in a way in which the
parent field would be accessed?

(If this does need to be done, then the principle of encapsulation would suggest
that it should happen automatically rather than to make the client remember to
clean up the data structure.)

https://codereview.chromium.org/109853003/diff/1/editor/util/plugins/com.goog...
editor/util/plugins/com.google.dart.java2dart/src/com/google/dart/java2dart/processor/PropertySemanticProcessor.java:104:
private static VariableDeclaration getFieldDeclaration(ClassDeclaration clazz,
String name) {
This, on the other hand, clearly belongs as a utility method defined on
ClassDeclaration.

https://codereview.chromium.org/109853003/diff/1/editor/util/plugins/com.goog...
editor/util/plugins/com.google.dart.java2dart/src/com/google/dart/java2dart/processor/PropertySemanticProcessor.java:651:
if (isDanglingNode(reference)) {
Instead of depending on the parent chain being broken, could we simply remove it
from the references when we remove it from the AST?

https://codereview.chromium.org/109853003/diff/1/pkg/analyzer/lib/src/generat...
File pkg/analyzer/lib/src/generated/ast.dart (right):

https://codereview.chromium.org/109853003/diff/1/pkg/analyzer/lib/src/generat...
pkg/analyzer/lib/src/generated/ast.dart:505:
AdjacentStrings({List<StringLiteral> strings}) : this.full(strings);
I never noticed this before, but why are we generating a constructor with an
optional argument when the original source doesn't include a zero-argument
constructor?

https://codereview.chromium.org/109853003/diff/1/pkg/analyzer/lib/src/generat...
pkg/analyzer/lib/src/generated/ast.dart:518: NodeList<StringLiteral> get strings
=> _strings;
Does this mean that what we were generating before is wrong, or is this a
regression?

[scheglov, 2013-12-10 18:21:08]

PTAL

https://codereview.chromium.org/109853003/diff/1/editor/util/plugins/com.goog...
File
editor/util/plugins/com.google.dart.java2dart/src/com/google/dart/java2dart/processor/PropertySemanticProcessor.java
(right):

https://codereview.chromium.org/109853003/diff/1/editor/util/plugins/com.goog...
editor/util/plugins/com.google.dart.java2dart/src/com/google/dart/java2dart/processor/PropertySemanticProcessor.java:95:
// TODO(scheglov) should ASTNode support this?
On 2013/12/10 14:57:41, Brian Wilkerson wrote:
> Why do we need to do this at all? If the parent's field is set to something
else
> (or if the node is removed from a NodeList) then the node is no longer
reachable
> via the AST. If it's then used to set the child of another node (or added to a
> different list) it's parent field will be re-set. Under what conditions is a
> node that was removed from an AST structure still accessed in a way in which
the
> parent field would be accessed?
> 
> (If this does need to be done, then the principle of encapsulation would
suggest
> that it should happen automatically rather than to make the client remember to
> clean up the data structure.)

I think you are right.
It is not nice to force each client to check for parent, etc.
I will add cleaning up in Context instead.

https://codereview.chromium.org/109853003/diff/1/editor/util/plugins/com.goog...
editor/util/plugins/com.google.dart.java2dart/src/com/google/dart/java2dart/processor/PropertySemanticProcessor.java:104:
private static VariableDeclaration getFieldDeclaration(ClassDeclaration clazz,
String name) {
On 2013/12/10 14:57:41, Brian Wilkerson wrote:
> This, on the other hand, clearly belongs as a utility method defined on
> ClassDeclaration.

Good idea.
https://codereview.chromium.org/108173006

https://codereview.chromium.org/109853003/diff/1/pkg/analyzer/lib/src/generat...
File pkg/analyzer/lib/src/generated/ast.dart (right):

https://codereview.chromium.org/109853003/diff/1/pkg/analyzer/lib/src/generat...
pkg/analyzer/lib/src/generated/ast.dart:505:
AdjacentStrings({List<StringLiteral> strings}) : this.full(strings);
On 2013/12/10 14:57:41, Brian Wilkerson wrote:
> I never noticed this before, but why are we generating a constructor with an
> optional argument when the original source doesn't include a zero-argument
> constructor?

At one point I thought that it is nice to provide a constructor with optional
named parameters.
But yes, this may promote making errors, such as NPEs.
Removed.

https://codereview.chromium.org/109853003/diff/1/pkg/analyzer/lib/src/generat...
pkg/analyzer/lib/src/generated/ast.dart:518: NodeList<StringLiteral> get strings
=> _strings;
On 2013/12/10 14:57:41, Brian Wilkerson wrote:
> Does this mean that what we were generating before is wrong, or is this a
> regression?

It was generated wrongly before.

In Java property "strings" is read-only, so to keep it just a field we would
need to mark it final.
However its initializer references "this", which is fine in Java, but not in
Dart.
So, we have to move the initializer into the constructor body and make property
read-only by providing only getter.

[Brian Wilkerson, 2013-12-10 19:19:10]

LGTM

[scheglov, 2013-12-10 21:40:04]

Committed patchset #2 manually as r31043 (presubmit successful).