[cfe,kernel] Add AstPrinter

This CL adds a Node.toText method together with an AstPrinter. These
facility and better toString implementation on AST nodes while allowing
for toString independent printing of AST to use in testing. This also
add support for an integrated toString of custom/internal nodes.

Some work is still needed in bringing the toString implementation on
all nodes to the old quality, and not all internal nodes have
customized textual representations yet. This work is left for future
CLs.

Change-Id: Ib0bf8a0bc02f489dfacdc8aa5f96da9c52f26058
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/150923
Commit-Queue: Johnni Winther <[email protected]>
Reviewed-by: Alexander Markov <[email protected]>
Reviewed-by: Jens Johansen <[email protected]>

Author: johnniwinther

pkg/_fe_analyzer_shared/lib/src/testing/id.dart

@@ -125,7 +125,9 @@ class IdValue {
   static const String stmtPrefix = "stmt: ";
   static const String errorPrefix = "error: ";
 
-  static IdValue decode(Uri sourceUri, Annotation annotation, String text) {
+  static IdValue decode(Uri sourceUri, Annotation annotation, String text,
+      {bool preserveWhitespaceInAnnotations: false,
+      bool preserveInfixWhitespace: false}) {
     int offset = annotation.offset;
     Id id;
     String expected;
@@ -182,8 +184,15 @@ class IdValue {
       id = new NodeId(offset, IdKind.node);
       expected = text;
     }
-    // Remove newlines.
-    expected = expected.replaceAll(new RegExp(r'\s*(\n\s*)+\s*'), '');
+    if (preserveWhitespaceInAnnotations) {
+      // Keep all whitespace.
+    } else if (preserveInfixWhitespace) {
+      // Remove heading and trailing whitespace.
+      expected = expected.trim();
+    } else {
+      // Remove unneeded whitespace.
+      expected = expected.replaceAll(new RegExp(r'\s*(\n\s*)+\s*'), '');
+    }
     return new IdValue(id, annotation, expected);
   }
 }

pkg/_fe_analyzer_shared/lib/src/testing/id_testing.dart

@@ -219,7 +219,9 @@ class MemberAnnotations<DataType> {
 // TODO(johnniwinther): Support an empty marker set.
 void computeExpectedMap(Uri sourceUri, String filename, AnnotatedCode code,
     Map<String, MemberAnnotations<IdValue>> maps,
-    {void onFailure(String message)}) {
+    {void onFailure(String message),
+    bool preserveWhitespaceInAnnotations: false,
+    bool preserveInfixWhitespaceInAnnotations: false}) {
   List<String> mapKeys = maps.keys.toList();
   Map<String, AnnotatedCode> split = splitByPrefixes(code, mapKeys);
 
@@ -229,7 +231,9 @@ void computeExpectedMap(Uri sourceUri, String filename, AnnotatedCode code,
     Map<Id, IdValue> expectedValues = fileAnnotations[sourceUri];
     for (Annotation annotation in code.annotations) {
       String text = annotation.text;
-      IdValue idValue = IdValue.decode(sourceUri, annotation, text);
+      IdValue idValue = IdValue.decode(sourceUri, annotation, text,
+          preserveWhitespaceInAnnotations: preserveWhitespaceInAnnotations,
+          preserveInfixWhitespace: preserveInfixWhitespaceInAnnotations);
       if (idValue.id.isGlobal) {
         if (fileAnnotations.globalData.containsKey(idValue.id)) {
           onFailure("Error in test '$filename': "
@@ -259,7 +263,9 @@ void computeExpectedMap(Uri sourceUri, String filename, AnnotatedCode code,
 TestData computeTestData(FileSystemEntity testFile,
     {Iterable<String> supportedMarkers,
     Uri createTestUri(Uri uri, String fileName),
-    void onFailure(String message)}) {
+    void onFailure(String message),
+    bool preserveWhitespaceInAnnotations: false,
+    bool preserveInfixWhitespaceInAnnotations: false}) {
   Uri entryPoint;
 
   String testName;
@@ -300,7 +306,10 @@ TestData computeTestData(FileSystemEntity testFile,
   }
   computeExpectedMap(entryPoint, testFile.uri.pathSegments.last,
       code[entryPoint], expectedMaps,
-      onFailure: onFailure);
+      onFailure: onFailure,
+      preserveWhitespaceInAnnotations: preserveWhitespaceInAnnotations,
+      preserveInfixWhitespaceInAnnotations:
+          preserveInfixWhitespaceInAnnotations);
   Map<String, String> memorySourceFiles = {
     entryPoint.path: code[entryPoint].sourceCode
   };
@@ -317,7 +326,10 @@ TestData computeTestData(FileSystemEntity testFile,
       code[libFileUri] = annotatedLibCode;
       computeExpectedMap(
           libFileUri, libFileName, annotatedLibCode, expectedMaps,
-          onFailure: onFailure);
+          onFailure: onFailure,
+          preserveWhitespaceInAnnotations: preserveWhitespaceInAnnotations,
+          preserveInfixWhitespaceInAnnotations:
+              preserveInfixWhitespaceInAnnotations);
     }
   }
 
@@ -713,7 +725,9 @@ Future<void> runTests<T>(Directory dataDir,
     void onFailure(String message),
     RunTestFunction<T> runTest,
     List<String> skipList,
-    Map<String, List<String>> skipMap}) async {
+    Map<String, List<String>> skipMap,
+    bool preserveWhitespaceInAnnotations: false,
+    bool preserveInfixWhitespaceInAnnotations: false}) async {
   MarkerOptions markerOptions =
       new MarkerOptions.fromDataDir(dataDir, shouldFindScript: shards == 1);
   // TODO(johnniwinther): Support --show to show actual data for an input.
@@ -775,7 +789,10 @@ Future<void> runTests<T>(Directory dataDir,
     TestData testData = computeTestData(entity,
         supportedMarkers: markerOptions.supportedMarkers,
         createTestUri: createTestUri,
-        onFailure: onFailure);
+        onFailure: onFailure,
+        preserveWhitespaceInAnnotations: preserveWhitespaceInAnnotations,
+        preserveInfixWhitespaceInAnnotations:
+            preserveInfixWhitespaceInAnnotations);
     print('Test: ${testData.testFileUri}');
 
     Map<String, TestResult<T>> results = await runTest(testData,

pkg/compiler/lib/src/ir/closure.dart

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:kernel/ast.dart' as ir;
+import 'package:kernel/src/printer.dart' as ir;
 import 'package:kernel/text/ast_to_text.dart' as ir show debugNodeToString;
 
 /// Collection of scope data collected for a single member.
@@ -419,6 +420,12 @@ class TypeVariableTypeWithContext implements ir.Node {
       'type=${type.toStringInternal()},context=${context.toStringInternal()},'
       'kind=$kind,typeDeclaration=${typeDeclaration.toStringInternal()}';
 
+  @override
+  String toText(ir.AstTextStrategy strategy) => type.toText(strategy);
+
+  @override
+  void toTextInternal(ir.AstPrinter printer) => type.toTextInternal(printer);
+
   @override
   String leakingDebugToString() => ir.debugNodeToString(this);
 }

pkg/compiler/lib/src/ir/constants.dart

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:kernel/ast.dart' as ir;
+import 'package:kernel/src/printer.dart' as ir;
 import 'package:kernel/type_environment.dart' as ir;
 import 'package:front_end/src/api_prototype/constant_evaluator.dart' as ir;
 import 'package:front_end/src/api_unstable/dart2js.dart' as ir;
@@ -204,4 +205,11 @@ class ConstantReference extends ir.TreeNode {
 
   @override
   String toStringInternal() => 'constant=${constant.toStringInternal()}';
+
+  @override
+  String toText(ir.AstTextStrategy strategy) => constant.toText(strategy);
+
+  @override
+  void toTextInternal(ir.AstPrinter printer) =>
+      constant.toTextInternal(printer);
 }

pkg/dev_compiler/test/nullable_inference_test.dart

@@ -9,6 +9,7 @@ import 'package:kernel/core_types.dart';
 import 'package:kernel/kernel.dart';
 import 'package:kernel/class_hierarchy.dart';
 import 'package:kernel/type_environment.dart';
+import 'package:kernel/src/printer.dart';
 import 'package:kernel/target/targets.dart';
 import 'package:test/test.dart';
 
@@ -17,6 +18,11 @@ import 'package:dev_compiler/src/kernel/nullable_inference.dart';
 import 'package:dev_compiler/src/kernel/js_typerep.dart';
 import 'package:dev_compiler/src/kernel/target.dart';
 
+const AstTextStrategy astTextStrategy = AstTextStrategy(
+    includeLibraryNamesInTypes: true,
+    includeLibraryNamesInMembers: true,
+    useMultiline: false);
+
 void main() {
   test('empty main', () async {
     await expectNotNull('main() {}', '');
@@ -72,7 +78,7 @@ void main() {
 
   test('constructor', () async {
     await expectNotNull(
-        'library a; class C {} main() { new C(); }', 'new a::C::•()');
+        'library a; class C {} main() { new C(); }', 'new a::C()');
   });
 
   group('operator', () {
@@ -249,8 +255,8 @@ void main() {
   });
 
   test('function expression', () async {
-    await expectNotNull('main() { () => null; f() {}; f; }',
-        '() → dart.core::Null? => null, f');
+    await expectNotNull(
+        'main() { () => null; f() {}; f; }', 'dart.core::Null? () => null, f');
   });
 
   test('cascades (kernel BlockExpression)', () async {
@@ -302,7 +308,7 @@ void main() {
         var y = (x = null) == null;
         print(x);
         print(y);
-      }''', '1, (x = null).{dart.core::Object::==}(null), y');
+      }''', '1, (x = null).{dart.core::Object.==}(null), y');
     });
     test('declaration from variable transitive', () async {
       await expectNotNull('''main() {
@@ -339,8 +345,8 @@ void main() {
       }''',
           // arithmetic operation results on `i` are themselves not null, even
           // though `i` is nullable.
-          '0, i.{dart.core::num::<}(10), 10, i = i.{dart.core::num::+}(1), '
-              'i.{dart.core::num::+}(1), 1, i.{dart.core::num::>=}(10), 10');
+          '0, i.{dart.core::num.<}(10), 10, i = i.{dart.core::num.+}(1), '
+              'i.{dart.core::num.+}(1), 1, i.{dart.core::num.>=}(10), 10');
     });
     test('for-in', () async {
       await expectNotNull('''main() {
@@ -362,7 +368,7 @@ void main() {
           print(z);
         }
         f(42);
-      }''', '0, () → void => dart.core::print("g"), "g", g, y, 1, z, f, 42');
+      }''', '0, void () => dart.core::print("g"), "g", g, y, 1, z, f, 42');
     });
     test('assignment to closure variable', () async {
       await expectNotNull('''main() {
@@ -391,8 +397,7 @@ void main() {
       await expectNotNull('''main() {
         var x = () => 42;
         var y = (() => x = null);
-      }''',
-          '() → dart.core::int* => 42, 42, () → dart.core::Null? => x = null');
+      }''', 'dart.core::int* () => 42, 42, dart.core::Null? () => x = null');
     });
     test('do not depend on unrelated variables', () async {
       await expectNotNull('''main() {
@@ -481,7 +486,7 @@ void main() {
       await expectNotNull(
           'library b; $imports class C { @notNull m() {} } '
               'main() { var c = new C(); c.m(); }',
-          'new b::C::•(), c.{b::C::m}(), c');
+          'new b::C(), c.{b::C.m}(), c');
     });
   });
 }
@@ -505,9 +510,9 @@ Future expectNotNull(String code, String expectedNotNull) async {
             // Print integer values as integers
             return BigInt.from(c.value).toString();
           }
-          return c.toConstantText();
+          return c.toText(astTextStrategy);
         }
-        return e.leakingDebugToString();
+        return e.toText(astTextStrategy);
       })
       // Filter out our own NotNull annotations.  The library prefix changes
       // per test, so just filter on the suffix.

pkg/front_end/lib/src/fasta/kernel/collections.dart

@@ -19,6 +19,8 @@ import 'package:kernel/ast.dart'
         transformList,
         visitList;
 
+import 'package:kernel/src/printer.dart';
+
 import 'package:kernel/type_environment.dart' show StaticTypeContext;
 
 import 'package:kernel/visitor.dart'
@@ -106,8 +108,8 @@ class SpreadElement extends Expression with ControlFlowElement {
   }
 
   @override
-  String toStringInternal() {
-    return "";
+  void toTextInternal(AstPrinter state) {
+    // TODO(johnniwinther): Implement this.
   }
 }
 
@@ -172,8 +174,8 @@ class IfElement extends Expression with ControlFlowElement {
   }
 
   @override
-  String toStringInternal() {
-    return "";
+  void toTextInternal(AstPrinter state) {
+    // TODO(johnniwinther): Implement this.
   }
 }
 
@@ -234,8 +236,8 @@ class ForElement extends Expression with ControlFlowElement {
   }
 
   @override
-  String toStringInternal() {
-    return "";
+  void toTextInternal(AstPrinter state) {
+    // TODO(johnniwinther): Implement this.
   }
 }
 
@@ -323,8 +325,8 @@ class ForInElement extends Expression with ControlFlowElement {
   }
 
   @override
-  String toStringInternal() {
-    return "";
+  void toTextInternal(AstPrinter state) {
+    // TODO(johnniwinther): Implement this.
   }
 }
 
@@ -351,6 +353,16 @@ mixin ControlFlowMapEntry implements MapEntry {
 
   @override
   R accept<R>(TreeVisitor<R> v) => v.defaultTreeNode(this);
+
+  @override
+  String toStringInternal() => toText(defaultAstTextStrategy);
+
+  @override
+  String toText(AstTextStrategy strategy) {
+    AstPrinter state = new AstPrinter(strategy);
+    toTextInternal(state);
+    return state.getText();
+  }
 }
 
 /// A spread element in a map literal.
@@ -387,8 +399,8 @@ class SpreadMapEntry extends TreeNode with ControlFlowMapEntry {
   }
 
   @override
-  String toStringInternal() {
-    return "";
+  void toTextInternal(AstPrinter state) {
+    // TODO(johnniwinther): Implement this.
   }
 }
 
@@ -433,8 +445,8 @@ class IfMapEntry extends TreeNode with ControlFlowMapEntry {
   }
 
   @override
-  String toStringInternal() {
-    return "";
+  void toTextInternal(AstPrinter state) {
+    // TODO(johnniwinther): Implement this.
   }
 }
 
@@ -480,8 +492,8 @@ class ForMapEntry extends TreeNode with ControlFlowMapEntry {
   }
 
   @override
-  String toStringInternal() {
-    return "";
+  void toTextInternal(AstPrinter state) {
+    // TODO(johnniwinther): Implement this.
   }
 }
 
@@ -554,8 +566,8 @@ class ForInMapEntry extends TreeNode with ControlFlowMapEntry {
   }
 
   @override
-  String toStringInternal() {
-    return "";
+  void toTextInternal(AstPrinter state) {
+    // TODO(johnniwinther): Implement this.
   }
 }
 

pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart

@@ -1899,7 +1899,7 @@ class ConstantEvaluator extends RecursiveVisitor<Constant> {
         if (constant is DoubleConstant && intFolder.isInt(constant)) {
           value = new BigInt.from(constant.value).toString();
         } else {
-          value = constant.toString();
+          value = constant.value.toString();
         }
         Object last = concatenated.last;
         if (last is StringBuffer) {

pkg/front_end/lib/src/fasta/kernel/forest.dart

@@ -7,6 +7,7 @@ library fasta.fangorn;
 import 'dart:core' hide MapEntry;
 
 import 'package:kernel/ast.dart';
+import 'package:kernel/src/printer.dart';
 
 import '../problems.dart' show unsupported;
 
@@ -761,7 +762,14 @@ class _VariablesDeclaration extends Statement {
   }
 
   @override
-  String toStringInternal() {
-    return "";
+  void toTextInternal(AstPrinter printer) {
+    for (int index = 0; index < declarations.length; index++) {
+      if (index > 0) {
+        printer.write(', ');
+      }
+      printer.writeVariableDeclaration(declarations[index],
+          includeModifiersAndType: index == 0);
+    }
+    printer.write(';');
   }
 }