Skip to content
This repository has been archived by the owner on Sep 16, 2022. It is now read-only.

Commit

Permalink
fix(Compiler): Fail with better errors on an invalid implicit provider.
Browse files Browse the repository at this point in the history
Previously, we would give a mysterious type error attempting to cast to ClassElement. We also now include the file entrypoint being parsed, and the source of the type itself in the error message.

Closes #1633.

PiperOrigin-RevId: 216255169
  • Loading branch information
matanlurey authored and alorenzen committed Oct 8, 2018
1 parent d65d6eb commit 99a34cb
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 12 deletions.
5 changes: 5 additions & 0 deletions angular/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,10 @@
messages with meanings that are formatted differently will now properly be
treated as the same message.

* [#1633][]: Using a function type or any non-class `Type` inside of the
`@GenerateInjector([...])` annotation would cause a non-ideal error to be
produced. It now includes more information where available.

[#434]: https://github.com/dart-lang/angular/issues/434
[#880]: https://github.com/dart-lang/angular/issues/880
[#930]: https://github.com/dart-lang/angular/issues/930
Expand All @@ -217,6 +221,7 @@
[#1591]: https://github.com/dart-lang/angular/issues/1591
[#1598]: https://github.com/dart-lang/angular/issues/1598
[#1625]: https://github.com/dart-lang/angular/issues/1625
[#1633]: https://github.com/dart-lang/angular/issues/1633

### Other improvements

Expand Down
3 changes: 3 additions & 0 deletions angular_compiler/lib/src/analyzer/di/injector.dart
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ class InjectorReader {
_throwParseError(e.constant);
} on NullFactoryException catch (e) {
_throwFactoryProvider(e.constant);
} on BuildError {
logWarning('An error occurred parsing providers on $doNotScope');
rethrow;
}
}
return _providers;
Expand Down
26 changes: 15 additions & 11 deletions angular_compiler/lib/src/analyzer/di/providers.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'package:analyzer/dart/constant/value.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:angular_compiler/cli.dart';
import 'package:meta/meta.dart';
import 'package:source_gen/source_gen.dart';

Expand Down Expand Up @@ -35,8 +36,8 @@ class ProviderReader {
throw ArgumentError.notNull();
}
if (isType(o)) {
// Represents "Foo", which is supported short-hand for "Provider(Foo)".
return _parseType(o);
// Represents "Foo", which is legacy short-hand for "ClassProvider(Foo)".
return _parseTypeAsImplicitClassProvider(o);
}
if (!isProvider(o)) {
final typeName = getTypeName(o.type);
Expand Down Expand Up @@ -172,16 +173,19 @@ class ProviderReader {
}

/// Returns a provider element representing a single type.
ProviderElement _parseType(DartObject o) {
ProviderElement _parseTypeAsImplicitClassProvider(DartObject o) {
final reader = ConstantReader(o);
final clazz = reader.typeValue.element as ClassElement;
final token = linkTypeOf(clazz.type);
return UseClassProviderElement(
TypeTokenElement(token),
linkTypeOf(typeArgumentOf(o)),
token,
dependencies: _dependencyReader.parseDependencies(clazz),
);
final element = reader.typeValue.element;
if (element is ClassElement) {
final token = linkTypeOf(element.type);
return UseClassProviderElement(
TypeTokenElement(token),
linkTypeOf(typeArgumentOf(o)),
token,
dependencies: _dependencyReader.parseDependencies(element),
);
}
return BuildError.throwForElement(element, 'Not a class element');
}
}

Expand Down
5 changes: 4 additions & 1 deletion angular_compiler/lib/src/cli/logging.dart
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ class BuildError extends Error {
// https://github.com/dart-lang/angular/issues/902#issuecomment-366330965
final source = element.source;
if (source == null || source.contents.data.isEmpty) {
logWarning('Could not find source $element: the next error may be terse');
final warning = source == null
? 'No source text available for $element'
: 'No source text available for $element (${source.uri})';
logWarning('$warning: the next error may be terse');
throw BuildError(message, trace);
}
final sourceUrl = source.uri;
Expand Down

0 comments on commit 99a34cb

Please sign in to comment.