Unify fasta->analyzer token translation logic.

pkg/front_end/lib/src/fasta/analyzer/token_utils.dart

Index: pkg/front_end/lib/src/fasta/analyzer/token_utils.dart
diff --git a/pkg/front_end/lib/src/fasta/analyzer/token_utils.dart b/pkg/front_end/lib/src/fasta/analyzer/token_utils.dart
index cfe197213b9e02886d784e50561d8d55f2b30bb4..8ebe76f2f0e9bd8978156731941293318ecffc38 100644
--- a/pkg/front_end/lib/src/fasta/analyzer/token_utils.dart
+++ b/pkg/front_end/lib/src/fasta/analyzer/token_utils.dart
@@ -5,42 +5,143 @@
 library fasta.analyzer.token_utils;
 
 import 'package:front_end/src/fasta/scanner/token.dart' show
+    KeywordToken,
     Token;
 
 import 'package:front_end/src/fasta/scanner/token_constants.dart';
 
-import 'package:analyzer/dart/ast/token.dart' as analyzer show
-    Token;
+import 'package:front_end/src/scanner/token.dart' as analyzer show
+    CommentToken,
+    Keyword,
+    KeywordToken,
+    KeywordTokenWithComment,
+    StringToken,
+    StringTokenWithComment,
+    Token,
+    TokenWithComment;
 
 import 'package:analyzer/dart/ast/token.dart' show
     TokenType;
 
-import 'package:analyzer/src/dart/ast/token.dart' as analyzer show
-    StringToken;
-
 import '../errors.dart' show
     internalError;
 
-analyzer.Token toAnalyzerToken(Token token) {
+analyzer.Token toAnalyzerToken(Token token,
+    [analyzer.CommentToken commentToken]) {
   if (token == null) return null;
+  analyzer.Token makeStringToken(TokenType tokenType) {
+    if (commentToken == null) {
+      return new analyzer.StringToken(tokenType, token.value, token.charOffset);
+    } else {
+      return new analyzer.StringTokenWithComment(
+          tokenType, token.value, token.charOffset, commentToken);
+    }
+  }
+
   switch (token.kind) {
-    case STRING_TOKEN:
-      return new analyzer.StringToken(
-          TokenType.STRING, token.value, token.charOffset);
+    case DOUBLE_TOKEN:
+      return makeStringToken(TokenType.DOUBLE);
+
+    case HEXADECIMAL_TOKEN:
+      return makeStringToken(TokenType.HEXADECIMAL);
 
     case IDENTIFIER_TOKEN:
-      return new analyzer.StringToken(
-          TokenType.IDENTIFIER, token.value, token.charOffset);
+      return makeStringToken(TokenType.IDENTIFIER);
 
     case INT_TOKEN:
-      return new analyzer.StringToken(
-          TokenType.INT, token.value, token.charOffset);
+      return makeStringToken(TokenType.INT);
+
+    case KEYWORD_TOKEN:
+      KeywordToken keywordToken = token;
+      var syntax = keywordToken.keyword.syntax;
+      var keyword = _keywordMap[syntax];
+      if (keyword == null) {
+        if (_pseudoKeywords.contains(syntax)) {

[ahe, 2017/02/10 14:12:40]

Would token.isIdentifier or token.isPseudo work here?

[Paul Berry, 2017/02/10 17:01:02]

Thanks.  token.isIdentifier is too broad (it includes things the spec calls
"built-in identifiers" such as `abstract`), but token.keyword.isPseudo is a good
suggestion.

I'm going to defer this to a follow-up CL because there are two words that the
spec, analyzer, and Fasta don't agree on how to classify (`deferred` and
`covariant`), and I want to do some more investigation and add some more tests.

[ahe, 2017/02/10 17:10:46]

I thought that might be the case.

One thing you can consider when you look at those words is to add additional
bools to the Keyword class.

[Paul Berry, 2017/02/10 17:20:24]

Thanks.  I was considering that, and I'm glad to hear that your thoughts run in
a similar direction.

+          // TODO(paulberry,ahe): fasta scans "async", "await", and "sync" as
+          // keywords.  They need to be identifiers since their meaning is only
+          // special in certain contexts.
+          return makeStringToken(TokenType.IDENTIFIER);
+        } else {
+          return internalError('Unknown keyword: $syntax');
+        }
+      }
+      if (commentToken == null) {
+        return new analyzer.KeywordToken(keyword, token.charOffset);
+      } else {
+        return new analyzer.KeywordTokenWithComment(
+            keyword, token.charOffset, commentToken);
+      }
+      break;
+
+    case STRING_TOKEN:
+      return makeStringToken(TokenType.STRING);
 
     default:
-      return new analyzer.Token(getTokenType(token), token.charOffset);
+      if (commentToken == null) {
+        return new analyzer.Token(getTokenType(token), token.charOffset);
+      } else {
+        return new analyzer.TokenWithComment(
+            getTokenType(token), token.charOffset, commentToken);
+      }
+      break;
   }
 }
 
+final _keywordMap = {
+  "assert": analyzer.Keyword.ASSERT,

[ahe, 2017/02/10 14:12:40]

I think you're not worried about performance right now, so feel free to ignore
this:

AFAIK, the built-in map is pretty slow, so one could try a string switch
instead. However, as far as I know, switches are implemented as a sequence of if
tests, so, at least in theory, this should be fastest:

analyzer.Keyword keywordFromString(String name) {
  if (identical("assert", name)) return analyzer.Keyword.BREAK;
  // And so on.
}

The scanner takes care to use string literals for all the keyword string, and
this is why it is safe to use identical.

[Paul Berry, 2017/02/10 17:01:02]

You're correct that I'm not worried about performance right now, especially for
this code (I think it remains to be seen whether this translation layer is going
to be needed in the long term, or whether we will unify Fasta and analyzer's
token representations).  I've added a TODO so that if this shows up in the
profiler, we won't forget your ideas for speeding it up.

[ahe, 2017/02/10 17:10:46]

FWIW, my biggest worry about unifying them is the previous token pointer. But it
should be easy to investigate the memory overhead in fasta if we add an
additional field to tokens, and it might be that bad as fasta doesn't keep them
in memory as dart2js does.

[Paul Berry, 2017/02/10 17:20:24]

Agreed--it's a complex tradeoff.  I don't intend to make any movements in that
direction without gathering data and opinions first :)

[ahe, 2017/02/10 17:22:45]

Data and opinions are good :-)

Just to be clear, I left out a negation above. I think the memory overhead might
*not* be that bad.

+  "break": analyzer.Keyword.BREAK,
+  "case": analyzer.Keyword.CASE,
+  "catch": analyzer.Keyword.CATCH,
+  "class": analyzer.Keyword.CLASS,
+  "const": analyzer.Keyword.CONST,
+  "continue": analyzer.Keyword.CONTINUE,
+  "default": analyzer.Keyword.DEFAULT,
+  "do": analyzer.Keyword.DO,
+  "else": analyzer.Keyword.ELSE,
+  "enum": analyzer.Keyword.ENUM,
+  "extends": analyzer.Keyword.EXTENDS,
+  "false": analyzer.Keyword.FALSE,
+  "final": analyzer.Keyword.FINAL,
+  "finally": analyzer.Keyword.FINALLY,
+  "for": analyzer.Keyword.FOR,
+  "if": analyzer.Keyword.IF,
+  "in": analyzer.Keyword.IN,
+  "new": analyzer.Keyword.NEW,
+  "null": analyzer.Keyword.NULL,
+  "rethrow": analyzer.Keyword.RETHROW,
+  "return": analyzer.Keyword.RETURN,
+  "super": analyzer.Keyword.SUPER,
+  "switch": analyzer.Keyword.SWITCH,
+  "this": analyzer.Keyword.THIS,
+  "throw": analyzer.Keyword.THROW,
+  "true": analyzer.Keyword.TRUE,
+  "try": analyzer.Keyword.TRY,
+  "var": analyzer.Keyword.VAR,
+  "void": analyzer.Keyword.VOID,
+  "while": analyzer.Keyword.WHILE,
+  "with": analyzer.Keyword.WITH,
+  "is": analyzer.Keyword.IS,
+  "abstract": analyzer.Keyword.ABSTRACT,
+  "as": analyzer.Keyword.AS,
+  "covariant": analyzer.Keyword.COVARIANT,
+  "dynamic": analyzer.Keyword.DYNAMIC,
+  "export": analyzer.Keyword.EXPORT,
+  "external": analyzer.Keyword.EXTERNAL,
+  "factory": analyzer.Keyword.FACTORY,
+  "get": analyzer.Keyword.GET,
+  "implements": analyzer.Keyword.IMPLEMENTS,
+  "import": analyzer.Keyword.IMPORT,
+  "library": analyzer.Keyword.LIBRARY,
+  "operator": analyzer.Keyword.OPERATOR,
+  "part": analyzer.Keyword.PART,
+  "set": analyzer.Keyword.SET,
+  "static": analyzer.Keyword.STATIC,
+  "typedef": analyzer.Keyword.TYPEDEF,
+  "deferred": analyzer.Keyword.DEFERRED,
+};
+
+final _pseudoKeywords = new Set<String>.from(['async', 'await', 'sync']);
+
 TokenType getTokenType(Token token) {
   switch (token.kind) {
     case EOF_TOKEN: return TokenType.EOF;
@@ -109,8 +210,8 @@ TokenType getTokenType(Token token) {
     case SLASH_EQ_TOKEN: return TokenType.SLASH_EQ;
     case STAR_TOKEN: return TokenType.STAR;
     case STAR_EQ_TOKEN: return TokenType.STAR_EQ;
-    // case STRING_INTERPOLATION_EXPRESSION_TOKEN:
-    //   return TokenType.STRING_INTERPOLATION_EXPRESSION;
+    case STRING_INTERPOLATION_TOKEN:
+      return TokenType.STRING_INTERPOLATION_EXPRESSION;
     case STRING_INTERPOLATION_IDENTIFIER_TOKEN:
       return TokenType.STRING_INTERPOLATION_IDENTIFIER;
     case TILDE_TOKEN: return TokenType.TILDE;