From 625d2c252b7a588c9851149fa1f344129991b8b4 Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Tue, 12 Jan 2021 16:05:17 -0800 Subject: [PATCH] fix(language-service): diagnostic and definition should work for absolute url (#40406) This commit fixes a bug in the **View Engine** implementation of `getSemanticDiagnostics` and `getDefinitionAndBoundSpan` for node in the decorator metadata that represents an external URL (`templateUrl` or `styleUrls`). The URL could be either relative or absolute, but the latter was not taken into account. Fix https://github.com/angular/vscode-ng-language-service/issues/1055 PR Close #40406 --- packages/language-service/src/definitions.ts | 5 ++--- packages/language-service/src/diagnostics.ts | 7 +++---- packages/language-service/src/utils.ts | 13 ++++++++++++- packages/language-service/test/definitions_spec.ts | 13 +++++++++++++ packages/language-service/test/diagnostics_spec.ts | 13 +++++++++++++ 5 files changed, 43 insertions(+), 8 deletions(-) diff --git a/packages/language-service/src/definitions.ts b/packages/language-service/src/definitions.ts index 620e954e5e..6a2653b50e 100644 --- a/packages/language-service/src/definitions.ts +++ b/packages/language-service/src/definitions.ts @@ -6,12 +6,12 @@ * found in the LICENSE file at https://angular.io/license */ -import * as path from 'path'; import * as ts from 'typescript'; // used as value and is provided at runtime import {locateSymbols} from './locate_symbol'; import {findTightestNode, getClassDeclFromDecoratorProp, getPropertyAssignmentFromValue} from './ts_utils'; import {AstResult, Span} from './types'; +import {extractAbsoluteFilePath} from './utils'; /** * Convert Angular Span to TypeScript TextSpan. Angular Span has 'start' and @@ -59,10 +59,9 @@ function getUrlFromProperty( return; } - const sf = urlNode.getSourceFile(); // Extract url path specified by the url node, which is relative to the TypeScript source file // the url node is defined in. - const url = path.join(path.dirname(sf.fileName), urlNode.text); + const url = extractAbsoluteFilePath(urlNode); // If the file does not exist, bail. It is possible that the TypeScript language service host // does not have a `fileExists` method, in which case optimistically assume the file exists. diff --git a/packages/language-service/src/diagnostics.ts b/packages/language-service/src/diagnostics.ts index adbf824a04..342a3585d7 100644 --- a/packages/language-service/src/diagnostics.ts +++ b/packages/language-service/src/diagnostics.ts @@ -7,7 +7,6 @@ */ import {NgAnalyzedModules} from '@angular/compiler'; -import * as path from 'path'; import * as ts from 'typescript'; import {createDiagnostic, Diagnostic} from './diagnostic_messages'; @@ -15,7 +14,7 @@ import {getTemplateExpressionDiagnostics} from './expression_diagnostics'; import {findPropertyValueOfType, findTightestNode} from './ts_utils'; import * as ng from './types'; import {TypeScriptServiceHost} from './typescript_host'; -import {offsetSpan, spanOf} from './utils'; +import {extractAbsoluteFilePath, offsetSpan, spanOf} from './utils'; /** * Return diagnostic information for the parsed AST of the template. @@ -161,8 +160,8 @@ function validateUrls( // picked up by the TS Language Server. continue; } - const curPath = urlNode.getSourceFile().fileName; - const url = path.join(path.dirname(curPath), urlNode.text); + + const url = extractAbsoluteFilePath(urlNode); if (tsLsHost.fileExists(url)) continue; // Exclude opening and closing quotes in the url span. diff --git a/packages/language-service/src/utils.ts b/packages/language-service/src/utils.ts index 9cbb0432fb..70ca4caa38 100644 --- a/packages/language-service/src/utils.ts +++ b/packages/language-service/src/utils.ts @@ -7,7 +7,8 @@ */ import {AstPath, BoundEventAst, CompileDirectiveSummary, CompileTypeMetadata, CssSelector, DirectiveAst, ElementAst, EmbeddedTemplateAst, HtmlAstPath, identifierName, Identifiers, Node, ParseSourceSpan, RecursiveTemplateAstVisitor, RecursiveVisitor, TemplateAst, TemplateAstPath, templateVisitAll, visitAll} from '@angular/compiler'; -import {getClassDeclFromDecoratorProp, getPropertyAssignmentFromValue} from './ts_utils'; +import * as path from 'path'; + import {AstResult, DiagnosticTemplateInfo, SelectorInfo, Span, Symbol, SymbolQuery} from './types'; interface SpanHolder { @@ -204,3 +205,13 @@ export function findOutputBinding( } } } + +/** + * Returns an absolute path from the text in `node`. If the text is already + * an absolute path, return it as is, otherwise join the path with the filename + * of the source file. + */ +export function extractAbsoluteFilePath(node: ts.StringLiteralLike) { + const url = node.text; + return path.isAbsolute(url) ? url : path.join(path.dirname(node.getSourceFile().fileName), url); +} diff --git a/packages/language-service/test/definitions_spec.ts b/packages/language-service/test/definitions_spec.ts index 2bb0dcd5f8..0a754d12fc 100644 --- a/packages/language-service/test/definitions_spec.ts +++ b/packages/language-service/test/definitions_spec.ts @@ -420,6 +420,19 @@ describe('definitions', () => { expect(def.textSpan).toEqual({start: 0, length: 0}); }); + it('should be able to find a template from an absolute url', () => { + const fileName = mockHost.addCode(` + @Component({ + templateUrl: '${TEST_TEMPLATE}', + }) + export class MyComponent {}`); + + const marker = mockHost.readFile(fileName)!.indexOf(TEST_TEMPLATE); + const result = ngService.getDefinitionAndBoundSpan(fileName, marker); + + expect(result?.definitions?.[0].fileName).toBe(TEST_TEMPLATE); + }); + it('should be able to find a stylesheet from a url', () => { const fileName = mockHost.addCode(` @Component({ diff --git a/packages/language-service/test/diagnostics_spec.ts b/packages/language-service/test/diagnostics_spec.ts index 09fea059b4..99b2ce9bb0 100644 --- a/packages/language-service/test/diagnostics_spec.ts +++ b/packages/language-service/test/diagnostics_spec.ts @@ -89,6 +89,19 @@ describe('diagnostics', () => { } }); + it('should not produce diagnostics for absolute template url', () => { + mockHost.override(APP_COMPONENT, ` + import {Component} from '@angular/core'; + + @Component({ + templateUrl: '${TEST_TEMPLATE}', + }) + export class AppComponent {} + `); + const diags = ngLS.getSemanticDiagnostics(APP_COMPONENT); + expect(diags).toEqual([]); + }); + it('should not produce diagnostics for slice pipe with arguments', () => { mockHost.override(TEST_TEMPLATE, `