Macro. Code optimizer. Update import directives.

Change-Id: I5599f243fd4ddcda046934b1584612b209d69815
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/350406
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>

pkg/_fe_analyzer_shared/lib/src/macros/code_optimizer.dart

@@ -55,18 +55,29 @@ abstract class CodeOptimizer {
 
     void walkScopes(_Scope scope) {
       for (_PrefixedName prefixedName in scope.prefixedNames) {
+        String prefix = prefixedName.prefix.token.lexeme;
         String name = prefixedName.name.token.lexeme;
         _NameStatus resolution = scope.resolve(name);
         if (resolution is _NameStatusImported) {
           if (resolution.imports.length == 1) {
             int prefixOffset = prefixedName.prefix.token.offset;
             edits.add(
-              new Edit(
+              new RemoveImportPrefixReferenceEdit(
                 offset: prefixOffset,
                 length: prefixedName.name.token.offset - prefixOffset,
-                replacement: '',
               ),
             );
+            resolution.imports.single.namesWithoutPrefix.add(name);
+          } else {
+            for (_Import import in resolution.imports) {
+              import.namesWithPrefix.add(name);
+            }
+          }
+        } else {
+          for (_Import import in listener.importScope.imports) {
+            if (import.prefix == prefix) {
+              import.namesWithPrefix.add(name);
+            }
           }
         }
       }
@@ -77,12 +88,64 @@ abstract class CodeOptimizer {
 
     walkScopes(listener.importScope);
 
-    edits.sort((a, b) => b.offset - a.offset);
+    Token? firstDeclarationToken = listener.libraryScope.firstDeclarationToken;
+    if (firstDeclarationToken == null) {
+      return [];
+    }
+
+    List<_Import> imports = listener.importScope.imports;
+    for (int i = 0; i < imports.length; i++) {
+      _Import import = imports[i];
+
+      // This should not happen in production.
+      // We use such "unused" import for tests.
+      if (import.namesWithoutPrefix.isEmpty) {
+        continue;
+      }
+
+      if (import.namesWithPrefix.isEmpty) {
+        // If no names require the prefix, remove it from the directive.
+        Token? toToken = i < imports.length - 1
+            ? imports[i + 1].importKeyword
+            : firstDeclarationToken;
+        if (import.uriStr == 'dart:core') {
+          int fromOffset = import.importKeyword.offset;
+          edits.add(
+            new RemoveDartCoreImportEdit(
+              offset: fromOffset,
+              length: toToken.offset - fromOffset,
+            ),
+          );
+        } else {
+          int fromOffset = import.uriToken.end;
+          edits.add(
+            new RemoveImportPrefixDeclarationEdit(
+              offset: import.uriToken.end,
+              length: import.semicolon.offset - fromOffset,
+            ),
+          );
+        }
+      } else if (import.namesWithoutPrefix.isNotEmpty) {
+        // If some names require the prefix, and some not, add a new import
+        // without a prefix, but hide those which require prefix.
+        List<String> namesToHide = import.namesWithPrefix.toList();
+        namesToHide.sort();
+        edits.add(
+          new ImportWithoutPrefixEdit(
+            offset: import.semicolon.end,
+            uriStr: import.uriStr,
+            namesToHide: namesToHide,
+          ),
+        );
+      }
+    }
+
+    edits.sort((a, b) => a.offset - b.offset);
     return edits;
   }
 }
 
-class Edit {
+sealed class Edit {
   final int offset;
   final int length;
   final String replacement;
@@ -103,6 +166,48 @@ class Edit {
   }
 }
 
+final class ImportWithoutPrefixEdit extends Edit {
+  final String uriStr;
+  final List<String> namesToHide;
+
+  ImportWithoutPrefixEdit({
+    required super.offset,
+    required this.uriStr,
+    required this.namesToHide,
+  }) : super(
+          length: 0,
+          replacement: '\nimport \'$uriStr\' hide ${namesToHide.join(', ')};',
+        );
+}
+
+final class RemoveDartCoreImportEdit extends RemoveEdit {
+  RemoveDartCoreImportEdit({
+    required super.offset,
+    required super.length,
+  });
+}
+
+sealed class RemoveEdit extends Edit {
+  RemoveEdit({
+    required super.offset,
+    required super.length,
+  }) : super(replacement: '');
+}
+
+final class RemoveImportPrefixDeclarationEdit extends RemoveEdit {
+  RemoveImportPrefixDeclarationEdit({
+    required super.offset,
+    required super.length,
+  });
+}
+
+final class RemoveImportPrefixReferenceEdit extends RemoveEdit {
+  RemoveImportPrefixReferenceEdit({
+    required super.offset,
+    required super.length,
+  });
+}
+
 class _ExtensionNoName {
   const _ExtensionNoName();
 }
@@ -119,14 +224,26 @@ class _Identifier {
 }
 
 class _Import {
+  final Token importKeyword;
+  final Token uriToken;
   final String uriStr;
   final String prefix;
   final Set<String> names;
+  final Token semicolon;
+
+  /// Names that are used with [prefix], but can be used without it.
+  final Set<String> namesWithoutPrefix = {};
+
+  /// Names that are used with [prefix], and the prefix cannot be removed.
+  final Set<String> namesWithPrefix = {};
 
   _Import({
+    required this.importKeyword,
+    required this.uriToken,
     required this.uriStr,
     required this.prefix,
     required this.names,
+    required this.semicolon,
   });
 }
 
@@ -169,6 +286,8 @@ class _InterpolationString {
 class _LibraryScope extends _NestedScope {
   final Set<String> globalNames = {};
 
+  Token? firstDeclarationToken;
+
   _LibraryScope({
     required super.parent,
   });
@@ -200,6 +319,8 @@ class _Listener extends Listener {
 
   @override
   void beginClassOrMixinOrNamedMixinApplicationPrelude(Token token) {
+    // TODO(scheglov): Not quite, ignores metadata and comments.
+    libraryScope.firstDeclarationToken ??= token;
     _scopeEnter();
   }
 
@@ -353,7 +474,8 @@ class _Listener extends Listener {
     _ImportPrefix prefix = pop() as _ImportPrefix;
     _StringLiteral uri = pop() as _StringLiteral;
 
-    String uriStr = uri.token.lexeme;
+    Token uriToken = uri.token;
+    String uriStr = uriToken.lexeme;
     if (uriStr.startsWith('\'') && uriStr.endsWith('\'')) {
       uriStr = uriStr.substring(1, uriStr.length - 1);
     } else {
@@ -362,8 +484,11 @@ class _Listener extends Listener {
 
     importScope.imports.add(
       new _Import(
+        importKeyword: importKeyword,
+        uriToken: uriToken,
         uriStr: uriStr,
         prefix: prefix.name.token.lexeme,
+        semicolon: semicolon!,
         names: getImportedNames(uriStr),
       ),
     );

pkg/_fe_analyzer_shared/test/macros/code_optimizer_test.dart

@@ -10,8 +10,8 @@ void main() {
   group('Class |', () {
     group('Method |', () {
       group('Return type |', () {
-        group('Not shadowed', () {
-          test('Simple', () {
+        group('Not shadowed |', () {
+          test('Last import, dart:core', () {
             assertEdits(code: r'''
 import 'dart:core' as prefix0;
 
@@ -19,16 +19,60 @@ class A {
   prefix0.String foo() {}
 }
 ''', expected: r'''
-44 +8 |prefix0.| -> ||
+RemoveDartCoreImportEdit
+  0 +32 |import 'dart:core' as prefix0;\n\n|
+RemoveImportPrefixReferenceEdit
+  44 +8 |prefix0.|
 ----------------
-import 'dart:core' as prefix0;
-
 class A {
   String foo() {}
 }
 ''');
           });
 
+          test('Last import, dart:math', () {
+            assertEdits(code: r'''
+import 'dart:math' as prefix0;
+
+class A {
+  prefix0.Random foo() {}
+}
+''', expected: r'''
+RemoveImportPrefixDeclarationEdit
+  18 +11 | as prefix0|
+RemoveImportPrefixReferenceEdit
+  44 +8 |prefix0.|
+----------------
+import 'dart:math';
+
+class A {
+  Random foo() {}
+}
+''');
+          });
+
+          test('First import, dart:math', () {
+            assertEdits(code: r'''
+import 'dart:core' as prefix0;
+import 'dart:math' as prefix1;
+
+class A {
+  prefix0.int foo() {}
+}
+''', expected: r'''
+RemoveDartCoreImportEdit
+  0 +31 |import 'dart:core' as prefix0;\n|
+RemoveImportPrefixReferenceEdit
+  75 +8 |prefix0.|
+----------------
+import 'dart:math' as prefix1;
+
+class A {
+  int foo() {}
+}
+''');
+          });
+
           test('Other class type parameter', () {
             assertEdits(code: r'''
 import 'dart:core' as prefix0;
@@ -39,10 +83,11 @@ class A {
 
 class B<String> {}
 ''', expected: r'''
-44 +8 |prefix0.| -> ||
+RemoveDartCoreImportEdit
+  0 +32 |import 'dart:core' as prefix0;\n\n|
+RemoveImportPrefixReferenceEdit
+  44 +8 |prefix0.|
 ----------------
-import 'dart:core' as prefix0;
-
 class A {
   String foo() {}
 }
@@ -63,10 +108,11 @@ class B {
   void bar<String>() {}
 }
 ''', expected: r'''
-44 +8 |prefix0.| -> ||
+RemoveDartCoreImportEdit
+  0 +32 |import 'dart:core' as prefix0;\n\n|
+RemoveImportPrefixReferenceEdit
+  44 +8 |prefix0.|
 ----------------
-import 'dart:core' as prefix0;
-
 class A {
   String foo() {}
 }
@@ -86,10 +132,11 @@ class A {
   void bar(String) {}
 }
 ''', expected: r'''
-44 +8 |prefix0.| -> ||
+RemoveDartCoreImportEdit
+  0 +32 |import 'dart:core' as prefix0;\n\n|
+RemoveImportPrefixReferenceEdit
+  44 +8 |prefix0.|
 ----------------
-import 'dart:core' as prefix0;
-
 class A {
   String foo() {}
   void bar(String) {}
@@ -106,10 +153,11 @@ class A {
   void bar<String>() {}
 }
 ''', expected: r'''
-44 +8 |prefix0.| -> ||
+RemoveDartCoreImportEdit
+  0 +32 |import 'dart:core' as prefix0;\n\n|
+RemoveImportPrefixReferenceEdit
+  44 +8 |prefix0.|
 ----------------
-import 'dart:core' as prefix0;
-
 class A {
   String foo() {}
   void bar<String>() {}
@@ -126,10 +174,11 @@ class A {
   prefix0.String foo() {}
 }
 ''', expected: r'''
-58 +8 |prefix0.| -> ||
+RemoveDartCoreImportEdit
+  0 +32 |import 'dart:core' as prefix0;\n\n|
+RemoveImportPrefixReferenceEdit
+  58 +8 |prefix0.|
 ----------------
-import 'dart:core' as prefix0;
-
 class A {
   A.String();
   String foo() {}
@@ -147,10 +196,11 @@ class A {
 
 enum X<String> { v }
 ''', expected: r'''
-44 +8 |prefix0.| -> ||
+RemoveDartCoreImportEdit
+  0 +32 |import 'dart:core' as prefix0;\n\n|
+RemoveImportPrefixReferenceEdit
+  44 +8 |prefix0.|
 ----------------
-import 'dart:core' as prefix0;
-
 class A {
   String foo() {}
 }
@@ -169,10 +219,11 @@ class A {
 
 extension X<String> on A {}
 ''', expected: r'''
-44 +8 |prefix0.| -> ||
+RemoveDartCoreImportEdit
+  0 +32 |import 'dart:core' as prefix0;\n\n|
+RemoveImportPrefixReferenceEdit
+  44 +8 |prefix0.|
 ----------------
-import 'dart:core' as prefix0;
-
 class A {
   String foo() {}
 }
@@ -191,10 +242,11 @@ class A {
 
 extension type X<String>(A it) {}
 ''', expected: r'''
-44 +8 |prefix0.| -> ||
+RemoveDartCoreImportEdit
+  0 +32 |import 'dart:core' as prefix0;\n\n|
+RemoveImportPrefixReferenceEdit
+  44 +8 |prefix0.|
 ----------------
-import 'dart:core' as prefix0;
-
 class A {
   String foo() {}
 }
@@ -213,10 +265,11 @@ class A {
 
 typedef F<String> = void Function();
 ''', expected: r'''
-44 +8 |prefix0.| -> ||
+RemoveDartCoreImportEdit
+  0 +32 |import 'dart:core' as prefix0;\n\n|
+RemoveImportPrefixReferenceEdit
+  44 +8 |prefix0.|
 ----------------
-import 'dart:core' as prefix0;
-
 class A {
   String foo() {}
 }
@@ -458,16 +511,54 @@ class A {
   prefix0.String foo() {}
   int String = 0;
 }
+''');
+          });
+
+          test('Partial 1/3', () {
+            assertEdits(code: r'''
+import 'dart:core' as prefix0;
+
+class A {
+  prefix0.bool foo1() {}
+  prefix0.int foo2() {}
+  prefix0.String foo3() {}
+}
+
+class String {}
+''', expected: r'''
+ImportWithoutPrefixEdit
+  30 |\nimport 'dart:core' hide String;|
+RemoveImportPrefixReferenceEdit
+  44 +8 |prefix0.|
+RemoveImportPrefixReferenceEdit
+  69 +8 |prefix0.|
+----------------
+import 'dart:core' as prefix0;
+import 'dart:core' hide String;
+
+class A {
+  bool foo1() {}
+  int foo2() {}
+  prefix0.String foo3() {}
+}
+
+class String {}
 ''');
           });
         });
       });
     });
   });
+
+  test('Update expectations', () {
+    // import '../../../analyzer/test/src/dart/resolution/node_text_expectations.dart';
+    // NodeTextExpectationsCollector.apply();
+  });
 }
 
 const _dartImports = {
   'dart:core': {'bool', 'double', 'int', 'String'},
+  'dart:math': {'Random'},
 };
 
 void assertEdits({
@@ -496,15 +587,33 @@ void assertEdits({
   );
 
   var buffer = StringBuffer();
-  for (var edit in edits) {
-    buffer.write('${edit.offset} +${edit.length}');
 
-    var toReplace = code.substring(edit.offset, edit.offset + edit.length);
-    buffer.write(' |${escape(toReplace)}|');
-
-    buffer.writeln(' -> |${escape(edit.replacement)}|');
+  void writeRemoveEdit(RemoveEdit edit) {
+    buffer.write('  ${edit.offset} +${edit.length}');
+    var removed = code.substring(edit.offset, edit.offset + edit.length);
+    buffer.writeln(' |${escape(removed)}|');
   }
 
+  for (var edit in edits) {
+    switch (edit) {
+      case RemoveDartCoreImportEdit():
+        buffer.writeln('RemoveDartCoreImportEdit');
+        writeRemoveEdit(edit);
+      case RemoveImportPrefixDeclarationEdit():
+        buffer.writeln('RemoveImportPrefixDeclarationEdit');
+        writeRemoveEdit(edit);
+      case RemoveImportPrefixReferenceEdit():
+        buffer.writeln('RemoveImportPrefixReferenceEdit');
+        writeRemoveEdit(edit);
+      case ImportWithoutPrefixEdit():
+        buffer.writeln('ImportWithoutPrefixEdit');
+        buffer.write('  ${edit.offset}');
+        buffer.writeln(' |${escape(edit.replacement)}|');
+    }
+  }
+
+  // Apply in reverse order.
+  edits = edits.reversed.toList();
   var optimized = Edit.applyList(edits, code);
   buffer.writeln('-' * 16);
   buffer.write(optimized);
@@ -513,6 +622,7 @@ void assertEdits({
   if (actual != expected) {
     print('-------- Actual --------');
     print('$actual------------------------');
+    // NodeTextExpectationsCollector.add(actual);
     fail('Not as expected');
   }
 }

pkg/analyzer/test/src/dart/resolution/node_text_expectations.dart

@@ -22,6 +22,10 @@ class NodeTextExpectationsCollector {
   static const updatingIsEnabled = false;
 
   static final assertMethods = [
+    _AssertMethod.forFunction(
+      methodName: 'assertEdits',
+      argument: _ArgumentNamed('expected'),
+    ),
     _AssertMethod(
       className: 'AnalysisContextCollectionTest',
       methodName: '_assertWorkspaceCollectionText',
@@ -187,7 +191,7 @@ class NodeTextExpectationsCollector {
     }
   }
 
-  static void _apply() {
+  static void apply() {
     for (final file in _files.values) {
       file.applyReplacements();
     }
@@ -202,7 +206,7 @@ class NodeTextExpectationsCollector {
 @reflectiveTest
 class UpdateNodeTextExpectations {
   test_applyReplacements() {
-    NodeTextExpectationsCollector._apply();
+    NodeTextExpectationsCollector.apply();
   }
 }
 
@@ -239,17 +243,20 @@ final class _ArgumentNamed extends _Argument {
 }
 
 class _AssertMethod {
-  final String className;
   final String methodName;
+  final String stackTracePattern;
   final _Argument argument;
 
   const _AssertMethod({
-    required this.className,
+    required String className,
     required this.methodName,
     required this.argument,
-  });
+  }) : stackTracePattern = '$className.$methodName';
 
-  String get stackTracePattern => '$className.$methodName';
+  const _AssertMethod.forFunction({
+    required this.methodName,
+    required this.argument,
+  }) : stackTracePattern = ' $methodName';
 }
 
 class _File {