Use FastaMessage instead of String. Part 1.

pkg/front_end/lib/src/fasta/fasta_codes.dart

Index: pkg/front_end/lib/src/fasta/fasta_codes.dart
diff --git a/pkg/front_end/lib/src/fasta/fasta_codes.dart b/pkg/front_end/lib/src/fasta/fasta_codes.dart
index 0604ca99aaa98761e3067027f70aa99bad32eba1..0a20f8cde1242fe2e0e43ad61a1447587cfd7bbd 100644
--- a/pkg/front_end/lib/src/fasta/fasta_codes.dart
+++ b/pkg/front_end/lib/src/fasta/fasta_codes.dart
@@ -8,7 +8,7 @@ import 'package:front_end/src/scanner/token.dart' show Token;
 
 part 'fasta_codes_generated.dart';
 
-class FastaCode<T> {
+abstract class FastaCode<T> {

[Siggi Cherem (dart-lang), 2017/07/07 21:17:08]

seems you can drop the T here now.

[ahe, 2017/07/10 13:43:50]

Actually, T was essential for ensuring type safety. But it's gone now.

[Siggi Cherem (dart-lang), 2017/07/10 16:19:47]

BTW - I was suggesting it when you didn't have anything depending on T in this
class, but to keep it in the subclasses.

Now that you have Code containing Template, and Template is parameterized,
should we add it back? For instance,

class Code<T> {
  ...
  final Template<T> template;

[ahe, 2017/07/10 16:57:26]

I know that nothing was depending on it, but for type safety, I had to have it
there.
Yes. You're right.

   final String name;
 
   final String template;
@@ -19,18 +19,43 @@ class FastaCode<T> {
 
   final String dart2jsCode;
 
-  final T format;
-
   const FastaCode(this.name,
-      {this.template,
-      this.tip,
-      this.analyzerCode,
-      this.dart2jsCode,
-      this.format});
+      {this.template, this.tip, this.analyzerCode, this.dart2jsCode});
 
   String toString() => name;
 }
 
+class UnboundFastaCode<T> extends FastaCode<T> {
+  final T bind;
+
+  const UnboundFastaCode(String name,
+      {String template,
+      String tip,
+      String analyzerCode,
+      String dart2jsCode,
+      this.bind})
+      : super(name,
+            template: template,
+            tip: tip,
+            analyzerCode: analyzerCode,
+            dart2jsCode: dart2jsCode);
+}
+
+class BoundFastaCode extends FastaCode<BoundFastaCode> {

[ahe, 2017/07/07 15:25:42]

I'm thinking about renaming this to NoArgumentsFastaCode

[Siggi Cherem (dart-lang), 2017/07/07 21:17:08]

+1 on doing some renames.

Some alternative suggestions:

  FastaCode => MessageBase or MessageDescriptor
  typedef BoundFastaCode => class Message (use an object)
  UnboundFastaCode => MessageTemplate
  FastaMessage => MessageWithLocation or BoundMessage or LocatedMessage
  bind => eval

So the code would look like this:

class MessageBase {
  final String name;
  final String template;
  ...
}

class MessageTemplate<T> extends MessageBase {
   /// A function that takes all the arguments needed by the MessageTemplate
   /// to produce an actual Message.
   T eval;
}

class Message extends MessageBase {
   ...
   LocatedMessage at(Uri file, int offset) => new LocatedMessage(uri, offset,
this);
}

class LocatedMessage {
  Uri uri;
  int offset;
  Message message;
}


My rationale:
  - no need to use the Fasta prefix IMO
  - MessageBase conveys that this is abstract, there are two implementations,
depending on whether arguments are expected.
  - "Template" implies arguments that are not bound
  - putting a message in a location is just wrapping an existing message.


Thoughts?

[ahe, 2017/07/10 13:43:50]

I did something like that.

+  const BoundFastaCode(String name,
+      {String template, String tip, String analyzerCode, String dart2jsCode})
+      : super(name,
+            template: template,
+            tip: tip,
+            analyzerCode: analyzerCode,
+            dart2jsCode: dart2jsCode);
+
+  FastaMessage call(Uri uri, int charOffset) {
+    return new FastaMessage(uri, charOffset, this,
+        message: template, tip: tip, arguments: const <String, dynamic>{});
+  }
+}
+
 class FastaMessage {
   final Uri uri;
 
@@ -47,3 +72,5 @@ class FastaMessage {
   const FastaMessage(this.uri, this.charOffset, this.code,
       {this.message, this.tip, this.arguments});
 }
+
+typedef BoundFastaMessage = FastaMessage Function(Uri, int);

[ahe, 2017/07/07 15:25:41]

I'm thinking about renaming this to BoundCode.