Make string juxtaposition combine properly with string interpolations.

frog/leg/tree/nodes.dart

Index: frog/leg/tree/nodes.dart
diff --git a/frog/leg/tree/nodes.dart b/frog/leg/tree/nodes.dart
index 7d066ff42172178f0c4c45ced9f9033107d32497..e117d45acde25c7299585669dc1ed00b2070a1e4 100644
--- a/frog/leg/tree/nodes.dart
+++ b/frog/leg/tree/nodes.dart
@@ -27,7 +27,7 @@ interface Visitor<R> {
   R visitLiteralMapEntry(LiteralMapEntry node);
   R visitLiteralNull(LiteralNull node);
   R visitLiteralString(LiteralString node);
-  R visitLiteralStringJuxtaposition(LiteralStringJuxtaposition node);
+  R visitStringJuxtaposition(StringJuxtaposition node);
   R visitModifiers(Modifiers node);
   R visitNamedArgument(NamedArgument node);
   R visitNewExpression(NewExpression node);
@@ -136,6 +136,7 @@ class Node implements Hashable {
   Statement asStatement() => null;
   StringInterpolation asStringInterpolation() => null;
   StringInterpolationPart asStringInterpolationPart() => null;
+  StringJuxtaposition asStringJuxtaposition() => null;
   SwitchCase asSwitchCase() => null;
   SwitchStatement asSwitchStatement() => null;
   Throw asThrow() => null;
@@ -728,182 +729,6 @@ class StringQuoting {
   }
 }
 
-class DartString implements Iterable<int> {
-  // This is a convenience constructor. If you need a const literal DartString,
-  // use [const LiteralDartString(string)] directly.
-  factory DartString.literal(String string) => new LiteralDartString(string);
-  factory DartString.rawString(SourceString source, int length) =>
-      new RawSourceDartString(source, length);
-  factory DartString.escapedString(SourceString source, int length) =>
-      new EscapedSourceDartString(source, length);
-  const DartString();
-  abstract int get length();
-  bool isEmpty() => length == 0;
-  abstract Iterator<int> iterator();
-  abstract String slowToString();
-
-  bool operator ==(var other) {
-    if (other is !DartString) return false;
-    DartString otherString = other;
-    if (length != otherString.length) return false;
-    Iterator it1 = iterator();
-    Iterator it2 = otherString.iterator();
-    while (it1.hasNext()) {
-      if (it1.next() != it2.next()) return false;
-    }
-    return true;
-  }
-  String toString() => "DartString#${length}:${slowToString()}";
-  abstract SourceString get source();
-}
-
-class LiteralDartString extends DartString {
-  final String string;
-  const LiteralDartString(this.string);
-  int get length() => string.length;
-  Iterator<int> iterator() => new StringCodeIterator(string);
-  String slowToString() => string;
-  SourceString get source() => new StringWrapper(string);
-}
-
-class SourceBasedDartString extends DartString {
-  String toStringCache = null;
-  final SourceString source;
-  final int length;
-  SourceBasedDartString(this.source, this.length);
-  abstract Iterator<int> iterator();
-}
-
-class RawSourceDartString extends SourceBasedDartString {
-  RawSourceDartString(source, length) : super(source, length);
-  Iterator<int> iterator() => source.iterator();
-  String slowToString() {
-    if (toStringCache !== null) return toStringCache;
-    toStringCache  = source.slowToString();
-    return toStringCache;
-  }
-}
-
-class EscapedSourceDartString extends SourceBasedDartString {
-  EscapedSourceDartString(source, length) : super(source, length);
-  Iterator<int> iterator() {
-    if (toStringCache !== null) return new StringCodeIterator(toStringCache);
-    return new StringEscapeIterator(source);
-  }
-  String slowToString() {
-    if (toStringCache !== null) return toStringCache;
-    StringBuffer buffer = new StringBuffer();
-    StringEscapeIterator it = new StringEscapeIterator(source);
-    while (it.hasNext()) {
-      buffer.addCharCode(it.next());
-    }
-    toStringCache = buffer.toString();
-    return toStringCache;
-  }
-}
-
-class ConsDartString extends DartString {
-  final DartString left;
-  final DartString right;
-  final int length;
-  int hashCache = null;
-  String toStringCache;
-  ConsDartString(DartString left, DartString right)
-      : this.left = left,
-        this.right = right,
-        length = left.length + right.length;
-
-  Iterator<int> iterator() => new ConsDartStringIterator(this);
-
-  String slowToString() {
-    if (toStringCache !== null) return toStringCache;
-    toStringCache = left.slowToString().concat(right.slowToString());
-    return toStringCache;
-  }
-  SourceString get source() => new StringWrapper(slowToString());
-}
-
-class ConsDartStringIterator implements Iterator<int> {
-  Iterator<int> current;
-  DartString right;
-  bool hasNextLookAhead;
-  ConsDartStringIterator(ConsDartString cons)
-      : current = cons.left.iterator(),
-        right = cons.right {
-    hasNextLookAhead = current.hasNext();
-    if (!hasNextLookAhead) {
-      nextPart();
-    }
-  }
-  bool hasNext() {
-    return hasNextLookAhead;
-  }
-  int next() {
-    assert(hasNextLookAhead);
-    int result = current.next();
-    hasNextLookAhead = current.hasNext();
-    if (!hasNextLookAhead) {
-      nextPart();
-    }
-    return result;
-  }
-  void nextPart() {
-    if (right !== null) {
-      current = right.iterator();
-      right = null;
-      hasNextLookAhead = current.hasNext();
-    }
-  }
-}
-
-/**
- *Iterator that returns the actual string contents of a string with escapes.
- */
-class StringEscapeIterator implements Iterator<int>{
-  final Iterator<int> source;
-  StringEscapeIterator(SourceString source) : this.source = source.iterator();
-  bool hasNext() => source.hasNext();
-  int next() {
-    int code = source.next();
-    if (code !== $BACKSLASH) {
-      return code;
-    }
-    code = source.next();
-    if (code === $n) return $LF;
-    if (code === $r) return $CR;
-    if (code === $t) return $TAB;
-    if (code === $b) return $BS;
-    if (code === $f) return $FF;
-    if (code === $v) return $VTAB;
-    if (code === $x) {
-      int value = hexDigitValue(source.next());
-      value = value * 16 + hexDigitValue(source.next());
-      return value;
-    }
-    if (code === $u) {
-      int value = 0;
-      code = source.next();
-      if (code === $OPEN_CURLY_BRACKET) {
-        for (code = source.next();
-             code != $CLOSE_CURLY_BRACKET;
-             code = source.next()) {
-           value = value * 16 + hexDigitValue(code);
-        }
-        return value;
-      }
-      // Four digit hex value.
-      value = hexDigitValue(code);
-      for (int i = 0; i < 3; i++) {
-        code = source.next();
-        value = value * 16 + hexDigitValue(code);
-      }
-      return value;
-    }
-    return code;
-  }
-}
-
-
 class LiteralString extends Literal<SourceString> {
   /** Set on validated string literals. */
   final DartString dartString = null;
@@ -1262,45 +1087,50 @@ class StringInterpolationPart extends Node {
   Token getEndToken() => string.getEndToken();
 }
 
-class LiteralStringJuxtaposition extends LiteralString {
-  // List of either StringLiteral or StringInterpolation.
-  final Link<Expression> literals;
+// A class representing juxtaposed string literals.

[ahe, 2012/03/15 09:02:56]

Documentation comment.

[Lasse Reichstein Nielsen, 2012/03/19 07:15:32]

Done.

+// The string literals can be both plain literals and string interpolations.
+class StringJuxtaposition extends Expression {
+  Expression first;
+  Expression second;
+  final DartString dartString;
+  // TODO(lrn): is this redundant? After parsing, dartString will be set unless
+  // one of first or second is a string interpolation (because any non-
+  // validating literal string would end after parsing).
+  final bool isInterpolation;

[ahe, 2012/03/15 09:02:56]

My guess: it is redundant! ;-)

[Lasse Reichstein Nielsen, 2012/03/19 07:15:32]

Refactored to be not redundant.

 
-  LiteralStringJuxtaposition(Link<Expression> literals)
-      : this.literals = literals,
-        super(literals.head.getBeginToken(), concatenateLiterals(literals));
+  StringJuxtaposition(Expression first, Expression second)
+      : this.first = first,
+        this.second = second,
+        this.dartString = concatenateLiterals(first, second),

[ahe, 2012/03/15 09:02:56]

As a matter of principle, doing this much work in a constructor rubs me the
wrong way. It seems like a premature optimization, that as a general pattern is
likely to cause problems if overused.

All the other constructors in this file are simply storing their arguments. This
is by design based on intuition I have built up over years. I'll try to think
more about why I prefer this style, but I'm having a hard time articulating it
right now.

I not convinced that strings deserve special treatment, and I'm worried that
this approach will be adopted in other places.

From the perspective of an IDE, you may want to find the innermost enclosing
expression or statement of the input cursor. For this purpose, the fields
dartString and isInterpolation are unnecessary.

On the other hand, we need this information early in the scanner_task when
reading #-tags. But I would rather have a method on Compiler that given an
Expression returns a String.

[Lasse Reichstein Nielsen, 2012/03/19 07:15:32]

I have made the computation of dartString and isInterpolation dynamic (a getter
that caches the result). This way they won't have any computational overhead.

+        this.isInterpolation = isEitherInterpolation(first, second);
 
-  static DartString concatenateLiterals(Link<Expression> literals) {
-    assert(!literals.isEmpty());
-    LiteralString literal = literals.head;
-    if (literals.tail.isEmpty()) {
-      return literal.dartString;
-    }
-    return new ConsDartString(literal.dartString,
-                              concatenateLiterals(literals.tail));
+  StringJuxtaposition asStringJuxtaposition() => this;
+
+  static DartString concatenateLiterals(Expression first, Expression second) {
+    DartString firstDartString = first.accept(const GetDartStringVisitor());
+    if (firstDartString === null) return null;
+    DartString secondDartString = second.accept(const GetDartStringVisitor());
+    if (secondDartString === null) return null;
+    return new DartString.concat(firstDartString,
+                                 secondDartString);
   }
 
-  SourceString get value() => null;
+  // Extract the string interpolations from the juxtaposed strings.
+  static bool isEitherInterpolation(Expression first, Expression second) {
+    return first.accept(const IsInterpolationVisitor())
+        || second.accept(const IsInterpolationVisitor());
+  }
 
-  accept(Visitor visitor) => visitor.visitLiteralStringJuxtaposition(this);
+  accept(Visitor visitor) => visitor.visitStringJuxtaposition(this);
 
-  visitChildren(Visitor visitor) {
-    for (Expression literal in literals) {
-      literal.accept(visitor);
-    }
+  void visitChildren(Visitor visitor) {
+    first.accept(visitor);
+    second.accept(visitor);
   }
 
-  Token getBeginToken() => literals.head.getBeginToken();
+  Token getBeginToken() => first.getBeginToken();
 
-  Token getEndToken() {
-    Link<Expression> current = literals;
-    Expression lastExpression = null;
-    while (!current.isEmpty()) {
-      lastExpression = current.head;
-      current = current.tail;
-    }
-    return lastExpression.getEndToken();
-  }
+  Token getEndToken() => second.getEndToken();
 }
 
 class EmptyStatement extends Statement {
@@ -1715,3 +1545,21 @@ class Initializers {
             node.selector.asIdentifier() !== null);
   }
 }
+
+class GetDartStringVisitor extends AbstractVisitor<DartString> {
+  const GetDartStringVisitor();
+  DartString visitNode(Node node) => null;
+  DartString visitStringInterpolation(StringInterpolation node) => null;
+  DartString visitStringJuxtaposition(StringJuxtaposition node)
+      => node.dartString;
+  DartString visitLiteralString(LiteralString node) => node.dartString;
+}
+
+class IsInterpolationVisitor extends AbstractVisitor<bool> {
+  const IsInterpolationVisitor();
+  bool visitNode(Node node) => false;
+  bool visitStringInterpolation(StringInterpolation node) => true;
+  bool visitStringJuxtaposition(StringJuxtaposition node)
+      => node.isInterpolation;
+  bool visitLiteralString(LiteralString node) => false;

[ahe, 2012/03/15 09:02:56]

Why is this needed?

[Lasse Reichstein Nielsen, 2012/03/19 07:15:32]

It's not.
This visitor is build to traverse sting-juxtapositions containing interpolations
and literals, so I thought it was more readable if it handled those explicitly,
but it's not important. I'll remove it (and visitStringInterpolation in the
GetDartStringVisiitor).

+}