dart-lang / sdk

The Dart SDK, including the VM, dart2js, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
9.94k stars 1.53k forks source link

DartFixKind.ORGANIZE_IMPORTS disagrees with LintNames.directives_ordering #43927

Open kevmoo opened 3 years ago

kevmoo commented 3 years ago

tl;dr

LintNames.directives_ordering (which I agree with) prefers code like this

import 'travis.dart';
import 'travis/lib.dart';

DartFixKind.ORGANIZE_IMPORTS orders code like this:

import 'travis/lib.dart';
import 'travis.dart';

I hacked up the test which I would like to pass. 😄

diff --git a/pkg/analysis_server/test/src/services/correction/fix/organize_imports_test.dart b/pkg/analysis_server/test/src/services/correction/fix/organize_imports_test.dart
index 868dbabae2f..a8d90b20c97 100644
--- a/pkg/analysis_server/test/src/services/correction/fix/organize_imports_test.dart
+++ b/pkg/analysis_server/test/src/services/correction/fix/organize_imports_test.dart
@@ -4,6 +4,7 @@

 import 'package:analysis_server/src/services/correction/fix.dart';
 import 'package:analysis_server/src/services/linter/lint_names.dart';
+import 'package:analyzer/src/error/codes.dart';
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';

@@ -40,4 +41,24 @@ import 'dart:io';
 void main(Stream<String> args) { }
 ''');
   }
+
+  Future<void> test_organizeImports2() async {
+    await resolveTestUnit('''
+import 'travis/lib.dart';
+
+import 'travis.dart';
+
+void main(Stream<String> args) { }
+''');
+    await assertHasFix(
+      '''
+import 'travis.dart';
+import 'travis/lib.dart';
+
+void main(Stream<String> args) { }
+''',
+      errorFilter: (input) =>
+          input.errorCode != CompileTimeErrorCode.URI_DOES_NOT_EXIST,
+    );
+  }
 }

CC @jwren @scheglov

kevmoo commented 3 years ago

CC @pq re LINT

bwilkerson commented 3 years ago

Just some history: The quick fix uses the ImportOrganizer, which also implements the "Sort Members in Dart File" feature in IDEs. The lint was written long after ImportOrganizer was and disagrees with it. IIRC, the Flutter team has yet another way of sorting imports that doesn't match either of the pre-existing orders.

While I strongly agree that it would be nice for everything to be consistent, there's an open question as to which pieces should be changed and to what.