Merge pull request #2865 from ybnd/poc-eslint-plugin-autofix-selectors

Introduce custom ESLint rules to apply and enforce new themed component selector convention

.eslintrc.json

@@ -11,7 +11,13 @@
     "eslint-plugin-jsonc",
     "eslint-plugin-rxjs",
     "eslint-plugin-simple-import-sort",
-    "eslint-plugin-import-newlines"
+    "eslint-plugin-import-newlines",
+    "eslint-plugin-jsonc",
+    "dspace-angular-ts",
+    "dspace-angular-html"
+  ],
+  "ignorePatterns": [
+    "lint/test/fixture"
   ],
   "overrides": [
     {
@@ -21,7 +27,8 @@
       "parserOptions": {
         "project": [
           "./tsconfig.json",
-          "./cypress/tsconfig.json"
+          "./cypress/tsconfig.json",
+          "./lint/tsconfig.json"
         ],
         "createDefaultProgram": true
       },
@@ -38,7 +45,10 @@
           "error",
           2,
           {
-            "SwitchCase": 1
+            "SwitchCase": 1,
+            "ignoredNodes": [
+              "ClassBody.body > PropertyDefinition[decorators.length > 0] > .key"
+            ]
           }
         ],
         "max-classes-per-file": [
@@ -212,6 +222,15 @@
         "@typescript-eslint/no-unsafe-return": "off",
         "@typescript-eslint/restrict-template-expressions": "off",
         "@typescript-eslint/require-await": "off",
+        "@typescript-eslint/no-base-to-string": [
+          "error",
+          {
+            "ignoredTypeNames": [
+              "ResourceType",
+              "Error"
+            ]
+          }
+        ],
 
         "deprecation/deprecation": "warn",
 
@@ -238,7 +257,12 @@
           "method"
         ],
 
-        "rxjs/no-nested-subscribe": "off"     // todo: go over _all_ cases
+        "rxjs/no-nested-subscribe": "off",     // todo: go over _all_ cases
+
+        // Custom DSpace Angular rules
+        "dspace-angular-ts/themed-component-classes": "error",
+        "dspace-angular-ts/themed-component-selectors": "error",
+        "dspace-angular-ts/themed-component-usages": "error"
       }
     },
     {
@@ -253,7 +277,10 @@
         "createDefaultProgram": true
       },
       "rules": {
-        "prefer-const": "off"
+        "prefer-const": "off",
+
+        // Custom DSpace Angular rules
+        "dspace-angular-ts/themed-component-usages": "error"
       }
     },
     {
@@ -262,7 +289,11 @@
       ],
       "extends": [
         "plugin:@angular-eslint/template/recommended"
-      ]
+      ],
+      "rules": {
+        // Custom DSpace Angular rules
+        "dspace-angular-html/themed-component-usages": "error"
+      }
     },
     {
       "files": [

.gitattributes

@@ -13,4 +13,7 @@
 *.css eol=lf
 *.scss eol=lf
 *.html eol=lf
-*.svg eol=lf
+*.svg eol=lf
+
+# Generated documentation should have LF line endings to reduce git noise
+docs/lint/**/*.md eol=lf

.github/workflows/build.yml

@@ -85,8 +85,14 @@ jobs:
       - name: Install Yarn dependencies
         run: yarn install --frozen-lockfile
 
+      - name: Build lint plugins
+        run: yarn run build:lint
+
+      - name: Run lint plugin tests
+        run: yarn run test:lint:nobuild
+
       - name: Run lint
-        run: yarn run lint --quiet
+        run: yarn run lint:nobuild --quiet
 
       - name: Check for circular dependencies
         run: yarn run check-circ-deps

angular.json

@@ -266,6 +266,8 @@
           "options": {
             "lintFilePatterns": [
               "src/**/*.ts",
+              "cypress/**/*.ts",
+              "lint/**/*.ts",
               "src/**/*.html",
               "src/**/*.json5"
             ]

cypress/e2e/login-modal.cy.ts

@@ -3,31 +3,31 @@ import { testA11y } from 'cypress/support/utils';
 const page = {
   openLoginMenu() {
     // Click the "Log In" dropdown menu in header
-    cy.get('ds-themed-header [data-test="login-menu"]').click();
+    cy.get('ds-header [data-test="login-menu"]').click();
   },
   openUserMenu() {
     // Once logged in, click the User menu in header
-    cy.get('ds-themed-header [data-test="user-menu"]').click();
+    cy.get('ds-header [data-test="user-menu"]').click();
   },
   submitLoginAndPasswordByPressingButton(email, password) {
     // Enter email
-    cy.get('ds-themed-header [data-test="email"]').type(email);
+    cy.get('ds-header [data-test="email"]').type(email);
     // Enter password
-    cy.get('ds-themed-header [data-test="password"]').type(password);
+    cy.get('ds-header [data-test="password"]').type(password);
     // Click login button
-    cy.get('ds-themed-header [data-test="login-button"]').click();
+    cy.get('ds-header [data-test="login-button"]').click();
   },
   submitLoginAndPasswordByPressingEnter(email, password) {
     // In opened Login modal, fill out email & password, then click Enter
-    cy.get('ds-themed-header [data-test="email"]').type(email);
-    cy.get('ds-themed-header [data-test="password"]').type(password);
-    cy.get('ds-themed-header [data-test="password"]').type('{enter}');
+    cy.get('ds-header [data-test="email"]').type(email);
+    cy.get('ds-header [data-test="password"]').type(password);
+    cy.get('ds-header [data-test="password"]').type('{enter}');
   },
   submitLogoutByPressingButton() {
     // This is the POST command that will actually log us out
     cy.intercept('POST', '/server/api/authn/logout').as('logout');
     // Click logout button
-    cy.get('ds-themed-header [data-test="logout-button"]').click();
+    cy.get('ds-header [data-test="logout-button"]').click();
     // Wait until above POST command responds before continuing
     // (This ensures next action waits until logout completes)
     cy.wait('@logout');
@@ -102,10 +102,10 @@ describe('Login Modal', () => {
     page.openLoginMenu();
 
     // Registration link should be visible
-    cy.get('ds-themed-header [data-test="register"]').should('be.visible');
+    cy.get('ds-header [data-test="register"]').should('be.visible');
 
     // Click registration link & you should go to registration page
-    cy.get('ds-themed-header [data-test="register"]').click();
+    cy.get('ds-header [data-test="register"]').click();
     cy.location('pathname').should('eq', '/register');
     cy.get('ds-register-email').should('exist');
 
@@ -119,10 +119,10 @@ describe('Login Modal', () => {
     page.openLoginMenu();
 
     // Forgot password link should be visible
-    cy.get('ds-themed-header [data-test="forgot"]').should('be.visible');
+    cy.get('ds-header [data-test="forgot"]').should('be.visible');
 
     // Click link & you should go to Forgot Password page
-    cy.get('ds-themed-header [data-test="forgot"]').click();
+    cy.get('ds-header [data-test="forgot"]').click();
     cy.location('pathname').should('eq', '/forgot');
     cy.get('ds-forgot-email').should('exist');
 

cypress/e2e/search-navbar.cy.ts

@@ -1,15 +1,15 @@
 const page = {
   fillOutQueryInNavBar(query) {
     // Click the magnifying glass
-    cy.get('ds-themed-header [data-test="header-search-icon"]').click();
+    cy.get('ds-header [data-test="header-search-icon"]').click();
     // Fill out a query in input that appears
-    cy.get('ds-themed-header [data-test="header-search-box"]').type(query);
+    cy.get('ds-header [data-test="header-search-box"]').type(query);
   },
   submitQueryByPressingEnter() {
-    cy.get('ds-themed-header [data-test="header-search-box"]').type('{enter}');
+    cy.get('ds-header [data-test="header-search-box"]').type('{enter}');
   },
   submitQueryByPressingIcon() {
-    cy.get('ds-themed-header [data-test="header-search-icon"]').click();
+    cy.get('ds-header [data-test="header-search-icon"]').click();
   },
 };
 

docs/lint/html/index.md

@@ -0,0 +1,4 @@
+[DSpace ESLint plugins](../../../lint/README.md) > HTML rules
+_______
+
+- [`dspace-angular-html/themed-component-usages`](./rules/themed-component-usages.md): Themeable components should be used via the selector of their `ThemedComponent` wrapper class

docs/lint/html/rules/themed-component-usages.md

@@ -0,0 +1,110 @@
+[DSpace ESLint plugins](../../../../lint/README.md) > [HTML rules](../index.md) > `dspace-angular-html/themed-component-usages`
+_______
+
+Themeable components should be used via the selector of their `ThemedComponent` wrapper class
+
+This ensures that custom themes can correctly override _all_ instances of this component.
+The only exception to this rule are unit tests, where we may want to use the base component in order to keep the test setup simple.
+
+
+_______
+
+[Source code](../../../../lint/src/rules/html/themed-component-usages.ts)
+
+### Examples
+
+
+#### Valid code
+
+##### use no-prefix selectors in HTML templates
+        
+```html
+<ds-test-themeable/>
+<ds-test-themeable></ds-test-themeable>
+<ds-test-themeable [test]="something"></ds-test-themeable>
+```
+
+##### use no-prefix selectors in TypeScript templates
+        
+```html
+@Component({
+  template: '<ds-test-themeable></ds-test-themeable>'
+})
+class Test {
+}
+```
+
+##### use no-prefix selectors in TypeScript test templates
+        
+Filename: `lint/test/fixture/src/test.spec.ts`
+
+```html
+@Component({
+  template: '<ds-test-themeable></ds-test-themeable>'
+})
+class Test {
+}
+```
+
+##### base selectors are also allowed in TypeScript test templates
+        
+Filename: `lint/test/fixture/src/test.spec.ts`
+
+```html
+@Component({
+  template: '<ds-base-test-themeable></ds-base-test-themeable>'
+})
+class Test {
+}
+```
+
+
+
+
+#### Invalid code  &amp; automatic fixes
+
+##### themed override selectors are not allowed in HTML templates
+        
+```html
+<ds-themed-test-themeable/>
+<ds-themed-test-themeable></ds-themed-test-themeable>
+<ds-themed-test-themeable [test]="something"></ds-themed-test-themeable>
+```
+Will produce the following error(s):
+```
+Themeable components should be used via their ThemedComponent wrapper's selector
+Themeable components should be used via their ThemedComponent wrapper's selector
+Themeable components should be used via their ThemedComponent wrapper's selector
+```
+        
+Result of `yarn lint --fix`:
+```html
+<ds-test-themeable/>
+<ds-test-themeable></ds-test-themeable>
+<ds-test-themeable [test]="something"></ds-test-themeable>
+```
+        
+
+##### base selectors are not allowed in HTML templates
+        
+```html
+<ds-base-test-themeable/>
+<ds-base-test-themeable></ds-base-test-themeable>
+<ds-base-test-themeable [test]="something"></ds-base-test-themeable>
+```
+Will produce the following error(s):
+```
+Themeable components should be used via their ThemedComponent wrapper's selector
+Themeable components should be used via their ThemedComponent wrapper's selector
+Themeable components should be used via their ThemedComponent wrapper's selector
+```
+        
+Result of `yarn lint --fix`:
+```html
+<ds-test-themeable/>
+<ds-test-themeable></ds-test-themeable>
+<ds-test-themeable [test]="something"></ds-test-themeable>
+```
+        
+
+

docs/lint/ts/index.md

@@ -0,0 +1,6 @@
+[DSpace ESLint plugins](../../../lint/README.md) > TypeScript rules
+_______
+
+- [`dspace-angular-ts/themed-component-classes`](./rules/themed-component-classes.md): Formatting rules for themeable component classes
+- [`dspace-angular-ts/themed-component-selectors`](./rules/themed-component-selectors.md): Themeable component selectors should follow the DSpace convention
+- [`dspace-angular-ts/themed-component-usages`](./rules/themed-component-usages.md): Themeable components should be used via their `ThemedComponent` wrapper class