Skip to content

Commit

Permalink
refactor(compiler): Remove deep imports in the language service (#54695)
Browse files Browse the repository at this point in the history
Previously, the language service relied on deep imports such as `@angular/compiler/render3/...`. This is bad form, because that creates a dependency on the package's internal structure. Additionally, this is not compatible with google3.

In this PR, I replace all the deep imports with shallow imports, in some cases adding the missing symbol to the `compiler.ts` exports.

PR Close #54695
  • Loading branch information
dylhunn authored and pkozlowski-opensource committed Mar 5, 2024
1 parent 33a6fab commit b40f1e5
Show file tree
Hide file tree
Showing 10 changed files with 161 additions and 163 deletions.
2 changes: 1 addition & 1 deletion packages/compiler/src/compiler.ts
Expand Up @@ -63,7 +63,7 @@ export {SourceMap} from './output/source_map';
export * from './injectable_compiler_2';
export * from './render3/partial/api';
export * from './render3/view/api';
export {BoundAttribute as TmplAstBoundAttribute, BoundEvent as TmplAstBoundEvent, BoundText as TmplAstBoundText, Content as TmplAstContent, Element as TmplAstElement, Icu as TmplAstIcu, Node as TmplAstNode, RecursiveVisitor as TmplAstRecursiveVisitor, Reference as TmplAstReference, Template as TmplAstTemplate, Text as TmplAstText, TextAttribute as TmplAstTextAttribute, Variable as TmplAstVariable, DeferredBlock as TmplAstDeferredBlock, DeferredBlockPlaceholder as TmplAstDeferredBlockPlaceholder, DeferredBlockLoading as TmplAstDeferredBlockLoading, DeferredBlockError as TmplAstDeferredBlockError, DeferredTrigger as TmplAstDeferredTrigger, BoundDeferredTrigger as TmplAstBoundDeferredTrigger, IdleDeferredTrigger as TmplAstIdleDeferredTrigger, ImmediateDeferredTrigger as TmplAstImmediateDeferredTrigger, HoverDeferredTrigger as TmplAstHoverDeferredTrigger, TimerDeferredTrigger as TmplAstTimerDeferredTrigger, InteractionDeferredTrigger as TmplAstInteractionDeferredTrigger, ViewportDeferredTrigger as TmplAstViewportDeferredTrigger, SwitchBlock as TmplAstSwitchBlock, SwitchBlockCase as TmplAstSwitchBlockCase, ForLoopBlock as TmplAstForLoopBlock, ForLoopBlockEmpty as TmplAstForLoopBlockEmpty, IfBlock as TmplAstIfBlock, IfBlockBranch as TmplAstIfBlockBranch, DeferredBlockTriggers as TmplAstDeferredBlockTriggers, UnknownBlock as TmplAstUnknownBlock} from './render3/r3_ast';
export {visitAll as tmplAstVisitAll, BlockNode as TmplAstBlockNode, BoundAttribute as TmplAstBoundAttribute, BoundEvent as TmplAstBoundEvent, BoundText as TmplAstBoundText, Content as TmplAstContent, Element as TmplAstElement, Icu as TmplAstIcu, Node as TmplAstNode, Visitor as TmplAstVisitor, RecursiveVisitor as TmplAstRecursiveVisitor, Reference as TmplAstReference, Template as TmplAstTemplate, Text as TmplAstText, TextAttribute as TmplAstTextAttribute, Variable as TmplAstVariable, DeferredBlock as TmplAstDeferredBlock, DeferredBlockPlaceholder as TmplAstDeferredBlockPlaceholder, DeferredBlockLoading as TmplAstDeferredBlockLoading, DeferredBlockError as TmplAstDeferredBlockError, DeferredTrigger as TmplAstDeferredTrigger, BoundDeferredTrigger as TmplAstBoundDeferredTrigger, IdleDeferredTrigger as TmplAstIdleDeferredTrigger, ImmediateDeferredTrigger as TmplAstImmediateDeferredTrigger, HoverDeferredTrigger as TmplAstHoverDeferredTrigger, TimerDeferredTrigger as TmplAstTimerDeferredTrigger, InteractionDeferredTrigger as TmplAstInteractionDeferredTrigger, ViewportDeferredTrigger as TmplAstViewportDeferredTrigger, SwitchBlock as TmplAstSwitchBlock, SwitchBlockCase as TmplAstSwitchBlockCase, ForLoopBlock as TmplAstForLoopBlock, ForLoopBlockEmpty as TmplAstForLoopBlockEmpty, IfBlock as TmplAstIfBlock, IfBlockBranch as TmplAstIfBlockBranch, DeferredBlockTriggers as TmplAstDeferredBlockTriggers, UnknownBlock as TmplAstUnknownBlock} from './render3/r3_ast';
export * from './render3/view/t2_api';
export * from './render3/view/t2_binder';
export {createCssSelectorFromNode} from './render3/view/util';
Expand Down
Expand Up @@ -6,8 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/

import {TmplAstBoundEvent} from '@angular/compiler';
import {ErrorCode, ngErrorCode} from '@angular/compiler-cli/src/ngtsc/diagnostics';
import {BoundEvent} from '@angular/compiler/src/render3/r3_ast';
import tss from 'typescript/lib/tsserverlibrary';

import {getTargetAtPosition, TargetNodeKind} from '../template_target';
Expand Down Expand Up @@ -85,7 +85,8 @@ export const fixInvalidBananaInBoxMeta: CodeActionMeta = {
},
};

function getTheBoundEventAtPosition(templateInfo: TemplateInfo, start: number): BoundEvent|null {
function getTheBoundEventAtPosition(templateInfo: TemplateInfo, start: number): TmplAstBoundEvent|
null {
// It's safe to get the bound event at the position `start + 1` because the `start` is at the
// start of the diagnostic, and the node outside the attribute key and value spans are skipped by
// the function `getTargetAtPosition`.
Expand All @@ -97,7 +98,7 @@ function getTheBoundEventAtPosition(templateInfo: TemplateInfo, start: number):
}

if (positionDetail.context.kind !== TargetNodeKind.AttributeInKeyContext ||
!(positionDetail.context.node instanceof BoundEvent)) {
!(positionDetail.context.node instanceof TmplAstBoundEvent)) {
return null;
}

Expand All @@ -107,7 +108,7 @@ function getTheBoundEventAtPosition(templateInfo: TemplateInfo, start: number):
/**
* Flip the invalid "box in a banana" `([thing])` to the correct "banana in a box" `[(thing)]`.
*/
function convertBoundEventToTsTextChange(node: BoundEvent): readonly tss.TextChange[] {
function convertBoundEventToTsTextChange(node: TmplAstBoundEvent): readonly tss.TextChange[] {
const name = node.name;
const boundSyntax = node.sourceSpan.toString();
const expectedBoundSyntax = boundSyntax.replace(`(${name})`, `[(${name.slice(1, -1)})]`);
Expand Down
5 changes: 2 additions & 3 deletions packages/language-service/src/codefixes/fix_missing_import.ts
Expand Up @@ -6,10 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ASTWithName} from '@angular/compiler';
import {ASTWithName, TmplAstElement} from '@angular/compiler';
import {ErrorCode as NgCompilerErrorCode, ngErrorCode} from '@angular/compiler-cli/src/ngtsc/diagnostics/index';
import {PotentialDirective, PotentialImportMode, PotentialPipe} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
import * as t from '@angular/compiler/src/render3/r3_ast'; // t for template AST
import ts from 'typescript';

import {getTargetAtPosition, TargetNodeKind} from '../template_target';
Expand Down Expand Up @@ -52,7 +51,7 @@ function getCodeActions(

let matches: Set<PotentialDirective>|Set<PotentialPipe>;
if (target.context.kind === TargetNodeKind.ElementInTagContext &&
target.context.node instanceof t.Element) {
target.context.node instanceof TmplAstElement) {
const allPossibleDirectives = checker.getPotentialTemplateDirectives(templateInfo.component);
matches = getDirectiveMatchesForElementTag(target.context.node, allPossibleDirectives);
} else if (
Expand Down
2 changes: 0 additions & 2 deletions packages/language-service/src/codefixes/fix_missing_member.ts
Expand Up @@ -6,8 +6,6 @@
* found in the LICENSE file at https://angular.io/license
*/

import {findFirstMatchingNode} from '@angular/compiler-cli/src/ngtsc/typecheck/src/comments';
import * as e from '@angular/compiler/src/expression_parser/ast'; // e for expression AST
import ts from 'typescript';
import tss from 'typescript/lib/tsserverlibrary';

Expand Down
3 changes: 1 addition & 2 deletions packages/language-service/src/completions.ts
Expand Up @@ -6,10 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/

import {AST, ASTWithSource, BindingPipe, BindingType, Call, EmptyExpr, ImplicitReceiver, LiteralPrimitive, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SafePropertyRead, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstText, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler';
import {AST, ASTWithSource, BindingPipe, BindingType, Call, EmptyExpr, ImplicitReceiver, LiteralPrimitive, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SafePropertyRead, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundEvent as BoundEvent, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstSwitchBlock as SwitchBlock, TmplAstTemplate, TmplAstText, TmplAstTextAttribute, TmplAstTextAttribute as TextAttribute, TmplAstUnknownBlock as UnknownBlock, TmplAstVariable} from '@angular/compiler';
import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core';
import {CompletionKind, PotentialDirective, SymbolKind, TemplateDeclarationSymbol} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
import {BoundEvent, DeferredBlock, SwitchBlock, TextAttribute, UnknownBlock} from '@angular/compiler/src/render3/r3_ast';
import ts from 'typescript';

import {addAttributeCompletionEntries, AsciiSortPriority, AttributeCompletionKind, buildAnimationCompletionEntries, buildAttributeCompletionTable, getAttributeCompletionSymbol} from './attribute_completions';
Expand Down
21 changes: 10 additions & 11 deletions packages/language-service/src/outlining_spans.ts
Expand Up @@ -6,11 +6,10 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ParseLocation, ParseSourceSpan} from '@angular/compiler';
import {ParseLocation, ParseSourceSpan, TmplAstBlockNode, TmplAstDeferredBlock, TmplAstForLoopBlock, TmplAstIfBlock, TmplAstNode, TmplAstRecursiveVisitor, tmplAstVisitAll} from '@angular/compiler';
import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core';
import {isExternalResource} from '@angular/compiler-cli/src/ngtsc/metadata';
import {isNamedClassDeclaration} from '@angular/compiler-cli/src/ngtsc/reflection';
import * as t from '@angular/compiler/src/render3/r3_ast'; // t for template AST
import ts from 'typescript';

import {getFirstComponentForTemplateFile, isTypeScriptFile, toTextSpan} from './utils';
Expand All @@ -22,7 +21,7 @@ export function getOutliningSpans(compiler: NgCompiler, fileName: string): ts.Ou
return [];
}

const templatesInFile: Array<t.Node[]> = [];
const templatesInFile: Array<TmplAstNode[]> = [];
for (const stmt of sf.statements) {
if (isNamedClassDeclaration(stmt)) {
const resources = compiler.getComponentResources(stmt);
Expand All @@ -47,19 +46,19 @@ export function getOutliningSpans(compiler: NgCompiler, fileName: string): ts.Ou
}
}

class BlockVisitor extends t.RecursiveVisitor {
readonly blocks = [] as Array<t.BlockNode>;
class BlockVisitor extends TmplAstRecursiveVisitor {
readonly blocks = [] as Array<TmplAstBlockNode>;

static getBlockSpans(templateNodes: t.Node[]): ts.OutliningSpan[] {
static getBlockSpans(templateNodes: TmplAstNode[]): ts.OutliningSpan[] {
const visitor = new BlockVisitor();
t.visitAll(visitor, templateNodes);
tmplAstVisitAll(visitor, templateNodes);
const {blocks} = visitor;
return blocks.map(block => {
let mainBlockSpan = block.sourceSpan;
// The source span of for loops and deferred blocks contain all parts (ForLoopBlockEmpty,
// DeferredBlockLoading, etc.). The folding range should only include the main block span for
// these.
if (block instanceof t.ForLoopBlock || block instanceof t.DeferredBlock) {
if (block instanceof TmplAstForLoopBlock || block instanceof TmplAstDeferredBlock) {
mainBlockSpan = block.mainBlockSpan;
}
return {
Expand All @@ -75,10 +74,10 @@ class BlockVisitor extends t.RecursiveVisitor {
});
}

visit(node: t.Node) {
if (node instanceof t.BlockNode
visit(node: TmplAstNode) {
if (node instanceof TmplAstBlockNode
// Omit `IfBlock` because we include the branches individually
&& !(node instanceof t.IfBlock)) {
&& !(node instanceof TmplAstIfBlock)) {
this.blocks.push(node);
}
}
Expand Down
5 changes: 2 additions & 3 deletions packages/language-service/src/quick_info.ts
Expand Up @@ -5,10 +5,9 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {AST, TmplAstBoundAttribute, TmplAstNode, TmplAstTextAttribute} from '@angular/compiler';
import {AST, TmplAstBlockNode, TmplAstBoundAttribute, TmplAstDeferredTrigger, TmplAstNode, TmplAstTextAttribute} from '@angular/compiler';
import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core';
import {DirectiveSymbol, DomBindingSymbol, ElementSymbol, InputBindingSymbol, OutputBindingSymbol, PipeSymbol, ReferenceSymbol, Symbol, SymbolKind, TcbLocation, VariableSymbol} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
import {BlockNode, DeferredTrigger} from '@angular/compiler/src/render3/r3_ast';
import ts from 'typescript';

import {DisplayInfoKind, SYMBOL_PUNC, SYMBOL_SPACE, SYMBOL_TEXT} from './display_parts';
Expand All @@ -26,7 +25,7 @@ export class QuickInfoBuilder {
private readonly positionDetails: TemplateTarget) {}

get(): ts.QuickInfo|undefined {
if (this.node instanceof DeferredTrigger || this.node instanceof BlockNode) {
if (this.node instanceof TmplAstDeferredTrigger || this.node instanceof TmplAstBlockNode) {
return createQuickInfoForBuiltIn(this.node, this.positionDetails.position);
}

Expand Down
14 changes: 8 additions & 6 deletions packages/language-service/src/quick_info_built_ins.ts
Expand Up @@ -5,8 +5,7 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {AST, Call, ImplicitReceiver, ParseSourceSpan, PropertyRead, ThisReceiver, TmplAstDeferredBlock, TmplAstDeferredBlockError, TmplAstDeferredBlockLoading, TmplAstDeferredBlockPlaceholder, TmplAstNode} from '@angular/compiler';
import {BlockNode, DeferredTrigger, ForLoopBlock, ForLoopBlockEmpty} from '@angular/compiler/src/render3/r3_ast';
import {AST, Call, ImplicitReceiver, ParseSourceSpan, PropertyRead, ThisReceiver, TmplAstBlockNode, TmplAstDeferredBlock, TmplAstDeferredBlockError, TmplAstDeferredBlockLoading, TmplAstDeferredBlockPlaceholder, TmplAstDeferredTrigger, TmplAstForLoopBlock, TmplAstForLoopBlockEmpty, TmplAstNode} from '@angular/compiler';
import ts from 'typescript';

import {DisplayInfoKind, SYMBOL_TEXT} from './display_parts';
Expand Down Expand Up @@ -50,9 +49,10 @@ export function createNgTemplateQuickInfo(node: TmplAstNode|AST): ts.QuickInfo {
}

export function createQuickInfoForBuiltIn(
node: DeferredTrigger|BlockNode, cursorPositionInTemplate: number): ts.QuickInfo|undefined {
node: TmplAstDeferredTrigger|TmplAstBlockNode, cursorPositionInTemplate: number): ts.QuickInfo|
undefined {
let partSpan: ParseSourceSpan;
if (node instanceof DeferredTrigger) {
if (node instanceof TmplAstDeferredTrigger) {
if (node.prefetchSpan !== null && isWithin(cursorPositionInTemplate, node.prefetchSpan)) {
partSpan = node.prefetchSpan;
} else if (
Expand All @@ -68,10 +68,12 @@ export function createQuickInfoForBuiltIn(
if (node instanceof TmplAstDeferredBlock || node instanceof TmplAstDeferredBlockError ||
node instanceof TmplAstDeferredBlockLoading ||
node instanceof TmplAstDeferredBlockPlaceholder ||
node instanceof ForLoopBlockEmpty && isWithin(cursorPositionInTemplate, node.nameSpan)) {
node instanceof TmplAstForLoopBlockEmpty &&
isWithin(cursorPositionInTemplate, node.nameSpan)) {
partSpan = node.nameSpan;
} else if (
node instanceof ForLoopBlock && isWithin(cursorPositionInTemplate, node.trackKeywordSpan)) {
node instanceof TmplAstForLoopBlock &&
isWithin(cursorPositionInTemplate, node.trackKeywordSpan)) {
partSpan = node.trackKeywordSpan;
} else {
return undefined;
Expand Down

0 comments on commit b40f1e5

Please sign in to comment.