feat(dart): Add a dev-mode check for undeclared lifecycle interfaces
Add a check in ReflectionCapabilities#interfaces which determines if the passed-in type implements a Lifecycle Interface but does not declare that it does so. See https://goo.gl/b07Kii for details. Closes #6849
This commit is contained in:
		
							parent
							
								
									8430927e6b
								
							
						
					
					
						commit
						1c20a62611
					
				| @ -1,9 +1,12 @@ | ||||
| library reflection.reflection_capabilities; | ||||
| 
 | ||||
| import 'package:angular2/src/facade/lang.dart'; | ||||
| import 'types.dart'; | ||||
| 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'; | ||||
| 
 | ||||
| var DOT_REGEX = new RegExp('\\.'); | ||||
| 
 | ||||
| @ -272,7 +275,9 @@ class ReflectionCapabilities implements PlatformReflectionCapabilities { | ||||
|   } | ||||
| 
 | ||||
|   List interfaces(type) { | ||||
|     return _interfacesFromMirror(reflectType(type)); | ||||
|     final clazz = reflectType(type); | ||||
|     _assertDeclaresLifecycleHooks(clazz); | ||||
|     return _interfacesFromMirror(clazz); | ||||
|   } | ||||
| 
 | ||||
|   List _interfacesFromMirror(classMirror) { | ||||
| @ -324,3 +329,103 @@ class ReflectionCapabilities implements PlatformReflectionCapabilities { | ||||
|     return '${(reflectClass(type).owner as LibraryMirror).uri}'; | ||||
|   } | ||||
| } | ||||
| 
 | ||||
| final _lifecycleHookMirrors = <ClassMirror>[ | ||||
|   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 = <ClassMirror>[]; | ||||
|   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<ClassMirror> 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(); | ||||
|   } | ||||
| } | ||||
|  | ||||
| @ -7,6 +7,7 @@ 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 { | ||||
| @ -65,6 +66,19 @@ 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; | ||||
| @ -214,6 +228,14 @@ 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(); | ||||
|            }); | ||||
|       }); | ||||
|     } | ||||
| 
 | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user