Use messages for (some) public API errors

pkg/front_end/lib/src/base/processed_options.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.
 
 import 'dart:async';
 
 import 'package:front_end/compilation_error.dart';
 import 'package:front_end/compiler_options.dart';
 import 'package:front_end/file_system.dart';
 import 'package:front_end/src/base/performace_logger.dart';
+import 'package:front_end/src/fasta/fasta_codes.dart';
 import 'package:front_end/src/fasta/ticker.dart';
 import 'package:front_end/src/fasta/translate_uri.dart';
 import 'package:front_end/src/incremental/byte_store.dart';
 import 'package:front_end/src/multi_root_file_system.dart';
 import 'package:kernel/kernel.dart'
     show Program, loadProgramFromBytes, CanonicalName;
 import 'package:kernel/target/targets.dart';
 import 'package:kernel/target/vm_fasta.dart';
 import 'package:package_config/packages_file.dart' as package_config;
 import 'package:source_span/source_span.dart' show SourceSpan;
@@ skipping 80 matching lines @@
   /// The logger to report compilation progress.
   PerformanceLog get logger {
     return _raw.logger;
   }
 
   /// The byte storage to get and put serialized data.
   ByteStore get byteStore {
     return _raw.byteStore;
   }
 
-  // TODO(sigmund): delete. We should use messages with error codes directly
-  // instead.
-  void reportError(String message) {
-    _raw.onError(new _CompilationError(message));
+  // TODO(sigmund): delete. Use reportMessage instead.
+  void deprecated_reportError(String error) {

[Siggi Cherem (dart-lang), 2017/07/11 03:57:13]

FYI -  I'm following Peter's style here, it is a transitional name to help us
migrate use sites.

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

Thank you, this is helpful as I'm grepping for that prefix.

+    _raw.onError(new _CompilationError(error));
   }
 
+  void reportMessage(LocatedMessage message) {
+    _raw.onError(message);
+  }
+
+  void reportMessageNoLocation(Message message) =>

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

reportMessageNoLocation -> reportMessageWithoutLocation?

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

Done.

+      reportMessage(message.withLocation(null, -1));
+
   /// Runs various validations checks on the input options. For instance,
   /// if an option is a path to a file, it checks that the file exists.
   Future<bool> validateOptions() async {
     for (var source in inputs) {
       if (source.scheme == 'file' &&
           !await fileSystem.entityForUri(source).exists()) {
-        reportError("Entry-point file not found: $source");
+        reportMessageNoLocation(
+            templateMissingInputFile.withArguments('$source'));
         return false;
       }
     }
 
     if (_raw.sdkRoot != null &&
         !await fileSystem.entityForUri(sdkRoot).exists()) {
-      reportError("SDK root directory not found: ${sdkRoot}");
+      reportMessageNoLocation(templateMissingSdkRoot.withArguments('$sdkRoot'));
       return false;
     }
 
     var summary = sdkSummary;
     if (summary != null && !await fileSystem.entityForUri(summary).exists()) {
-      reportError("SDK summary not found: ${summary}");
+      reportMessageNoLocation(
+          templateMissingSdkSummary.withArguments('$summary'));
       return false;
     }
 
     if (compileSdk && summary != null) {
-      reportError(
-          "The compileSdk and sdkSummary options are mutually exclusive");
+      reportMessageNoLocation(messageCombinedCompileSdkAndSummary);

[Paul Berry, 2017/07/11 16:00:59]

IMO there should be a distinction between errors that are likely due to the user
doing something wrong vs. errors that are almost certainly due to a bug in our
code.  The former kinds of errors make sense to report using our error reporting
mechanism.  For the latter, I think we should have a separate "internal error"
reporting mechanism.

For this case here (compileSdk is `true` and a summary was provided), the only
way it could occur is due to a bug in the code invoking the front-end, so from
the user's point of view it's an internal error.

Having said that, it might perhaps make sense to have a single user-visible
error code called "InternalError" (with room for a string containing details for
the benefit of people developing the front end and its clients).  *If* we decide
to do that, then we could use that InternalError code here, with a hardcoded
string detailing that the problem is that both compileSdk and a summary were
specified.

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

Good points!

I like however having precise messages when we know what the problem is. In
particular, I'd like to get to the point where we have a searchable database for
errors, even internal ones.

How about we add a bool to [Code] that indicates if the error is internal?

       return false;
     }
     return true;
   }
 
   /// Determine whether to generate code for the SDK when compiling a
   /// whole-program.
   bool get compileSdk => _raw.compileSdk;
 
   FileSystem _fileSystem;
@@ skipping 185 matching lines @@
             'Invalid access to $uri: '
             'the file is accessed in a modular hermetic build, '
             'but it was not explicitly listed as an input.');
 
   @override
   String toString() => message;
 }
 
 /// An error that only contains a message and no error location.
 class _CompilationError implements CompilationError {
-  String get correction => null;
+  String get tip => null;
   SourceSpan get span => null;
   final String message;
   _CompilationError(this.message);
 
   String toString() => message;
 }