Skip to content

Commit 00abeb1

Browse files
astashovastashov
committed
Deemphasize reexported symbols [#1158]
@Hixie noticed, that when a library exports another library, but both libraries are being docced together, the result is that each class that is exported by both libraries is listed twice. We should still list classes and other symbols in the libraries that reexport them (since they still part of public API of those libs), but we only gonna show "canonical" symbols in Search, and also we won't create duplicate HTML pages for reexported symbols. We define the reexported symbols in the following way: 1. A defines Foo. 2. Foo doesn't appear in the docs for A because A is a private library, i.e., in lib/src/../a.dart 3. B exports Foo from A. 4. Foo appears in the docs for B. 5. C exports Foo from B. 6. Foo appears in the docs for C. So, Foo of lib C shouldn't have its own page and also shouldn't appear in Search, because Foo was exported from B and B already has docs. All links to Foo from HTML docs of C should point to the Foo of B. To solve that, we build directed "export graph", where we track what libs export what libs. Then, to find the "canonical" lib, we go up the graph until we find first documented lib - that's going to be the canonical one. Then, we filter out non-canonical libs when building the search index, and also use canonical lib in `.href` getters, this way ensuring we always link to symbols in canonical libs and don't create HTML files for symbols from non-canonical libs. Testing: Unit tests, also built test docs (`testing/test_package_docs`), made sure we don't create files for non-canonical stuff (i.e. we create `fake/Cool` files, but not `ex/Cool`). Also, built Flutter docs, ensured we don't create `material/BoxDecoration`, `rendering/BoxDecoration` and `widgets/BoxDecoration`, and only create `painting/BoxDecoration` files, and all the links point to `painting/BoxDecoration`.
1 parent 06ee58a commit 00abeb1

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

61 files changed

+154
-1423
lines changed

‎lib/src/export_graph.dart‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import 'package:analyzer/dart/element/element.dart';
2+
3+
class _ExportGraphNode {
4+
final LibraryElement libraryElement;
5+
Set<_ExportGraphNode> exportedBy = new Set();
6+
Set<_ExportGraphNode> exports = new Set();
7+
8+
_ExportGraphNode(this.libraryElement);
9+
10+
_ExportGraphNode canonicalLibraryElement(Iterable<LibraryElement> libraryElements) {
11+
if (libraryElements.contains(libraryElement)) {
12+
return this;
13+
} else {
14+
return exportedBy.toList().firstWhere((l) => l.canonicalLibraryElement(libraryElements) != null);
15+
}
16+
}
17+
}
18+
19+
void _buildSubGraph(Map<String, _ExportGraphNode> map, LibraryElement libraryElement) {
20+
if (!map.containsKey(libraryElement.source.fullName)) {
21+
map[libraryElement.source.fullName] = new _ExportGraphNode(libraryElement);
22+
}
23+
final node = map[libraryElement.source.fullName];
24+
libraryElement.exports.forEach((ExportElement export) {
25+
final exportedLibraryElement = export.exportedLibrary;
26+
if (!map.containsKey(exportedLibraryElement.source.fullName)) {
27+
map[exportedLibraryElement.source.fullName] = new _ExportGraphNode(exportedLibraryElement);
28+
}
29+
final childNode = map[exportedLibraryElement.source.fullName];
30+
childNode.exportedBy.add(node);
31+
node.exports.add(childNode);
32+
_buildSubGraph(map, exportedLibraryElement);
33+
});
34+
}
35+
36+
class ExportGraph {
37+
final Map<String, _ExportGraphNode> map = {};
38+
final Iterable<LibraryElement> packageLibraryElements;
39+
40+
ExportGraph(this.packageLibraryElements) {
41+
packageLibraryElements.forEach((LibraryElement libraryElement) {
42+
_buildSubGraph(map, libraryElement);
43+
});
44+
}
45+
46+
LibraryElement canonicalLibraryElement(LibraryElement libraryElement) {
47+
if (map.containsKey(libraryElement.source.fullName)) {
48+
final node = map[libraryElement.source.fullName];
49+
return node.canonicalLibraryElement(packageLibraryElements)?.libraryElement;
50+
} else {
51+
return libraryElement;
52+
}
53+
}
54+
}

‎lib/src/html/html_generator_instance.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class HtmlGeneratorInstance implements HtmlOptions {
5050

5151
void _generateSearchIndex() {
5252
File jsonFile = _createOutputFile(out, 'index.json');
53-
String json = JSON.encode(documentedElements.map((ModelElement e) {
53+
String json = JSON.encode(documentedElements.where((e) => e.isCanonical).map((ModelElement e) {
5454
Map data = {
5555
'name': e.name,
5656
'qualifiedName': e.name,

‎lib/src/model.dart‎

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import 'markdown_processor.dart' show Documentation;
2626
import 'model_utils.dart';
2727
import 'package_meta.dart' show PackageMeta, FileContents;
2828
import 'utils.dart';
29+
import 'export_graph.dart';
2930

3031
Map<String, Map<String, List<Map<String, dynamic>>>> __crossdartJson;
3132

@@ -98,7 +99,7 @@ class Accessor extends ModelElement
9899

99100
@override
100101
String get href =>
101-
'${library.dirName}/${_accessor.enclosingElement.name}/${name}.html';
102+
'${canonicalLibrary.dirName}/${_accessor.enclosingElement.name}/${name}.html';
102103

103104
bool get isGetter => _accessor.isGetter;
104105

@@ -310,7 +311,7 @@ class Class extends ModelElement implements EnclosedElement {
310311
bool get hasSupertype => supertype != null;
311312

312313
@override
313-
String get href => '${library.dirName}/$fileName';
314+
String get href => '${canonicalLibrary.dirName}/$fileName';
314315

315316
/// Returns all the implementors of the class specified.
316317
List<Class> get implementors =>
@@ -663,7 +664,7 @@ class Constructor extends ModelElement
663664

664665
@override
665666
String get href =>
666-
'${library.dirName}/${_constructor.enclosingElement.name}/$name.html';
667+
'${canonicalLibrary.dirName}/${_constructor.enclosingElement.name}/$name.html';
667668

668669
@override
669670
bool get isConst => _constructor.isConst;
@@ -790,7 +791,7 @@ class EnumField extends Field {
790791

791792
@override
792793
String get href =>
793-
'${library.dirName}/${(enclosingElement as Class).fileName}';
794+
'${canonicalLibrary.dirName}/${(enclosingElement as Class).fileName}';
794795

795796
@override
796797
String get linkedName => name;
@@ -844,12 +845,21 @@ class Field extends ModelElement
844845
@override
845846
bool get hasSetter => _field.setter != null;
846847

848+
@override
849+
Library get canonicalLibrary {
850+
if (isInherited) {
851+
return enclosingElement.canonicalLibrary;
852+
} else {
853+
return super.canonicalLibrary;
854+
}
855+
}
856+
847857
@override
848858
String get href {
849859
if (enclosingElement is Class) {
850-
return '${library.dirName}/${enclosingElement.name}/$_fileName';
860+
return '${canonicalLibrary.dirName}/${enclosingElement.name}/$_fileName';
851861
} else if (enclosingElement is Library) {
852-
return '${library.dirName}/$_fileName';
862+
return '${canonicalLibrary.dirName}/$_fileName';
853863
} else {
854864
throw new StateError(
855865
'$name is not in a class or library, instead it is a ${enclosingElement.element}');
@@ -1284,6 +1294,15 @@ class Method extends ModelElement
12841294
}).toList();
12851295
}
12861296

1297+
@override
1298+
Library get canonicalLibrary {
1299+
if (isInherited) {
1300+
return enclosingElement.canonicalLibrary;
1301+
} else {
1302+
return super.canonicalLibrary;
1303+
}
1304+
}
1305+
12871306
@override
12881307
ModelElement get enclosingElement {
12891308
if (_enclosingClass == null) {
@@ -1301,7 +1320,7 @@ class Method extends ModelElement
13011320
}
13021321

13031322
@override
1304-
String get href => '${library.dirName}/${enclosingElement.name}/${fileName}';
1323+
String get href => '${canonicalLibrary.dirName}/${enclosingElement.name}/${fileName}';
13051324

13061325
bool get isInherited => _isInherited;
13071326

@@ -1456,6 +1475,17 @@ abstract class ModelElement implements Comparable, Nameable, Documentable {
14561475
return _rawDocs;
14571476
}
14581477

1478+
Library _canonicalLibrary;
1479+
Library get canonicalLibrary {
1480+
if (_canonicalLibrary == null) {
1481+
final libraryElement = package.exportGraph.canonicalLibraryElement(element.library) ?? library.element;
1482+
_canonicalLibrary = package.libraries.firstWhere((l) => l.element == libraryElement, orElse: () => library);
1483+
}
1484+
return _canonicalLibrary;
1485+
}
1486+
1487+
bool get isCanonical => library == canonicalLibrary;
1488+
14591489
@override
14601490
String get documentationAsHtml => _documentation.asHtml;
14611491

@@ -1897,7 +1927,7 @@ class ModelFunction extends ModelElement
18971927
String get fileName => "$name.html";
18981928

18991929
@override
1900-
String get href => '${library.dirName}/$fileName';
1930+
String get href => '${canonicalLibrary.dirName}/$fileName';
19011931

19021932
@override
19031933
bool get isStatic => _func.isStatic;
@@ -2137,6 +2167,11 @@ class Package implements Nameable, Documentable {
21372167
}
21382168
return _allModelElements;
21392169
}
2170+
2171+
ExportGraph _exportGraph;
2172+
ExportGraph get exportGraph {
2173+
return (_exportGraph ??= new ExportGraph(libraries.map((l) => l.element as LibraryElement)));
2174+
}
21402175
}
21412176

21422177
class PackageCategory implements Comparable<PackageCategory> {
@@ -2178,13 +2213,13 @@ class Parameter extends ModelElement implements EnclosedElement {
21782213
var p = _parameter.enclosingElement;
21792214

21802215
if (p is FunctionElement) {
2181-
return '${library.dirName}/${p.name}.html';
2216+
return '${canonicalLibrary.dirName}/${p.name}.html';
21822217
} else {
21832218
// TODO: why is this logic here?
21842219
var name = Operator.friendlyNames.containsKey(p.name)
21852220
? Operator.friendlyNames[p.name]
21862221
: p.name;
2187-
return '${library.dirName}/${p.enclosingElement.name}/' +
2222+
return '${canonicalLibrary.dirName}/${p.enclosingElement.name}/' +
21882223
'${name}.html#${htmlId}';
21892224
}
21902225
}
@@ -2353,7 +2388,7 @@ class TopLevelVariable extends ModelElement
23532388
bool get hasSetter => _variable.setter != null;
23542389

23552390
@override
2356-
String get href => '${library.dirName}/$_fileName';
2391+
String get href => '${canonicalLibrary.dirName}/$_fileName';
23572392

23582393
@override
23592394
bool get isConst => _variable.isConst;
@@ -2404,7 +2439,7 @@ class Typedef extends ModelElement
24042439
String get fileName => '$name.html';
24052440

24062441
@override
2407-
String get href => '${library.dirName}/$fileName';
2442+
String get href => '${canonicalLibrary.dirName}/$fileName';
24082443

24092444
@override
24102445
String get kind => 'typedef';
@@ -2435,7 +2470,7 @@ class TypeParameter extends ModelElement {
24352470

24362471
@override
24372472
String get href =>
2438-
'${library.dirName}/${_typeParameter.enclosingElement.name}/$name';
2473+
'${canonicalLibrary.dirName}/${_typeParameter.enclosingElement.name}/$name';
24392474

24402475
@override
24412476
String get kind => 'type parameter';

‎test/model_test.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@ void main() {
636636
.firstWhere((m) => m.name == 'returnCool', orElse: () => null);
637637
expect(returnCool, isNotNull);
638638
expect(returnCool.linkedReturnType,
639-
equals('<a href="ex/Cool-class.html">Cool</a>'));
639+
equals('<a href="fake/Cool-class.html">Cool</a>'));
640640
});
641641

642642
test('F has a single instance method', () {

‎testing/test_package_docs/ex/Animal-class.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ <h5><a href="ex/ex-library.html">ex</a></h5>
108108
<li><a href="ex/Cat-class.html">Cat</a></li>
109109
<li><a href="ex/CatString-class.html">CatString</a></li>
110110
<li><a href="ex/ConstantCat-class.html">ConstantCat</a></li>
111-
<li><a href="ex/Cool-class.html">Cool</a></li>
111+
<li><a href="fake/Cool-class.html">Cool</a></li>
112112
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
113113
<li><a href="ex/Dog-class.html">Dog</a></li>
114114
<li><a href="ex/E-class.html">E</a></li>

‎testing/test_package_docs/ex/Apple-class.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ <h5><a href="ex/ex-library.html">ex</a></h5>
112112
<li><a href="ex/Cat-class.html">Cat</a></li>
113113
<li><a href="ex/CatString-class.html">CatString</a></li>
114114
<li><a href="ex/ConstantCat-class.html">ConstantCat</a></li>
115-
<li><a href="ex/Cool-class.html">Cool</a></li>
115+
<li><a href="fake/Cool-class.html">Cool</a></li>
116116
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
117117
<li><a href="ex/Dog-class.html">Dog</a></li>
118118
<li><a href="ex/E-class.html">E</a></li>

‎testing/test_package_docs/ex/B-class.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ <h5><a href="ex/ex-library.html">ex</a></h5>
110110
<li><a href="ex/Cat-class.html">Cat</a></li>
111111
<li><a href="ex/CatString-class.html">CatString</a></li>
112112
<li><a href="ex/ConstantCat-class.html">ConstantCat</a></li>
113-
<li><a href="ex/Cool-class.html">Cool</a></li>
113+
<li><a href="fake/Cool-class.html">Cool</a></li>
114114
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
115115
<li><a href="ex/Dog-class.html">Dog</a></li>
116116
<li><a href="ex/E-class.html">E</a></li>

‎testing/test_package_docs/ex/COLOR-constant.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ <h5><a href="ex/ex-library.html">ex</a></h5>
103103
<li><a href="ex/Cat-class.html">Cat</a></li>
104104
<li><a href="ex/CatString-class.html">CatString</a></li>
105105
<li><a href="ex/ConstantCat-class.html">ConstantCat</a></li>
106-
<li><a href="ex/Cool-class.html">Cool</a></li>
106+
<li><a href="fake/Cool-class.html">Cool</a></li>
107107
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
108108
<li><a href="ex/Dog-class.html">Dog</a></li>
109109
<li><a href="ex/E-class.html">E</a></li>

‎testing/test_package_docs/ex/COLOR_GREEN-constant.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ <h5><a href="ex/ex-library.html">ex</a></h5>
103103
<li><a href="ex/Cat-class.html">Cat</a></li>
104104
<li><a href="ex/CatString-class.html">CatString</a></li>
105105
<li><a href="ex/ConstantCat-class.html">ConstantCat</a></li>
106-
<li><a href="ex/Cool-class.html">Cool</a></li>
106+
<li><a href="fake/Cool-class.html">Cool</a></li>
107107
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
108108
<li><a href="ex/Dog-class.html">Dog</a></li>
109109
<li><a href="ex/E-class.html">E</a></li>

‎testing/test_package_docs/ex/COLOR_ORANGE-constant.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ <h5><a href="ex/ex-library.html">ex</a></h5>
103103
<li><a href="ex/Cat-class.html">Cat</a></li>
104104
<li><a href="ex/CatString-class.html">CatString</a></li>
105105
<li><a href="ex/ConstantCat-class.html">ConstantCat</a></li>
106-
<li><a href="ex/Cool-class.html">Cool</a></li>
106+
<li><a href="fake/Cool-class.html">Cool</a></li>
107107
<li><a href="ex/Deprecated-class.html">Deprecated</a></li>
108108
<li><a href="ex/Dog-class.html">Dog</a></li>
109109
<li><a href="ex/E-class.html">E</a></li>

0 commit comments

Comments
 (0)