Skip to content

Commit 68d0506

Browse files
srawlinsCommit Queue
authored andcommitted
analyzer: Extract out two common utilities for URIs
* Uri.isSamePackageAs(Uri) and Uri.isImplementation are each used a few times. Not as much as I would expect, but this still simplifies some code. Change-Id: Ie9020916c793852a0df9d04154420948d942d951 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/413801 Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Paul Berry <[email protected]>
1 parent af8729c commit 68d0506

File tree

11 files changed

+128
-103
lines changed

11 files changed

+128
-103
lines changed

pkg/analysis_server/lib/src/services/correction/dart/import_library.dart

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ import 'package:analyzer/source/source_range.dart';
2121
import 'package:analyzer/src/dart/ast/extensions.dart';
2222
import 'package:analyzer/src/dart/element/type.dart';
2323
import 'package:analyzer/src/dart/resolver/applicable_extensions.dart';
24+
import 'package:analyzer/utilities/extensions/uri.dart';
2425
import 'package:analyzer_plugin/src/utilities/change_builder/change_builder_dart.dart';
25-
import 'package:analyzer_plugin/src/utilities/library.dart';
2626
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
2727
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
2828
import 'package:analyzer_plugin/utilities/range_factory.dart';
@@ -369,8 +369,7 @@ class ImportLibrary extends MultiCorrectionProducer {
369369
}
370370
// If both files are in the same package's 'lib' folder, also include a
371371
// relative import.
372-
var includeRelativeUri = canBeRelativeImport(
373-
librarySource.uri,
372+
var includeRelativeUri = librarySource.uri.isSamePackageAs(
374373
libraryElement2.uri,
375374
);
376375
// Add the fix(es).
@@ -388,6 +387,10 @@ class ImportLibrary extends MultiCorrectionProducer {
388387
return producers;
389388
}
390389

390+
/// Whether [path] appears to be a package-implementation source file.
391+
///
392+
/// Note that this is approximate; without knowing the containing package's
393+
/// location, this can give false positives.
391394
bool _isLibSrcPath(String path) {
392395
var parts = resourceProvider.pathContext.split(path);
393396
for (var i = 0; i < parts.length - 2; i++) {

pkg/analyzer/api.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5352,6 +5352,10 @@ package:analyzer/src/task/api/model.dart:
53525352
source (getter: Source?)
53535353
package:analyzer/src/workspace/workspace.dart:
53545354
Workspace (non-public)
5355+
package:analyzer/utilities/extensions/uri.dart:
5356+
UriExtension (extension on Uri):
5357+
isImplementation (getter: bool)
5358+
isSamePackageAs (method: bool Function(Uri))
53555359
dart:async:
53565360
Future (referenced)
53575361
Stream (referenced)
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
extension UriExtension on Uri {
6+
/// Whether this URI represents a path in a package's private "implementation"
7+
/// directory.
8+
bool get isImplementation =>
9+
(isScheme('package') || !hasAbsolutePath) &&
10+
pathSegments.length > 2 &&
11+
pathSegments[1] == 'src';
12+
13+
/// Whether this URI and [other] are each 'package:' URIs referencing the same
14+
/// package name.
15+
bool isSamePackageAs(Uri other) {
16+
return isScheme('package') &&
17+
other.isScheme('package') &&
18+
pathSegments.isNotEmpty &&
19+
other.pathSegments.isNotEmpty &&
20+
pathSegments.first == other.pathSegments.first;
21+
}
22+
}

pkg/analyzer/test/test_all.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import 'generated/test_all.dart' as generated;
1111
import 'instrumentation/test_all.dart' as instrumentation;
1212
import 'source/test_all.dart' as source;
1313
import 'src/test_all.dart' as src;
14+
import 'utilities/test_all.dart' as utilities;
1415
import 'verify_diagnostics_test.dart' as verify_diagnostics;
1516
import 'verify_docs_test.dart' as verify_docs;
1617
import 'verify_tests_test.dart' as verify_tests;
@@ -24,6 +25,7 @@ main() {
2425
instrumentation.main();
2526
source.main();
2627
src.main();
28+
utilities.main();
2729
verify_diagnostics.main();
2830
verify_docs.main();
2931
verify_tests.main();
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:test_reflective_loader/test_reflective_loader.dart';
6+
7+
import 'uri_test.dart' as uri;
8+
9+
main() {
10+
defineReflectiveSuite(() {
11+
uri.main();
12+
}, name: 'extensions');
13+
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:analyzer/utilities/extensions/uri.dart';
6+
import 'package:test/test.dart';
7+
import 'package:test_reflective_loader/test_reflective_loader.dart';
8+
9+
main() {
10+
defineReflectiveSuite(() {
11+
defineReflectiveTests(UriExtensionTest);
12+
});
13+
}
14+
15+
@reflectiveTest
16+
class UriExtensionTest {
17+
void test_isImplementation_packageScheme_insideSrc() {
18+
expect(Uri.parse('package:foo/src/foo.dart').isImplementation, isTrue);
19+
}
20+
21+
void test_isImplementation_packageScheme_outsideSrc() {
22+
expect(Uri.parse('package:foo/foo.dart').isImplementation, isFalse);
23+
}
24+
25+
void test_isImplementation_relative() {
26+
expect(Uri.parse('foo.dart').isImplementation, isFalse);
27+
}
28+
29+
void test_samePackage_nonPackageScheme() {
30+
expect(
31+
Uri.parse('file://foo/foo.dart')
32+
.isSamePackageAs(Uri.parse('package:foo/bar.dart')),
33+
isFalse,
34+
);
35+
}
36+
37+
void test_samePackage_packageScheme_notSame() {
38+
expect(
39+
Uri.parse('package:foo/foo.dart')
40+
.isSamePackageAs(Uri.parse('package:bar/bar.dart')),
41+
isFalse,
42+
);
43+
}
44+
45+
void test_samePackage_packageScheme_same() {
46+
expect(
47+
Uri.parse('package:foo/foo.dart')
48+
.isSamePackageAs(Uri.parse('package:foo/bar.dart')),
49+
isTrue,
50+
);
51+
}
52+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:test_reflective_loader/test_reflective_loader.dart';
6+
7+
import 'extensions/test_all.dart' as extensions;
8+
9+
main() {
10+
defineReflectiveSuite(() {
11+
extensions.main();
12+
}, name: 'utilities');
13+
}

pkg/analyzer_plugin/lib/src/utilities/library.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88
/// In other words, returns whether each is a 'package:' URI referencing the
99
/// same package name.
1010
bool canBeRelativeImport(Uri library1, Uri library2) {
11+
// TODO(srawlins): Replace this with `isSamePackageAs` from
12+
// 'package:analyzer/src/utilities/extensions/uri.dart', once that is
13+
// available on pub.
1114
return library1.isScheme('package') &&
1215
library2.isScheme('package') &&
1316
library1.pathSegments.isNotEmpty &&

pkg/linter/lib/src/rules/implementation_imports.dart

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,35 +5,12 @@
55
import 'package:analyzer/dart/ast/ast.dart';
66
import 'package:analyzer/dart/ast/visitor.dart';
77
import 'package:analyzer/dart/element/element2.dart';
8+
import 'package:analyzer/utilities/extensions/uri.dart';
89

910
import '../analyzer.dart';
1011

1112
const _desc = r"Don't import implementation files from another package.";
1213

13-
bool isImplementation(Uri? uri) {
14-
var segments = uri?.pathSegments ?? const <String>[];
15-
if (segments.length > 2) {
16-
if (segments[1] == 'src') {
17-
return true;
18-
}
19-
}
20-
return false;
21-
}
22-
23-
bool isPackage(Uri? uri) => uri?.scheme == 'package';
24-
25-
bool samePackage(Uri? uri1, Uri? uri2) {
26-
if (uri1 == null || uri2 == null) {
27-
return false;
28-
}
29-
var segments1 = uri1.pathSegments;
30-
var segments2 = uri2.pathSegments;
31-
if (segments1.isEmpty || segments2.isEmpty) {
32-
return false;
33-
}
34-
return segments1.first == segments2.first;
35-
}
36-
3714
class ImplementationImports extends LintRule {
3815
ImplementationImports()
3916
: super(name: LintNames.implementation_imports, description: _desc);
@@ -49,8 +26,8 @@ class ImplementationImports extends LintRule {
4926
var libraryUri = context.libraryElement2?.uri;
5027
if (libraryUri == null) return;
5128

52-
// If the source URI is not a `package` URI bail out.
53-
if (!isPackage(libraryUri)) return;
29+
// If the source URI is not a `package` URI, bail out.
30+
if (libraryUri.scheme != 'package') return;
5431

5532
var visitor = _Visitor(this, libraryUri);
5633
registry.addImportDirective(this, visitor);
@@ -65,16 +42,15 @@ class _Visitor extends SimpleAstVisitor<void> {
6542

6643
@override
6744
void visitImportDirective(ImportDirective node) {
68-
Uri? importUri;
6945
if (node.libraryImport?.uri case DirectiveUriWithSource importedLibrary) {
70-
importUri = importedLibrary.source.uri;
71-
}
46+
var importUri = importedLibrary.source.uri;
7247

73-
// Test for 'package:*/src/'.
74-
if (!isImplementation(importUri)) return;
48+
// Test for 'package:*/src/'.
49+
if (!importUri.isImplementation) return;
7550

76-
if (!samePackage(importUri, sourceUri)) {
77-
rule.reportLint(node.uri);
51+
if (!importUri.isSamePackageAs(sourceUri)) {
52+
rule.reportLint(node.uri);
53+
}
7854
}
7955
}
8056
}

pkg/linter/lib/src/rules/prefer_relative_imports.dart

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@
55
import 'package:analyzer/dart/ast/ast.dart';
66
import 'package:analyzer/dart/ast/visitor.dart';
77
import 'package:analyzer/dart/element/element2.dart';
8+
import 'package:analyzer/utilities/extensions/uri.dart';
89
import 'package:path/path.dart' as path;
910

1011
import '../analyzer.dart';
11-
import 'implementation_imports.dart' show samePackage;
1212

1313
const _desc = r'Prefer relative imports for files in `lib/`.';
1414

@@ -51,9 +51,10 @@ class _Visitor extends SimpleAstVisitor<void> {
5151
var importUri = importedLibrary.relativeUri;
5252
if (!importUri.isScheme('package')) return false;
5353

54-
if (!samePackage(importUri, sourceUri)) return false;
54+
if (!importUri.isSamePackageAs(sourceUri)) return false;
5555

56-
// TODO(pq): context.package.contains(source) should work (but does not)
56+
// TODO(pq): `context.package.contains(source)` should work (but does
57+
// not).
5758
var packageRoot = context.package?.root;
5859
return packageRoot != null &&
5960
path.isWithin(packageRoot, importedLibrary.source.fullName);

0 commit comments

Comments
 (0)