linter: support doc-directives in directives_ordering
Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try
Change-Id: Ieb3b072a3fbd8d56751c29a39c979807f07cd2c6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/376821
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
diff --git a/pkg/linter/lib/src/rules/directives_ordering.dart b/pkg/linter/lib/src/rules/directives_ordering.dart
index 9ee40fd..c544430 100644
--- a/pkg/linter/lib/src/rules/directives_ordering.dart
+++ b/pkg/linter/lib/src/rules/directives_ordering.dart
@@ -108,13 +108,15 @@
```
''';
+const _docImportKeyword = '@docImport';
+
const _exportKeyword = 'export';
const _importKeyword = 'import';
-/// Compares directives by package then file in package.
+/// Compares directives by package name, then file name in the package.
///
-/// Package is everything until the first `/`.
+/// The package name is everything until the first '/'.
int compareDirectives(String a, String b) {
if (!a.startsWith('package:') || !b.startsWith('package:')) {
if (!a.startsWith('/') && !b.startsWith('/')) {
@@ -217,23 +219,6 @@
}
}
-// ignore: unused_element
-class _PackageBox {
- final String _packageName;
-
- _PackageBox(this._packageName);
-
- // ignore: unused_element
- bool _isNotOwnPackageDirective(NamespaceDirective node) =>
- _isPackageDirective(node) && !_isOwnPackageDirective(node);
-
- bool _isOwnPackageDirective(NamespaceDirective node) {
- var uriContent = node.uri.stringValue;
- return uriContent != null &&
- uriContent.startsWith('package:$_packageName/');
- }
-}
-
class _Visitor extends SimpleAstVisitor<void> {
final DirectivesOrdering rule;
@@ -250,45 +235,45 @@
void _checkDartDirectiveGoFirst(
Set<AstNode> lintedNodes, CompilationUnit node) {
- void reportImport(NamespaceDirective directive) {
- if (lintedNodes.add(directive)) {
- rule._reportLintWithDartDirectiveGoFirstMessage(
- directive, _importKeyword);
+ for (var import in node.importDirectives.withDartUrisSkippingTheFirstSet) {
+ if (lintedNodes.add(import)) {
+ rule._reportLintWithDartDirectiveGoFirstMessage(import, _importKeyword);
}
}
- void reportExport(NamespaceDirective directive) {
- if (lintedNodes.add(directive)) {
- rule._reportLintWithDartDirectiveGoFirstMessage(
- directive, _exportKeyword);
+ for (var export in node.exportDirectives.withDartUrisSkippingTheFirstSet) {
+ if (lintedNodes.add(export)) {
+ rule._reportLintWithDartDirectiveGoFirstMessage(export, _exportKeyword);
}
}
- Iterable<NamespaceDirective> getNodesToLint(
- Iterable<NamespaceDirective> directives) =>
- directives.skipWhile(_isDartDirective).where(_isDartDirective);
-
- getNodesToLint(_getImportDirectives(node)).forEach(reportImport);
-
- getNodesToLint(_getExportDirectives(node)).forEach(reportExport);
+ for (var import
+ in node.docImportDirectives.withDartUrisSkippingTheFirstSet) {
+ if (lintedNodes.add(import)) {
+ rule._reportLintWithDartDirectiveGoFirstMessage(
+ import, _docImportKeyword);
+ }
+ }
}
void _checkDirectiveSectionOrderedAlphabetically(
Set<AstNode> lintedNodes, CompilationUnit node) {
- var importDirectives = _getImportDirectives(node);
- var exportDirectives = _getExportDirectives(node);
-
- var dartImports = importDirectives.where(_isDartDirective);
- var dartExports = exportDirectives.where(_isDartDirective);
-
- var relativeImports = importDirectives.where(_isRelativeDirective);
- var relativeExports = exportDirectives.where(_isRelativeDirective);
+ var dartImports = node.importDirectives.where(_isDartDirective);
+ var dartExports = node.exportDirectives.where(_isDartDirective);
+ var dartDocImports = node.docImportDirectives.where(_isDartDirective);
_checkSectionInOrder(lintedNodes, dartImports);
_checkSectionInOrder(lintedNodes, dartExports);
+ _checkSectionInOrder(lintedNodes, dartDocImports);
+
+ var relativeImports = node.importDirectives.where(_isRelativeDirective);
+ var relativeExports = node.exportDirectives.where(_isRelativeDirective);
+ var relativeDocImports =
+ node.docImportDirectives.where(_isRelativeDirective);
_checkSectionInOrder(lintedNodes, relativeImports);
_checkSectionInOrder(lintedNodes, relativeExports);
+ _checkSectionInOrder(lintedNodes, relativeDocImports);
// See: https://github.com/dart-lang/linter/issues/3395
// (`DartProject` removal)
@@ -297,11 +282,13 @@
// will have ecosystem impact.
// Not a pub package. Package directives should be sorted in one block.
- var packageImports = importDirectives.where(_isPackageDirective);
- var packageExports = exportDirectives.where(_isPackageDirective);
+ var packageImports = node.importDirectives.where(_isPackageDirective);
+ var packageExports = node.exportDirectives.where(_isPackageDirective);
+ var packageDocImports = node.docImportDirectives.where(_isPackageDirective);
_checkSectionInOrder(lintedNodes, packageImports);
_checkSectionInOrder(lintedNodes, packageExports);
+ _checkSectionInOrder(lintedNodes, packageDocImports);
// The following is relying on projectName which is meant to come from
// a `DartProject` instance (but was not since the project was always null)
@@ -328,75 +315,90 @@
void _checkExportDirectiveAfterImportDirective(
Set<AstNode> lintedNodes, CompilationUnit node) {
- void reportDirective(Directive directive) {
+ for (var directive in node.directives.reversed
+ .skipWhile(_isPartDirective)
+ .skipWhile(_isExportDirective)
+ .where(_isExportDirective)) {
if (lintedNodes.add(directive)) {
rule._reportLintWithExportDirectiveAfterImportDirectiveMessage(
directive);
}
}
-
- node.directives.reversed
- .skipWhile(_isPartDirective)
- .skipWhile(_isExportDirective)
- .where(_isExportDirective)
- .forEach(reportDirective);
}
void _checkPackageDirectiveBeforeRelative(
Set<AstNode> lintedNodes, CompilationUnit node) {
- void reportImport(NamespaceDirective directive) {
- if (lintedNodes.add(directive)) {
+ for (var import
+ in node.importDirectives.withPackageUrisSkippingAbsoluteUris) {
+ if (lintedNodes.add(import)) {
rule._reportLintWithPackageDirectiveBeforeRelativeMessage(
- directive, _importKeyword);
+ import, _importKeyword);
}
}
- void reportExport(NamespaceDirective directive) {
- if (lintedNodes.add(directive)) {
+ for (var export
+ in node.exportDirectives.withPackageUrisSkippingAbsoluteUris) {
+ if (lintedNodes.add(export)) {
rule._reportLintWithPackageDirectiveBeforeRelativeMessage(
- directive, _exportKeyword);
+ export, _exportKeyword);
}
}
- Iterable<NamespaceDirective> getNodesToLint(
- Iterable<NamespaceDirective> directives) =>
- directives
- .where(_isNotDartDirective)
- .skipWhile(_isAbsoluteDirective)
- .where(_isPackageDirective);
-
- getNodesToLint(_getImportDirectives(node)).forEach(reportImport);
-
- getNodesToLint(_getExportDirectives(node)).forEach(reportExport);
+ for (var import
+ in node.docImportDirectives.withPackageUrisSkippingAbsoluteUris) {
+ if (lintedNodes.add(import)) {
+ rule._reportLintWithPackageDirectiveBeforeRelativeMessage(
+ import, _docImportKeyword);
+ }
+ }
}
void _checkSectionInOrder(
Set<AstNode> lintedNodes, Iterable<NamespaceDirective> nodes) {
- void reportDirective(NamespaceDirective directive) {
- if (lintedNodes.add(directive)) {
- rule._reportLintWithDirectiveSectionOrderedAlphabeticallyMessage(
- directive);
- }
- }
+ if (nodes.isEmpty) return;
- NamespaceDirective? previousDirective;
- for (var directive in nodes) {
- if (previousDirective != null) {
- var previousUri = previousDirective.uri.stringValue;
- var directiveUri = directive.uri.stringValue;
- if (previousUri != null &&
- directiveUri != null &&
- compareDirectives(previousUri, directiveUri) > 0) {
- reportDirective(directive);
+ var previousUri = nodes.first.uri.stringValue;
+ for (var directive in nodes.skip(1)) {
+ var directiveUri = directive.uri.stringValue;
+ if (previousUri != null &&
+ directiveUri != null &&
+ compareDirectives(previousUri, directiveUri) > 0) {
+ if (lintedNodes.add(directive)) {
+ rule._reportLintWithDirectiveSectionOrderedAlphabeticallyMessage(
+ directive);
}
}
- previousDirective = directive;
+ previousUri = directive.uri.stringValue;
}
}
+}
- Iterable<ExportDirective> _getExportDirectives(CompilationUnit node) =>
- node.directives.whereType<ExportDirective>();
+extension on CompilationUnit {
+ Iterable<ImportDirective> get docImportDirectives {
+ var libraryDirective = directives.whereType<LibraryDirective>().firstOrNull;
+ if (libraryDirective == null) return const [];
+ var docComment = libraryDirective.documentationComment;
+ if (docComment == null) return const [];
+ return docComment.docImports.map((e) => e.import);
+ }
- Iterable<ImportDirective> _getImportDirectives(CompilationUnit node) =>
- node.directives.whereType<ImportDirective>();
+ Iterable<ExportDirective> get exportDirectives =>
+ directives.whereType<ExportDirective>();
+
+ Iterable<ImportDirective> get importDirectives =>
+ directives.whereType<ImportDirective>();
+}
+
+extension on Iterable<NamespaceDirective> {
+ /// The directives with 'dart:' URIs, skipping the first such set of
+ /// directives.
+ Iterable<NamespaceDirective> get withDartUrisSkippingTheFirstSet =>
+ skipWhile(_isDartDirective).where(_isDartDirective);
+
+ /// The directives with 'package:' URIs, after the first set of directives
+ /// with absolute URIs.
+ Iterable<NamespaceDirective> get withPackageUrisSkippingAbsoluteUris =>
+ where(_isNotDartDirective)
+ .skipWhile(_isAbsoluteDirective)
+ .where(_isPackageDirective);
}
diff --git a/pkg/linter/test/rules/directives_ordering_test.dart b/pkg/linter/test/rules/directives_ordering_test.dart
index 26ef477..aefad87 100644
--- a/pkg/linter/test/rules/directives_ordering_test.dart
+++ b/pkg/linter/test/rules/directives_ordering_test.dart
@@ -26,6 +26,20 @@
@override
String get lintRule => 'directives_ordering';
+ test_dartDirectivesGoFirst_docImports() async {
+ newFile('$testPackageLibPath/a.dart', '');
+ await assertDiagnostics(r'''
+/// @docImport 'dart:math';
+/// @docImport 'a.dart';
+/// @docImport 'dart:html';
+/// @docImport 'dart:isolate';
+library;
+''', [
+ lint(61, 19),
+ lint(89, 22),
+ ]);
+ }
+
test_dartDirectivesGoFirst_exports() async {
newFile('$testPackageLibPath/a.dart', '');
await assertDiagnostics(r'''
@@ -117,6 +131,21 @@
''');
}
+ test_packageDirectivesGoBeforeRelative_docImports() async {
+ newFile('$testPackageLibPath/a.dart', '');
+ newFile('$testPackageLibPath/b.dart', '');
+ await assertDiagnostics(r'''
+/// @docImport 'dart:math';
+/// @docImport 'package:js/js.dart';
+/// @docImport 'a.dart';
+/// @docImport 'package:meta/meta.dart';
+/// @docImport 'b.dart';
+library;
+''', [
+ lint(98, 32),
+ ]);
+ }
+
test_packageDirectivesGoBeforeRelative_exports() async {
newFile('$testPackageLibPath/a.dart', '');
newFile('$testPackageLibPath/b.dart', '');
@@ -179,6 +208,18 @@
]);
}
+ test_sortDirectiveSectionsAlphabetically_dartSchema_docImport() async {
+ await assertDiagnostics(r'''
+/// @docImport 'dart:html';
+/// @docImport 'dart:isolate';
+/// @docImport 'dart:convert';
+/// @docImport 'dart:math';
+library;
+''', [
+ lint(67, 22),
+ ]);
+ }
+
test_sortDirectiveSectionsAlphabetically_dartSchema_export() async {
await assertDiagnostics(r'''
export 'dart:isolate';
