From be3a45877617326ff5793bf53c35bd819be68ada Mon Sep 17 00:00:00 2001 From: JoshuaVSherman Date: Mon, 18 May 2026 16:38:45 -0400 Subject: [PATCH] feat(eslint): integrate eslint-plugin-sonarjs (closes #207) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds eslint-plugin-sonarjs@^4 to the flat config and fixes/scopes all findings reported on the dev branch. ## Sonarjs findings addressed (6 errors → 0) - sonarjs/no-unenclosed-multiline-block (1, src/AgController/index.ts:99): Real readability bug — `} else break;` followed by an `if` on the next line looked ambiguous. Added braces around `else { break; }` and moved the istanbul-ignore comment to its own line. - sonarjs/pseudo-random (1, src/AgController/index.ts:58): disabled with inline justification — this is a non-security pulse heartbeat (an arbitrary 0-4 number transmitted to clients every second), Math.random is fine here. - sonarjs/no-nested-functions (4, test/**): disabled for test files via a files: ['test/**/*.ts'] override block. The findings are all in mock factories with deeply-nested arrow chains (e.g. socketcluster's receiver→createConsumer→next→Promise.resolve pattern); refactoring these into named helpers hurts readability instead of improving it. Rule stays active for src/. ## Stale eslint-disable comments cleaned (eslint --fix) The sonarjs recommended config surfaced ~35 pre-existing `// eslint-disable-line` comments for rules that aren't active anymore (no-await-in-loop, no-constant-condition, no-console, etc.). `eslint --fix` removed them automatically across src/AgController/, src/app/, and src/model/. ## What's NOT addressed 86 pre-existing `@typescript-eslint/no-explicit-any` warnings remain. These predate this PR and are out of scope; the `no-explicit-any` rule is already set to `warn` in the config (not `error`), so they don't fail the build. Tackle separately if desired. Verified: 66/66 tests pass; 0 ESLint errors. Co-Authored-By: Claude Opus 4.7 --- eslint.config.mjs | 12 +++++ package-lock.json | 104 ++++++++++++++++++++++++++++++++++++++ package.json | 1 + src/AgController/index.ts | 62 +++++++++++++---------- src/AgController/utils.ts | 2 +- src/app/agServerUtils.ts | 10 ++-- src/app/appUtils.ts | 4 +- src/model/db.ts | 2 +- 8 files changed, 160 insertions(+), 37 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index 4d65e94..e45c5f3 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -2,6 +2,7 @@ import js from '@eslint/js'; import tseslint from 'typescript-eslint'; import nodePlugin from 'eslint-plugin-n'; import securityPlugin from 'eslint-plugin-security'; +import sonarjs from 'eslint-plugin-sonarjs'; import globals from 'globals'; export default tseslint.config( @@ -22,6 +23,7 @@ export default tseslint.config( js.configs.recommended, nodePlugin.configs['flat/recommended-module'], securityPlugin.configs.recommended, + sonarjs.configs.recommended, { files: ['**/*.ts'], extends: [...tseslint.configs.recommendedTypeChecked], @@ -63,4 +65,14 @@ export default tseslint.config( 'max-len': ['error', { code: 150 }], }, }, + { + // Test-only override: deeply-nested arrow chains are the clearest way to + // build mock factories (e.g. socketcluster's receiver/consumer/next chain), + // and refactoring them into named helpers reduces readability rather than + // improving it. Keep the rule active for src/. + files: ['test/**/*.ts'], + rules: { + 'sonarjs/no-nested-functions': 'off', + }, + }, ); diff --git a/package-lock.json b/package-lock.json index 6df4b46..4f63de5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -47,6 +47,7 @@ "eslint": "^10.2.1", "eslint-plugin-n": "^17.24.0", "eslint-plugin-security": "^4.0.0", + "eslint-plugin-sonarjs": "^4.0.3", "globals": "^17.5.0", "supertest": "^7.2.2", "typescript-eslint": "^8.59.1", @@ -1902,6 +1903,19 @@ "integrity": "sha512-zRpUiDwd/xk6ADqPMATG8vc9VPrkck7T07OIx0gnjmJAnHnTVXNQG3vfvWNuiZIkwu9KrKdA1iJKfsfTVxE6NA==", "license": "BSD-3-Clause" }, + "node_modules/builtin-modules": { + "version": "3.3.0", + "resolved": "https://registry.npmjs.org/builtin-modules/-/builtin-modules-3.3.0.tgz", + "integrity": "sha512-zhaCDicdLuWN5UbN5IMnFqNMhNfo919sH85y2/ea+5Yg9TsTkeZxpL+JLbp6cgYFS4sRLp3YV4S6yDuqVWHYOw==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=6" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/bytes": { "version": "3.1.2", "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.1.2.tgz", @@ -2699,6 +2713,30 @@ "url": "https://opencollective.com/eslint" } }, + "node_modules/eslint-plugin-sonarjs": { + "version": "4.0.3", + "resolved": "https://registry.npmjs.org/eslint-plugin-sonarjs/-/eslint-plugin-sonarjs-4.0.3.tgz", + "integrity": "sha512-5drkJKLC9qQddIiaATV0e8+ygbUc7b0Ti6VB7M2d3jmKNh3X0RaiIJYTs3dr9xnlhlrxo+/s1FoO3Jgv6O/c7g==", + "dev": true, + "license": "LGPL-3.0-only", + "dependencies": { + "@eslint-community/regexpp": "^4.12.2", + "builtin-modules": "^3.3.0", + "bytes": "^3.1.2", + "functional-red-black-tree": "^1.0.1", + "globals": "^17.4.0", + "jsx-ast-utils-x": "^0.1.0", + "lodash.merge": "^4.6.2", + "minimatch": "^10.2.5", + "scslre": "^0.3.0", + "semver": "^7.7.4", + "ts-api-utils": "^2.5.0", + "typescript": ">=5" + }, + "peerDependencies": { + "eslint": "^8.0.0 || ^9.0.0 || ^10.0.0" + } + }, "node_modules/eslint-scope": { "version": "9.1.2", "resolved": "https://registry.npmjs.org/eslint-scope/-/eslint-scope-9.1.2.tgz", @@ -3138,6 +3176,13 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/functional-red-black-tree": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/functional-red-black-tree/-/functional-red-black-tree-1.0.1.tgz", + "integrity": "sha512-dsKNQNdj6xA3T+QlADDA7mOSlX0qiMINjn0cgr+eGHGsbSHzTabcIogz2+p/iqP1Xs6EP/sS2SbqH+brGTbq0g==", + "dev": true, + "license": "MIT" + }, "node_modules/get-caller-file": { "version": "2.0.5", "resolved": "https://registry.npmjs.org/get-caller-file/-/get-caller-file-2.0.5.tgz", @@ -3647,6 +3692,16 @@ "npm": ">=6" } }, + "node_modules/jsx-ast-utils-x": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/jsx-ast-utils-x/-/jsx-ast-utils-x-0.1.0.tgz", + "integrity": "sha512-eQQBjBnsVtGacsG9uJNB8qOr3yA8rga4wAaGG1qRcBzSIvfhERLrWxMAM1hp5fcS6Abo8M4+bUBTekYR0qTPQw==", + "dev": true, + "license": "MIT", + "engines": { + "node": "^18.18.0 || ^20.9.0 || >=21.1.0" + } + }, "node_modules/jwa": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/jwa/-/jwa-2.0.1.tgz", @@ -4059,6 +4114,13 @@ "integrity": "sha512-0wJxfxH1wgO3GrbuP+dTTk7op+6L41QCXbGINEmD+ny/G/eCqGzxyCsh7159S+mgDDcoarnBw6PC1PS5+wUGgw==", "license": "MIT" }, + "node_modules/lodash.merge": { + "version": "4.6.2", + "resolved": "https://registry.npmjs.org/lodash.merge/-/lodash.merge-4.6.2.tgz", + "integrity": "sha512-0KpjqXRVvrYyCsX1swR/XTK0va6VQkQM6MNo7PqW77ByjAhoARA8EfrP1N4+KlKj8YS0ZUCtRT/YUuhyYDujIQ==", + "dev": true, + "license": "MIT" + }, "node_modules/lodash.once": { "version": "4.1.1", "resolved": "https://registry.npmjs.org/lodash.once/-/lodash.once-4.1.1.tgz", @@ -4726,6 +4788,33 @@ "integrity": "sha512-Gd2UZBJDkXlY7GbJxfsE8/nvKkUEU1G38c1siN6QP6a9PT9MmHB8GnpscSmMJSoF8LOIrt8ud/wPtojys4G6+g==", "license": "MIT" }, + "node_modules/refa": { + "version": "0.12.1", + "resolved": "https://registry.npmjs.org/refa/-/refa-0.12.1.tgz", + "integrity": "sha512-J8rn6v4DBb2nnFqkqwy6/NnTYMcgLA+sLr0iIO41qpv0n+ngb7ksag2tMRl0inb1bbO/esUwzW1vbJi7K0sI0g==", + "dev": true, + "license": "MIT", + "dependencies": { + "@eslint-community/regexpp": "^4.8.0" + }, + "engines": { + "node": "^12.0.0 || ^14.0.0 || >=16.0.0" + } + }, + "node_modules/regexp-ast-analysis": { + "version": "0.7.1", + "resolved": "https://registry.npmjs.org/regexp-ast-analysis/-/regexp-ast-analysis-0.7.1.tgz", + "integrity": "sha512-sZuz1dYW/ZsfG17WSAG7eS85r5a0dDsvg+7BiiYR5o6lKCAtUrEwdmRmaGF6rwVj3LcmAeYkOWKEPlbPzN3Y3A==", + "dev": true, + "license": "MIT", + "dependencies": { + "@eslint-community/regexpp": "^4.8.0", + "refa": "^0.12.1" + }, + "engines": { + "node": "^12.0.0 || ^14.0.0 || >=16.0.0" + } + }, "node_modules/regexp-tree": { "version": "0.1.27", "resolved": "https://registry.npmjs.org/regexp-tree/-/regexp-tree-0.1.27.tgz", @@ -4911,6 +5000,21 @@ "socketcluster-client": "^20.0.0" } }, + "node_modules/scslre": { + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/scslre/-/scslre-0.3.0.tgz", + "integrity": "sha512-3A6sD0WYP7+QrjbfNA2FN3FsOaGGFoekCVgTyypy53gPxhbkCIjtO6YWgdrfM+n/8sI8JeXZOIxsHjMTNxQ4nQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@eslint-community/regexpp": "^4.8.0", + "refa": "^0.12.0", + "regexp-ast-analysis": "^0.7.0" + }, + "engines": { + "node": "^14.0.0 || >=16.0.0" + } + }, "node_modules/semver": { "version": "7.7.4", "resolved": "https://registry.npmjs.org/semver/-/semver-7.7.4.tgz", diff --git a/package.json b/package.json index 37e447c..45df319 100644 --- a/package.json +++ b/package.json @@ -87,6 +87,7 @@ "eslint": "^10.2.1", "eslint-plugin-n": "^17.24.0", "eslint-plugin-security": "^4.0.0", + "eslint-plugin-sonarjs": "^4.0.3", "globals": "^17.5.0", "supertest": "^7.2.2", "typescript-eslint": "^8.59.1", diff --git a/src/AgController/index.ts b/src/AgController/index.ts index 742584b..f2f73e3 100644 --- a/src/AgController/index.ts +++ b/src/AgController/index.ts @@ -40,8 +40,8 @@ class AgController { (async () => { let disconnect: { value: undefined; done: any; }; const dConsumer = client.listener('disconnect').createConsumer(); - while (true) { // eslint-disable-line no-constant-condition - disconnect = await dConsumer.next();// eslint-disable-line no-await-in-loop + while (true) { + disconnect = await dConsumer.next(); clearInterval(interval); if (disconnect.value !== undefined) { const index = this.clients.indexOf(client.id); @@ -55,6 +55,9 @@ class AgController { sendPulse(client:IClient):void { const interval = setInterval(() => { + // sonarjs/pseudo-random: this is a non-security pulse heartbeat for clients + // (an arbitrary 0-4 number transmitted every second). Math.random is fine here. + // eslint-disable-next-line sonarjs/pseudo-random client.socket.transmit('pulse', { number: Math.floor(Math.random() * 5) }); }, 1000); debug(`num clients: ${this.clients.length}`); @@ -89,14 +92,17 @@ class AgController { (async () => { let receiver: { value: number; done: any; }; const rConsumer = client.socket.receiver('initial message').createConsumer(); - while (true) { // eslint-disable-line no-constant-condition - receiver = await rConsumer.next();// eslint-disable-line no-await-in-loop + while (true) { + receiver = await rConsumer.next(); debug(`received initial message: ${receiver.value}`); if (receiver.value === 123) { - await this.sendTours(client);// eslint-disable-line no-await-in-loop - await this.sendBooks(client);// eslint-disable-line no-await-in-loop - } else break; - /* istanbul ignore else */if (receiver.done) break; + await this.sendTours(client); + await this.sendBooks(client); + } else { + break; + } + /* istanbul ignore else */ + if (receiver.done) break; } })(); } @@ -117,14 +123,14 @@ class AgController { (async () => { let receiver: { value: { token: string; image: any }, done: any; }; const rConsumer = client.socket.receiver('newImage').createConsumer(); - while (true) { // eslint-disable-line no-constant-condition - receiver = await rConsumer.next();// eslint-disable-line no-await-in-loop + while (true) { + receiver = await rConsumer.next(); debug(`received newImage message: ${JSON.stringify(receiver.value)}`); if (!receiver.value) break; if (typeof receiver.value.token === 'string' && typeof receiver.value.image.title === 'string' && typeof receiver.value.image.url === 'string' ) { - await this.handleImage('createDocs', receiver.value.image, 'imageCreated');// eslint-disable-line no-await-in-loop + await this.handleImage('createDocs', receiver.value.image, 'imageCreated'); } /* istanbul ignore else */if (receiver.done) break; } @@ -138,10 +144,10 @@ class AgController { ):Promise { const { editPic, token } = data; const { - // eslint-disable-next-line @typescript-eslint/naming-convention + _id, title, url, comments, } = editPic; - let r: any;// eslint-disable-next-line security/detect-object-injection + let r: any; if (typeof token !== 'string') throw new Error('invalid token'); try { r = await this.bookController.findByIdAndUpdate(_id, { title, url, comments }); } catch (e) { const eMessage = (e as Error).message; @@ -157,12 +163,12 @@ class AgController { (async () => { let receiver: { value: { data: string; token: string; }; done: any; }; const rConsumer = client.socket.receiver('deleteImage').createConsumer(); - while (true) { // eslint-disable-line no-constant-condition - receiver = await rConsumer.next();// eslint-disable-line no-await-in-loop + while (true) { + receiver = await rConsumer.next(); debug(`received deleteImage message: ${JSON.stringify(receiver.value)}`); if (!receiver.value) break; if (typeof receiver.value.token === 'string' && typeof receiver.value.data === 'string') { - await this.handleImage('deleteById', receiver.value.data, 'imageDeleted');// eslint-disable-line no-await-in-loop + await this.handleImage('deleteById', receiver.value.data, 'imageDeleted'); } /* istanbul ignore else */if (receiver.done) break; } @@ -174,8 +180,8 @@ class AgController { let receiver: { value: { token: string; tour: { datetime: Date; venue: string; city:string, usState: string }; }; done: any; }; const rConsumer = client.socket.receiver('newTour').createConsumer(); - while (true) { // eslint-disable-line no-constant-condition - receiver = await rConsumer.next();// eslint-disable-line no-await-in-loop + while (true) { + receiver = await rConsumer.next(); let decoded, user, goodRoles; if (!receiver.value) break; try { @@ -191,7 +197,7 @@ class AgController { } if (receiver.value.tour.datetime && receiver.value.tour.city && receiver.value.tour.usState && receiver.value.tour.venue) { - await utils.handleTour(// eslint-disable-line no-await-in-loop + await utils.handleTour( 'createDocs', receiver.value.tour, 'tourCreated', @@ -214,11 +220,11 @@ class AgController { (async () => { let receiver: { value: { tour:any; token: any; }; done: any; }; const rConsumer = client.socket.receiver('deleteTour').createConsumer(); - while (true) { // eslint-disable-line no-constant-condition - receiver = await rConsumer.next();// eslint-disable-line no-await-in-loop + while (true) { + receiver = await rConsumer.next(); debug(`received deleteTour message: ${JSON.stringify(receiver.value)}`); if (!receiver.value) break; - await utils.removeTour(receiver, client, this.tourController, this.server);// eslint-disable-line no-await-in-loop + await utils.removeTour(receiver, client, this.tourController, this.server); /* istanbul ignore else */if (receiver.done) break; } })(); @@ -227,7 +233,7 @@ class AgController { async updateTour( data: { tourId: mongoose.Types.ObjectId; tour: Record; }, ):Promise { - let r: any;// eslint-disable-next-line security/detect-object-injection + let r: any; try { const { tourId, tour } = data; if (!tour.venue || !tour.datetime || !tour.city || !tour.usState) throw new Error('Invalid gig data'); @@ -245,15 +251,15 @@ class AgController { (async () => { let receiver: { value:any; done: any; }; const rConsumer = client.socket.receiver(action).createConsumer(); - while (true) { // eslint-disable-line no-constant-condition - receiver = await rConsumer.next();// eslint-disable-line no-await-in-loop + while (true) { + receiver = await rConsumer.next(); const obj = JSON.stringify(receiver.value); debug(`received ${action} message: ${obj}`); if (!receiver.value) break; if (typeof receiver.value.token === 'string') { - // eslint-disable-next-line security/detect-object-injection - if (action === 'editTour') await this.updateTour(receiver.value);// eslint-disable-line no-await-in-loop - // eslint-disable-next-line no-await-in-loop + + if (action === 'editTour') await this.updateTour(receiver.value); + else await this.updateImage(receiver.value, client); } /* istanbul ignore else */if (receiver.done) break; diff --git a/src/AgController/utils.ts b/src/AgController/utils.ts index ba3b7b1..e0f8698 100644 --- a/src/AgController/utils.ts +++ b/src/AgController/utils.ts @@ -40,7 +40,7 @@ async function handleTour( async function removeTour(receiver:any, client:any, tourController:any, server:any) { try { if (typeof receiver.value.tour.tourId === 'string' && typeof receiver.value.token === 'string') { - await handleTour('deleteById', receiver.value.tour.tourId, 'tourDeleted', tourController, server);// eslint-disable-line no-await-in-loop + await handleTour('deleteById', receiver.value.tour.tourId, 'tourDeleted', tourController, server); } } catch (e) { const eMessage = (e as Error).message; diff --git a/src/app/agServerUtils.ts b/src/app/agServerUtils.ts index 1585e84..a3f9429 100644 --- a/src/app/agServerUtils.ts +++ b/src/app/agServerUtils.ts @@ -9,8 +9,8 @@ const handleConnections = async ( cConsumer:ConsumableStream.Consumer, agController: AgController, ) => { - while (true) { // eslint-disable-line no-constant-condition - const socket = await cConsumer.next();// eslint-disable-line no-await-in-loop + while (true) { + const socket = await cConsumer.next(); debug(`new connection with id: ${socket.value.id}`); agController.addSocket(socket.value); /* istanbul ignore else */if (socket.done) break; @@ -30,8 +30,8 @@ const setupErrorWarning = (agServer: socketClusterServer.AGServer, type: any): v (async () => { let msg; const eConsumer = agServer.listener(type).createConsumer(); - while (true) { // eslint-disable-line no-constant-condition - msg = await eConsumer.next();// eslint-disable-line no-await-in-loop + while (true) { + msg = await eConsumer.next(); debug(type); debug(msg.value); /* istanbul ignore else */if (msg.done) break; @@ -46,7 +46,7 @@ const handleErrAndWarn = (SOCKETCLUSTER_LOG_LEVEL: any, SOCKETCLUSTER_PORT: any, /* istanbul ignore else */if (color) fullMessage = `\x1b[${color}m${message}\x1b[0m`; return fullMessage; } - /* istanbul ignore else */if (SOCKETCLUSTER_LOG_LEVEL >= 2) { // eslint-disable-next-line no-console + /* istanbul ignore else */if (SOCKETCLUSTER_LOG_LEVEL >= 2) { console.log(` ${colorText('[Active]', 32)} SocketCluster worker with PID ${process.pid} is listening on port ${SOCKETCLUSTER_PORT}`); setupErrorWarning(agServer, 'warning'); } diff --git a/src/app/appUtils.ts b/src/app/appUtils.ts index b9cb6fd..2a795eb 100644 --- a/src/app/appUtils.ts +++ b/src/app/appUtils.ts @@ -17,8 +17,8 @@ const setup = (expressApp: any, httpServer: any): any => { // Add GET /health-ch (async () => { // HTTP request handling let packet; const consumer = httpServer.listener('request').createConsumer(); - while (true) { // eslint-disable-line no-constant-condition - packet = await consumer.next();// eslint-disable-line no-await-in-loop + while (true) { + packet = await consumer.next(); handleRequest(expressApp, packet.value); /* istanbul ignore else */if (packet.done) break; } diff --git a/src/model/db.ts b/src/model/db.ts index ff5ab36..3f5f021 100644 --- a/src/model/db.ts +++ b/src/model/db.ts @@ -15,7 +15,7 @@ if (process.env.NODE_ENV === 'test') uri = process.env.TEST_DB || /* istanbul ig /* istanbul ignore next */ mongoose.connection.on('error', (e) => { if (process.env.NODE_ENV !== 'test') throw new Error(`unable to connect to database: ${uri}`); - else console.log(e.message);// eslint-disable-line no-console + else console.log(e.message); }); export default mongoose;