feat(language-service): provide diagnostics for invalid styleUrls (#32674)

Similar to diagnostics for invalid templateUrls, check that styleUrls
actually point to a valid Url.

Closes #32564.

PR Close #32674
This commit is contained in:
ayazhafiz 2019-09-13 13:58:01 -05:00 committed by Andrew Kushnir
parent cd4021ba41
commit 4c168ed9ba
2 changed files with 54 additions and 4 deletions

View File

@ -116,7 +116,7 @@ export function getDeclarationDiagnostics(
span: declarationSpan, span: declarationSpan,
}); });
} }
const {template, templateUrl} = metadata.template !; const {template, templateUrl, styleUrls} = metadata.template !;
if (template === null && !templateUrl) { if (template === null && !templateUrl) {
results.push({ results.push({
kind: ng.DiagnosticKind.Error, kind: ng.DiagnosticKind.Error,
@ -138,7 +138,7 @@ export function getDeclarationDiagnostics(
// TODO: We should create an enum of the various properties a directive can have to use // TODO: We should create an enum of the various properties a directive can have to use
// instead of string literals. We can then perform a mass migration of all literal usages. // instead of string literals. We can then perform a mass migration of all literal usages.
const templateUrlNode = findPropertyValueOfType( const templateUrlNode = findPropertyValueOfType(
directiveIdentifier.parent, 'templateUrl', ts.isStringLiteralLike); directiveIdentifier.parent, 'templateUrl', ts.isLiteralExpression);
if (!templateUrlNode) { if (!templateUrlNode) {
logImpossibleState(`templateUrl ${templateUrl} exists but its TypeScript node doesn't`); logImpossibleState(`templateUrl ${templateUrl} exists but its TypeScript node doesn't`);
return []; return [];
@ -146,6 +146,19 @@ export function getDeclarationDiagnostics(
results.push(...validateUrls([templateUrlNode], host.tsLsHost)); results.push(...validateUrls([templateUrlNode], host.tsLsHost));
} }
if (styleUrls.length > 0) {
// Find styleUrls value from the directive call expression, which is the parent of the
// directive identifier.
const styleUrlsNode = findPropertyValueOfType(
directiveIdentifier.parent, 'styleUrls', ts.isArrayLiteralExpression);
if (!styleUrlsNode) {
logImpossibleState(`styleUrls property exists but its TypeScript node doesn't'`);
return [];
}
results.push(...validateUrls(styleUrlsNode.elements, host.tsLsHost));
}
} else if (!directives.has(declaration.type)) { } else if (!directives.has(declaration.type)) {
results.push({ results.push({
kind: ng.DiagnosticKind.Error, kind: ng.DiagnosticKind.Error,
@ -168,7 +181,7 @@ export function getDeclarationDiagnostics(
* @return diagnosed url errors, if any * @return diagnosed url errors, if any
*/ */
function validateUrls( function validateUrls(
urls: ts.StringLiteralLike[], tsLsHost: Readonly<ts.LanguageServiceHost>): ng.Diagnostic[] { urls: ArrayLike<ts.Expression>, tsLsHost: Readonly<ts.LanguageServiceHost>): ng.Diagnostic[] {
if (!tsLsHost.fileExists) { if (!tsLsHost.fileExists) {
return []; return [];
} }
@ -176,7 +189,13 @@ function validateUrls(
const allErrors: ng.Diagnostic[] = []; const allErrors: ng.Diagnostic[] = [];
// TODO(ayazhafiz): most of this logic can be unified with the logic in // TODO(ayazhafiz): most of this logic can be unified with the logic in
// definitions.ts#getUrlFromProperty. Create a utility function to be used by both. // definitions.ts#getUrlFromProperty. Create a utility function to be used by both.
for (const urlNode of urls) { for (let i = 0; i < urls.length; ++i) {
const urlNode = urls[i];
if (!ts.isStringLiteralLike(urlNode)) {
// If a non-string value is assigned to a URL node (like `templateUrl`), a type error will be
// picked up by the TS Language Server.
continue;
}
const curPath = urlNode.getSourceFile().fileName; const curPath = urlNode.getSourceFile().fileName;
const url = path.join(path.dirname(curPath), urlNode.text); const url = path.join(path.dirname(curPath), urlNode.text);
if (tsLsHost.fileExists(url)) continue; if (tsLsHost.fileExists(url)) continue;

View File

@ -507,6 +507,37 @@ describe('diagnostics', () => {
diagnostics.find(d => d.messageText === 'URL does not point to a valid file'); diagnostics.find(d => d.messageText === 'URL does not point to a valid file');
expect(urlDiagnostic).toBeUndefined(); expect(urlDiagnostic).toBeUndefined();
}); });
it('should report errors for invalid styleUrls', () => {
const fileName = mockHost.addCode(`
@Component({
styleUrls: ['«notAFile»'],
})
export class MyComponent {}`);
const marker = mockHost.getReferenceMarkerFor(fileName, 'notAFile');
const diagnostics = ngLS.getDiagnostics(fileName) !;
const urlDiagnostic =
diagnostics.find(d => d.messageText === 'URL does not point to a valid file');
expect(urlDiagnostic).toBeDefined();
const {start, length} = urlDiagnostic !;
expect(start).toBe(marker.start);
expect(length).toBe(marker.length);
});
it('should not report errors for valid styleUrls', () => {
const fileName = '/app/app.component.ts';
mockHost.override(fileName, `
@Component({
styleUrls: ['./test.css', './test.css'],
})
export class MyComponent {}`);
const diagnostics = ngLS.getDiagnostics(fileName) !;
expect(diagnostics.length).toBe(0);
});
}); });
// https://github.com/angular/vscode-ng-language-service/issues/235 // https://github.com/angular/vscode-ng-language-service/issues/235