Merge pull request #2343 from ybnd/more-eslint

Stricter ESLint configuration for code quality, consistency & cleaner merges

Author: tdonohue

.eslintrc.json

@@ -8,7 +8,10 @@
     "eslint-plugin-deprecation",
     "unused-imports",
     "eslint-plugin-lodash",
-    "eslint-plugin-jsonc"
+    "eslint-plugin-jsonc",
+    "eslint-plugin-rxjs",
+    "eslint-plugin-simple-import-sort",
+    "eslint-plugin-import-newlines"
   ],
   "overrides": [
     {
@@ -27,17 +30,29 @@
         "plugin:@typescript-eslint/recommended",
         "plugin:@typescript-eslint/recommended-requiring-type-checking",
         "plugin:@angular-eslint/recommended",
-        "plugin:@angular-eslint/template/process-inline-templates"
+        "plugin:@angular-eslint/template/process-inline-templates",
+        "plugin:rxjs/recommended"
       ],
       "rules": {
+        "indent": [
+          "error",
+          2,
+          {
+            "SwitchCase": 1
+          }
+        ],
         "max-classes-per-file": [
           "error",
           1
         ],
         "comma-dangle": [
-          "off",
+          "error",
           "always-multiline"
         ],
+        "object-curly-spacing": [
+          "error",
+          "always"
+        ],
         "eol-last": [
           "error",
           "always"
@@ -104,15 +119,13 @@
             "allowTernary": true
           }
         ],
-        "prefer-const": "off",            // todo: re-enable & fix errors (more strict than it used to be in TSLint)
+        "prefer-const": "error",
+        "no-case-declarations": "error",
+        "no-extra-boolean-cast": "error",
         "prefer-spread": "off",
         "no-underscore-dangle": "off",
-
-        // todo: disabled rules from eslint:recommended, consider re-enabling & fixing
         "no-prototype-builtins": "off",
         "no-useless-escape": "off",
-        "no-case-declarations": "off",
-        "no-extra-boolean-cast": "off",
 
         "@angular-eslint/directive-selector": [
           "error",
@@ -183,7 +196,7 @@
         ],
         "@typescript-eslint/type-annotation-spacing": "error",
         "@typescript-eslint/unified-signatures": "error",
-        "@typescript-eslint/ban-types": "warn",  // todo: deal with {} type issues & re-enable
+        "@typescript-eslint/ban-types": "error",
         "@typescript-eslint/no-floating-promises": "warn",
         "@typescript-eslint/no-misused-promises": "warn",
         "@typescript-eslint/restrict-plus-operands": "warn",
@@ -203,14 +216,45 @@
 
         "deprecation/deprecation": "warn",
 
+        "simple-import-sort/imports": "error",
+        "simple-import-sort/exports": "error",
         "import/order": "off",
+        "import/first": "error",
+        "import/newline-after-import": "error",
+        "import/no-duplicates": "error",
         "import/no-deprecated": "warn",
         "import/no-namespace": "error",
+        "import-newlines/enforce": [
+          "error",
+          {
+            "items": 1,
+            "semi": true,
+            "forceSingleLine": true
+          }
+        ],
+
         "unused-imports/no-unused-imports": "error",
         "lodash/import-scope": [
           "error",
           "method"
-        ]
+        ],
+
+        "rxjs/no-nested-subscribe": "off"     // todo: go over _all_ cases
+      }
+    },
+    {
+      "files": [
+        "*.spec.ts"
+      ],
+      "parserOptions": {
+        "project": [
+          "./tsconfig.json",
+          "./cypress/tsconfig.json"
+        ],
+        "createDefaultProgram": true
+      },
+      "rules": {
+        "prefer-const": "off"
       }
     },
     {
@@ -219,12 +263,7 @@
       ],
       "extends": [
         "plugin:@angular-eslint/template/recommended"
-      ],
-      "rules": {
-        // todo: re-enable & fix errors
-        "@angular-eslint/template/no-negated-async": "off",
-        "@angular-eslint/template/eqeqeq": "off"
-      }
+      ]
     },
     {
       "files": [

cypress/support/e2e.ts

@@ -21,7 +21,6 @@ import './commands';
 import 'cypress-axe';
 import { DSPACE_XSRF_COOKIE } from 'src/app/core/xsrf/xsrf.constants';
 
-
 // Runs once before all tests
 before(() => {
   // Cypress doesn't have access to the running application in Node.js.

package.json

@@ -170,9 +170,12 @@
     "eslint": "^8.39.0",
     "eslint-plugin-deprecation": "^1.4.1",
     "eslint-plugin-import": "^2.27.5",
+    "eslint-plugin-import-newlines": "^1.3.1",
     "eslint-plugin-jsdoc": "^45.0.0",
     "eslint-plugin-jsonc": "^2.6.0",
     "eslint-plugin-lodash": "^7.4.0",
+    "eslint-plugin-rxjs": "^5.0.3",
+    "eslint-plugin-simple-import-sort": "^10.0.0",
     "eslint-plugin-unused-imports": "^2.0.0",
     "express-static-gzip": "^2.1.7",
     "jasmine-core": "^3.8.0",

src/app/access-control/access-control-routing-paths.ts

@@ -1,5 +1,5 @@
-import { URLCombiner } from '../core/url-combiner/url-combiner';
 import { getAccessControlModuleRoute } from '../app-routing-paths';
+import { URLCombiner } from '../core/url-combiner/url-combiner';
 
 export const EPERSON_PATH = 'epeople';
 

src/app/access-control/access-control-routing.module.ts

@@ -1,20 +1,20 @@
 import { NgModule } from '@angular/core';
 import { RouterModule } from '@angular/router';
-import { EPeopleRegistryComponent } from './epeople-registry/epeople-registry.component';
-import { GroupFormComponent } from './group-registry/group-form/group-form.component';
-import { GroupsRegistryComponent } from './group-registry/groups-registry.component';
-import { EPERSON_PATH, GROUP_PATH } from './access-control-routing-paths';
+
 import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.resolver';
-import { GroupPageGuard } from './group-registry/group-page.guard';
+import { GroupAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/group-administrator.guard';
+import { SiteAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/site-administrator.guard';
 import {
-  GroupAdministratorGuard
-} from '../core/data/feature-authorization/feature-authorization-guard/group-administrator.guard';
-import {
-  SiteAdministratorGuard
-} from '../core/data/feature-authorization/feature-authorization-guard/site-administrator.guard';
+  EPERSON_PATH,
+  GROUP_PATH,
+} from './access-control-routing-paths';
 import { BulkAccessComponent } from './bulk-access/bulk-access.component';
+import { EPeopleRegistryComponent } from './epeople-registry/epeople-registry.component';
 import { EPersonFormComponent } from './epeople-registry/eperson-form/eperson-form.component';
 import { EPersonResolver } from './epeople-registry/eperson-resolver.service';
+import { GroupFormComponent } from './group-registry/group-form/group-form.component';
+import { GroupPageGuard } from './group-registry/group-page.guard';
+import { GroupsRegistryComponent } from './group-registry/groups-registry.component';
 
 @NgModule({
   imports: [
@@ -23,10 +23,10 @@ import { EPersonResolver } from './epeople-registry/eperson-resolver.service';
         path: EPERSON_PATH,
         component: EPeopleRegistryComponent,
         resolve: {
-          breadcrumb: I18nBreadcrumbResolver
+          breadcrumb: I18nBreadcrumbResolver,
         },
         data: { title: 'admin.access-control.epeople.title', breadcrumbKey: 'admin.access-control.epeople' },
-        canActivate: [SiteAdministratorGuard]
+        canActivate: [SiteAdministratorGuard],
       },
       {
         path: `${EPERSON_PATH}/create`,
@@ -51,40 +51,40 @@ import { EPersonResolver } from './epeople-registry/eperson-resolver.service';
         path: GROUP_PATH,
         component: GroupsRegistryComponent,
         resolve: {
-          breadcrumb: I18nBreadcrumbResolver
+          breadcrumb: I18nBreadcrumbResolver,
         },
         data: { title: 'admin.access-control.groups.title', breadcrumbKey: 'admin.access-control.groups' },
-        canActivate: [GroupAdministratorGuard]
+        canActivate: [GroupAdministratorGuard],
       },
       {
         path: `${GROUP_PATH}/create`,
         component: GroupFormComponent,
         resolve: {
-          breadcrumb: I18nBreadcrumbResolver
+          breadcrumb: I18nBreadcrumbResolver,
         },
         data: { title: 'admin.access-control.groups.title.addGroup', breadcrumbKey: 'admin.access-control.groups.addGroup' },
-        canActivate: [GroupAdministratorGuard]
+        canActivate: [GroupAdministratorGuard],
       },
       {
         path: `${GROUP_PATH}/:groupId/edit`,
         component: GroupFormComponent,
         resolve: {
-          breadcrumb: I18nBreadcrumbResolver
+          breadcrumb: I18nBreadcrumbResolver,
         },
         data: { title: 'admin.access-control.groups.title.singleGroup', breadcrumbKey: 'admin.access-control.groups.singleGroup' },
-        canActivate: [GroupPageGuard]
+        canActivate: [GroupPageGuard],
       },
       {
         path: 'bulk-access',
         component: BulkAccessComponent,
         resolve: {
-          breadcrumb: I18nBreadcrumbResolver
+          breadcrumb: I18nBreadcrumbResolver,
         },
         data: { title: 'admin.access-control.bulk-access.title', breadcrumbKey: 'admin.access-control.bulk-access' },
-        canActivate: [SiteAdministratorGuard]
+        canActivate: [SiteAdministratorGuard],
       },
-    ])
-  ]
+    ]),
+  ],
 })
 /**
  * Routing module for the AccessControl section of the admin sidebar

src/app/access-control/access-control.module.ts

@@ -1,23 +1,27 @@
 import { CommonModule } from '@angular/common';
 import { NgModule } from '@angular/core';
+import { AbstractControl } from '@angular/forms';
 import { RouterModule } from '@angular/router';
+import { NgbAccordionModule } from '@ng-bootstrap/ng-bootstrap';
+import {
+  DYNAMIC_ERROR_MESSAGES_MATCHER,
+  DynamicErrorMessagesMatcher,
+} from '@ng-dynamic-forms/core';
+
+import { AccessControlFormModule } from '../shared/access-control-form-container/access-control-form.module';
+import { FormModule } from '../shared/form/form.module';
+import { SearchModule } from '../shared/search/search.module';
 import { SharedModule } from '../shared/shared.module';
 import { AccessControlRoutingModule } from './access-control-routing.module';
+import { BulkAccessBrowseComponent } from './bulk-access/browse/bulk-access-browse.component';
+import { BulkAccessComponent } from './bulk-access/bulk-access.component';
+import { BulkAccessSettingsComponent } from './bulk-access/settings/bulk-access-settings.component';
 import { EPeopleRegistryComponent } from './epeople-registry/epeople-registry.component';
 import { EPersonFormComponent } from './epeople-registry/eperson-form/eperson-form.component';
 import { GroupFormComponent } from './group-registry/group-form/group-form.component';
 import { MembersListComponent } from './group-registry/group-form/members-list/members-list.component';
 import { SubgroupsListComponent } from './group-registry/group-form/subgroup-list/subgroups-list.component';
 import { GroupsRegistryComponent } from './group-registry/groups-registry.component';
-import { FormModule } from '../shared/form/form.module';
-import { DYNAMIC_ERROR_MESSAGES_MATCHER, DynamicErrorMessagesMatcher } from '@ng-dynamic-forms/core';
-import { AbstractControl } from '@angular/forms';
-import { BulkAccessComponent } from './bulk-access/bulk-access.component';
-import { NgbAccordionModule } from '@ng-bootstrap/ng-bootstrap';
-import { BulkAccessBrowseComponent } from './bulk-access/browse/bulk-access-browse.component';
-import { BulkAccessSettingsComponent } from './bulk-access/settings/bulk-access-settings.component';
-import { SearchModule } from '../shared/search/search.module';
-import { AccessControlFormModule } from '../shared/access-control-form-container/access-control-form.module';
 
 /**
  * Condition for displaying error messages on email form field
@@ -55,9 +59,9 @@ export const ValidateEmailErrorStateMatcher: DynamicErrorMessagesMatcher =
   providers: [
     {
       provide: DYNAMIC_ERROR_MESSAGES_MATCHER,
-      useValue: ValidateEmailErrorStateMatcher
+      useValue: ValidateEmailErrorStateMatcher,
     },
-  ]
+  ],
 })
 /**
  * This module handles all components related to the access control pages

src/app/access-control/bulk-access/browse/bulk-access-browse.component.spec.ts

@@ -1,16 +1,22 @@
-import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
 import { NO_ERRORS_SCHEMA } from '@angular/core';
-
-import { of } from 'rxjs';
-import { NgbAccordionModule, NgbNavModule } from '@ng-bootstrap/ng-bootstrap';
+import {
+  ComponentFixture,
+  TestBed,
+  waitForAsync,
+} from '@angular/core/testing';
+import {
+  NgbAccordionModule,
+  NgbNavModule,
+} from '@ng-bootstrap/ng-bootstrap';
 import { TranslateModule } from '@ngx-translate/core';
+import { of } from 'rxjs';
 
-import { BulkAccessBrowseComponent } from './bulk-access-browse.component';
+import { buildPaginatedList } from '../../../core/data/paginated-list.model';
+import { PageInfo } from '../../../core/shared/page-info.model';
 import { SelectableListService } from '../../../shared/object-list/selectable-list/selectable-list.service';
 import { SelectableObject } from '../../../shared/object-list/selectable-list/selectable-list.service.spec';
-import { PageInfo } from '../../../core/shared/page-info.model';
-import { buildPaginatedList } from '../../../core/data/paginated-list.model';
 import { createSuccessfulRemoteDataObject } from '../../../shared/remote-data.utils';
+import { BulkAccessBrowseComponent } from './bulk-access-browse.component';
 
 describe('BulkAccessBrowseComponent', () => {
   let component: BulkAccessBrowseComponent;
@@ -31,13 +37,13 @@ describe('BulkAccessBrowseComponent', () => {
       imports: [
         NgbAccordionModule,
         NgbNavModule,
-        TranslateModule.forRoot()
+        TranslateModule.forRoot(),
       ],
       declarations: [BulkAccessBrowseComponent],
-      providers: [ { provide: SelectableListService, useValue: selectableListService }, ],
+      providers: [ { provide: SelectableListService, useValue: selectableListService } ],
       schemas: [
-        NO_ERRORS_SCHEMA
-      ]
+        NO_ERRORS_SCHEMA,
+      ],
     }).compileComponents();
   }));
 
@@ -72,7 +78,7 @@ describe('BulkAccessBrowseComponent', () => {
       'elementsPerPage': 5,
       'totalElements': 2,
       'totalPages': 1,
-      'currentPage': 1
+      'currentPage': 1,
     }), [selected1, selected2]) ;
     const rd = createSuccessfulRemoteDataObject(list);