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/uri_translator.dart';
 import 'package:front_end/src/fasta/uri_translator_impl.dart';
+import 'package:front_end/src/fasta/problems.dart' show unimplemented;
 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;
+import 'package:source_span/source_span.dart' show SourceSpan, SourceLocation;
 
 /// All options needed for the front end implementation.
 ///
 /// This includes: all of [CompilerOptions] in a form useful to the
 /// implementation, default values for options that were not provided,
 /// and information derived from how the compiler was invoked (like the
 /// entry-points given to the compiler and whether a modular or whole-program
 /// API was used).
 ///
 /// The intent is that the front end should immediately wrap any incoming
@@ skipping 72 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,ahe): delete in favor of reportMessage.
+  void deprecated_reportError(String error) {
+    _raw.onError(new _StringMessage(error));
   }
 
+  void reportMessage(LocatedMessage message) {
+    _raw.onError(new _CompilationMessage(message));

[ahe, 2017/07/13 00:03:38]

We need to think about how these methods deal with _raw.onError throwing an
exception.

We do not want to report that as a crash if we add crash reporting like in
dart2js.

[Siggi Cherem (dart-lang), 2017/07/18 00:32:57]

Acknowledged.

+  }
+
+  void reportMessageWithoutLocation(Message message) =>
+      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");
+        reportMessageWithoutLocation(
+            templateMissingInputFile.withArguments('$source'));

[ahe, 2017/07/13 00:03:38]

There's a #uri parameter you can use now. It will automatically make the URI
relative, but preserve the full URI in the arguments object.

[Siggi Cherem (dart-lang), 2017/07/18 00:32:57]

Done.

         return false;
       }
     }
 
     if (_raw.sdkRoot != null &&
         !await fileSystem.entityForUri(sdkRoot).exists()) {
-      reportError("SDK root directory not found: ${sdkRoot}");
+      reportMessageWithoutLocation(
+          templateMissingSdkRoot.withArguments('$sdkRoot'));

[ahe, 2017/07/13 00:03:38]

Ditto.

[Siggi Cherem (dart-lang), 2017/07/18 00:32:57]

Done.

       return false;
     }
 
     var summary = sdkSummary;
     if (summary != null && !await fileSystem.entityForUri(summary).exists()) {
-      reportError("SDK summary not found: ${summary}");
+      reportMessageWithoutLocation(
+          templateMissingSdkSummary.withArguments('$summary'));

[ahe, 2017/07/13 00:03:38]

Ditto.

[Siggi Cherem (dart-lang), 2017/07/18 00:32:57]

Done.

       return false;
     }
 
     if (compileSdk && summary != null) {
-      reportError(
-          "The compileSdk and sdkSummary options are mutually exclusive");
+      reportMessageWithoutLocation(
+          templateInternalProblemUnsupported.withArguments(
+              "The compileSdk and sdkSummary options are mutually exclusive"));

[ahe, 2017/07/13 00:03:37]

I think this should be its own error code, but I can live with this.

I think so because this error is likely to be seen by users of the API.

[Siggi Cherem (dart-lang), 2017/07/18 00:32:57]

Done.

       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 82 matching lines @@
   }
 
   /// Get the package map which maps package names to URIs.
   ///
   /// This is an asynchronous getter since file system operations may be
   /// required to locate/read the packages file.
   Future<Map<String, Uri>> _getPackages() async {
     if (_packages == null) {
       if (_raw.packagesFileUri == null) {
         // TODO(sigmund,paulberry): implement
-        throw new UnimplementedError('search for .packages');
+        return unimplemented('search for .packages');
       } else if (_raw.packagesFileUri.path.isEmpty) {
         _packages = {};
       } else {
         var contents =
             await fileSystem.entityForUri(_raw.packagesFileUri).readAsBytes();
         _packages = package_config.parse(contents, _raw.packagesFileUri);
       }
     }
     return _packages;
   }
 
   /// Get the location of the SDK.
   Uri _normalizeSdkRoot() {
     // If an SDK summary location was provided, the SDK itself should not be
     // needed.
     assert(_raw.sdkSummary == null);
     if (_raw.sdkRoot == null) {
       // TODO(paulberry): implement the algorithm for finding the SDK
       // automagically.
-      throw new UnimplementedError('infer the default sdk location');
+      return unimplemented('infer the default sdk location');
     }
     var root = _raw.sdkRoot;
     if (!root.path.endsWith('/')) {
       root = root.replace(path: root.path + '/');
     }
     return root;
   }
 
   /// Get or infer the location of the SDK summary.
   Uri _computeSdkSummaryUri() {
@@ skipping 60 matching lines @@
       : super(
             uri,
             '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;
-  SourceSpan get span => null;
-  final String message;
-  _CompilationError(this.message);
+/// Wraps a [LocatedMessage] to implement the public [CompilationError] API.
+class _CompilationMessage implements CompilationError {
+  final LocatedMessage original;

[ahe, 2017/07/13 00:03:38]

This should probably be private as you've stated several times that properties
of this object aren't exposed in the public API.

[Siggi Cherem (dart-lang), 2017/07/18 00:32:57]

Ok, I've added this to the CL where I'm unifying CompilerContext and
ProcessedOptions.

+
+  String get message => original.message;
+
+  String get tip => original.tip;
+
+  SourceSpan get span =>
+      new SourceLocation(original.charOffset, sourceUrl: original.uri)
+          .pointSpan();
+
+  _CompilationMessage(this.original);
 
   String toString() => message;
 }
+
+/// Wraps an error message as a [CompilationError] API.

[ahe, 2017/07/13 00:03:38]

Use templateUnspecified instead?

[Siggi Cherem (dart-lang), 2017/07/18 00:32:57]

This is now gone.

+class _StringMessage implements CompilationError {
+  final String message;
+  String get tip => null;
+  SourceSpan get span => null;
+
+  _StringMessage(this.message);
+
+  String toString() => message;
+}