From 4c168ed9ba74d672c5bddf3f398876c3c484334a Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Fri, 13 Sep 2019 13:58:01 -0500 Subject: [PATCH] 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 --- packages/language-service/src/diagnostics.ts | 27 +++++++++++++--- .../language-service/test/diagnostics_spec.ts | 31 +++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/packages/language-service/src/diagnostics.ts b/packages/language-service/src/diagnostics.ts index 5b6dc6f348..dbee32ddd3 100644 --- a/packages/language-service/src/diagnostics.ts +++ b/packages/language-service/src/diagnostics.ts @@ -116,7 +116,7 @@ export function getDeclarationDiagnostics( span: declarationSpan, }); } - const {template, templateUrl} = metadata.template !; + const {template, templateUrl, styleUrls} = metadata.template !; if (template === null && !templateUrl) { results.push({ 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 // instead of string literals. We can then perform a mass migration of all literal usages. const templateUrlNode = findPropertyValueOfType( - directiveIdentifier.parent, 'templateUrl', ts.isStringLiteralLike); + directiveIdentifier.parent, 'templateUrl', ts.isLiteralExpression); if (!templateUrlNode) { logImpossibleState(`templateUrl ${templateUrl} exists but its TypeScript node doesn't`); return []; @@ -146,6 +146,19 @@ export function getDeclarationDiagnostics( 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)) { results.push({ kind: ng.DiagnosticKind.Error, @@ -168,7 +181,7 @@ export function getDeclarationDiagnostics( * @return diagnosed url errors, if any */ function validateUrls( - urls: ts.StringLiteralLike[], tsLsHost: Readonly): ng.Diagnostic[] { + urls: ArrayLike, tsLsHost: Readonly): ng.Diagnostic[] { if (!tsLsHost.fileExists) { return []; } @@ -176,7 +189,13 @@ function validateUrls( const allErrors: ng.Diagnostic[] = []; // 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. - 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 url = path.join(path.dirname(curPath), urlNode.text); if (tsLsHost.fileExists(url)) continue; diff --git a/packages/language-service/test/diagnostics_spec.ts b/packages/language-service/test/diagnostics_spec.ts index 39aa793d04..584c4ae205 100644 --- a/packages/language-service/test/diagnostics_spec.ts +++ b/packages/language-service/test/diagnostics_spec.ts @@ -507,6 +507,37 @@ describe('diagnostics', () => { diagnostics.find(d => d.messageText === 'URL does not point to a valid file'); 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