From e856173b1818a9e092c1d9ac929fe5135bdef818 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 7 Mar 2018 13:41:46 -0800 Subject: [PATCH] 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. --- .travis.yml | 3 +- CHANGELOG.md | 6 ++ lib/source_gen.dart | 2 +- lib/src/type_checker.dart | 126 +++++++++++++++++++++++++++++------- pubspec.yaml | 2 +- test/type_checker_test.dart | 27 ++++++-- 6 files changed, 134 insertions(+), 32 deletions(-) diff --git a/.travis.yml b/.travis.yml index 418b907b..7e4cc440 100644 --- a/.travis.yml +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 8163106c..c35e0d44 100644 --- a/CHANGELOG.md +++ b/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()`, diff --git a/lib/source_gen.dart b/lib/source_gen.dart index 6cefe040..d7cc2ebf 100644 --- a/lib/source_gen.dart +++ b/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; diff --git a/lib/src/type_checker.dart b/lib/src/type_checker.dart index d1539379..dd5b6c66 100644 --- a/lib/src/type_checker.dart +++ b/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 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 annotationsOf( + Element element, { + bool throwOnUnresolved, + }) => + _annotationsWhere( + element, + isAssignableFromType, + throwOnUnresolved: throwOnUnresolved, + ); + + Iterable _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 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 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'); +} diff --git a/pubspec.yaml b/pubspec.yaml index d752238f..f6d6d6d7 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: source_gen -version: 0.7.6-dev +version: 0.7.6 author: Dart Team description: Automated source code generation for Dart. homepage: https://github.com/dart-lang/source_gen diff --git a/test/type_checker_test.dart b/test/type_checker_test.dart index 6ba17aca..dd118d18 100644 --- a/test/type_checker_test.dart +++ b/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(), + 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());