Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(language-service): fix go to definition for template variables and references #40455

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 17 additions & 10 deletions packages/language-service/ivy/definitions.ts
Expand Up @@ -14,7 +14,7 @@ import * as ts from 'typescript';

import {getTargetAtPosition, TargetNodeKind} from './template_target';
import {findTightestNode, getParentClassDeclaration} from './ts_utils';
import {flatMap, getDirectiveMatchesForAttribute, getDirectiveMatchesForElementTag, getTemplateInfoAtPosition, getTextSpanOfNode, isDollarEvent, isTypeScriptFile, TemplateInfo, toTextSpan} from './utils';
import {flatMap, getDirectiveMatchesForAttribute, getDirectiveMatchesForElementTag, getTemplateInfoAtPosition, getTemplateLocationFromShimLocation, getTextSpanOfNode, isDollarEvent, isTypeScriptFile, TemplateInfo, toTextSpan} from './utils';

interface DefinitionMeta {
node: AST|TmplAstNode;
Expand Down Expand Up @@ -100,15 +100,22 @@ export class DefinitionBuilder {
case SymbolKind.Reference: {
const definitions: ts.DefinitionInfo[] = [];
if (symbol.declaration !== node) {
definitions.push({
name: symbol.declaration.name,
containerName: '',
containerKind: ts.ScriptElementKind.unknown,
kind: ts.ScriptElementKind.variableElement,
textSpan: getTextSpanOfNode(symbol.declaration),
contextSpan: toTextSpan(symbol.declaration.sourceSpan),
fileName: symbol.declaration.sourceSpan.start.file.url,
});
const shimLocation = symbol.kind === SymbolKind.Variable ? symbol.localVarLocation :
symbol.referenceVarLocation;
const mapping = getTemplateLocationFromShimLocation(
this.compiler.getTemplateTypeChecker(), shimLocation.shimPath,
shimLocation.positionInShimFile);
if (mapping !== null) {
definitions.push({
name: symbol.declaration.name,
containerName: '',
containerKind: ts.ScriptElementKind.unknown,
kind: ts.ScriptElementKind.variableElement,
textSpan: getTextSpanOfNode(symbol.declaration),
contextSpan: toTextSpan(symbol.declaration.sourceSpan),
fileName: mapping.templateUrl,
});
}
}
if (symbol.kind === SymbolKind.Variable) {
definitions.push(
Expand Down
23 changes: 5 additions & 18 deletions packages/language-service/ivy/references.ts
Expand Up @@ -14,7 +14,7 @@ import * as ts from 'typescript';

import {getTargetAtPosition, TargetNodeKind} from './template_target';
import {findTightestNode} from './ts_utils';
import {getDirectiveMatchesForAttribute, getDirectiveMatchesForElementTag, getTemplateInfoAtPosition, isWithin, TemplateInfo, toTextSpan} from './utils';
import {getDirectiveMatchesForAttribute, getDirectiveMatchesForElementTag, getTemplateInfoAtPosition, getTemplateLocationFromShimLocation, isWithin, TemplateInfo, toTextSpan} from './utils';

interface FilePosition {
fileName: string;
Expand Down Expand Up @@ -376,27 +376,14 @@ export class ReferencesAndRenameBuilder {
// TODO(atscott): Determine how to consistently resolve paths. i.e. with the project
// serverHost or LSParseConfigHost in the adapter. We should have a better defined way to
// normalize paths.
const mapping = templateTypeChecker.getTemplateMappingAtShimLocation({
shimPath: absoluteFrom(shimDocumentSpan.fileName),
positionInShimFile: shimDocumentSpan.textSpan.start,
});
const mapping = getTemplateLocationFromShimLocation(
templateTypeChecker, absoluteFrom(shimDocumentSpan.fileName),
shimDocumentSpan.textSpan.start);
if (mapping === null) {
return null;
}
const {templateSourceMapping, span} = mapping;

let templateUrl: AbsoluteFsPath;
if (templateSourceMapping.type === 'direct') {
templateUrl = absoluteFromSourceFile(templateSourceMapping.node.getSourceFile());
} else if (templateSourceMapping.type === 'external') {
templateUrl = absoluteFrom(templateSourceMapping.templateUrl);
} else {
// This includes indirect mappings, which are difficult to map directly to the code
// location. Diagnostics similarly return a synthetic template string for this case rather
// than a real location.
return null;
}

const {span, templateUrl} = mapping;
if (requiredNodeText !== undefined && span.toString() !== requiredNodeText) {
return null;
}
Expand Down
22 changes: 22 additions & 0 deletions packages/language-service/ivy/test/definitions_spec.ts
Expand Up @@ -13,6 +13,28 @@ import {extractCursorInfo, LanguageServiceTestEnvironment} from './env';
import {assertFileNames, createModuleWithDeclarations, humanizeDocumentSpanLike} from './test_utils';

describe('definitions', () => {
it('gets definition for template reference in overridden template', () => {
initMockFileSystem('Native');
const templateFile = {contents: '', name: absoluteFrom('/app.html')};
const appFile = {
name: absoluteFrom('/app.ts'),
contents: `
import {Component} from '@angular/core';

@Component({templateUrl: '/app.html'})
export class AppCmp {}
`,
};

const env = createModuleWithDeclarations([appFile], [templateFile]);
const {cursor} = env.overrideTemplateWithCursor(
absoluteFrom('/app.ts'), 'AppCmp', '<input #myInput /> {{myIn¦put.value}}');
env.expectNoSourceDiagnostics();
const {definitions} = env.ngLS.getDefinitionAndBoundSpan(absoluteFrom('/app.html'), cursor)!;
expect(definitions![0].name).toEqual('myInput');
assertFileNames(Array.from(definitions!), ['app.html']);
});

it('returns the pipe class as definition when checkTypeOfPipes is false', () => {
initMockFileSystem('Native');
const {cursor, text} = extractCursorInfo('{{"1/1/2020" | dat¦e}}');
Expand Down
2 changes: 1 addition & 1 deletion packages/language-service/ivy/test/test_utils.ts
Expand Up @@ -60,7 +60,7 @@ export function humanizeDocumentSpanLike<T extends ts.DocumentSpan>(
env.host.readFile(item.fileName)) ??
'';
if (!fileContents) {
throw new Error('Could not read file ${entry.fileName}');
throw new Error(`Could not read file ${item.fileName}`);
}
return {
...item,
Expand Down
31 changes: 30 additions & 1 deletion packages/language-service/ivy/utils.ts
Expand Up @@ -7,9 +7,10 @@
*/
import {AbsoluteSourceSpan, CssSelector, ParseSourceSpan, SelectorMatcher, TmplAstBoundEvent} from '@angular/compiler';
import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core';
import {absoluteFrom, absoluteFromSourceFile, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
import {isExternalResource} from '@angular/compiler-cli/src/ngtsc/metadata';
import {DeclarationNode} from '@angular/compiler-cli/src/ngtsc/reflection';
import {DirectiveSymbol} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
import {DirectiveSymbol, TemplateTypeChecker} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
import * as e from '@angular/compiler/src/expression_parser/ast'; // e for expression AST
import * as t from '@angular/compiler/src/render3/r3_ast'; // t for template AST
import * as ts from 'typescript';
Expand Down Expand Up @@ -346,3 +347,31 @@ export function isWithin(position: number, span: AbsoluteSourceSpan|ParseSourceS
// like ¦start and end¦ where ¦ is the cursor.
return start <= position && position <= end;
}

/**
* For a given location in a shim file, retrieves the corresponding file url for the template and
* the span in the template.
*/
export function getTemplateLocationFromShimLocation(
templateTypeChecker: TemplateTypeChecker, shimPath: AbsoluteFsPath,
positionInShimFile: number): {templateUrl: AbsoluteFsPath, span: ParseSourceSpan}|null {
const mapping =
templateTypeChecker.getTemplateMappingAtShimLocation({shimPath, positionInShimFile});
if (mapping === null) {
return null;
}
const {templateSourceMapping, span} = mapping;

let templateUrl: AbsoluteFsPath;
if (templateSourceMapping.type === 'direct') {
templateUrl = absoluteFromSourceFile(templateSourceMapping.node.getSourceFile());
} else if (templateSourceMapping.type === 'external') {
templateUrl = absoluteFrom(templateSourceMapping.templateUrl);
} else {
// This includes indirect mappings, which are difficult to map directly to the code
// location. Diagnostics similarly return a synthetic template string for this case rather
// than a real location.
return null;
}
return {templateUrl, span};
}