fix(eslint-plugin): [relative-url-prefix] valid relative urls being reported (#456)

Author: rafaelss95

packages/eslint-plugin/src/rules/relative-url-prefix.ts

@@ -1,25 +1,21 @@
 import type { TSESTree } from '@typescript-eslint/experimental-utils';
 import { createESLintRule } from '../utils/create-eslint-rule';
 import { COMPONENT_CLASS_DECORATOR } from '../utils/selectors';
-import {
-  getDecoratorProperty,
-  isArrayExpression,
-  isLiteralWithStringValue,
-} from '../utils/utils';
+import { isArrayExpression, isLiteralWithStringValue } from '../utils/utils';
 
 type Options = [];
 export type MessageIds = 'relativeUrlPrefix';
 export const RULE_NAME = 'relative-url-prefix';
 
 const STYLE_GUIDE_LINK = 'https://angular.io/styleguide#style-05-04';
-const RELATIVE_URL_PREFIX_MATCHER = /^\.{1,2}\/[^./]/;
+const RELATIVE_URL_PREFIX_MATCHER = /^\.\.?\/.+/;
 
 export default createESLintRule<Options, MessageIds>({
   name: RULE_NAME,
   meta: {
     type: 'suggestion',
     docs: {
-      description: `The ./ and ../ prefix is standard syntax for relative URLs; don't depend on Angular's current ability to do without that prefix. See more at ${STYLE_GUIDE_LINK}.`,
+      description: `The ./ and ../ prefix is standard syntax for relative URLs; don't depend on Angular's current ability to do without that prefix. See more at ${STYLE_GUIDE_LINK}`,
       category: 'Best Practices',
       recommended: false,
     },
@@ -31,43 +27,35 @@ export default createESLintRule<Options, MessageIds>({
   defaultOptions: [],
   create(context) {
     return {
-      [COMPONENT_CLASS_DECORATOR](node: TSESTree.Decorator) {
-        const templateUrlProperty = getDecoratorProperty(node, 'templateUrl');
-        if (
-          templateUrlProperty &&
-          isLiteralWithStringValue(templateUrlProperty.value)
-        ) {
-          if (
-            !RELATIVE_URL_PREFIX_MATCHER.test(templateUrlProperty.value.value)
-          ) {
-            context.report({
-              node: templateUrlProperty.value,
-              messageId: 'relativeUrlPrefix',
-            });
-          }
-        }
+      [`${COMPONENT_CLASS_DECORATOR} Property[key.name='templateUrl']`]({
+        value,
+      }: TSESTree.Property) {
+        if (!isUrlInvalid(value)) return;
 
-        const styleUrlsProperty = getDecoratorProperty(node, 'styleUrls');
-        if (styleUrlsProperty) {
-          if (
-            styleUrlsProperty.value &&
-            isArrayExpression(styleUrlsProperty.value) &&
-            styleUrlsProperty.value.elements.length > 0
-          ) {
-            styleUrlsProperty.value.elements.forEach((e) => {
-              if (
-                isLiteralWithStringValue(e) &&
-                !RELATIVE_URL_PREFIX_MATCHER.test(e.value)
-              ) {
-                context.report({
-                  node: e,
-                  messageId: 'relativeUrlPrefix',
-                });
-              }
-            });
-          }
-        }
+        context.report({
+          node: value,
+          messageId: 'relativeUrlPrefix',
+        });
+      },
+      [`${COMPONENT_CLASS_DECORATOR} Property[key.name='styleUrls']`]({
+        value,
+      }: TSESTree.Property) {
+        if (!isArrayExpression(value)) return;
+
+        value.elements.filter(isUrlInvalid).forEach((element) => {
+          context.report({
+            node: element,
+            messageId: 'relativeUrlPrefix',
+          });
+        });
       },
     };
   },
 });
+
+function isUrlInvalid(node: TSESTree.Property | TSESTree.Property['value']) {
+  return (
+    !isLiteralWithStringValue(node) ||
+    !RELATIVE_URL_PREFIX_MATCHER.test(node.value)
+  );
+}

packages/eslint-plugin/tests/rules/relative-url-prefix.test.ts

@@ -12,78 +12,74 @@ import rule, { RULE_NAME } from '../../src/rules/relative-url-prefix';
 const ruleTester = new RuleTester({
   parser: '@typescript-eslint/parser',
 });
-
 const messageId: MessageIds = 'relativeUrlPrefix';
 
 ruleTester.run(RULE_NAME, rule, {
   valid: [
     `
     @Component({
-      styleUrls: ['./foobar.css']
+      styleUrls: [
+        './foo.css',
+        '../bar.css',
+        '../../baz.scss',
+        '../../../baz.sass',
+        './../test.css',
+        '.././angular.sass'
+      ]
     })
     class Test {}
-`,
+    `,
     `
     @Component({
-      styleUrls: ['../foobar.css']
+      templateUrl: './foobar.html'
     })
     class Test {}
-`,
+    `,
     `
     @Component({
-      styleUrls: ['./foo.css', './bar.css', './whatyouwant.css']
+      templateUrl: '../foobar.html'
     })
     class Test {}
-`,
+    `,
     `
     @Component({
-      templateUrl: './foobar.html'
+      templateUrl: '../../foobar.html'
     })
     class Test {}
-`,
+    `,
     `
     @Component({
-      templateUrl: '../foobar.html'
+      templateUrl: '../../../foobar.html'
     })
     class Test {}
-`,
+    `,
+    `
+    @Component({
+      templateUrl: './../foobar.html'
+    })
+    class Test {}
+    `,
+    `
+    @Component({
+      templateUrl: '.././foobar.html'
+    })
+    class Test {}
+    `,
   ],
   invalid: [
     convertAnnotatedSourceToFailureCase({
-      description: `it should fail when a relative URL isn't prefixed with ./`,
+      description: 'it should fail if one of "styleUrls" is absolute',
       annotatedSource: `
         @Component({
-          styleUrls: ['foobar.css']
-                      ~~~~~~~~~~~~
-        })
-        class Test {}
-      `,
-      messageId,
-    }),
-    convertAnnotatedSourceToFailureCase({
-      description: `it should fail when a relative URL isn't prefixed with ./`,
-      annotatedSource: `
-        @Component({
-          styleUrls: ['./../foobar.css']
-                      ~~~~~~~~~~~~~~~~~
-        })
-        class Test {}
-      `,
-      messageId,
-    }),
-    convertAnnotatedSourceToFailureCase({
-      description: `it should fail when one relative URL isn't prefixed with ./`,
-      annotatedSource: `
-        @Component({
-          styleUrls: ['./foo.css', 'bar.css', './whatyouwant.css']
+          styleUrls: ['./foo.css', 'bar.css', '../baz.scss', '../../test.css']
                                    ~~~~~~~~~
         })
         class Test {}
       `,
       messageId,
     }),
     convertAnnotatedSourceToFailureCase({
-      description: `it should fail when a relative URL isn't prefixed with ./`,
+      description: 'it should fail if "templateUrl" is absolute',
       annotatedSource: `
         @Component({
           templateUrl: 'foobar.html'
@@ -93,16 +89,5 @@ ruleTester.run(RULE_NAME, rule, {
       `,
       messageId,
     }),
-    convertAnnotatedSourceToFailureCase({
-      description: `it should fail when a relative URL isn't prefixed with ./`,
-      annotatedSource: `
-        @Component({
-          templateUrl: '.././foobar.html'
-                       ~~~~~~~~~~~~~~~~~~
-        })
-        class Test {}
-      `,
-      messageId,
-    }),
   ],
 });