Skip to content

Commit

Permalink
Do not sort own package imports differently from other package imports
Browse files Browse the repository at this point in the history
- I would rather follow Effective Dart guidelines. directives_ordering
no longer separates own package from other package. See
https://dart.dev/guides/language/effective-dart/style#ordering
- prefer_relative_imports seems like a better alternative for packages
that wish to separate own package imports
https://dart.dev/guides/language/effective-dart/usage#prefer-relative-import-paths
  • Loading branch information
regenvanwalbeek-wf committed Jul 14, 2022
1 parent 4b4d9d8 commit 806b81d
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 79 deletions.
3 changes: 1 addition & 2 deletions lib/src/tools/import_cleaner_tool.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import '../utils/import_cleaner/import_cleaner.dart';
class ImportCleanerTool extends DevTool {
List<Glob> directoriesToInclude = [];
int _exitCode = 0;
String packageName;

@override
final ArgParser argParser = ArgParser(allowTrailingOptions: true)
Expand Down Expand Up @@ -133,7 +132,7 @@ class ImportCleanerTool extends DevTool {

String _safelyCleanImports(String fileContents) {
try {
return cleanImports(fileContents, currentPackageName: this.packageName);
return cleanImports(fileContents);
} on ArgumentError {
return null;
}
Expand Down
25 changes: 3 additions & 22 deletions lib/src/utils/import_cleaner/import.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ class Import {
/// The AST node that represents an import in a file.
final ImportDirective directive;

/// The library running the cleaner
final String package_name;

/// Comments that appear before the import that should stay with the import when sorted.
List<Token> beforeComments = [];

Expand All @@ -32,32 +29,16 @@ class Import {
/// If the import is an external package import. Memoized for performance.
bool _isExternalPkgImport;
bool get isExternalPkgImport {
return _isExternalPkgImport ??=
target.startsWith('package:') && !_checkIfCurrentImport(target);
return _isExternalPkgImport ??= target.startsWith('package:');
}

/// If the import is a relative import. Memoized for performance.
bool _isRelativeImport;
bool get isRelativeImport {
return _isRelativeImport ??=
!isExternalPkgImport && !isDartImport && !isCurrentPackageImport;
}

bool _checkIfCurrentImport(String target) {
if (package_name == null) {
return false;
}

return target.contains('package:$package_name');
}

/// If the import is a current package import. Memoized for performance.
bool _isCurrentPackageImport;
bool get isCurrentPackageImport {
return _isCurrentPackageImport ??= _checkIfCurrentImport(target);
return _isRelativeImport ??= !isExternalPkgImport && !isDartImport;
}

Import(this.directive, {this.package_name});
Import(this.directive);

/// The character offset of the end of this import in source text (includes comments associated with this import).
int end() {
Expand Down
27 changes: 4 additions & 23 deletions lib/src/utils/import_cleaner/import_cleaner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@ import 'import_collector.dart';
/// Takes in a file as a string and cleans up the imports. Sorts imports and removes double quotes.
///
/// Throws an ArgumentError if [sourceFileContents] cannot be parsed.
String cleanImports(String sourceFileContents, {String currentPackageName}) {
final imports = parseString(content: sourceFileContents)
.unit
.accept(ImportCollector(currentPackageName: currentPackageName));
String cleanImports(String sourceFileContents) {
final imports =
parseString(content: sourceFileContents).unit.accept(ImportCollector());
if (imports.isEmpty) {
return sourceFileContents;
}
Expand Down Expand Up @@ -92,14 +91,11 @@ String _getSortedImportString(String sourceFileContents, List<Import> imports) {
imports.indexWhere((import) => import.isRelativeImport);
final firstPkgImportIdx =
imports.indexWhere((import) => import.isExternalPkgImport);
final firstCurrentPkgImportIdx =
imports.indexWhere((import) => import.isCurrentPackageImport);
for (var importIndex = 0; importIndex < imports.length; importIndex++) {
final import = imports[importIndex];
if (importIndex != 0 &&
(importIndex == firstRelativeImportIdx ||
importIndex == firstPkgImportIdx ||
importIndex == firstCurrentPkgImportIdx)) {
importIndex == firstPkgImportIdx)) {
sortedReplacement.write('\n');
}

Expand Down Expand Up @@ -140,21 +136,6 @@ int _importComparator(Import first, Import second) {
return 1;
}

// Check for name
final firstIsCurrent = first.isCurrentPackageImport;
final secondIsCurrent = second.isCurrentPackageImport;
if (firstIsCurrent && secondIsCurrent) {
return first.target.compareTo(second.target);
}

if (firstIsCurrent && !secondIsCurrent) {
return -1;
}

if (!firstIsCurrent && secondIsCurrent) {
return 1;
}

// Neither are dart imports or pkg imports. Must be relative path imports...
return first.target.compareTo(second.target);
}
5 changes: 1 addition & 4 deletions lib/src/utils/import_cleaner/import_collector.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,10 @@ import 'import.dart';
/// Imports are returned in the order they appear in a file when passed to `CompilationUnit.accept`.
class ImportCollector extends SimpleAstVisitor<List<Import>> {
final List<Import> _imports = [];
final String currentPackageName;

ImportCollector({this.currentPackageName});

@override
List<Import> visitImportDirective(ImportDirective node) {
_imports.add(Import(node, package_name: this.currentPackageName));
_imports.add(Import(node));
return null;
}

Expand Down
32 changes: 4 additions & 28 deletions test/utils/import_cleaner/import_cleaner_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,15 @@ void main() {
_TestCase('22. empty file', uncleanImports22, cleanImports22),
_TestCase('23. no imports', uncleanImports23, cleanImports23),
_TestCase('24. comments between imports', uncleanImports24, cleanImports24),
_TestCase('25. sorts current package imports correctly', uncleanImports25,
cleanImports25,
packageName: 'current_package'),
];

group('cleanImports', () {
for (final testCase in testCases) {
test(testCase.description, () {
expect(
cleanImports(testCase.unclean,
currentPackageName: testCase.packageName),
equals(testCase.clean));
cleanImports(testCase.unclean),
equals(testCase.clean),
);
});
}
});
Expand All @@ -70,10 +67,8 @@ class _TestCase {
final String description;
final String unclean;
final String clean;
final String packageName;

const _TestCase(this.description, this.unclean, this.clean,
{this.packageName});
const _TestCase(this.description, this.unclean, this.clean);
}

const uncleanImports1 = '''
Expand Down Expand Up @@ -788,22 +783,3 @@ void main() {
// content
}
''';

const cleanImports25 = '''
import 'dart:a.dart';
import 'dart:b.dart';
import 'package:args/dart_stuff.dart';
import 'package:glob/dart_stuff.dart';
import 'package:current_package/a.dart';
import 'dart_async_mocks.dart';
import 'dart_sync_mocks.dart';
import 'package_things_a.dart';
import 'package_things_b.dart';
void main() {
// content
}
''';

0 comments on commit 806b81d

Please sign in to comment.