Use FastaMessage instead of String. Part 1.

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

 // Copyright (c) 2017, 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 fasta.codes;
 
 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;
 
   final String tip;
 
   final String analyzerCode;
 
   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;
 
   final int charOffset;
 
   final String message;
 
   final String tip;

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

could we turn this into:

String get tip => code.tip;

?

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

No, the tip is also subject to template expansion.

 
   final FastaCode code;
 
   final Map<String, dynamic> arguments;
 
   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.