Allow cascades with repeated method names to be one line.

Allow cascades with repeated method names to be one line.

BUG=
R=rnystrom@google.com

Committed: 1195508ee8b9ca4135e15f333ef22d9c409e8bf6

[Bob Nystrom, 2015-03-20 19:59:41]

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart
File lib/src/source_visitor.dart (right):

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart#n...
lib/src/source_visitor.dart:386: bool _consistentNames(NodeList<Expression>
sections) {
Let's call this "_allowInlineCascade". Also:

- Doc comment.
- Just take a List<Expression> instead of NodeList.

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart#n...
lib/src/source_visitor.dart:389: // We could be more forgiving about what
consistutes sections with
"constitutes"

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart#n...
lib/src/source_visitor.dart:393: if (expression is! MethodInvocation) return
false;
Document this. It's to disallow getters/setters, right?

Or better, just:

if (sections.any((section) => section is! MethodInvocation)) return false;

var name = sections.first.methodName.name;
return sections.all((section) => section.methodName.name == name);

https://codereview.appspot.com/212600043/diff/1/test/whitespace/cascades.stmt
File test/whitespace/cascades.stmt (right):

https://codereview.appspot.com/212600043/diff/1/test/whitespace/cascades.stmt...
test/whitespace/cascades.stmt:12: >>> multiple cascades get the same line when
the method name is the same list
Remove "list".

https://codereview.appspot.com/212600043/diff/1/test/whitespace/cascades.stmt...
test/whitespace/cascades.stmt:24: >>> cascades indent contained blocks (and
force multi-line)
Test that setters with the same name still get split.

[jacobr, 2015-03-20 20:20:53]

ptal

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart
File lib/src/source_visitor.dart (right):

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart#n...
lib/src/source_visitor.dart:386: bool _consistentNames(NodeList<Expression>
sections) {
On 2015/03/20 19:59:41, Bob Nystrom wrote:
> Let's call this "_allowInlineCascade". Also:
> 
> - Doc comment.
> - Just take a List<Expression> instead of NodeList.

done

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart#n...
lib/src/source_visitor.dart:389: // We could be more forgiving about what
consistutes sections with
On 2015/03/20 19:59:41, Bob Nystrom wrote:
> "constitutes"

Done.

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart#n...
lib/src/source_visitor.dart:393: if (expression is! MethodInvocation) return
false;
On 2015/03/20 19:59:40, Bob Nystrom wrote:
> Document this. It's to disallow getters/setters, right?
> 
> Or better, just:
> 
> if (sections.any((section) => section is! MethodInvocation)) return false;
> 
> var name = sections.first.methodName.name;
> return sections.all((section) => section.methodName.name == name);

your suggested code is not analyzer warning clean. otherwise I agree it is
slightly better although less efficient.

https://codereview.appspot.com/212600043/diff/1/test/whitespace/cascades.stmt
File test/whitespace/cascades.stmt (right):

https://codereview.appspot.com/212600043/diff/1/test/whitespace/cascades.stmt...
test/whitespace/cascades.stmt:24: >>> cascades indent contained blocks (and
force multi-line)
On 2015/03/20 19:59:41, Bob Nystrom wrote:
> Test that setters with the same name still get split.

Done.

[Bob Nystrom, 2015-03-20 21:05:48]

You'll need to add this to the changelog too.

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart
File lib/src/source_visitor.dart (right):

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart#n...
lib/src/source_visitor.dart:388: String name = null;
"var name;"

Nit: but add a blank line above it too, please.

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart#n...
lib/src/source_visitor.dart:395: name = expression.methodName.toSource();
"toSource()" -> "name".

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart#n...
lib/src/source_visitor.dart:396: } else if (name !=
expression.methodName.toSource()) {
Here too.

https://codereview.appspot.com/212600043/diff/20001/test/whitespace/cascades....
File test/whitespace/cascades.stmt (right):

https://codereview.appspot.com/212600043/diff/20001/test/whitespace/cascades....
test/whitespace/cascades.stmt:24: >>> cascades indent contained blocks (and
force multi-line) multiple cascades get their own line even when property names
are the same
"cascaded setters are always multi-line even with the same names"

[jacobr, 2015-03-20 23:16:09]

ptal

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart
File lib/src/source_visitor.dart (right):

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart#n...
lib/src/source_visitor.dart:388: String name = null;
On 2015/03/20 21:05:48, Bob Nystrom wrote:
> "var name;"
> 
> Nit: but add a blank line above it too, please.

Done.

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart#n...
lib/src/source_visitor.dart:395: name = expression.methodName.toSource();
On 2015/03/20 21:05:48, Bob Nystrom wrote:
> "toSource()" -> "name".

This is fixed in patch set I uploaded with my code review comments.

https://codereview.appspot.com/212600043/diff/1/lib/src/source_visitor.dart#n...
lib/src/source_visitor.dart:396: } else if (name !=
expression.methodName.toSource()) {
On 2015/03/20 21:05:48, Bob Nystrom wrote:
> Here too.

This is fixed in patch set I uploaded with my code review comments.

https://codereview.appspot.com/212600043/diff/20001/test/whitespace/cascades....
File test/whitespace/cascades.stmt (right):

https://codereview.appspot.com/212600043/diff/20001/test/whitespace/cascades....
test/whitespace/cascades.stmt:24: >>> cascades indent contained blocks (and
force multi-line) multiple cascades get their own line even when property names
are the same
On 2015/03/20 21:05:48, Bob Nystrom wrote:
> "cascaded setters are always multi-line even with the same names"

Done.

[Bob Nystrom, 2015-03-23 20:01:18]

Don't forget to add it to the changelog too, then LGTM.

https://codereview.appspot.com/212600043/diff/60001/lib/src/source_visitor.dart
File lib/src/source_visitor.dart (right):

https://codereview.appspot.com/212600043/diff/60001/lib/src/source_visitor.da...
lib/src/source_visitor.dart:391: String name = null;
var name;

[jacobr, 2015-03-23 21:18:51]

Committed patchset #5 (id:80001) manually as
1195508ee8b9ca4135e15f333ef22d9c409e8bf6 (presubmit successful).