Add better errors to TypeChecker. (#317)

* Add better errors to TypeChecker.

* Add bug.

* Fix tests.

* Only dartfmt in dev.

* Fix test and travis.

* Address feedback.

Author: matanlurey

.travis.yml

@@ -2,8 +2,7 @@ language: dart
 
 dart:
   - dev
-  # Flutter Alpha @ v0.0.23
-  - 2.0.0-dev.19.0
+
 dart_task:
   - test
   - dartfmt

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 0.7.6
+
+* `TypeChecker` now throws an `UnresolvedAnnotationException` with a more
+  detailed exception body (and properties useful for further debugging) instead
+  of `Could not resolve @null`.
+
 ## 0.7.5+1
 
 * `LibraryBuilder` and `PartBuilder` now have a more readable `toString()`,

lib/source_gen.dart

@@ -11,5 +11,5 @@ export 'src/generator.dart';
 export 'src/generator_for_annotation.dart';
 export 'src/library.dart' show AnnotatedElement, LibraryReader;
 export 'src/span_for_element.dart' show spanForElement;
-export 'src/type_checker.dart' show TypeChecker;
+export 'src/type_checker.dart' show TypeChecker, UnresolvedAnnotationException;
 export 'src/utils.dart' show typeNameOf;

lib/src/type_checker.dart

@@ -2,11 +2,15 @@
 // 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:mirrors';
+import 'dart:mirrors' hide SourceLocation;
 
 import 'package:analyzer/dart/constant/value.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
+// TODO(https://github.com/dart-lang/sdk/issues/32454):
+// ignore: implementation_imports
+import 'package:analyzer/src/dart/element/element.dart';
+import 'package:source_span/source_span.dart';
 
 import 'utils.dart';
 
@@ -74,7 +78,8 @@ abstract class TypeChecker {
 
   /// Returns the first constant annotating [element] that is exactly this type.
   ///
-  /// Throws on unresolved annotations unless [throwOnUnresolved] is `false`.
+  /// Throws [UnresolvedAnnotationException] on unresolved annotations unless
+  /// [throwOnUnresolved] is explicitly set to `false` (default is `true`).
   DartObject firstAnnotationOfExact(Element element, {bool throwOnUnresolved}) {
     if (element.metadata.isEmpty) {
       return null;
@@ -86,42 +91,70 @@ abstract class TypeChecker {
 
   /// Returns if a constant annotating [element] is exactly this type.
   ///
-  /// Throws on unresolved annotations unless [throwOnUnresolved] is `false`.
+  /// Throws [UnresolvedAnnotationException] on unresolved annotations unless
+  /// [throwOnUnresolved] is explicitly set to `false` (default is `true`).
   bool hasAnnotationOfExact(Element element, {bool throwOnUnresolved}) =>
       firstAnnotationOfExact(element, throwOnUnresolved: throwOnUnresolved) !=
       null;
 
-  DartObject _computeConstantValue(ElementAnnotation annotation,
-      {bool throwOnUnresolved}) {
+  DartObject _computeConstantValue(
+    Element element,
+    int annotationIndex, {
+    bool throwOnUnresolved,
+  }) {
     throwOnUnresolved ??= true;
+    final annotation = element.metadata[annotationIndex];
     final result = annotation.computeConstantValue();
     if (result == null && throwOnUnresolved) {
-      throw new StateError(
-          'Could not resolve $annotation. An import or dependency may be '
-          'missing or invalid.');
+      throw new UnresolvedAnnotationException._from(element, annotationIndex);
     }
     return result;
   }
 
   /// Returns annotating constants on [element] assignable to this type.
   ///
-  /// Throws on unresolved annotations unless [throwOnUnresolved] is `false`.
-  Iterable<DartObject> annotationsOf(Element element,
-          {bool throwOnUnresolved}) =>
-      element.metadata
-          .map((annotation) => _computeConstantValue(annotation,
-              throwOnUnresolved: throwOnUnresolved))
-          .where((a) => a?.type != null && isAssignableFromType(a.type));
+  /// Throws [UnresolvedAnnotationException] on unresolved annotations unless
+  /// [throwOnUnresolved] is explicitly set to `false` (default is `true`).
+  Iterable<DartObject> annotationsOf(
+    Element element, {
+    bool throwOnUnresolved,
+  }) =>
+      _annotationsWhere(
+        element,
+        isAssignableFromType,
+        throwOnUnresolved: throwOnUnresolved,
+      );
+
+  Iterable<DartObject> _annotationsWhere(
+    Element element,
+    bool Function(DartType) predicate, {
+    bool throwOnUnresolved,
+  }) sync* {
+    for (var i = 0; i < element.metadata.length; i++) {
+      final value = _computeConstantValue(
+        element,
+        i,
+        throwOnUnresolved: throwOnUnresolved,
+      );
+      if (value?.type != null && predicate(value.type)) {
+        yield value;
+      }
+    }
+  }
 
   /// Returns annotating constants on [element] of exactly this type.
   ///
-  /// Throws on unresolved annotations unless [throwOnUnresolved] is `false`.
-  Iterable<DartObject> annotationsOfExact(Element element,
-          {bool throwOnUnresolved}) =>
-      element.metadata
-          .map((annotation) => _computeConstantValue(annotation,
-              throwOnUnresolved: throwOnUnresolved))
-          .where((a) => a?.type != null && isExactlyType(a.type));
+  /// Throws [UnresolvedAnnotationException] on unresolved annotations unless
+  /// [throwOnUnresolved] is explicitly set to `false` (default is `true`).
+  Iterable<DartObject> annotationsOfExact(
+    Element element, {
+    bool throwOnUnresolved,
+  }) =>
+      _annotationsWhere(
+        element,
+        isExactlyType,
+        throwOnUnresolved: throwOnUnresolved,
+      );
 
   /// Returns `true` if the type of [element] can be assigned to this type.
   bool isAssignableFrom(Element element) =>
@@ -242,3 +275,52 @@ class _AnyChecker extends TypeChecker {
   @override
   bool isExactly(Element element) => _checkers.any((c) => c.isExactly(element));
 }
+
+/// Exception thrown when [TypeChecker] fails to resolve a metadata annotation.
+///
+/// Methods such as [TypeChecker.firstAnnotationOf] may throw this exception
+/// when one or more annotations are not resolvable. This is usually a sign that
+/// something was misspelled, an import is missing, or a dependency was not
+/// defined (for build systems such as Bazel).
+class UnresolvedAnnotationException implements Exception {
+  /// Element that was annotated with something we could not resolve.
+  final Element annotatedElement;
+
+  /// Source span of the annotation that was not resolved.
+  final SourceSpan annotationSource;
+
+  // TODO: Remove internal API once ElementAnnotation has source information.
+  // https://github.com/dart-lang/sdk/issues/32454
+  static SourceSpan _getSourceSpanFrom(ElementAnnotation annotation) {
+    final internals = annotation as ElementAnnotationImpl;
+    final astNode = internals.annotationAst;
+    final contents = annotation.source.contents.data;
+    final start = astNode.offset;
+    final end = start + astNode.length;
+    return new SourceSpan(
+      new SourceLocation(start, sourceUrl: annotation.source.uri),
+      new SourceLocation(end, sourceUrl: annotation.source.uri),
+      contents.substring(start, end),
+    );
+  }
+
+  /// Creates an exception from an annotation ([annotationIndex]) that was not
+  /// resolvable while traversing [Element.metadata] on [annotatedElement].
+  factory UnresolvedAnnotationException._from(
+    Element annotatedElement,
+    int annotationIndex,
+  ) {
+    final annotation = annotatedElement.metadata[annotationIndex];
+    final sourceSpan = _getSourceSpanFrom(annotation);
+    return new UnresolvedAnnotationException._(annotatedElement, sourceSpan);
+  }
+
+  const UnresolvedAnnotationException._(
+    this.annotatedElement,
+    this.annotationSource,
+  );
+
+  @override
+  String toString() => annotationSource
+      .message('Could not resolve annotation for $annotatedElement');
+}

pubspec.yaml

@@ -1,5 +1,5 @@
 name: source_gen
-version: 0.7.6-dev
+version: 0.7.6
 author: Dart Team <misc@dartlang.org>
 description: Automated source code generation for Dart.
 homepage: https://github.com/dart-lang/source_gen

test/type_checker_test.dart

@@ -195,8 +195,16 @@ void main() {
     final classX = library.getType('X');
     final $deprecated = const TypeChecker.fromRuntime(Deprecated);
 
-    expect(() => $deprecated.annotationsOf(classX), throwsStateError,
-        reason: 'deprecated was spelled wrong; no annotation can be resolved');
+    expect(
+      () => $deprecated.annotationsOf(classX),
+      throwsA(allOf(
+          const isInstanceOf<UnresolvedAnnotationException>(),
+          predicate((e) => e
+              .toString()
+              .contains('Could not resolve annotation for class X')),
+          predicate((e) => e.toString().contains('@depracated')))),
+      reason: 'deprecated was spelled wrong; no annotation can be resolved',
+    );
   });
 
   test('should check multiple checkers', () {
@@ -313,10 +321,14 @@ void main() {
     });
 
     test('should throw by default', () {
-      expect(() => $A.firstAnnotationOf($ExampleOfA), throwsStateError);
-      expect(() => $A.annotationsOf($ExampleOfA), throwsStateError);
-      expect(() => $A.firstAnnotationOfExact($ExampleOfA), throwsStateError);
-      expect(() => $A.annotationsOfExact($ExampleOfA), throwsStateError);
+      expect(() => $A.firstAnnotationOf($ExampleOfA),
+          throwsUnresolvedAnnotationException);
+      expect(() => $A.annotationsOf($ExampleOfA),
+          throwsUnresolvedAnnotationException);
+      expect(() => $A.firstAnnotationOfExact($ExampleOfA),
+          throwsUnresolvedAnnotationException);
+      expect(() => $A.annotationsOfExact($ExampleOfA),
+          throwsUnresolvedAnnotationException);
     });
 
     test('should not throw if `throwOnUnresolved` == false', () {
@@ -342,3 +354,6 @@ void main() {
     });
   });
 }
+
+final throwsUnresolvedAnnotationException =
+    throwsA(const isInstanceOf<UnresolvedAnnotationException>());