Skip to content

Commit 625d2c2

Browse files
kyliauatscott
authored andcommitted
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 angular/vscode-ng-language-service#1055 PR Close #40406
1 parent b48eabd commit 625d2c2

File tree

5 files changed

+43
-8
lines changed

5 files changed

+43
-8
lines changed

packages/language-service/src/definitions.ts

+2-3
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import * as path from 'path';
109
import * as ts from 'typescript'; // used as value and is provided at runtime
1110

1211
import {locateSymbols} from './locate_symbol';
1312
import {findTightestNode, getClassDeclFromDecoratorProp, getPropertyAssignmentFromValue} from './ts_utils';
1413
import {AstResult, Span} from './types';
14+
import {extractAbsoluteFilePath} from './utils';
1515

1616
/**
1717
* Convert Angular Span to TypeScript TextSpan. Angular Span has 'start' and
@@ -59,10 +59,9 @@ function getUrlFromProperty(
5959
return;
6060
}
6161

62-
const sf = urlNode.getSourceFile();
6362
// Extract url path specified by the url node, which is relative to the TypeScript source file
6463
// the url node is defined in.
65-
const url = path.join(path.dirname(sf.fileName), urlNode.text);
64+
const url = extractAbsoluteFilePath(urlNode);
6665

6766
// If the file does not exist, bail. It is possible that the TypeScript language service host
6867
// does not have a `fileExists` method, in which case optimistically assume the file exists.

packages/language-service/src/diagnostics.ts

+3-4
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,14 @@
77
*/
88

99
import {NgAnalyzedModules} from '@angular/compiler';
10-
import * as path from 'path';
1110
import * as ts from 'typescript';
1211

1312
import {createDiagnostic, Diagnostic} from './diagnostic_messages';
1413
import {getTemplateExpressionDiagnostics} from './expression_diagnostics';
1514
import {findPropertyValueOfType, findTightestNode} from './ts_utils';
1615
import * as ng from './types';
1716
import {TypeScriptServiceHost} from './typescript_host';
18-
import {offsetSpan, spanOf} from './utils';
17+
import {extractAbsoluteFilePath, offsetSpan, spanOf} from './utils';
1918

2019
/**
2120
* Return diagnostic information for the parsed AST of the template.
@@ -161,8 +160,8 @@ function validateUrls(
161160
// picked up by the TS Language Server.
162161
continue;
163162
}
164-
const curPath = urlNode.getSourceFile().fileName;
165-
const url = path.join(path.dirname(curPath), urlNode.text);
163+
164+
const url = extractAbsoluteFilePath(urlNode);
166165
if (tsLsHost.fileExists(url)) continue;
167166

168167
// Exclude opening and closing quotes in the url span.

packages/language-service/src/utils.ts

+12-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
*/
88

99
import {AstPath, BoundEventAst, CompileDirectiveSummary, CompileTypeMetadata, CssSelector, DirectiveAst, ElementAst, EmbeddedTemplateAst, HtmlAstPath, identifierName, Identifiers, Node, ParseSourceSpan, RecursiveTemplateAstVisitor, RecursiveVisitor, TemplateAst, TemplateAstPath, templateVisitAll, visitAll} from '@angular/compiler';
10-
import {getClassDeclFromDecoratorProp, getPropertyAssignmentFromValue} from './ts_utils';
10+
import * as path from 'path';
11+
1112
import {AstResult, DiagnosticTemplateInfo, SelectorInfo, Span, Symbol, SymbolQuery} from './types';
1213

1314
interface SpanHolder {
@@ -204,3 +205,13 @@ export function findOutputBinding(
204205
}
205206
}
206207
}
208+
209+
/**
210+
* Returns an absolute path from the text in `node`. If the text is already
211+
* an absolute path, return it as is, otherwise join the path with the filename
212+
* of the source file.
213+
*/
214+
export function extractAbsoluteFilePath(node: ts.StringLiteralLike) {
215+
const url = node.text;
216+
return path.isAbsolute(url) ? url : path.join(path.dirname(node.getSourceFile().fileName), url);
217+
}

packages/language-service/test/definitions_spec.ts

+13
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,19 @@ describe('definitions', () => {
420420
expect(def.textSpan).toEqual({start: 0, length: 0});
421421
});
422422

423+
it('should be able to find a template from an absolute url', () => {
424+
const fileName = mockHost.addCode(`
425+
@Component({
426+
templateUrl: '${TEST_TEMPLATE}',
427+
})
428+
export class MyComponent {}`);
429+
430+
const marker = mockHost.readFile(fileName)!.indexOf(TEST_TEMPLATE);
431+
const result = ngService.getDefinitionAndBoundSpan(fileName, marker);
432+
433+
expect(result?.definitions?.[0].fileName).toBe(TEST_TEMPLATE);
434+
});
435+
423436
it('should be able to find a stylesheet from a url', () => {
424437
const fileName = mockHost.addCode(`
425438
@Component({

packages/language-service/test/diagnostics_spec.ts

+13
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,19 @@ describe('diagnostics', () => {
8989
}
9090
});
9191

92+
it('should not produce diagnostics for absolute template url', () => {
93+
mockHost.override(APP_COMPONENT, `
94+
import {Component} from '@angular/core';
95+
96+
@Component({
97+
templateUrl: '${TEST_TEMPLATE}',
98+
})
99+
export class AppComponent {}
100+
`);
101+
const diags = ngLS.getSemanticDiagnostics(APP_COMPONENT);
102+
expect(diags).toEqual([]);
103+
});
104+
92105
it('should not produce diagnostics for slice pipe with arguments', () => {
93106
mockHost.override(TEST_TEMPLATE, `
94107
<div *ngFor="let h of heroes | slice:0:1">

0 commit comments

Comments
 (0)