Skip to content

Commit a4bd66d

Browse files
chore(lint): add eslint-plugin-sonarjs (core) — Sonar-grade rules in normal lint, free + local
Brings Sonar's bug/smell detection into ESLint so it runs everywhere — including PRO — with zero cloud cost and no leak (a private SonarCloud project is paid-by-LOC; core stays on free public Automatic Analysis, pro is scanned locally by these rules). - extends plugin:sonarjs/recommended-legacy; most rules at the recommended `error` (forward guard on new code). no-duplicate-string OFF (fights RN style literals); five already-tripped legacy rules at `warn` with a burn-down + ratchet plan (GAPS_BACKLOG). ESLint v8 has no native suppression baseline, hence warn-then-ratchet. - test override: no-identical-functions + cognitive-complexity off for tests (duplicate arrange/act is fine); real-bug rules stay ON for tests. Real bugs the plugin caught on day one and we FIXED (not warned): - openAICompatibleProvider.test.ts: `expect(....length >= 0).toBe(true)` tautology (always true) → assert the terminal outcome (onComplete fired, no abort). - streamingStateMachine.test.ts: dead `releasers` collection (declared+reset, never used) → removed. - pro/ui/McpServerModal.tsx: `borderRadius: open ? 8 : 8` no-op ternary → fixed in the pro repo (branch fix/sonarjs-redundant-ternary; pointer bump via pro's own PR). Also removes the redundant SonarCloud CI job: core uses Automatic Analysis + Codecov (a CI scan would only duplicate Codecov's coverage). sonar-project.properties already points at the off-grid-ai org for Automatic Analysis. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent a17b633 commit a4bd66d

7 files changed

Lines changed: 101 additions & 50 deletions

File tree

.eslintrc.js

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
11
module.exports = {
22
root: true,
3-
extends: '@react-native',
3+
extends: [
4+
'@react-native',
5+
// SonarJS: Sonar-grade bug/smell detection in normal lint — free, local, and it covers
6+
// PRO too (pro has no cloud Sonar project; a private cloud project is paid-by-LOC). Most
7+
// rules stay at `error` (recommended default) as a forward guard; the handful already
8+
// tripped on legacy code are `warn` below with a logged burn-down (GAPS_BACKLOG).
9+
'plugin:sonarjs/recommended-legacy',
10+
],
411
plugins: [
512
'react-native',
613
'react',
714
'react-hooks',
15+
'sonarjs',
816
],
917
env: {
1018
jest: true,
@@ -43,6 +51,20 @@ module.exports = {
4351
'react-native/no-color-literals': 'error',
4452
'react-native/no-raw-text': 'error',
4553
'react-native/no-single-element-style-arrays': 'error',
54+
55+
// SonarJS — every rule stays at the recommended `error` (a real forward guard on new code)
56+
// EXCEPT the two handled here:
57+
// - no-duplicate-string OFF: it fights RN styling — 'space-between'/'center'/'row' and color
58+
// literals repeat by design across StyleSheet objects; a constant per style value is noise,
59+
// not clarity. The one low-value SonarJS rule for this codebase.
60+
// - the rest are `warn` (already tripped on legacy core; burn-down in docs/GAPS_BACKLOG.md,
61+
// ratchet each back to `error` as its count hits zero).
62+
'sonarjs/no-duplicate-string': 'off',
63+
'sonarjs/prefer-single-boolean-return': 'warn',
64+
'sonarjs/no-nested-template-literals': 'warn',
65+
'sonarjs/no-collapsible-if': 'warn',
66+
'sonarjs/prefer-immediate-return': 'warn',
67+
'sonarjs/no-duplicated-branches': 'warn',
4668
},
4769
overrides: [
4870
{
@@ -56,6 +78,11 @@ module.exports = {
5678
'react-native/no-inline-styles': 'off',
5779
'react-native/no-raw-text': 'off',
5880
'react-native/no-color-literals': 'off',
81+
// Duplicate test bodies (identical arrange/act across cases) are acceptable and clearer
82+
// than over-DRYing tests; the real-bug SonarJS rules (mischeck, unused-collection, etc.)
83+
// stay ON for tests — they caught a tautology assertion + a dead collection here.
84+
'sonarjs/no-identical-functions': 'off',
85+
'sonarjs/cognitive-complexity': 'off',
5986
},
6087
},
6188
],

.github/workflows/ci.yml

Lines changed: 4 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -195,42 +195,7 @@ jobs:
195195
- name: Build Android Release
196196
run: cd android && ./gradlew assembleRelease
197197

198-
sonar:
199-
# SonarCloud CI scan for CORE ONLY (org off-grid-ai, project off-grid-ai_off-grid-ai-mobile).
200-
# It reads the code and posts findings back to the PR (a check status + a summary comment via
201-
# the SonarCloud GitHub App) — it never pushes code.
202-
#
203-
# PRO IS DELIBERATELY NOT SCANNED HERE. This is a PUBLIC project, and pro/ is private paid
204-
# source. Two guardrails keep it out: (1) we do NOT check out the pro submodule in this job
205-
# (submodules: false), so pro/ is an empty gitlink with nothing to scan; (2) pro is not in
206-
# sonar.sources. Pro gets its own PRIVATE Sonar project in the pro repo — never here.
207-
# Requires Automatic Analysis to be OFF (it is) + the SONAR_TOKEN secret + the GitHub App.
208-
runs-on: ubuntu-latest
209-
if: ${{ github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository }}
210-
env:
211-
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
212-
213-
steps:
214-
- name: Checkout code (core only — NO pro submodule)
215-
uses: actions/checkout@v4
216-
with:
217-
submodules: false # never pull private pro/ into this public project
218-
fetch-depth: 0 # Sonar needs full history for new-code / blame attribution
219-
220-
- name: Setup Node.js
221-
uses: actions/setup-node@v4
222-
with:
223-
node-version: '20'
224-
cache: 'npm'
225-
226-
- name: Install dependencies
227-
run: npm ci
228-
229-
- name: Generate JS coverage (core only — pro not checked out, so excluded)
230-
run: npx jest --coverage --forceExit
231-
232-
- name: SonarCloud scan
233-
if: ${{ env.SONAR_TOKEN != '' }}
234-
uses: SonarSource/sonarcloud-github-action@master
235-
env:
236-
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
198+
# NOTE: SonarCloud runs via Automatic Analysis (SonarCloud-side, on the public core project) —
199+
# no CI job. A CI scan would only add coverage import, which Codecov (in the `test` job) already
200+
# does, so it'd be redundant complexity. Pro is scanned locally by eslint-plugin-sonarjs (free,
201+
# private, no leak). See docs/GAPS_BACKLOG.md.

__tests__/integration/audio/streamingStateMachine.test.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ jest.mock('@offgrid/core/utils/logger', () => ({
2020
type SpeakMode = 'resolve' | 'hang' | 'throw';
2121
let speakMode: SpeakMode = 'resolve';
2222
let enginePhase = 'ready';
23-
const releasers: Array<() => void> = [];
2423
const mockEngine = {
2524
speak: jest.fn(() => {
2625
if (speakMode === 'throw') return Promise.reject(new Error('std::exception'));
@@ -68,7 +67,6 @@ beforeEach(async () => {
6867
speakMode = 'resolve';
6968
enginePhase = 'ready';
7069
mockCanLoad.mockReturnValue(false);
71-
releasers.length = 0;
7270
// clearAllMocks resets call history but NOT implementations — restore the
7371
// speakMode-driven impl so a prior test's mockImplementation can't leak in.
7472
mockEngine.speak.mockImplementation(() => {

__tests__/unit/services/providers/openAICompatibleProvider.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -443,8 +443,11 @@ describe('OpenAICompatibleProvider', () => {
443443
// Stop generation (should abort)
444444
await provider.stopGeneration();
445445

446-
// Generation should have completed without error
447-
expect(wasAborted || onComplete.mock.calls.length >= 0).toBe(true);
446+
// Generation completed (fast) before the stop → onComplete fired and the abort never had to
447+
// trigger. (The prior assertion `onComplete.mock.calls.length >= 0` was a tautology — always
448+
// true — so it proved nothing; assert the real terminal outcome instead.)
449+
expect(onComplete).toHaveBeenCalled();
450+
expect(wasAborted).toBe(false);
448451
});
449452
});
450453

docs/GAPS_BACKLOG.md

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,25 +15,68 @@ Verdict legend:
1515

1616
Added dependency-cruiser as the STANDING GATE for the architectural boundaries we kept
1717
re-establishing by hand (`.dependency-cruiser.js`; `npm run depcruise`; CI `architecture`
18-
job + pre-push). Rules are at `error`; the existing debt is captured in
19-
`.dependency-cruiser-known-violations.json` (68 violations) so the gate PASSES on current
18+
job + pre-push). AGGRESSIVE ruleset, all at `error`; existing debt captured in
19+
`.dependency-cruiser-known-violations.json` (66 violations) so the gate PASSES on current
2020
debt but FAILS on anything NEW. This is the honest register of that baselined debt - burn it
2121
down, never regenerate the baseline to hide a new violation.
2222

2323
| Rule | Count | Verdict | Note |
2424
|---|---|---|---|
2525
| no-circular | 61 | fix-the-guard (own PR) | Mostly cycles routed through barrel `index.ts` files (`services/index`, `stores/index`) + intra-screen hook cycles (HomeScreen hooks ⇄ useHomeScreen). Break by importing the concrete module, not the barrel, and extracting shared hook state down a layer. Large - dedicated PR(s). |
2626
| utils-stay-pure | 3 | fix-the-guard | `utils/proPrompt→stores/appStore`, `utils/imageModelIntegrity→services/modelLoadErrors`, `utils/downloadAggregate→stores/downloadStore`. A "pure" util reaching into a store/service - move the impure bit up or the shared data down. |
27-
| no-backward-layering-utils | 1 | fix-the-guard | `services/loadModelWithOverride→components/CustomAlert`: a service imports a UI component to show an alert. Service should return a decision; the caller renders the alert (SoC §A). |
28-
| no-orphans (warn) | 3 | instrument-and-revisit | `screens/ChatScreen/toolUsage.ts`, `config/revenueCatKeys.ts`, `bootstrap/proStub.js` - grep-verify each is truly unreferenced (proStub is likely a pro-gating shell) before delete. |
27+
| no-backward-layering-core | 1 | fix-the-guard | `services/loadModelWithOverride→components/CustomAlert`: a service imports a UI component to show an alert. Service should return a decision; the caller renders the alert (SoC §A). |
28+
| components-are-leaf-ui | 1 | fix-the-guard | `components/models/VoiceModelsSheet→screens/ModelsScreen/VoiceModelsUpsell`: a reusable component imports a screen. Move VoiceModelsUpsell into components/ (or pass it as a prop). |
2929

30-
Engine-DIP violations found by the gate on day one (screens importing concrete `litert`)
31-
were FIXED in this PR, not baselined (see the Engine DIP section) - the gate proving its value.
30+
Resolved by the gate on day one (NOT baselined):
31+
- Engine-DIP: screens importing concrete `litert` → routed through services/engines (see Engine DIP section).
32+
- Dead code: `screens/ChatScreen/toolUsage.ts` (shouldUseToolsForMessage, zero prod callers) deleted with its test.
33+
- Phantom-dep false positives excluded (not debt): `whisper.rn` (declared; `.rn` defeats the resolver),
34+
`@offgrid/pro` (private open-core submodule wired via metro haste through the one bootstrap loader).
35+
36+
Aggressive rules also active with ZERO current violations (pure forward-guards): not-to-test-from-prod,
37+
not-to-dev-dep, no-phantom-deps, no-deprecated-core (warn).
3238

3339
NOTE: the gate catches the IMPORT-edge half of the DIP rule. The VALUE-branch half
3440
(`model.engine === 'litert'` comparing a store value) is not an edge - guard it with an
3541
ESLint `no-restricted-syntax` rule (follow-up).
3642

43+
## SonarJS in ESLint - burn-down - 2026-07-10 (PR #510)
44+
45+
Added `eslint-plugin-sonarjs` (recommended-legacy) so Sonar-grade bug/smell rules run in normal
46+
lint - FREE, LOCAL, and it covers PRO too (pro has no cloud Sonar project; a private cloud project
47+
is paid-by-LOC). Most rules are at the recommended `error` (forward guard on new code). Six rules
48+
already tripped on legacy core are relaxed to `warn` - ratchet each back to `error` as its count
49+
hits zero. ESLint v8 has no native suppression baseline, hence warn-then-ratchet rather than a
50+
baseline file.
51+
52+
Real bugs SonarJS caught immediately and we FIXED (not warned):
53+
- `openAICompatibleProvider.test.ts`: `expect(... .length >= 0).toBe(true)` - a tautology assertion
54+
(always true, could never fail). Rewrote to assert the terminal outcome (onComplete fired, no abort).
55+
- `streamingStateMachine.test.ts`: `releasers` - a collection declared + reset but never pushed/read
56+
(dead). Removed.
57+
- `pro/ui/McpServerModal.tsx`: `borderRadius: open ? 8 : 8` - redundant ternary (same value both
58+
branches). Fixed to `borderRadius: 8`. (Fixed in the pro submodule - the "SonarJS catches pro" win.)
59+
60+
`no-duplicate-string` is OFF (not warn): it fights RN styling — style literals ('space-between',
61+
'center', color values) repeat by design across StyleSheet objects; a constant per value is noise.
62+
Also unblocks pro's `--max-warnings=0` pre-commit hook.
63+
64+
Warn-level burn-down (core src, ratchet to error as each hits zero):
65+
| Rule | Count |
66+
|---|---|
67+
| sonarjs/prefer-single-boolean-return | 9 |
68+
| sonarjs/no-nested-template-literals | 6 |
69+
| sonarjs/no-collapsible-if | 2 |
70+
| sonarjs/prefer-immediate-return | 1 |
71+
| sonarjs/no-duplicated-branches | 1 |
72+
73+
Test override: `no-identical-functions` + `cognitive-complexity` are OFF for test files (duplicate
74+
arrange/act across cases is clearer than over-DRYed tests); the real-bug rules stay ON for tests.
75+
76+
SonarCloud: CORE uses Automatic Analysis (public project, free) + Codecov for coverage - NO CI
77+
scan job (it would only duplicate Codecov's coverage). PRO is covered by the SonarJS ESLint rules
78+
above, locally - never sent to any cloud project.
79+
3780
---
3881

3982
## Dead-code recon - 2026-07-06

package-lock.json

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@
9999
"dependency-cruiser": "^18.0.0",
100100
"eslint": "^8.19.0",
101101
"eslint-plugin-react-compiler": "^19.1.0-rc.2",
102+
"eslint-plugin-sonarjs": "^1.0.4",
102103
"husky": "^9.1.7",
103104
"jest": "^29.6.3",
104105
"lint-staged": "^15.5.2",

0 commit comments

Comments
 (0)