Fix LSP Organize Imports so removing unused imports works

Using only a parsed file meant we didn't get any "unused import" hints, which meant the unused imports were not removed. Change-Id: Id5c24ac59f38885d08756747ef9165102ad54388 Reviewed-on: https://dart-review.googlesource.com/c/86884 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Commit-Queue: Danny Tuppeny <dantup@google.com>
dart-lang · Dec 11, 2018 · b924d75 · b924d75
1 parent 7d0823a
commit b924d75
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 30 deletions.
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/commands/organize_imports.dart b/pkg/analysis_server/lib/src/lsp/handlers/commands/organize_imports.dart
@@ -15,7 +15,7 @@ class OrganizeImportsCommandHandler extends SimpleEditCommandHandler {
   OrganizeImportsCommandHandler(LspAnalysisServer server) : super(server);
 
   @override
-  String get commandName => 'Sort Members';
+  String get commandName => 'Organize Imports';
 
   @override
   Future<ErrorOr<void>> handle(List<dynamic> arguments) async {
@@ -33,29 +33,23 @@ class OrganizeImportsCommandHandler extends SimpleEditCommandHandler {
     final path = arguments.single;
     final docIdentifier = server.getVersionedDocumentIdentifier(path);
 
-    var driver = server.getAnalysisDriver(path);
-    final result = await driver?.parseFile(path);
-    if (result == null) {
-      return ErrorOr.error(new ResponseError(
-        ServerErrorCodes.FileNotAnalyzed,
-        '$commandName is only available for analyzed files',
-        null,
-      ));
-    }
-    final code = result.content;
-    final unit = result.unit;
+    final result = await requireUnit(path);
+    return result.mapResult((result) {
+      final code = result.content;
+      final unit = result.unit;
 
-    if (hasScanParseErrors(result)) {
-      return ErrorOr.error(new ResponseError(
-        ServerErrorCodes.FileHasErrors,
-        'Unable to $commandName because the file contains parse errors',
-        path,
-      ));
-    }
+      if (hasScanParseErrors(result.errors)) {
+        return ErrorOr.error(new ResponseError(
+          ServerErrorCodes.FileHasErrors,
+          'Unable to $commandName because the file contains parse errors',
+          path,
+        ));
+      }
 
-    final sorter = new DirectiveOrganizer(code, unit, result.errors);
-    final edits = sorter.organize();
+      final organizer = new DirectiveOrganizer(code, unit, result.errors);
+      final edits = organizer.organize();
 
-    return await sendEditsToClient(docIdentifier, unit, edits);
+      return sendEditsToClient(docIdentifier, unit, edits);
+    });
   }
 }
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/commands/simple_edit_handler.dart b/pkg/analysis_server/lib/src/lsp/handlers/commands/simple_edit_handler.dart
@@ -8,7 +8,6 @@ import 'package:analysis_server/src/lsp/constants.dart';
 import 'package:analysis_server/src/lsp/handlers/handlers.dart';
 import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
 import 'package:analysis_server/src/lsp/mapping.dart';
-import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/error/error.dart' as engine;
 import 'package:analyzer/src/dart/scanner/scanner.dart' as engine;
@@ -71,8 +70,8 @@ abstract class SimpleEditCommandHandler
     }
   }
 
-  bool hasScanParseErrors(ParsedUnitResult result) {
-    return result.errors.any((error) =>
+  bool hasScanParseErrors(List<engine.AnalysisError> errors) {
+    return errors.any((error) =>
         error.errorCode is engine.ScannerErrorCode ||
         error.errorCode is engine.ParserErrorCode);
   }

diff --git a/pkg/analysis_server/lib/src/lsp/handlers/commands/sort_members.dart b/pkg/analysis_server/lib/src/lsp/handlers/commands/sort_members.dart
@@ -45,7 +45,7 @@ class SortMembersCommandHandler extends SimpleEditCommandHandler {
     final code = result.content;
     final unit = result.unit;
 
-    if (hasScanParseErrors(result)) {
+    if (hasScanParseErrors(result.errors)) {
       return ErrorOr.error(new ResponseError(
         ServerErrorCodes.FileHasErrors,
         'Unable to $commandName because the file contains parse errors',

diff --git a/pkg/analysis_server/test/lsp/code_actions_source_test.dart b/pkg/analysis_server/test/lsp/code_actions_source_test.dart
@@ -22,12 +22,19 @@ main() {
 class OrganizeImportsSourceCodeActionsTest extends SourceCodeActionsTest {
   test_appliesCorrectEdits_withDocumentChangesSupport() async {
     const content = '''
-import 'dart:convert';
+import 'dart:math';
 import 'dart:async';
+import 'dart:convert';
+
+Future foo;
+int minified(int x, int y) => min(x, y);
     ''';
     const expectedContent = '''
 import 'dart:async';
-import 'dart:convert';
+import 'dart:math';
+
+Future foo;
+int minified(int x, int y) => min(x, y);
     ''';
     await newFile(mainFilePath, content: content);
     await initializeWithDocumentChangesSupport();
@@ -72,12 +79,19 @@ import 'dart:convert';
 
   test_appliesCorrectEdits_withoutDocumentChangesSupport() async {
     const content = '''
-import 'dart:convert';
+import 'dart:math';
 import 'dart:async';
+import 'dart:convert';
+
+Future foo;
+int minified(int x, int y) => min(x, y);
     ''';
     const expectedContent = '''
 import 'dart:async';
-import 'dart:convert';
+import 'dart:math';
+
+Future foo;
+int minified(int x, int y) => min(x, y);
     ''';
     await newFile(mainFilePath, content: content);
     await initialize();