Use messages for (some) public API errors

pkg/front_end/lib/src/fasta/kernel/kernel_target.dart

 // Copyright (c) 2016, 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.kernel_target;
 
 import 'dart:async' show Future;
 
 import 'package:front_end/file_system.dart';
 
@@ skipping 79 matching lines @@
 
   final DillTarget dillTarget;
 
   /// Shared with [CompilerContext].
   final Map<String, Source> uriToSource;
 
   SourceLoader<Library> loader;
 
   Program program;
 
-  final List<String> errors = <String>[];
+  final List<LocatedMessage> errors = <LocatedMessage>[];

[Siggi Cherem (dart-lang), 2017/07/12 00:35:15]

note: this is now possible because I made deprecated_InputError implement
LocatedMessage

[ahe, 2017/07/12 13:14:43]

I'd like to hold back on this change. I'm pretty close to achieving the same
effect, but without making deprecated_InputError implement LocatedMessage.

[Siggi Cherem (dart-lang), 2017/07/12 21:54:59]

OK, I reverted this.

 
   final TypeBuilder dynamicType =
       new KernelNamedTypeBuilder("dynamic", null, -1, null);
 
   bool get strongMode => backendTarget.strongMode;
 
   bool get disableTypeInference => backendTarget.disableTypeInference;
 
   KernelTarget(
       this.fileSystem, DillTarget dillTarget, TranslateUri uriTranslator,
       [Map<String, Source> uriToSource])
       : dillTarget = dillTarget,
         uriToSource = uriToSource ?? CompilerContext.current.uriToSource,
         super(dillTarget.ticker, uriTranslator, dillTarget.backendTarget) {
     resetCrashReporting();
     loader = createLoader();
   }
 
   void deprecated_addError(file, int charOffset, String message) {
     Uri uri = file is String ? Uri.parse(file) : file;
     deprecated_InputError error =
         new deprecated_InputError(uri, charOffset, message);
-    String formatterMessage = error.deprecated_format();
-    print(formatterMessage);
-    errors.add(formatterMessage);
+    errors.add(error);
+    print(error.deprecated_format());

[ahe, 2017/07/12 13:14:43]

I'm pretty close to removing this and all the other calls to print from this
class.

[Siggi Cherem (dart-lang), 2017/07/12 21:54:59]

fantastic!, reverted to prevent conflicts.

   }
 
   SourceLoader<Library> createLoader() =>
       new SourceLoader<Library>(fileSystem, this);
 
   void addSourceInformation(
       Uri uri, List<int> lineStarts, List<int> sourceCode) {
     String fileUri = relativizeUri(uri);
     uriToSource[fileUri] = new Source(lineStarts, sourceCode);
   }
@@ skipping 77 matching lines @@
     cls.mixedInType = null;
     builder.supertype = new KernelNamedTypeBuilder("Object", null,
         builder.charOffset, builder.fileUri ?? Uri.parse(cls.fileUri))
       ..builder = objectClassBuilder;
     builder.interfaces = null;
     builder.mixedInType = null;
   }
 
   void handleInputError(deprecated_InputError error, {bool isFullProgram}) {
     if (error != null) {
-      String message = error.deprecated_format();
-      print(message);
-      errors.add(message);
+      errors.add(error);
+      print(error.deprecated_format());
     }
     program = erroneousProgram(isFullProgram);
   }
 
   @override
   Future<Program> buildOutlines({CanonicalName nameRoot}) async {
     if (loader.first == null) return null;
     try {
       loader.createTypeInferenceEngine();
       await loader.buildOutlines();
@@ skipping 68 matching lines @@
   /// replace [program].
   void handleRecoverableErrors(List<LocatedMessage> recoverableErrors) {
     if (recoverableErrors.isEmpty) return;
     KernelLibraryBuilder mainLibrary = loader.first;
     if (mainLibrary == null) {
       program = erroneousProgram(true);
       return;
     }
     List<Expression> expressions = <Expression>[];
     for (LocatedMessage error in recoverableErrors) {
+      errors.add(error);
       String message = deprecated_formatUnexpected(
           error.uri, error.charOffset, error.message);
-      errors.add(message);
       expressions.add(new StringLiteral(message));
     }
     mainLibrary.library.addMember(new Field(new Name("#errors"),
         initializer: new ListLiteral(expressions, isConst: true),
         isConst: true));
   }
 
   Program erroneousProgram(bool isFullProgram) {
     Uri uri = loader.first?.uri ?? Uri.parse("error:error");
     Uri fileUri = loader.first?.fileUri ?? uri;
     KernelLibraryBuilder library =
         new KernelLibraryBuilder(uri, fileUri, loader, false);
     loader.first = library;
     if (isFullProgram) {
       // If this is an outline, we shouldn't add an executable main
       // method. Similarly considerations apply to separate compilation. It
       // could also make sense to add a way to mark .dill files as having
       // compile-time errors.
       KernelProcedureBuilder mainBuilder = new KernelProcedureBuilder(null, 0,
           null, "main", null, null, ProcedureKind.Method, library, -1, -1, -1);
       library.addBuilder(mainBuilder.name, mainBuilder, -1);
-      mainBuilder.body = new ExpressionStatement(
-          new Throw(new StringLiteral("${errors.join('\n')}")));
+      mainBuilder.body = new ExpressionStatement(new Throw(
+          new StringLiteral("${errors.map((e) => e.message).join('\n')}")));

[Siggi Cherem (dart-lang), 2017/07/12 00:35:15]

this changes the generated code a bit, one of our tests shows the difference in
the `.expect` files.

[ahe, 2017/07/12 13:14:43]

What's currently stored in the .expect files helps me catch problems when I
break the compiler's output. One can argue that it isn't the purpose of the
.expect files, but as I consider alternative, I'm actually starting to believe
that having formatted messages in the .expect files is a convenient way to
ensure that everything works as intended.

[Siggi Cherem (dart-lang), 2017/07/12 21:54:59]

reverted this for now as well

     }
     library.build(loader.coreLibrary);
     return link(<Library>[library.library]);
   }
 
   /// Creates a program by combining [libraries] with the libraries of
   /// `dillTarget.loader.program`.
   Program link(List<Library> libraries, {CanonicalName nameRoot}) {
     Map<String, Source> uriToSource =
         new Map<String, Source>.from(this.uriToSource);
@@ skipping 282 matching lines @@
   void runBuildTransformations() {
     backendTarget.performModularTransformationsOnLibraries(
         loader.coreTypes, loader.hierarchy, loader.libraries,
         logger: (String msg) => ticker.logMs(msg));
     backendTarget.performGlobalTransformations(loader.coreTypes, program,
         logger: (String msg) => ticker.logMs(msg));
   }
 
   void verify() {
     var verifyErrors = verifyProgram(program);
-    errors.addAll(verifyErrors.map((error) => '$error'));
+    errors.addAll(verifyErrors);
     ticker.logMs("Verified program");
   }
 
   /// Return `true` if the given [library] was built by this [KernelTarget]
   /// from sources, and not loaded from a [DillTarget].
   bool isSourceLibrary(Library library) {
     return loader.libraries.contains(library);
   }
 }
 
 /// Looks for a constructor call that matches `super()` from a constructor in
 /// [cls]. Such a constructor may have optional arguments, but no required
 /// arguments.
 Constructor defaultSuperConstructor(Class cls) {
   Class superclass = cls.superclass;
   while (superclass != null && superclass.isMixinApplication) {
     superclass = superclass.superclass;
   }
   for (Constructor constructor in superclass.constructors) {
     if (constructor.name.name.isEmpty) {
       return constructor.function.requiredParameterCount == 0
           ? constructor
           : null;
     }
   }
   return null;
 }