cargdev/angular
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(compiler): always return a mutable clone from Scope#resolve (#38857)

Browse Source 
This change prevents comments from a resolved node from appearing at
each location the resolved expression is used and also prevents callers
of `Scope#resolve` from accidentally modifying / adding comments to the
declaration site.

PR Close #38857
This commit is contained in:
Andrew Scott 2020-09-15 10:07:28 -07:00 committed by Andrew Kushnir
parent 722699fb0c
commit 129107191c
2 changed files with 34 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
View File

@ -418,10 +418,10 @@ class TcbReferenceOp extends TcbOp {
execute(): ts.Identifier { execute(): ts.Identifier {
const id = this.tcb.allocateId(); const id = this.tcb.allocateId();
let initializer = ts.getMutableClone( let initializer =
this.target instanceof TmplAstTemplate || this.target instanceof TmplAstElement ? this.target instanceof TmplAstTemplate || this.target instanceof TmplAstElement ?
this.scope.resolve(this.target) : this.scope.resolve(this.target) :
this.scope.resolve(this.host, this.target)); this.scope.resolve(this.host, this.target);
// The reference is either to an element, an <ng-template> node, or to a directive on an // The reference is either to an element, an <ng-template> node, or to a directive on an
// element or template. // element or template.
@ -1121,7 +1121,8 @@ class Scope {
/** /**
* Look up a `ts.Expression` representing the value of some operation in the current `Scope`, * Look up a `ts.Expression` representing the value of some operation in the current `Scope`,
* including any parent scope(s). * including any parent scope(s). This method always returns a mutable clone of the
* `ts.Expression` with the comments cleared.
* *
* @param node a `TmplAstNode` of the operation in question. The lookup performed will depend on * @param node a `TmplAstNode` of the operation in question. The lookup performed will depend on
* the type of this node: * the type of this node:
@ -1142,7 +1143,18 @@ class Scope {
// Attempt to resolve the operation locally. // Attempt to resolve the operation locally.
const res = this.resolveLocal(node, directive); const res = this.resolveLocal(node, directive);
if (res !== null) { if (res !== null) {
return res; // We want to get a clone of the resolved expression and clear the trailing comments
// so they don't continue to appear in every place the expression is used.
// As an example, this would otherwise produce:
// var _t1 /**T:DIR*/ /*1,2*/ = _ctor1();
// _t1 /**T:DIR*/ /*1,2*/.input = 'value';
//
// In addition, returning a clone prevents the consumer of `Scope#resolve` from
// attaching comments at the declaration site.
const clone = ts.getMutableClone(res);
ts.setSyntheticTrailingComments(clone, []);
return clone;
} else if (this.parent !== null) { } else if (this.parent !== null) {
// Check with the parent. // Check with the parent.
return this.parent.resolve(node, directive); return this.parent.resolve(node, directive);
@ -1547,7 +1559,7 @@ class TcbExpressionTranslator {
return null; return null;
} }
const expr = ts.getMutableClone(this.scope.resolve(binding)); const expr = this.scope.resolve(binding);
addParseSpanInfo(expr, ast.sourceSpan); addParseSpanInfo(expr, ast.sourceSpan);
return expr; return expr;
} }

16
packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts
View File

@ -165,6 +165,22 @@ describe('type check blocks diagnostics', () => {
.toContain('((_t1 /*23,24*/) || (_t1 /*28,29*/) /*23,29*/);'); .toContain('((_t1 /*23,24*/) || (_t1 /*28,29*/) /*23,29*/);');
}); });
}); });
describe('attaching comments for generic directive inputs', () => {
it('should be correct for directive refs', () => {
const DIRECTIVES: TestDeclaration[] = [{
type: 'directive',
name: 'MyComponent',
selector: 'my-cmp',
isComponent: true,
isGeneric: true,
inputs: {'inputA': 'inputA'},
}];
const TEMPLATE = `<my-cmp [inputA]="''"></my-cmp>`;
expect(tcbWithSpans(TEMPLATE, DIRECTIVES))
.toContain('_t1.inputA = ("" /*18,20*/) /*8,21*/;');
});
});
}); });
}); });