Support for modifiers.

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

Index: pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart
diff --git a/pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart b/pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart
index 3310536f2688c8f64d8d722e8c7aa6f09951bb13..c55e5bb336e9ee0ae2293ab3f39851263fb09165 100644
--- a/pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart
+++ b/pkg/front_end/lib/src/fasta/analyzer/ast_builder.dart
@@ -509,10 +509,10 @@ class AstBuilder extends ScopeListener {
     }
     SimpleIdentifier name = pop();
     TypeName type = pop();
-    pop(); // Modifiers.
+    Token keyword = _popOptionalSingleModifier();
     pop(); // Metadata.
     SimpleFormalParameter node = ast.simpleFormalParameter(
-        null, null, toAnalyzerToken(thisKeyword), type, name);
+        null, null, toAnalyzerToken(keyword), type, name);
     scope[name.name] = name.staticElement = new AnalyzerParameterElement(node);
     push(node);
   }
@@ -610,13 +610,12 @@ class AstBuilder extends ScopeListener {
 
   void handleModifier(Token token) {
     debugEvent("Modifier");
-    // TODO(ahe): Don't ignore modifiers.
+    push(token);
   }
 
   void handleModifiers(int count) {
     debugEvent("Modifiers");
-    // TODO(ahe): Don't ignore modifiers.
-    push(NullValue.Modifiers);
+    push(popList(count) ?? const <Token>[]);
   }
 
   FunctionBody _endFunctionBody() {
@@ -649,17 +648,14 @@ class AstBuilder extends ScopeListener {
     SimpleIdentifier name = pop();
     analyzer.Token propertyKeyword = toAnalyzerToken(getOrSet);
     TypeAnnotation returnType = pop();
-    // TODO(paulberry): handle modifiers.
-    var modifiers = pop();
-    assert(modifiers == null);
-    analyzer.Token externalKeyword = null;
+    Token externalKeyword = _popOptionalSingleModifier();
     List<Annotation> metadata = pop();
     // TODO(paulberry): capture doc comments.  See dartbug.com/28851.
     Comment comment = null;
     push(ast.functionDeclaration(
         comment,
         metadata,
-        externalKeyword,
+        toAnalyzerToken(externalKeyword),
         returnType,
         propertyKeyword,
         name,
@@ -840,16 +836,14 @@ class AstBuilder extends ScopeListener {
     } else {
       classKeyword = beginToken;
     }
-    var modifiers = pop();
-    assert(modifiers == null); // TODO(paulberry)
-    analyzer.Token abstractKeyword;
+    Token abstractKeyword = _popOptionalSingleModifier();
     List<Annotation> metadata = pop();
     // TODO(paulberry): capture doc comments.  See dartbug.com/28851.
     Comment comment = null;
     push(ast.classDeclaration(
         comment,
         metadata,
-        abstractKeyword,
+        toAnalyzerToken(abstractKeyword),
         toAnalyzerToken(classKeyword),
         name,
         typeParameters,
@@ -873,8 +867,8 @@ class AstBuilder extends ScopeListener {
   }
 
   @override
-  void endNamedMixinApplication(
-      Token beginToken, Token equalsToken, Token implementsKeyword, Token endToken) {
+  void endNamedMixinApplication(Token beginToken, Token equalsToken,
+      Token implementsKeyword, Token endToken) {
     debugEvent("NamedMixinApplication");
     ImplementsClause implementsClause;
     if (implementsKeyword != null) {
@@ -898,9 +892,7 @@ class AstBuilder extends ScopeListener {
     } else {
       classKeyword = beginToken;
     }
-    var modifiers = pop();
-    assert(modifiers == null); // TODO(paulberry)
-    analyzer.Token abstractKeyword;
+    Token abstractKeyword = _popOptionalSingleModifier();
     List<Annotation> metadata = pop();
     // TODO(paulberry): capture doc comments.  See dartbug.com/28851.
     Comment comment = null;
@@ -911,7 +903,7 @@ class AstBuilder extends ScopeListener {
         name,
         typeParameters,
         equals,
-        abstractKeyword,
+        toAnalyzerToken(abstractKeyword),
         superclass,
         withClause,
         implementsClause,
@@ -1015,16 +1007,9 @@ class AstBuilder extends ScopeListener {
     debugEvent("TopLevelFields");
     List<VariableDeclaration> variables = popList(count);
     TypeAnnotation type = pop();
-    var keyword;
-    if (optional('var', beginToken) ||
-        optional('const', beginToken) ||
-        optional('final', beginToken)) {
-      keyword = beginToken;
-    }
+    Token keyword = _popOptionalSingleModifier();
     var variableList = ast.variableDeclarationList(
         null, null, toAnalyzerToken(keyword), type, variables);
-    var modifiers = pop();
-    assert(modifiers == null); // TODO(paulberry)
     List<Annotation> metadata = pop();
     // TODO(paulberry): capture doc comments.  See dartbug.com/28851.
     Comment comment = null;
@@ -1068,12 +1053,27 @@ class AstBuilder extends ScopeListener {
     var name = pop();
     analyzer.Token propertyKeyword = toAnalyzerToken(getOrSet);
     TypeAnnotation returnType = pop();
-    // TODO(paulberry): handle modifiers.
-    var modifiers = pop();
-    assert(modifiers == null);
-    Token externalKeyword = null; // TODO(paulberry)
-    Token constKeyword = null; // TODO(paulberry)
-    Token factoryKeyword = null; // TODO(paulberry)
+
+    Token externalKeyword = null;
+    Token constKeyword = null;
+    Token factoryKeyword = null;
+    List<Token> modifiers = pop();
+    for (Token modifier in modifiers) {
+      String value = modifier.stringValue;
+      if (identical('external', value)) {
+        // TODO(scheglov): Check the order and uniqueness.
+        externalKeyword = modifier;
+      } else if (identical('const', value)) {
+        // TODO(scheglov): Check the order and uniqueness.
+        constKeyword = modifier;
+      } else if (identical('factory', value)) {
+        // TODO(scheglov): Check the order and uniqueness.
+        factoryKeyword = modifier;
+      } else {
+        // TODO(scheglov): Report error.

[Paul Berry, 2017/02/21 23:09:11]

The TODO is good, but for safety, let's also use "internalError" here so that if
we forget to come back and fix this, we won't ever accept invalid programs.

[scheglov, 2017/02/22 03:08:23]

Done.

[ahe, 2017/02/22 09:08:17]

Alternatively, try this:

printUnexpected(uri, modifier.charOffset, "Unexpected modifier: '${value}'.");

Later on, I plan to run through all these text strings and convert them to
proper error messages.

[scheglov, 2017/02/22 16:32:46]

There are 331 usage of internalError() and just 2 usages of printUnexpected() in
the codebase. So, it seems that internalError() is more popular.

[ahe, 2017/02/27 08:39:26]

We talked about the differences between internalError and printUnexpected in a
meeting, but as my memory sometimes plays tricks on me, I've realized that
there's an even better way to report compile-time errors.

internalError is used when the compiler is broken. Here's its documentation:

/// Used to report an internal error.
///
/// Internal errors should be avoided as best as possible, but are preferred
/// over assertion failures. Favor error messages that starts with "Internal
/// error: " and a short description that may help a developer debug the issue.
/// This method should be called instead of using `throw`, as this allows us to
/// ensure that there are no throws anywhere in the codebase.

So that might not be the right thing to use here as it seems you've identified a
compile-time error.

I haven't documented [printUnexpected], and I'll do so, but I've documented
[inputError] which is kinda similar. The latter stops the compilation, that is,
a fatal compile-time error that the compiler can't recover from. Obviously, this
isn't ideal, so I added [printUnexpected], which just prints a compile-time
error.

However, when you're in BodyBuilder or AstBuilder, you always have access to the
current library builder, so you can use LibraryBuilder.addCompileTimeError.

So finally, I can conclude that LibraryBuilder.addCompileTimeError would
probably be the right method to use here :-)

I'll create a CL where I update the documentation.

+      }
+    }
+
     List<Annotation> metadata = pop();
     // TODO(paulberry): capture doc comments.  See dartbug.com/28851.
     Comment comment = null;
@@ -1134,6 +1134,23 @@ class AstBuilder extends ScopeListener {
         parameters,
         toAnalyzerToken(endToken)));
   }
+
+  /**
+   * Pop the modifiers list, if the list is empty return `null`, if the list
+   * has one item return it; otherwise return `null`.
+   */
+  Token _popOptionalSingleModifier() {
+    List<Token> modifiers = pop();
+    if (modifiers.length == 0) {
+      return null;
+    } else if (modifiers.length == 1) {
+      // TODO(scheglov): Verify that the modifier is valid.
+      return modifiers[0];
+    } else {
+      // TODO(scheglov): Report error.

[Paul Berry, 2017/02/21 23:09:11]

Ditto.

[scheglov, 2017/02/22 03:08:24]

Done.

+      return null;
+    }
+  }
 }
 
 /// Data structure placed on the stack to represent a class body.