Handle mutually recursive re-exports in dartdocgen

[ak] Handle mutually recursive re-exports in dartdocgen

BUG=
R=tjblasi@google.com

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

[Alan Knight, 2015-02-19 00:46:13]

alanknight@google.com changed reviewers:
+ tjblasi@google.com

[Alan Knight, 2015-02-19 00:46:13]

This is, we're pretty sure, legal in the language, and fails for Angular with
core_dom/static_keys.dart re-exporting an entry from directive/static_keys.dart
which also re-exports the core_dom/static_keys.dart. Just add in a visited map
and short-circuit things we've already seen.

[tjblasi, 2015-02-19 00:52:56]

lgtm

https://codereview.chromium.org/929353004/diff/1/pkg/docgen/lib/src/models/mo...
File pkg/docgen/lib/src/models/model_helpers.dart (right):

https://codereview.chromium.org/929353004/diff/1/pkg/docgen/lib/src/models/mo...
pkg/docgen/lib/src/models/model_helpers.dart:220: LibrarySourceMirror library,
Map visited) {
It seems a bit strange that this is a required param since non-recursive calls
don't really need to specify it.

I'd put in a weak vote for:
- Have calcExportedItems call _calcExportedItemsRecursive(...) which accepts the
visited map
- Make it an optional positional parameter & check if (visited == null) visited
= {};

Or do nothing, I don't feel strongly about it :)

[Alan Knight, 2015-02-19 01:07:04]

https://codereview.chromium.org/929353004/diff/1/pkg/docgen/lib/src/models/mo...
File pkg/docgen/lib/src/models/model_helpers.dart (right):

https://codereview.chromium.org/929353004/diff/1/pkg/docgen/lib/src/models/mo...
pkg/docgen/lib/src/models/model_helpers.dart:220: LibrarySourceMirror library,
Map visited) {
On 2015/02/19 00:52:55, tjblasi wrote:
> It seems a bit strange that this is a required param since non-recursive calls
> don't really need to specify it.
> 
> I'd put in a weak vote for:
> - Have calcExportedItems call _calcExportedItemsRecursive(...) which accepts
the
> visited map
> - Make it an optional positional parameter & check if (visited == null)
visited
> = {};
> 
> Or do nothing, I don't feel strongly about it :)

We can't know in advance if it's recursive or not. Or at least not without
re-doing much of the same work. And there are only two callers - the one just
below that passes it through, and the one that starts it all off with the empty
map. So it seemed simplest just to always pass it.

[Alan Knight, 2015-02-19 01:07:28]

Committed patchset #1 (id:1) manually as 43861 (presubmit successful).