Skip to content

Commit 0ad9830

Browse files
authored
Fix issue where excluded packages can sometimes default to remote linking (#2387)
* remote linking flutter bug * Fix dartfmt and class count test * Remove workaround for #1431
1 parent 8f349b7 commit 0ad9830

File tree

8 files changed

+13
-23
lines changed

8 files changed

+13
-23
lines changed

lib/src/model/class.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,12 +179,12 @@ class Class extends Container
179179
var result = <Class>{};
180180
var seen = <Class>{};
181181

182-
// Recursively adds [implementor] if public, or the impelentors of
182+
// Recursively adds [implementor] if public, or the implementors of
183183
// [implementor] if not.
184184
void addToResult(Class implementor) {
185185
if (seen.contains(implementor)) return;
186186
seen.add(implementor);
187-
if (implementor.isPublic) {
187+
if (implementor.isPublicAndPackageDocumented) {
188188
result.add(implementor);
189189
} else {
190190
model_utils

lib/src/model/model_element.dart

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,8 +1036,7 @@ abstract class ModelElement extends Canonicalization
10361036
@override
10371037
Package get package => library?.package;
10381038

1039-
bool get isPublicAndPackageDocumented =>
1040-
isPublic && library.packageGraph.packageDocumentedFor(this);
1039+
bool get isPublicAndPackageDocumented => isPublic && package.isDocumented;
10411040

10421041
List<Parameter> _allParameters;
10431042

lib/src/model/package.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,10 @@ class Package extends LibraryContainer
188188
_documentedWhere = DocumentLocation.local;
189189
}
190190
} else {
191-
if (config.linkToRemote && config.linkToUrl.isNotEmpty && isPublic) {
191+
if (config.linkToRemote &&
192+
config.linkToUrl.isNotEmpty &&
193+
isPublic &&
194+
!packageGraph.config.isPackageExcluded(name)) {
192195
_documentedWhere = DocumentLocation.remote;
193196
} else {
194197
_documentedWhere = DocumentLocation.missing;

lib/src/model/package_graph.dart

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -277,20 +277,6 @@ class PackageGraph {
277277
bool allLibrariesAdded = false;
278278
bool _localDocumentationBuilt = false;
279279

280-
Set<String> _allRootDirs;
281-
282-
/// Returns true if there's at least one library documented in the package
283-
/// that has the same package path as the library for the given element.
284-
///
285-
/// Usable as a cross-check for dartdoc's canonicalization to generate
286-
/// warnings for ModelElement.isPublicAndPackageDocumented.
287-
bool packageDocumentedFor(ModelElement element) {
288-
_allRootDirs ??= {
289-
...(publicLibraries.map((l) => l.packageMeta?.resolvedDir))
290-
};
291-
return _allRootDirs.contains(element.library.packageMeta?.resolvedDir);
292-
}
293-
294280
PackageWarningCounter get packageWarningCounter => _packageWarningCounter;
295281

296282
final Set<Tuple3<Element, PackageWarning, String>> _warnAlreadySeen = {};

test/end2end/dartdoc_test.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,12 @@ void main() {
206206
});
207207

208208
test('basic interlinking test', () async {
209-
var dartdoc = await buildDartdoc([], _testPackageDir, tempDir);
209+
var dartdoc = await buildDartdoc(
210+
['--exclude-packages=args'], _testPackageDir, tempDir);
210211
var results = await dartdoc.generateDocs();
211212
var p = results.packageGraph;
212213
var meta = p.publicPackages.firstWhere((p) => p.name == 'meta');
214+
var args = p.publicPackages.firstWhere((p) => p.name == 'args');
213215
var useSomethingInAnotherPackage = p.publicLibraries
214216
.firstWhere((l) => l.name == 'fake')
215217
.properties
@@ -219,6 +221,7 @@ void main() {
219221
.properties
220222
.firstWhere((p) => p.name == 'useSomethingInTheSdk');
221223
expect(meta.documentedWhere, equals(DocumentLocation.remote));
224+
expect(args.documentedWhere, equals(DocumentLocation.missing));
222225
expect(
223226
useSomethingInAnotherPackage.modelType.linkedName,
224227
matches(

test/end2end/model_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1513,7 +1513,7 @@ void main() {
15131513
});
15141514

15151515
test('correctly finds all the classes', () {
1516-
expect(classes, hasLength(37));
1516+
expect(classes, hasLength(38));
15171517
});
15181518

15191519
test('abstract', () {

testing/test_package/lib/example.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'dart:math';
88
import 'src/mylib.dart' show Helper;
99
import 'package:test_package_imported/main.dart';
1010

11+
export 'package:args/args.dart' show ArgParser;
1112
export 'dart:core' show deprecated, Deprecated;
1213
import 'package:meta/meta.dart' show protected, factory;
1314

tool/grind.dart

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,8 +1106,6 @@ Future<WarningsCollection> _buildDartdocFlutterPluginDocs() async {
11061106
[
11071107
'--enable-asserts',
11081108
path.join(Directory.current.path, 'bin', 'dartdoc.dart'),
1109-
'--exclude-packages',
1110-
'Dart', // TODO(jcollins-g): dart-lang/dartdoc#1431
11111109
'--json',
11121110
'--link-to-remote',
11131111
'--output',

0 commit comments

Comments
 (0)