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

…2674) Similar to diagnostics for invalid templateUrls, check that styleUrls actually point to a valid Url. Closes #32564. PR Close #32674
angular · Sep 17, 2019 · 4c168ed · 4c168ed
1 parent cd4021b
commit 4c168ed
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 4 deletions.
diff --git 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,14 +138,27 @@ 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 [];
         }
 
         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,15 +181,21 @@ export function getDeclarationDiagnostics(
  * @return diagnosed url errors, if any
  */
 function validateUrls(
-    urls: ts.StringLiteralLike[], tsLsHost: Readonly<ts.LanguageServiceHost>): ng.Diagnostic[] {
+    urls: ArrayLike<ts.Expression>, tsLsHost: Readonly<ts.LanguageServiceHost>): ng.Diagnostic[] {
   if (!tsLsHost.fileExists) {
     return [];
   }
 
   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
@@ -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