From 9925aa89dca9f6548cc3900647873cef114a3a60 Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Tue, 2 Aug 2016 14:38:31 -0700 Subject: [PATCH] fix(compiler): Report references to non-exported symbols. Includes fixes to places now reported as errors. Part of #8310 --- .../select_multiple_control_value_accessor.ts | 4 +- .../forms-deprecated/directives/validators.ts | 2 +- .../compiler-cli/src/static_reflector.ts | 4 ++ .../select_multiple_control_value_accessor.ts | 4 +- tools/@angular/tsc-wrapped/src/collector.ts | 6 +++ .../tsc-wrapped/test/collector.spec.ts | 42 +++++++++++++++++++ 6 files changed, 57 insertions(+), 5 deletions(-) diff --git a/modules/@angular/common/src/forms-deprecated/directives/select_multiple_control_value_accessor.ts b/modules/@angular/common/src/forms-deprecated/directives/select_multiple_control_value_accessor.ts index 6117a3cad5..1883e599bf 100644 --- a/modules/@angular/common/src/forms-deprecated/directives/select_multiple_control_value_accessor.ts +++ b/modules/@angular/common/src/forms-deprecated/directives/select_multiple_control_value_accessor.ts @@ -6,14 +6,14 @@ * found in the LICENSE file at https://angular.io/license */ -import {Directive, ElementRef, Host, Input, OnDestroy, Optional, Renderer, forwardRef} from '@angular/core'; +import {Directive, ElementRef, Host, Input, OnDestroy, OpaqueToken, Optional, Renderer, Type, forwardRef} from '@angular/core'; import {MapWrapper} from '../../facade/collection'; import {StringWrapper, isBlank, isPresent, isPrimitive, isString, looseIdentical} from '../../facade/lang'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from './control_value_accessor'; -const SELECT_MULTIPLE_VALUE_ACCESSOR = { +export const SELECT_MULTIPLE_VALUE_ACCESSOR = { provide: NG_VALUE_ACCESSOR, useExisting: forwardRef(() => SelectMultipleControlValueAccessor), multi: true diff --git a/modules/@angular/common/src/forms-deprecated/directives/validators.ts b/modules/@angular/common/src/forms-deprecated/directives/validators.ts index 6292b47878..109a5ab3de 100644 --- a/modules/@angular/common/src/forms-deprecated/directives/validators.ts +++ b/modules/@angular/common/src/forms-deprecated/directives/validators.ts @@ -35,7 +35,7 @@ import {NG_VALIDATORS, Validators} from '../validators'; */ export interface Validator { validate(c: AbstractControl): {[key: string]: any}; } -const REQUIRED = Validators.required; +export const REQUIRED = Validators.required; export const REQUIRED_VALIDATOR: any = { provide: NG_VALIDATORS, diff --git a/modules/@angular/compiler-cli/src/static_reflector.ts b/modules/@angular/compiler-cli/src/static_reflector.ts index b4862ef723..0286cb3662 100644 --- a/modules/@angular/compiler-cli/src/static_reflector.ts +++ b/modules/@angular/compiler-cli/src/static_reflector.ts @@ -594,6 +594,10 @@ function expandedMessage(error: any): string { error.context && error.context.name ? `Calling function '${error.context.name}', f` : 'F'; return prefix + 'unction calls are not supported. Consider replacing the function or lambda with a reference to an exported function'; + case 'Reference to a local symbol': + if (error.context && error.context.name) { + return `Reference to a local (non-exported) symbol '${error.context.name}'. Consider exporting the symbol`; + } } return error.message; } diff --git a/modules/@angular/forms/src/directives/select_multiple_control_value_accessor.ts b/modules/@angular/forms/src/directives/select_multiple_control_value_accessor.ts index 964627e160..cda2b1905e 100644 --- a/modules/@angular/forms/src/directives/select_multiple_control_value_accessor.ts +++ b/modules/@angular/forms/src/directives/select_multiple_control_value_accessor.ts @@ -6,14 +6,14 @@ * found in the LICENSE file at https://angular.io/license */ -import {Directive, ElementRef, Host, Input, OnDestroy, Optional, Renderer, forwardRef} from '@angular/core'; +import {Directive, ElementRef, Host, Input, OnDestroy, OpaqueToken, Optional, Renderer, Type, forwardRef} from '@angular/core'; import {MapWrapper} from '../facade/collection'; import {StringWrapper, isBlank, isPresent, isPrimitive, isString, looseIdentical} from '../facade/lang'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from './control_value_accessor'; -const SELECT_MULTIPLE_VALUE_ACCESSOR = { +export const SELECT_MULTIPLE_VALUE_ACCESSOR = { provide: NG_VALUE_ACCESSOR, useExisting: forwardRef(() => SelectMultipleControlValueAccessor), multi: true diff --git a/tools/@angular/tsc-wrapped/src/collector.ts b/tools/@angular/tsc-wrapped/src/collector.ts index 4130a772c3..4017403869 100644 --- a/tools/@angular/tsc-wrapped/src/collector.ts +++ b/tools/@angular/tsc-wrapped/src/collector.ts @@ -297,13 +297,19 @@ export class MetadataCollector { } else { varValue = errorSym('Variable not initialized', nameNode); } + let exported = false; if (variableStatement.flags & ts.NodeFlags.Export || variableDeclaration.flags & ts.NodeFlags.Export) { if (!metadata) metadata = {}; metadata[nameNode.text] = varValue; + exported = true; } if (isPrimitive(varValue)) { locals.define(nameNode.text, varValue); + } else if (!exported) { + locals.define( + nameNode.text, + errorSym('Reference to a local symbol', nameNode, {name: nameNode.text})); } } else { // Destructuring (or binding) declarations are not supported, diff --git a/tools/@angular/tsc-wrapped/test/collector.spec.ts b/tools/@angular/tsc-wrapped/test/collector.spec.ts index 0d6ea26b09..6dc153a3e2 100644 --- a/tools/@angular/tsc-wrapped/test/collector.spec.ts +++ b/tools/@angular/tsc-wrapped/test/collector.spec.ts @@ -24,6 +24,7 @@ describe('Collector', () => { 'exported-functions.ts', 'exported-enum.ts', 'exported-consts.ts', + 'local-symbol-ref.ts', 're-exports.ts', 'static-field-reference.ts', 'static-method.ts', @@ -486,6 +487,28 @@ describe('Collector', () => { {from: 'angular2/core'} ]); }); + + it('should collect an error symbol if collecting a reference to a non-exported symbol', () => { + let source = program.getSourceFile('/local-symbol-ref.ts'); + let metadata = collector.getMetadata(source); + expect(metadata.metadata).toEqual({ + REQUIRED_VALIDATOR: { + __symbolic: 'error', + message: 'Reference to a local symbol', + line: 3, + character: 9, + context: {name: 'REQUIRED'} + }, + SomeComponent: { + __symbolic: 'class', + decorators: [{ + __symbolic: 'call', + expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Component'}, + arguments: [{providers: [{__symbolic: 'reference', name: 'REQUIRED_VALIDATOR'}]}] + }] + } + }); + }); }); // TODO: Do not use \` in a template literal as it confuses clang-format @@ -799,6 +822,22 @@ const FILES: Directory = { export {Foo as OtherModule} from './static-field-reference.ts'; export * from 'angular2/core'; `, + 'local-symbol-ref.ts': ` + import {Component, Validators} from 'angular2/core'; + + const REQUIRED = Validators.required; + + export const REQUIRED_VALIDATOR: any = { + provide: 'SomeToken', + useValue: REQUIRED, + multi: true + }; + + @Component({ + providers: [REQUIRED_VALIDATOR] + }) + export class SomeComponent {} + `, 'node_modules': { 'angular2': { 'core.d.ts': ` @@ -849,6 +888,9 @@ const FILES: Directory = { export interface OnInit { ngOnInit(): any; } + export class Validators { + static required(): void; + } `, 'common.d.ts': ` export declare class NgFor {