Dart formatter checkpoint.

pkg/analyzer_experimental/lib/src/services/formatter_impl.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library formatter_impl;
 
 
 import 'dart:io';
 
 import 'package:analyzer_experimental/analyzer.dart';
@@ skipping 65 matching lines @@
   /// Format the specified portion (from [offset] with [length]) of the given
   /// [source] string, optionally providing an [indentationLevel].
   String format(CodeKind kind, String source, {int offset, int end,
     int indentationLevel:0});
 
 }
 
 class CodeFormatterImpl implements CodeFormatter, AnalysisErrorListener {
 
   final FormatterOptions options;
   final List<AnalysisError> errors = <AnalysisError>[];

[scheglov, 2013/06/19 22:12:05]

Remove type annotation for the final field.

[pquitslund, 2013/06/20 22:34:52]

Done.

+  final EditRecorder recorder;
 
-  CodeFormatterImpl(this.options);
+  CodeFormatterImpl(FormatterOptions options) : this.options = options,

[scheglov, 2013/06/19 22:12:05]

May be "this.options" and remove initializer?

[pquitslund, 2013/06/20 22:34:52]

The trouble is that I'd like to use options in my creation of the EditRecorder
AND I'd like recorder to be final.  Unfortunately, I can't access options in the
initializer block and that's the only place I can assign to final fields...

(Sorry if this is confusing.  Feel free to walk over if you want me to show you
what's up.)

+      recorder = new EditRecorder(options);
 
   String format(CodeKind kind, String source, {int offset, int end,
       int indentationLevel:0}) {
 
     var start = tokenize(source);
-    _checkForErrors();
+    checkForErrors();
 
     var node = parse(kind, start);
-    _checkForErrors();
+    checkForErrors();
 
-    // To be continued...
-
-    return source;
+    var formatter = new FormattingEngine(options);
+    return formatter.format(source, node, start, kind, recorder);
   }
 
   ASTNode parse(CodeKind kind, Token start) {
 
     var parser = new Parser(null, this);
 
     switch (kind) {
       case CodeKind.COMPILATION_UNIT:
         return parser.parseCompilationUnit(start);
       case CodeKind.STATEMENT:
         return parser.parseStatement(start);
     }
 
     throw new FormatterException('Unsupported format kind: $kind');
   }
 
-  _checkForErrors() {
+  checkForErrors() {
     if (errors.length > 0) {
       throw new FormatterException.forError(errors);
     }
   }
 
   void onError(AnalysisError error) {
     errors.add(error);
   }
 
   Token tokenize(String source) {
     var scanner = new StringScanner(null, source, this);
     return scanner.tokenize();
   }
 
 }
 
-/// Placeholder class to hold a reference to the Class object representing
-/// the Dart keyword void.
-class Void extends Object {
-
-}
-
 
 /// Records a sequence of edits to a source string that will cause the string
 /// to be formatted when applied.
 class EditRecorder {
 
   final FormatterOptions options;
+  final EditStore editStore;
 
   int column = 0;
 
   int sourceIndex = 0;
   String source = '';
 
   Token currentToken;
 
-  int indentationLevel = 0;
   int numberOfIndentations = 0;
 
-  bool isIndentNeeded = false;
+  bool needsIndent = false;
 
-  EditRecorder(this.options);
+  EditRecorder(this.options): editStore = new EditStore();

[Brian Wilkerson, 2013/06/19 21:50:52]

Just curiosity, but why did you move away from using a field formal parameter in
CodeFormatterImpl, but not here? And how did you decide which of these
possibilities to use:

EditRecorder(this.options): editStore = new EditStore();
EditRecorder(FormatterOptions options): this.options = options, editStore = new
EditStore();
EditRecorder(FormatterOptions options): this(options, new EditStore());

or any variation that uses a block body,
or combining this with the 'forStore' constructor into a single

EditRecorder(this.options, [this.editStore = new EditStore()]);

[pquitslund, 2013/06/20 22:34:52]

Discussed (exhaustively) in person!

+
+  EditRecorder.forStore(this.options, this.editStore);
+
+  /// Add an [Edit] that describes a textual [replacement] of a text
+  /// interval starting at the given [offset] spanning the given [length].
+  void addEdit(int offset, int length, String replacement) {
+    editStore.addEdit(offset, length, replacement);
+  }
+
+  /// Advance past the given expected [token] (or fail if not matched).
+  void advance(Token token) {
+    if (currentToken.lexeme == token.lexeme) {
+
+      // TODO(pquitslund) emit comments
+//      if (needsIndent) {
+//        advanceIndent();
+//        needsIndent = false;
+//      }
+      // Record writing a token at the current edit location
+      advanceChars(token.length);
+      currentToken = currentToken.next;
+    } else {
+      wrongToken(token.lexeme);
+    }
+  }
+
+  /// Move indices past indent, adding an edit if needed to adjust indentation
+  void advanceIndent() {
+//    var indentWidth = options.indentPerLevel * indentationLevel;
+//    var indentString = getIndentString(indentWidth);
+//    var sourceIndentWidth = 0;
+//    for (var i = 0; i < source.length; i++) {
+//      if (isIndentChar(source[sourceIndex + i])) {
+//        sourceIndentWidth += 1;
+//      } else {
+//        break;
+//      }
+//    }
+//    var hasSameIndent = sourceIndentWidth == indentWidth;
+//    if (hasSameIndent) {
+//      for (var i = 0; i < indentWidth; i++) {
+//        if (source[sourceIndex + i] != indentString[i]) {
+//          hasSameIndent = false;
+//          break;
+//        }
+//      }
+//      if (hasSameIndent) {
+//        advanceChars(indentWidth);
+//        return;
+//      }
+//    }
+//    addEdit(sourceIndex, sourceIndentWidth, indentString);
+//    column += indentWidth;
+//    sourceIndex += sourceIndentWidth;
+
+    var indent = options.indentPerLevel * numberOfIndentations;
+
+    spaces(indent);
+  }
+
+  String getIndentString(int indentWidth) {
+
+    // TODO(pquitslund) a temporary workaround
+    if (indentWidth < 0) {
+      return '';
+    }
+
+    // TODO(pquitslund) allow indent with tab chars
+
+    // Fetch a precomputed indent string
+    if (indentWidth < SPACES.length) {
+      return SPACES[indentWidth];
+    }
+
+    // Build un-precomputed strings dynamically
+    var sb = new StringBuffer();
+    for (var i=0; i < indentWidth; ++i) {

[scheglov, 2013/06/19 22:12:05]

Whitespaces before/after '='.

[pquitslund, 2013/06/20 22:34:52]

Done.

+      sb.write(' ');
+    }
+    return sb.toString();
+  }
+
+  /// Advance past the given expected [token] (or fail if not matched).
+  void advanceToken(String token) {
+    if (currentToken.lexeme == token) {
+      advance(currentToken);
+    } else {
+      wrongToken(token);
+    }
+  }
+
+  /// Advance [column] and [sourceIndex] indices by [len] characters.
+  void advanceChars(int len) {
+    column += len;
+    sourceIndex += len;
+  }
 
   /// Count the number of whitespace chars beginning at the current
   /// [sourceIndex].
   int countWhitespace() {
     var count = 0;
     for (var i = sourceIndex; i < source.length; ++i) {
       if (isIndentChar(source[i])) {
         ++count;
       } else {
         break;
       }
     }
     return count;
   }
 
-  /// Indent.
+  /// Update indent indices.
   void indent() {
-    indentationLevel += options.indentPerLevel;
     numberOfIndentations++;
   }
 
   /// Test if there is a newline at the given source [index].
   bool isNewlineAt(int index) {
     if (index < 0 || index + NEW_LINE.length > source.length) {
       return false;
     }
     for (var i = 0; i < NEW_LINE.length; i++) {
       if (source[index] != NEW_LINE[i]) {
         return false;
       }
     }
     return true;
   }
 
+  /// Newline.
+  void newline() {
+    // TODO(pquitslund) emit comments
+    needsIndent = true;
+    // If there is a newline before the edit location, do nothing.
+    if (isNewlineAt(sourceIndex - NEW_LINE.length)) {
+      return;
+    }
+    // If there is a newline after the edit location, advance over it.
+    if (isNewlineAt(sourceIndex)) {
+      advanceChars(NEW_LINE.length);
+      return;
+    }
+    // Otherwise, replace whitespace with a newline.
+    var charsToReplace = countWhitespace();
+    if (isNewlineAt(sourceIndex + charsToReplace)) {
+      ++charsToReplace;

[Brian Wilkerson, 2013/06/19 21:50:52]

Does this want to be incremented by NEW_LINE.length?

[pquitslund, 2013/06/20 22:34:52]

Yes!  Good catch. :)

+    }
+    addEdit(sourceIndex, charsToReplace, NEW_LINE);
+    advanceChars(charsToReplace);
+  }
+
+
+  /// Un-indent.
+  void unindent() {
+    numberOfIndentations--;
+  }
+
+  /// Space.
+  void space() {
+    // TODO(pquitslund) emit comments
+//    // If there is a space before the edit location, do nothing.
+//    if (isSpaceAt(sourceIndex - 1)) {
+//      return;
+//    }
+//    // If there is a space after the edit location, advance over it.
+//    if (isSpaceAt(sourceIndex)) {
+//      advance(1);
+//      return;
+//    }
+    // Otherwise, replace spaces with a single space.
+    spaces(1);
+  }
+
+  /// Spaces.
+  void spaces(int num) {
+    var charsToReplace = countWhitespace();
+    addEdit(sourceIndex, charsToReplace, SPACES[num]);
+    advanceChars(charsToReplace);
+  }
+
+  wrongToken(String token) {
+    throw new FormatterException('expected token: "${token}", '
+                                 'actual: "${currentToken}"');
+  }
+
+  String toString() =>
+      new EditOperation().apply(editStore.edits,
+                                source.substring(0, sourceIndex));
+
 }
 
 const SPACE = ' ';
+final SPACES = [
+          '',
+          ' ',
+          '  ',
+          '   ',
+          '    ',
+          '     ',
+          '      ',
+          '       ',
+          '        ',
+          '         ',
+          '          ',
+          '           ',
+          '            ',
+          '             ',
+          '              ',
+          '               ',
+          '                ',
+];
+
 
 bool isIndentChar(String ch) => ch == SPACE; // TODO(pquitslund) also check tab
 
 
 /// Manages stored [Edit]s.
 class EditStore {
 
+  const EditStore();

[Brian Wilkerson, 2013/06/19 21:50:52]

I wouldn't make this a 'const' constructor because edit stores are mutable, but
perhaps I'm thinking of 'const' the wrong way.

[pquitslund, 2013/06/20 22:34:52]

Actually, I think I AM!  Honestly, this is an artifact of an attempt to make
this constructor callable from with a constructor initializer block.  Fixed now.

+
   /// The underlying sequence of [Edit]s.
   final edits = <Edit>[];
 
   /// Add the given [Edit] to the end of the edit sequence.
   void add(Edit edit) {
     edits.add(edit);
   }
 
   /// Add an [Edit] that describes a textual [replacement] of a text interval
   /// starting at the given [offset] spanning the given [length].
@@ skipping 17 matching lines @@
   /// Reset cached state.
   void reset() {
     edits.clear();
   }
 
   String toString() => 'EditStore( ${edits.toString()} )';
 
 }
 
 
-
 /// Describes a text edit.
 class Edit {
 
   /// The offset at which to apply the edit.
   final int offset;
 
   /// The length of the text interval to replace.
   final int length;
 
   /// The replacement text.
   final String replacement;
 
   /// Create an edit.
   const Edit(this.offset, this.length, this.replacement);
 
   /// Create an edit for the given [range].
   Edit.forRange(SourceRange range, String replacement):
     this(range.offset, range.length, replacement);
 
   String toString() => '${offset < 0 ? '(' : 'X('} offset: ${offset} , '
                          'length ${length}, replacement :> ${replacement} <:)';
 
 }
 
+/// Applies a sequence of [edits] to a [document].
+class EditOperation {
+
+  String apply(List<Edit> edits, String document) {
+
+    var edit;
+    for (var i = edits.length - 1; i >= 0; --i) {
+      edit = edits[i];
+      document = replace(document, edit.offset,
+                         edit.offset + edit.length, edit.replacement);
+    }
+
+    return document;
+  }
+
+}
+
+
+String replace(String str, int start, int end, String replacement) =>
+    str.substring(0, start) + replacement + str.substring(end);
+
+
 /// An AST visitor that drives formatting heuristics.
-class FormattingEngine extends RecursiveASTVisitor<Void> {
+class FormattingEngine extends RecursiveASTVisitor {
 
   final FormatterOptions options;
 
+  CodeKind kind;
+  EditRecorder recorder;
+
   FormattingEngine(this.options);
 
+  String format(String source, ASTNode node, Token start, CodeKind kind,
+      EditRecorder recorder) {
+
+    this.kind = kind;
+    this.recorder = recorder;
+
+    recorder..source = source
+            ..currentToken = start;

[Brian Wilkerson, 2013/06/19 21:50:52]

This looks weird. I would have expected the state of the recorder to have been
configured before passing it to this method and for the 'source' and 'start'
arguments to be removed. (The reference to 'source' below would presumably be
replaced by 'recorder.source'.)

[pquitslund, 2013/06/20 22:34:52]

Agreed.  Still working on how this entry point should look.  Thanks!

+
+    node.accept(this);
+
+    var editor = new EditOperation();
+    return editor.apply(recorder.editStore.edits, source);
+  }
+
+
+  visitClassDeclaration(ClassDeclaration node) {
+
+    recorder.advanceIndent();
+
+    if (node.documentationComment != null) {
+      node.documentationComment.accept(this);
+    }
+
+    recorder..advance(node.classKeyword)..space();
+
+    node.name.accept(this);
+
+    if (node.typeParameters != null) {
+      node.typeParameters.accept(this);
+    }
+    recorder.space();
+
+    if (node.extendsClause != null) {
+      node.extendsClause.accept(this);
+      recorder.space();
+    }
+

[Brian Wilkerson, 2013/06/19 21:50:52]

You missed the withClause.

[pquitslund, 2013/06/20 22:34:52]

Coming real soon.  (Driven by accompanying tests.)

+    if (node.implementsClause != null) {
+      node.implementsClause.accept(this);
+      recorder.space();
+    }
+
+    recorder..advance(node.leftBracket)
+            ..indent();
+
+    for (var member in node.members) {
+      recorder.newline();

[Brian Wilkerson, 2013/06/19 21:50:52]

Do you want to advanceIndent() after the newline, or are you expecting every
member to do that before starting to write themselves out?

[pquitslund, 2013/06/20 22:34:52]

Actually, yes.  That is cleaner.  Thanks!

+      member.accept(this);
+    }
+
+    recorder..unindent()
+            ..newline()
+            ..advanceIndent()
+            ..advance(node.rightBracket);
+  }
+
+
+  visitBlockFunctionBody(BlockFunctionBody node) {
+    recorder..advance(node.beginToken)

[Brian Wilkerson, 2013/06/19 21:50:52]

The begin token for a BlockFunctionBody is the '{' for the block. I don't think
you want to advance over it here, but leave it for the block to do. (I don't
think you ever want to use 'beginToken' or 'endToken'; either there are more
specific getters or they belong to a child.)

+            ..indent()
+            ..newline();
+    node.block.accept(this);
+    recorder..unindent()
+            ..advanceIndent()
+            ..advance(node.endToken);
+  }
+
+
+  visitBlock(Block block) {
+
+  }
+
+
+  visitExpressionFunctionBody(ExpressionFunctionBody node) {
+    recorder..advance(node.beginToken)

[Brian Wilkerson, 2013/06/19 21:50:52]

Here too: use 'functionDefinition' and 'semicolon'.

[pquitslund, 2013/06/20 22:34:52]

Thanks!  

+            ..indent()
+            ..newline();
+    node.expression.accept(this);
+    recorder..unindent()
+            ..advanceIndent()
+            ..advance(node.endToken);
+  }
+
+
+  visitMethodDeclaration(MethodDeclaration node) {
+
+    recorder.advanceIndent();

[Brian Wilkerson, 2013/06/19 21:50:52]

The rule for AST nodes is that whitespace before and after the node belongs to
the parent. I think you want to follow something similar here: the whitespace
before and after a node should be written while visiting the parent of the node.
I think it will lead to fewer mistakes, because you'll have more context when
deciding what whitespace to write.

[pquitslund, 2013/06/20 22:34:52]

Done.

+
+    if (node.modifierKeyword != null) {
+      recorder.advance(node.modifierKeyword);
+      recorder.space();
+    }
+
+    if (node.returnType != null) {
+      recorder.advance(node.returnType.beginToken);

[Brian Wilkerson, 2013/06/19 21:50:52]

I think you want to visit the returnType at this point.

[pquitslund, 2013/06/20 22:34:52]

Done.

+      recorder.space();
+    }
+
+    recorder.advance(node.name.beginToken);
+
+    node.parameters.accept(this);
+
+    recorder.space();
+
+    node.body.accept(this);
+  }
+
+
+  visitFormalParameterList(FormalParameterList node) {
+    recorder.advance(node.beginToken);
+    //...
+    recorder.advance(node.endToken);
+  }
+
+
+  visitSimpleIdentifier(SimpleIdentifier node) {
+    recorder.advance(node.token);
+  }
+
 }