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
This commit is contained in:
Keen Yee Liau 2021-01-12 16:05:17 -08:00 committed by atscott
parent b48eabddb8
commit 625d2c252b
5 changed files with 43 additions and 8 deletions

View File

@ -6,12 +6,12 @@
* found in the LICENSE file at https://angular.io/license * 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 * as ts from 'typescript'; // used as value and is provided at runtime
import {locateSymbols} from './locate_symbol'; import {locateSymbols} from './locate_symbol';
import {findTightestNode, getClassDeclFromDecoratorProp, getPropertyAssignmentFromValue} from './ts_utils'; import {findTightestNode, getClassDeclFromDecoratorProp, getPropertyAssignmentFromValue} from './ts_utils';
import {AstResult, Span} from './types'; import {AstResult, Span} from './types';
import {extractAbsoluteFilePath} from './utils';
/** /**
* Convert Angular Span to TypeScript TextSpan. Angular Span has 'start' and * Convert Angular Span to TypeScript TextSpan. Angular Span has 'start' and
@ -59,10 +59,9 @@ function getUrlFromProperty(
return; return;
} }
const sf = urlNode.getSourceFile();
// Extract url path specified by the url node, which is relative to the TypeScript source file // Extract url path specified by the url node, which is relative to the TypeScript source file
// the url node is defined in. // 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 // 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. // does not have a `fileExists` method, in which case optimistically assume the file exists.

View File

@ -7,7 +7,6 @@
*/ */
import {NgAnalyzedModules} from '@angular/compiler'; import {NgAnalyzedModules} from '@angular/compiler';
import * as path from 'path';
import * as ts from 'typescript'; import * as ts from 'typescript';
import {createDiagnostic, Diagnostic} from './diagnostic_messages'; import {createDiagnostic, Diagnostic} from './diagnostic_messages';
@ -15,7 +14,7 @@ import {getTemplateExpressionDiagnostics} from './expression_diagnostics';
import {findPropertyValueOfType, findTightestNode} from './ts_utils'; import {findPropertyValueOfType, findTightestNode} from './ts_utils';
import * as ng from './types'; import * as ng from './types';
import {TypeScriptServiceHost} from './typescript_host'; 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. * Return diagnostic information for the parsed AST of the template.
@ -161,8 +160,8 @@ function validateUrls(
// picked up by the TS Language Server. // picked up by the TS Language Server.
continue; continue;
} }
const curPath = urlNode.getSourceFile().fileName;
const url = path.join(path.dirname(curPath), urlNode.text); const url = extractAbsoluteFilePath(urlNode);
if (tsLsHost.fileExists(url)) continue; if (tsLsHost.fileExists(url)) continue;
// Exclude opening and closing quotes in the url span. // Exclude opening and closing quotes in the url span.

View File

@ -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 {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'; import {AstResult, DiagnosticTemplateInfo, SelectorInfo, Span, Symbol, SymbolQuery} from './types';
interface SpanHolder { 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);
}

View File

@ -420,6 +420,19 @@ describe('definitions', () => {
expect(def.textSpan).toEqual({start: 0, length: 0}); 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', () => { it('should be able to find a stylesheet from a url', () => {
const fileName = mockHost.addCode(` const fileName = mockHost.addCode(`
@Component({ @Component({

View File

@ -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', () => { it('should not produce diagnostics for slice pipe with arguments', () => {
mockHost.override(TEST_TEMPLATE, ` mockHost.override(TEST_TEMPLATE, `
<div *ngFor="let h of heroes | slice:0:1"> <div *ngFor="let h of heroes | slice:0:1">