From 7455b907d12bb4c6d2dbe5ad7960b08d916ccfab Mon Sep 17 00:00:00 2001 From: Vikram Subramanian Date: Thu, 3 Mar 2016 18:00:18 -0800 Subject: [PATCH] Revert "feat(dart): Add a dev-mode check for undeclared lifecycle interfaces" This reverts commit a3d762913482f2deb6c59220e87c52b686d1b4fa. Needs co-ordination with google3 changes. --- .../reflection/reflection_capabilities.dart | 111 +----------------- .../test/core/reflection/reflector_spec.ts | 22 ---- 2 files changed, 3 insertions(+), 130 deletions(-) diff --git a/modules/angular2/src/core/reflection/reflection_capabilities.dart b/modules/angular2/src/core/reflection/reflection_capabilities.dart index e5709670d8..e390aa6ac8 100644 --- a/modules/angular2/src/core/reflection/reflection_capabilities.dart +++ b/modules/angular2/src/core/reflection/reflection_capabilities.dart @@ -1,12 +1,9 @@ library reflection.reflection_capabilities; -import 'dart:mirrors'; - -import 'package:angular2/src/core/linker/interfaces.dart'; import 'package:angular2/src/facade/lang.dart'; - -import 'platform_reflection_capabilities.dart'; import 'types.dart'; +import 'dart:mirrors'; +import 'platform_reflection_capabilities.dart'; var DOT_REGEX = new RegExp('\\.'); @@ -275,9 +272,7 @@ class ReflectionCapabilities implements PlatformReflectionCapabilities { } List interfaces(type) { - final clazz = reflectType(type); - _assertDeclaresLifecycleHooks(clazz); - return _interfacesFromMirror(clazz); + return _interfacesFromMirror(reflectType(type)); } List _interfacesFromMirror(classMirror) { @@ -329,103 +324,3 @@ class ReflectionCapabilities implements PlatformReflectionCapabilities { return '${(reflectClass(type).owner as LibraryMirror).uri}'; } } - -final _lifecycleHookMirrors = [ - reflectType(AfterContentChecked), - reflectType(AfterContentInit), - reflectType(AfterViewChecked), - reflectType(AfterViewInit), - reflectType(DoCheck), - reflectType(OnChanges), - reflectType(OnDestroy), - reflectType(OnInit), -]; - -/// Checks whether [clazz] implements lifecycle ifaces without declaring them. -/// -/// Due to Dart implementation details, lifecycle hooks are only called when a -/// class explicitly declares that it implements the associated interface. -/// See https://goo.gl/b07Kii for details. -void _assertDeclaresLifecycleHooks(ClassMirror clazz) { - final missingDeclarations = []; - for (var iface in _lifecycleHookMirrors) { - if (!_checkDeclares(clazz, iface: iface) && - _checkImplements(clazz, iface: iface)) { - missingDeclarations.add(iface); - } - } - if (missingDeclarations.isNotEmpty) { - throw new MissingInterfaceError(clazz, missingDeclarations); - } -} - -/// Returns whether [clazz] declares that it implements [iface]. -/// -/// Returns `false` if [clazz] implements [iface] but does not declare it. -/// Returns `false` if [clazz]'s superclass declares that it -/// implements [iface]. -bool _checkDeclares(ClassMirror clazz, {ClassMirror iface: null}) { - if (iface == null) { - throw new ArgumentError.notNull('iface'); - } - return clazz.superinterfaces.contains(iface); -} - -/// Returns whether [clazz] implements [iface]. -/// -/// Returns `true` if [clazz] implements [iface] and does not declare it. -/// Returns `true` if [clazz]'s superclass implements [iface]. -/// -/// This is an approximation of a JavaScript feature check: -/// ```js -/// var matches = true; -/// for (var prop in iface) { -/// if (iface.hasOwnProperty(prop)) { -/// matches = matches && clazz.hasOwnProperty(prop); -/// } -/// } -/// return matches; -/// ``` -bool _checkImplements(ClassMirror clazz, {ClassMirror iface: null}) { - if (iface == null) { - throw new ArgumentError.notNull('iface'); - } - - var matches = true; - iface.declarations.forEach((symbol, declarationMirror) { - if (!matches) return; - if (declarationMirror.isConstructor || declarationMirror.isPrivate) return; - matches = clazz.declarations.keys.contains(symbol); - }); - if (!matches && clazz.superclass != null) { - matches = _checkImplements(clazz.superclass, iface: iface); - } - if (!matches && clazz.mixin != clazz) { - matches = _checkImplements(clazz.mixin, iface: iface); - } - - return matches; -} - -/// Error thrown when a class implements a lifecycle iface it does not declare. -class MissingInterfaceError extends Error { - final ClassMirror clazz; - final List missingDeclarations; - - MissingInterfaceError(this.clazz, this.missingDeclarations); - - @override - String toString() { - final buf = new StringBuffer(); - buf.write('${clazz.simpleName} implements '); - if (missingDeclarations.length == 1) { - buf.write('an interface but does not declare it: '); - } else { - buf.write('interfaces but does not declare them: '); - } - buf.write( - missingDeclarations.map((d) => d.simpleName.toString()).join(', ')); - buf.write('. See https://goo.gl/b07Kii for more info.'); - return buf.toString(); - } -} diff --git a/modules/angular2/test/core/reflection/reflector_spec.ts b/modules/angular2/test/core/reflection/reflector_spec.ts index 2b82edd2fd..d1b9e64e92 100644 --- a/modules/angular2/test/core/reflection/reflector_spec.ts +++ b/modules/angular2/test/core/reflection/reflector_spec.ts @@ -7,7 +7,6 @@ import { beforeEach, browserDetection } from 'angular2/testing_internal'; -import {OnInit} from 'angular2/core'; import {Reflector, ReflectionInfo} from 'angular2/src/core/reflection/reflection'; import {ReflectionCapabilities} from 'angular2/src/core/reflection/reflection_capabilities'; import { @@ -66,19 +65,6 @@ class SuperClassImplementingInterface implements Interface2 {} class ClassImplementingInterface extends SuperClassImplementingInterface implements Interface {} -// Classes used to test our runtime check for classes that implement lifecycle interfaces but do not -// declare them. -// See https://github.com/angular/angular/pull/6879 and https://goo.gl/b07Kii for details. -class ClassDoesNotDeclareOnInit { - ngOnInit() {} -} - -class SuperClassImplementingOnInit implements OnInit { - ngOnInit() {} -} - -class SubClassDoesNotDeclareOnInit extends SuperClassImplementingOnInit {} - export function main() { describe('Reflector', () => { var reflector; @@ -228,14 +214,6 @@ export function main() { var p = reflector.interfaces(ClassWithDecorators); expect(p).toEqual([]); }); - - it("should throw for undeclared lifecycle interfaces", - () => { expect(() => reflector.interfaces(ClassDoesNotDeclareOnInit)).toThrowError(); }); - - it("should throw for class inheriting a lifecycle impl and not declaring the interface", - () => { - expect(() => reflector.interfaces(SubClassDoesNotDeclareOnInit)).toThrowError(); - }); }); }