feat(eslint): integrate eslint-plugin-sonarjs (closes #207)#211
Merged
Conversation
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
eslint-plugin-sonarjs@^4to the flat config (sonarjs.configs.recommended).dev: 1 readability bug fix (no-unenclosed-multiline-block insrc/AgController/index.ts:99), 1 inline disable with justification (pseudo-random for the pulse heartbeat), and 1 test-files-only override block disablingno-nested-functionsfor mock factories.eslint --fixcleaned up ~35 stale// eslint-disable-linecomments for rules that aren't active anymore.Not addressed (out of scope)
86 pre-existing
@typescript-eslint/no-explicit-anywarnings remain. They predate this PR and the rule is already configured aswarn, noterror, so they don't fail the build. Separate cleanup if desired.Test plan
npx eslint .→ 0 errors (86 pre-existingno-explicit-anywarnings remain)npm test→ 66/66 tests passsrc/AgController/index.ts:99-104— braces aroundelse { break; }and the istanbul-ignore comment moved to its own line.Closes #207.
🤖 Generated with Claude Code