From 03063f3c1e86ffea53d77281f784f55b9d75394b Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 7 Mar 2018 11:32:41 -0800 Subject: [PATCH 1/6] Add better errors to TypeChecker. --- CHANGELOG.md | 6 ++ lib/source_gen.dart | 2 +- lib/src/type_checker.dart | 124 +++++++++++++++++++++++++++++------- pubspec.yaml | 2 +- test/type_checker_test.dart | 17 ++++- 5 files changed, 125 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8163106c..7a3ec208 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 0.7.5+2 + +* `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..6c167e7f 100644 --- a/lib/src/type_checker.dart +++ b/lib/src/type_checker.dart @@ -2,11 +2,14 @@ // 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'; +// ignore: implementation_imports +import 'package:analyzer/src/dart/element/element.dart'; +import 'package:source_span/source_span.dart'; import 'utils.dart'; @@ -74,7 +77,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 +90,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 +274,51 @@ 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. + 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..f4c9afe6 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: source_gen -version: 0.7.6-dev +version: 0.7.5+2 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..70081d37 100644 --- a/test/type_checker_test.dart +++ b/test/type_checker_test.dart @@ -195,8 +195,21 @@ 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( + '@depreacated', + )), + ), + ), + reason: 'deprecated was spelled wrong; no annotation can be resolved', + ); }); test('should check multiple checkers', () { From 9bfb1cf9df4bd1dc38c373f6746452e16bf1f46b Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 7 Mar 2018 11:34:19 -0800 Subject: [PATCH 2/6] Add bug. --- lib/src/type_checker.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/src/type_checker.dart b/lib/src/type_checker.dart index 6c167e7f..dd5b6c66 100644 --- a/lib/src/type_checker.dart +++ b/lib/src/type_checker.dart @@ -7,6 +7,7 @@ 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'; @@ -289,6 +290,7 @@ class UnresolvedAnnotationException implements Exception { 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; From 52a959c64a782ed833ad1ed675e98072ca9633cf Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 7 Mar 2018 11:40:45 -0800 Subject: [PATCH 3/6] Fix tests. --- .travis.yml | 1 + test/type_checker_test.dart | 30 ++++++++++++++++-------------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/.travis.yml b/.travis.yml index 418b907b..0358bfe5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,7 @@ dart: - dev # Flutter Alpha @ v0.0.23 - 2.0.0-dev.19.0 + dart_task: - test - dartfmt diff --git a/test/type_checker_test.dart b/test/type_checker_test.dart index 70081d37..ac611185 100644 --- a/test/type_checker_test.dart +++ b/test/type_checker_test.dart @@ -197,17 +197,12 @@ void main() { expect( () => $deprecated.annotationsOf(classX), - throwsA( - allOf( + throwsA(allOf( const isInstanceOf(), - predicate((e) => e.toString().contains( - 'Could not resolve annotation for "class X"', - )), - predicate((e) => e.toString().contains( - '@depreacated', - )), - ), - ), + predicate((e) => e + .toString() + .contains('Could not resolve annotation for class X')), + predicate((e) => e.toString().contains('@depreacated')))), reason: 'deprecated was spelled wrong; no annotation can be resolved', ); }); @@ -326,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', () { @@ -355,3 +354,6 @@ void main() { }); }); } + +final throwsUnresolvedAnnotationException = + throwsA(const isInstanceOf()); From 6df91abe7a3a867c94392c0e5319dfd24d08c0f0 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 7 Mar 2018 11:41:50 -0800 Subject: [PATCH 4/6] Only dartfmt in dev. --- .travis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 0358bfe5..a846e3ae 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,7 +7,8 @@ dart: dart_task: - test - - dartfmt + - dartfmt: + dart: dev - dartanalyzer # Only building master means that we don't run two builds for each pull request. From 42c4ffe9f2b624ed203135539b83232bca7ba29d Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 7 Mar 2018 11:59:59 -0800 Subject: [PATCH 5/6] Fix test and travis. --- .travis.yml | 5 +---- test/type_checker_test.dart | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index a846e3ae..7e4cc440 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,13 +2,10 @@ language: dart dart: - dev - # Flutter Alpha @ v0.0.23 - - 2.0.0-dev.19.0 dart_task: - test - - dartfmt: - dart: dev + - dartfmt - dartanalyzer # Only building master means that we don't run two builds for each pull request. diff --git a/test/type_checker_test.dart b/test/type_checker_test.dart index ac611185..dd118d18 100644 --- a/test/type_checker_test.dart +++ b/test/type_checker_test.dart @@ -202,7 +202,7 @@ void main() { predicate((e) => e .toString() .contains('Could not resolve annotation for class X')), - predicate((e) => e.toString().contains('@depreacated')))), + predicate((e) => e.toString().contains('@depracated')))), reason: 'deprecated was spelled wrong; no annotation can be resolved', ); }); From 8aa636b6a77e8e5ca46ac134ca84f8bc60215e29 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 7 Mar 2018 13:41:25 -0800 Subject: [PATCH 6/6] Address feedback. --- CHANGELOG.md | 2 +- pubspec.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a3ec208..c35e0d44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## 0.7.5+2 +## 0.7.6 * `TypeChecker` now throws an `UnresolvedAnnotationException` with a more detailed exception body (and properties useful for further debugging) instead diff --git a/pubspec.yaml b/pubspec.yaml index f4c9afe6..f6d6d6d7 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: source_gen -version: 0.7.5+2 +version: 0.7.6 author: Dart Team description: Automated source code generation for Dart. homepage: https://github.com/dart-lang/source_gen