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

Commit

Permalink
fix(Compiler): Refuse invalid function (null) in FactoryProvider.
Browse files Browse the repository at this point in the history
Turns a (mysterious) runtime error into a compile-time one. Shouldn't impact users.

Closes #1500.
Closes #1581.

PiperOrigin-RevId: 209173141
  • Loading branch information
matanlurey committed Aug 17, 2018
1 parent bb12380 commit 4b1b7cc
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 2 deletions.
1 change: 1 addition & 0 deletions _tests/build.debug.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ targets:
- test/core/di/provider_test.dart*
- test/regression/906_incorrect_injectable_warning_test.dart*
- test/regression/519_missing_query_selector_test.dart*
- test/regression/1500_factory_provider_null_test.dart*
- test/regression/1538_deferred_template_test.dart*

angular:
Expand Down
1 change: 1 addition & 0 deletions _tests/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ targets:
- test/core/di/provider_test.dart*
- test/regression/906_incorrect_injectable_warning_test.dart*
- test/regression/519_missing_query_selector_test.dart*
- test/regression/1500_factory_provider_null_test.dart*
- test/regression/1538_deferred_template_test.dart*
angular:
options:
Expand Down
21 changes: 21 additions & 0 deletions _tests/test/regression/1500_factory_provider_null_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
@TestOn('vm')
import 'package:_tests/compiler.dart';
import 'package:test/test.dart';

// See https://github.com/dart-lang/angular/issues/1500.
void main() {
test('should disallow a value of "null" for FactoryProvider', () async {
await compilesExpecting("""
import '$ngImport';
class Service {}
@GenerateInjector([
FactoryProvider(Service, null),
])
final InjectorFactory injectorFactory = null; // OK for compiler tests.
""", errors: [
contains('an explicit value of `null` was passed in where a function is'),
]);
});
}
4 changes: 4 additions & 0 deletions angular/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,12 @@
`*ngFor="let item of items;"` - note the trailing `;`), throws a proper
unexpected token error instead of a confusing type error during recovery.

* [#1500][]: Configuring a provider with `FactoryProvider(Foo, null)` is now
a compile-time error, instead of a misleading runtime error.

[#434]: https://github.com/dart-lang/angular/issues/434
[#880]: https://github.com/dart-lang/angular/issues/880
[#1500]: https://github.com/dart-lang/angular/issues/1500
[#1502]: https://github.com/dart-lang/angular/issues/1502
[#1538]: https://github.com/dart-lang/angular/issues/1538
[#1539]: https://github.com/dart-lang/angular/issues/1539
Expand Down
2 changes: 2 additions & 0 deletions angular_compiler/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
* Catches an (invalid) `null` token of a provider and throws a better error.

* Catches an (invalid) `null` value of the function for `FactoryProvider`.

## 0.4.0

### New Features
Expand Down
10 changes: 10 additions & 0 deletions angular_compiler/lib/src/analyzer/di/injector.dart
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,14 @@ class InjectorReader {
);
}

@alwaysThrows
void _throwFactoryProvider(DartObject context) {
BuildError.throwForElement(
field,
'Invalid provider ($context): an explicit value of `null` was passed in '
'where a function is expected.');
}

/// Providers that are part of the provided list of the annotation.
Iterable<ProviderElement> get providers {
if (_providers == null) {
Expand All @@ -96,6 +104,8 @@ class InjectorReader {
_providers = moduleReader.deduplicateProviders(module.flatten());
} on NullTokenException catch (e) {
_throwParseError(e.constant);
} on NullFactoryException catch (e) {
_throwFactoryProvider(e.constant);
}
}
return _providers;
Expand Down
16 changes: 14 additions & 2 deletions angular_compiler/lib/src/analyzer/di/providers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ class ProviderReader {
}
if (isType(o)) {
// Represents "Foo", which is supported short-hand for "Provider(Foo)".
// TODO(matanl): Validate that Foo has @Injectable() when flag is set.
return _parseType(o);
}
if (!isProvider(o)) {
Expand All @@ -50,7 +49,7 @@ class ProviderReader {
final reader = ConstantReader(o);
final value = reader.read('token');
if (value.isNull) {
throw new NullTokenException(o);
throw NullTokenException(o);
}
final token = _tokenReader.parseTokenObject(value.objectValue);
final useClass = reader.read('useClass');
Expand Down Expand Up @@ -81,6 +80,11 @@ class ProviderReader {
}
// Base case: const Provider(Foo) with no fields set.
if (token is TypeTokenElement) {
// Ensure this isn't a FactoryProvider with a null function:
// https://github.com/dart-lang/angular/issues/1500
if (!$Provider.isExactlyType(o.type)) {
throw NullFactoryException(o);
}
return _parseUseClass(token, o, reader.read('token').typeValue.element);
}
throw UnsupportedError('Could not parse provider: $o.');
Expand Down Expand Up @@ -338,3 +342,11 @@ class NullTokenException implements Exception {

const NullTokenException(this.constant);
}

/// Thrown when a value of `null` is read for `FactoryProvider`.
class NullFactoryException implements Exception {
/// Constant whose `.token` property was resolved to `null`.
final DartObject constant;

const NullFactoryException(this.constant);
}

0 comments on commit 4b1b7cc

Please sign in to comment.