Skip to content

Commit

Permalink
refactor(compiler-cli): rename ShimLocation to TcbLocation (#45454)
Browse files Browse the repository at this point in the history
Inline type check blocks (TCBs) are emitted into the original source file, but
node positions would still be represented as a `ShimLocation` with a `shimPath`
corresponding with the type-checking shim file. This results in inconsistencies,
as the `positionInShimFile` field of `ShimLocation` would not correspond with
the `shimPath` of that `ShimLocation`.

This commit is a precursor to letting `ShimLocation` also represent the correct
location for inline type check blocks, by renaming the interface to
`TcbLocation`. A followup commit addresses the actual inconsistency.

PR Close #45454
  • Loading branch information
JoostK authored and dylhunn committed Mar 29, 2022
1 parent e55e98d commit b2758d7
Show file tree
Hide file tree
Showing 16 changed files with 137 additions and 137 deletions.
12 changes: 6 additions & 6 deletions packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {ErrorCode} from '../../diagnostics';
import {FullTemplateMapping, NgTemplateDiagnostic, TypeCheckableDirectiveMeta} from './api';
import {GlobalCompletion} from './completion';
import {DirectiveInScope, PipeInScope} from './scope';
import {ElementSymbol, ShimLocation, Symbol, TemplateSymbol} from './symbols';
import {ElementSymbol, Symbol, TcbLocation, TemplateSymbol} from './symbols';

/**
* Interface to the Angular Template Type Checker to extract diagnostics and intelligence from the
Expand Down Expand Up @@ -56,7 +56,7 @@ export interface TemplateTypeChecker {
* Given a `shim` and position within the file, returns information for mapping back to a template
* location.
*/
getTemplateMappingAtShimLocation(shimLocation: ShimLocation): FullTemplateMapping|null;
getTemplateMappingAtTcbLocation(tcbLocation: TcbLocation): FullTemplateMapping|null;

/**
* Get all `ts.Diagnostic`s currently available that pertain to the given component.
Expand Down Expand Up @@ -113,19 +113,19 @@ export interface TemplateTypeChecker {


/**
* For the given expression node, retrieve a `ShimLocation` that can be used to perform
* For the given expression node, retrieve a `TcbLocation` that can be used to perform
* autocompletion at that point in the expression, if such a location exists.
*/
getExpressionCompletionLocation(
expr: PropertyRead|SafePropertyRead, component: ts.ClassDeclaration): ShimLocation|null;
expr: PropertyRead|SafePropertyRead, component: ts.ClassDeclaration): TcbLocation|null;

/**
* For the given node represents a `LiteralPrimitive`(the `TextAttribute` represents a string
* literal), retrieve a `ShimLocation` that can be used to perform autocompletion at that point in
* literal), retrieve a `TcbLocation` that can be used to perform autocompletion at that point in
* the node, if such a location exists.
*/
getLiteralCompletionLocation(
strNode: LiteralPrimitive|TmplAstTextAttribute, component: ts.ClassDeclaration): ShimLocation
strNode: LiteralPrimitive|TmplAstTextAttribute, component: ts.ClassDeclaration): TcbLocation
|null;

/**
Expand Down
6 changes: 3 additions & 3 deletions packages/compiler-cli/src/ngtsc/typecheck/api/completion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {TmplAstReference, TmplAstVariable} from '@angular/compiler';

import {ShimLocation} from './symbols';
import {TcbLocation} from './symbols';

/**
* An autocompletion source of any kind.
Expand Down Expand Up @@ -60,7 +60,7 @@ export interface GlobalCompletion {
* A location within the type-checking shim where TypeScript's completion APIs can be used to
* access completions for the template's component context (component class members).
*/
componentContext: ShimLocation;
componentContext: TcbLocation;

/**
* `Map` of local references and variables that are visible at the requested level of the
Expand All @@ -76,5 +76,5 @@ export interface GlobalCompletion {
* A location within the type-checking shim where TypeScript's completion APIs can be used to
* access completions for the AST node of the cursor position (primitive constants).
*/
nodeContext: ShimLocation|null;
nodeContext: TcbLocation|null;
}
35 changes: 19 additions & 16 deletions packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,19 @@ export type Symbol = InputBindingSymbol|OutputBindingSymbol|ElementSymbol|Refere
*/
export type TemplateDeclarationSymbol = ReferenceSymbol|VariableSymbol;

/** Information about where a `ts.Node` can be found in the type check block shim file. */
export interface ShimLocation {
/**
* Information about where a `ts.Node` can be found in the type check file. This can either be
* a type-checking shim file, or an original source file for inline type check blocks.
*/
export interface TcbLocation {
/**
* The fully qualified path of the file which contains the generated TypeScript type check
* code for the component's template.
*/
shimPath: AbsoluteFsPath;
tcbPath: AbsoluteFsPath;

/** The location in the shim file where node appears. */
positionInShimFile: number;
/** The location in the file where node appears. */
positionInFile: number;
}

/**
Expand All @@ -62,7 +65,7 @@ export interface TsNodeSymbolInfo {
tsSymbol: ts.Symbol|null;

/** The position of the most relevant part of the template node. */
shimLocation: ShimLocation;
tcbLocation: TcbLocation;
}

/**
Expand All @@ -81,7 +84,7 @@ export interface ExpressionSymbol {
tsSymbol: ts.Symbol|null;

/** The position of the most relevant part of the expression. */
shimLocation: ShimLocation;
tcbLocation: TcbLocation;
}

/** Represents either an input or output binding in a template. */
Expand All @@ -101,7 +104,7 @@ export interface BindingSymbol {
target: DirectiveSymbol|ElementSymbol|TemplateSymbol;

/** The location in the shim file where the field access for the binding appears. */
shimLocation: ShimLocation;
tcbLocation: TcbLocation;
}

/**
Expand Down Expand Up @@ -171,15 +174,15 @@ export interface ReferenceSymbol {
* ```
* This `targetLocation` is `[_t1 variable declaration].getStart()`.
*/
targetLocation: ShimLocation;
targetLocation: TcbLocation;

/**
* The location in the TCB for the identifier node in the reference variable declaration.
* For example, given a variable declaration statement for a template reference:
* `var _t2 = _t1`, this location is `[_t2 node].getStart()`. This location can
* be used to find references to the variable within the template.
*/
referenceVarLocation: ShimLocation;
referenceVarLocation: TcbLocation;
}

/**
Expand Down Expand Up @@ -211,13 +214,13 @@ export interface VariableSymbol {
/**
* The location in the shim file for the identifier that was declared for the template variable.
*/
localVarLocation: ShimLocation;
localVarLocation: TcbLocation;

/**
* The location in the shim file for the initializer node of the variable that represents the
* template variable.
*/
initializerLocation: ShimLocation;
initializerLocation: TcbLocation;
}

/**
Expand All @@ -236,7 +239,7 @@ export interface ElementSymbol {
directives: DirectiveSymbol[];

/** The location in the shim file for the variable that holds the type of the element. */
shimLocation: ShimLocation;
tcbLocation: TcbLocation;

templateNode: TmplAstElement;
}
Expand All @@ -261,7 +264,7 @@ export interface DirectiveSymbol extends DirectiveInScope {
tsType: ts.Type;

/** The location in the shim file for the variable that holds the type of the directive. */
shimLocation: ShimLocation;
tcbLocation: TcbLocation;
}

/**
Expand Down Expand Up @@ -292,7 +295,7 @@ export interface PipeSymbol {
tsSymbol: ts.Symbol|null;

/** The position of the transform call in the template. */
shimLocation: ShimLocation;
tcbLocation: TcbLocation;

/** The symbol for the pipe class as an instance that appears in the TCB. */
classSymbol: ClassSymbol;
Expand All @@ -307,5 +310,5 @@ export interface ClassSymbol {
tsSymbol: SymbolWithValueDeclaration;

/** The position for the variable declaration for the class instance. */
shimLocation: ShimLocation;
tcbLocation: TcbLocation;
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ class NullishCoalescingNotNullableCheck extends
if (symbol.kind !== SymbolKind.Expression) {
return [];
}
const span =
ctx.templateTypeChecker.getTemplateMappingAtShimLocation(symbol.shimLocation)!.span;
const span = ctx.templateTypeChecker.getTemplateMappingAtTcbLocation(symbol.tcbLocation)!.span;
const diagnostic = ctx.makeTemplateDiagnostic(
span,
`The left side of this nullish coalescing operation does not include 'null' or 'undefined' in its type, therefore the '??' operator can be safely removed.`);
Expand Down
16 changes: 8 additions & 8 deletions packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {ClassDeclaration, isNamedClassDeclaration, ReflectionHost} from '../../r
import {ComponentScopeReader, TypeCheckScopeRegistry} from '../../scope';
import {isShim} from '../../shims';
import {getSourceFileOrNull, isSymbolWithValueDeclaration} from '../../util/src/typescript';
import {DirectiveInScope, ElementSymbol, FullTemplateMapping, GlobalCompletion, NgTemplateDiagnostic, OptimizeFor, PipeInScope, ProgramTypeCheckAdapter, ShimLocation, Symbol, TemplateDiagnostic, TemplateId, TemplateSymbol, TemplateTypeChecker, TypeCheckableDirectiveMeta, TypeCheckingConfig} from '../api';
import {DirectiveInScope, ElementSymbol, FullTemplateMapping, GlobalCompletion, NgTemplateDiagnostic, OptimizeFor, PipeInScope, ProgramTypeCheckAdapter, Symbol, TcbLocation, TemplateDiagnostic, TemplateId, TemplateSymbol, TemplateTypeChecker, TypeCheckableDirectiveMeta, TypeCheckingConfig} from '../api';
import {makeTemplateDiagnostic} from '../diagnostics';

import {CompletionEngine} from './completion';
Expand Down Expand Up @@ -151,20 +151,20 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
return null;
}

getTemplateMappingAtShimLocation({shimPath, positionInShimFile}: ShimLocation):
FullTemplateMapping|null {
const records = this.getFileAndShimRecordsForPath(absoluteFrom(shimPath));
getTemplateMappingAtTcbLocation({tcbPath, positionInFile}: TcbLocation): FullTemplateMapping
|null {
const records = this.getFileAndShimRecordsForPath(absoluteFrom(tcbPath));
if (records === null) {
return null;
}
const {fileRecord} = records;

const shimSf = this.programDriver.getProgram().getSourceFile(absoluteFrom(shimPath));
const shimSf = this.programDriver.getProgram().getSourceFile(absoluteFrom(tcbPath));
if (shimSf === undefined) {
return null;
}
return getTemplateMapping(
shimSf, positionInShimFile, fileRecord.sourceManager, /*isDiagnosticsRequest*/ false);
shimSf, positionInFile, fileRecord.sourceManager, /*isDiagnosticsRequest*/ false);
}

generateAllTypeCheckBlocks() {
Expand Down Expand Up @@ -270,7 +270,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
}

getExpressionCompletionLocation(
ast: PropertyRead|SafePropertyRead, component: ts.ClassDeclaration): ShimLocation|null {
ast: PropertyRead|SafePropertyRead, component: ts.ClassDeclaration): TcbLocation|null {
const engine = this.getOrCreateCompletionEngine(component);
if (engine === null) {
return null;
Expand All @@ -280,7 +280,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
}

getLiteralCompletionLocation(
node: LiteralPrimitive|TmplAstTextAttribute, component: ts.ClassDeclaration): ShimLocation
node: LiteralPrimitive|TmplAstTextAttribute, component: ts.ClassDeclaration): TcbLocation
|null {
const engine = this.getOrCreateCompletionEngine(component);
if (engine === null) {
Expand Down
38 changes: 19 additions & 19 deletions packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {AST, EmptyExpr, ImplicitReceiver, LiteralPrimitive, PropertyRead, Proper
import ts from 'typescript';

import {AbsoluteFsPath} from '../../file_system';
import {CompletionKind, GlobalCompletion, ReferenceCompletion, ShimLocation, VariableCompletion} from '../api';
import {CompletionKind, GlobalCompletion, ReferenceCompletion, TcbLocation, VariableCompletion} from '../api';

import {ExpressionIdentifier, findFirstMatchingNode} from './comments';
import {TemplateData} from './context';
Expand All @@ -22,7 +22,7 @@ import {TemplateData} from './context';
* surrounding TS program have changed.
*/
export class CompletionEngine {
private componentContext: ShimLocation|null;
private componentContext: TcbLocation|null;

/**
* Cache of completions for various levels of the template, including the root template (`null`).
Expand All @@ -32,10 +32,10 @@ export class CompletionEngine {
new Map<TmplAstTemplate|null, Map<string, ReferenceCompletion|VariableCompletion>>();

private expressionCompletionCache =
new Map<PropertyRead|SafePropertyRead|LiteralPrimitive|TmplAstTextAttribute, ShimLocation>();
new Map<PropertyRead|SafePropertyRead|LiteralPrimitive|TmplAstTextAttribute, TcbLocation>();


constructor(private tcb: ts.Node, private data: TemplateData, private shimPath: AbsoluteFsPath) {
constructor(private tcb: ts.Node, private data: TemplateData, private tcbPath: AbsoluteFsPath) {
// Find the component completion expression within the TCB. This looks like: `ctx. /* ... */;`
const globalRead = findFirstMatchingNode(this.tcb, {
filter: ts.isPropertyAccessExpression,
Expand All @@ -44,11 +44,11 @@ export class CompletionEngine {

if (globalRead !== null) {
this.componentContext = {
shimPath: this.shimPath,
tcbPath: this.tcbPath,
// `globalRead.name` is an empty `ts.Identifier`, so its start position immediately follows
// the `.` in `ctx.`. TS autocompletion APIs can then be used to access completion results
// for the component context.
positionInShimFile: globalRead.name.getStart(),
positionInFile: globalRead.name.getStart(),
};
} else {
this.componentContext = null;
Expand All @@ -74,16 +74,16 @@ export class CompletionEngine {
return null;
}

let nodeContext: ShimLocation|null = null;
let nodeContext: TcbLocation|null = null;
if (node instanceof EmptyExpr) {
const nodeLocation = findFirstMatchingNode(this.tcb, {
filter: ts.isIdentifier,
withSpan: node.sourceSpan,
});
if (nodeLocation !== null) {
nodeContext = {
shimPath: this.shimPath,
positionInShimFile: nodeLocation.getStart(),
tcbPath: this.tcbPath,
positionInFile: nodeLocation.getStart(),
};
}
}
Expand All @@ -95,8 +95,8 @@ export class CompletionEngine {
});
if (nodeLocation) {
nodeContext = {
shimPath: this.shimPath,
positionInShimFile: nodeLocation.getStart(),
tcbPath: this.tcbPath,
positionInFile: nodeLocation.getStart(),
};
}
}
Expand All @@ -108,7 +108,7 @@ export class CompletionEngine {
};
}

getExpressionCompletionLocation(expr: PropertyRead|PropertyWrite|SafePropertyRead): ShimLocation
getExpressionCompletionLocation(expr: PropertyRead|PropertyWrite|SafePropertyRead): TcbLocation
|null {
if (this.expressionCompletionCache.has(expr)) {
return this.expressionCompletionCache.get(expr)!;
Expand Down Expand Up @@ -146,15 +146,15 @@ export class CompletionEngine {
return null;
}

const res: ShimLocation = {
shimPath: this.shimPath,
positionInShimFile: tsExpr.name.getEnd(),
const res: TcbLocation = {
tcbPath: this.tcbPath,
positionInFile: tsExpr.name.getEnd(),
};
this.expressionCompletionCache.set(expr, res);
return res;
}

getLiteralCompletionLocation(expr: LiteralPrimitive|TmplAstTextAttribute): ShimLocation|null {
getLiteralCompletionLocation(expr: LiteralPrimitive|TmplAstTextAttribute): TcbLocation|null {
if (this.expressionCompletionCache.has(expr)) {
return this.expressionCompletionCache.get(expr)!;
}
Expand Down Expand Up @@ -186,9 +186,9 @@ export class CompletionEngine {
// In the shimFile, if `tsExpr` is a string, the position should be in the quotes.
positionInShimFile -= 1;
}
const res: ShimLocation = {
shimPath: this.shimPath,
positionInShimFile,
const res: TcbLocation = {
tcbPath: this.tcbPath,
positionInFile: positionInShimFile,
};
this.expressionCompletionCache.set(expr, res);
return res;
Expand Down

0 comments on commit b2758d7

Please sign in to comment.