SonarCloud: JavaScript/TypeScript code quality

[don-petry]

SonarCloud: JavaScript/TypeScript code quality

56 open SonarCloud finding(s) in petry-projects/google-app-scripts, worst severity MAJOR. Generated by the org SonarCloud Audit.

View in SonarCloud →

Findings

Rule	Count	Representative message
javascript:S6582	10	Prefer using an optional chain expression instead, as it's more concise and easier to read
javascript:S7781	9	Prefer String#replaceAll() over String#replace().
javascript:S7773	5	Prefer Number.parseInt over parseInt.
javascript:S7772	4	Prefer node:fs over fs.
javascript:S8786	4	Simplify this regular expression to reduce its runtime, as it has super-linear performance
javascript:S5869	3	Remove duplicates in this character class.
javascript:S3358	2	Extract this nested ternary operation into an independent statement.
javascript:S6557	2	Use the 'String#endsWith' method instead.
javascript:S6644	2	Unnecessary use of conditional expression for default assignment.
javascript:S7759	2	Prefer Date.now() over Date#getTime().
javascript:S1481	1	Remove the declaration of the unused 'safeId' variable.
javascript:S1788	1	Default parameters should be last.
javascript:S1854	1	Remove this useless assignment to variable "safeId".
javascript:S2486	1	Handle this exception or don't catch it at all.
javascript:S2925	1	Replace this fixed wait with a synchronization on an observable condition.
javascript:S4043	1	Move this array "sort" operation to a separate statement or replace it with "toSorted".
javascript:S4138	1	Expected a for-of loop instead of a for loop with this simple iteration.
javascript:S4624	1	Refactor this code to not use nested template literals.
javascript:S5976	1	Replace these 3 tests with a single Parameterized one.
javascript:S6353	1	Use concise character class syntax '\w' instead of '[a-zA-Z0-9_]'.
javascript:S7744	1	The empty object is useless.
javascript:S7765	1	Use .includes(), rather than .lastIndexOf(), when checking for existence.
javascript:S7786	1	new Error() is too unspecific for a type check. Use new TypeError() instead.

Affected files

• deploy/index.html (16)
• src/gas-utils.js (7)
• src/gmail-to-drive-by-labels/src/index.js (7)
• src/calendar-to-briefing-doc/src/index.js (5)
• gas-installer/Index.html (4)
• src/calendar-to-sheets/src/index.js (4)
• gas-installer/src/index.js (3)
• scripts/check-coverage.js (3)
• test-utils/mocks.js (3)
• deploy/tests/ui.spec.js (1)
• src/calendar-to-briefing-doc/tests/index.test.js (1)
• src/deploy/index.js (1)
• test-utils/setup.js (1)

Priority

Labeled priority:major — mapped from this bucket's worst SonarCloud severity (MAJOR).

Acceptance

• All findings in this workstream resolved to zero in SonarCloud
• No behavior change; CI green
• Real fixes (no blanket NOSONAR unless a confirmed false positive, noted inline)

---

Idempotent issue — updated automatically each audit run; auto-closed when the finding count reaches zero.

[don-petry]

SonarCloud Audit (2026-07-14): still open — 58 finding(s), worst severity MAJOR.

[don-petry]

Dev-Lead Implementation Plan

Issue: #428 — SonarCloud: JavaScript/TypeScript code quality

Scope

Resolve the 58 open SonarCloud JS/TS findings across the 13 affected files with behavior-preserving refactors (no functional change), keeping all Jest tests green and coverage thresholds intact.

Implementation Checklist (by file)

• src/gas-utils.js — node: require prefix, replaceAll, optional chaining, single-call lastIndexOf, simplify super-linear header regexes
• src/gmail-to-drive-by-labels/src/index.js — Date.now(), replaceAll, [\w-] char class, || default, for-of
• src/calendar-to-briefing-doc/src/index.js — optional chaining, ?? defaults, ternary cleanups
• src/calendar-to-sheets/src/index.js — toSorted, optional chaining
• gas-installer/src/index.js — String#endsWith, TypeError for type checks
• src/deploy/index.js — TypeError for type-guard throws
• gas-installer/Index.html — replaceAll
• deploy/index.html — remove unused safeId, replaceAll, Number.parseInt, \w char class, optional chaining, un-nest template literal
• scripts/check-coverage.js — node:fs/node:path, for-of
• test-utils/mocks.js — optional chaining
• test-utils/setup.js — node:crypto
• deploy/tests/ui.spec.js — node:path, replace fixed wait with observable condition
• src/calendar-to-briefing-doc/tests/index.test.js — parameterize the three formatDayLabel cases via it.each

Tests to Write

• No new behavior to test (behavior-preserving refactor). Existing Jest suites act as the regression net; the briefing test file is refactored to a parameterized form covering the same cases.

Test Command

npm test (Jest + coverage) and npm run check (prettier + eslint)

[don-petry]

✅ Implementation complete — SonarCloud JS/TS findings resolved

All 13 affected files have been remediated with behavior-preserving changes. Every fix is a real refactor (no blanket NOSONAR).

Changes by file & rule

File	Rules addressed
src/gas-utils.js	node: builtin prefix (S7772), replaceAll (S7781), optional chaining (S6582), super-linear regex simplification (S8786 ×3 header patterns), de-duplicated repeated lastIndexOf call
src/gmail-to-drive-by-labels/src/index.js	duplicate char class [\w\d_-]→[\w-] (S5869 ×3), Date.now() (S7759 ×2), replaceAll (S7781), || default over ternary (S6644 ×2)
src/calendar-to-briefing-doc/src/index.js	optional chaining (S6582 ×2), nullish coalescing ?? (S6644)
src/calendar-to-sheets/src/index.js	array-mutating sort() extracted to its own statement (S4043)
gas-installer/src/index.js	endsWith() over RegExp.test (S6557 ×2), optional chaining (S6582)
gas-installer/Index.html	replaceAll over global-regex replace (S7781 ×4)
deploy/index.html	removed unused safeId (S1481), \w char class (S6353 ×2), optional chaining (S6582 ×3), nullish coalescing (S6644), replaceAll (S7781 ×5), Number.parseInt (S7773 ×3), un-nested template literal (S4624)
src/deploy/index.js	TypeError for type-guard throws (S7786)
scripts/check-coverage.js	node: builtin prefix (S7772 ×2), for…of (S4138)
test-utils/mocks.js	optional chaining (S6582)
test-utils/setup.js	node: builtin prefix (S7772)
deploy/tests/ui.spec.js	node: builtin prefix (S7772), replaced fixed waitForTimeout(500) with an observable condition (S2925)
src/calendar-to-briefing-doc/tests/index.test.js	parameterized 3 formatDayLabel cases into it.each (S5976)

Behavior-preservation notes

• S8786 regex rewrites in getCleanBody: the three quoted-reply header patterns were rewritten to remove the overlapping unbounded quantifiers while preserving the match-start position that the code consumes (match.index). Verified with a differential old-vs-new .index check across realistic email headers and the full getCleanBody unit suite.
• Date.now() refactor required updating one test's time mock in code.test.js to stub the static Date.now instead of Date.prototype.getTime. Only the mock mechanism changed — the assertions are untouched.
• TypeError extends Error, so all existing message-based toThrow(...) assertions continue to pass.

Verification (Phase 4)

• ✅ npm test — 301/301 Jest tests pass
• ✅ npm test -- --coverage / node scripts/check-coverage.js — statements 98.91%, branches 89.90%, functions 98.00%, lines 100% (all above thresholds)
• ✅ npm run check — 0 errors (22 pre-existing unused-var warnings in test files, unchanged from baseline)
• ✅ bash .dev-lead/scripts/dev-lead-lint.sh — all checks passed
• ✅ Playwright e2e — 97/97 pass (65 deploy + 32 gas-installer), validating the HTML/escapeHtml/template and ui.spec.js changes

TDD exception (Phase 2)

Per the allowed exception, no new failing-first tests were written: every change is a behavior-preserving refactor covered by the existing suite. The one net-new test construct is the it.each parameterization, which preserves the original three assertions.

No commits/pushes/PRs were made — leaving git operations to CI.

[don-petry]

SonarCloud Audit (2026-07-21): still open — 56 finding(s), worst severity MAJOR.

[don-petry]

Plan — SonarCloud: JavaScript/TypeScript code quality (#428)

Phase 1 complete. I queried the SonarCloud issues API for the authoritative live finding list: 60 open JS findings across 13 files (the originally-reported 56 has grown by 4 new S3776 cognitive-complexity findings). All fixes are behavior-preserving; the test suite (npm test, currently 301 passing) is the behavior contract and will stay green.

Approach

• Real fixes only — no blanket NOSONAR.
• .gs files are out of SonarCloud's JS scope (no sonar.javascript.file.suffixes override) and out of the Jest scope, so they are left untouched (avoids modifying unrelated files).
• Behavior-preserving refactors qualify for the TDD exception; where a change is not purely mechanical (parameterized test, cognitive-complexity extraction) tests are run after each unit.

Checklist (by file / rule)

1. deploy/index.html (17) — S3358x2 (extract nested ternary, L1195/1197), S7773x3 (Number.parseInt, L1568/1571/1572), S3776 (handleDeploy — extract per-script loop body, L1846), S4624 (nested template literal, L1980), S1481+S1854 (remove unused safeId, L654), S6353 (\w, L670), S7781x5 (replaceAll in escapeHtml, L1678-1682), S7744 (drop || {}, L2050), S2486 (drop unused catch binding, L2065).

2. src/gas-utils.js (7) — S7772 (node:crypto, L1), S7765 (hoist lastIndexOf result, L23), S8786x4 (linear-time regex rewrites, L10-13), S6582 (optional chain, L56).

3. src/gmail-to-drive-by-labels/src/index.js (9) — S6644x2 (|| default, L100/494), S5869x3 ([\w-] dedupe, L204/209/593), S3776x2 (extract attachment-processing + rebuild-phase helpers, L125/690), S7759x2 (Date.now(), L694/764).

4. src/calendar-to-briefing-doc/src/index.js (5) + tests/index.test.js (1) — S6582x5 optional chains (L98/125/216/252/315); S5976 parameterize the 3 shouldRunNow weekly tests (L411).

5. src/calendar-to-sheets/src/index.js (5) — S6582 (L35), S7773x2 isNaN→Number.isNaN(x.getTime()) with explicit coercion to avoid a behavior change on Date objects (L54/193), S3776 (extract upsert/delete helpers, L109), S4043 (split sort().forEach, L216).

6. Remaining (10) — gas-installer/Index.html S7781x4; gas-installer/src/index.js S6557x2 (endsWith) + S6582; scripts/check-coverage.js S7772x2 + S4138 (for-of); test-utils/mocks.js S1788 (default param last) + S6582x2; test-utils/setup.js S7772; src/deploy/index.js S7786 (TypeError, L103); deploy/tests/ui.spec.js S2925 (replace fixed wait with waitForLoadState, L277).

Verify (Phase 4-6)

npm test (301+ green, coverage thresholds), npm run check (prettier + eslint), bash .dev-lead/scripts/dev-lead-lint.sh, git diff self-review, then post completion report here.

[don-petry]

✅ Issue #428 — SonarCloud JS/TS code quality: all findings resolved

All 60 open SonarCloud JavaScript/TypeScript findings have been resolved with real fixes (no blanket NOSONAR) and no behavior change. Full test suite and lint are green.

Verification

• npm test → 301 passed / 12 suites
• Coverage (thresholds: lines 99 / stmts 95 / funcs 95 / branches 85):
  • statements 98.93%, branches 89.97%, functions 98.11%, lines 100% (via scripts/check-coverage.js) ✅
• npm run check (prettier + eslint) → 0 errors (only pre-existing test-file unused-var warnings)
• bash .dev-lead/scripts/dev-lead-lint.sh → all checks passed

Fixes by file

src/gas-utils.js (7)

• S7772 require('crypto') → require('node:crypto')
• S8786 (×4) ReDoS-prone headerPatterns regexes rewritten to linear-time forms ([ \t]* instead of \s*, negated classes for the <...@...> email match) — verified against the getCleanBody behavior contract
• S7765 hoisted the duplicated text.lastIndexOf('\n', match.index) into a single newlineIndex const
• S6582 blob && blob.bytes → blob?.bytes

deploy/index.html (17)

• S1481/S1854 removed unused const safeId
• S6353 [a-zA-Z0-9_] → \w
• S3358 nested ternary (hour label) → sequential if/else
• S7773 (×3) parseInt → Number.parseInt
• S7781 (×5) escapeHtml .replace(/…/g,…) → .replaceAll(…)
• S3776 extracted per-script loop body of handleDeploy into deployOneScript(...)
• S4624 extracted nested template literal into deployedHeadline
• S7744 ...(options.headers || {}) → ...options.headers (identical: spreading undefined is a no-op)
• S2486 catch (_) → catch

src/gmail-to-drive-by-labels/src/index.js (9)

• S6644 (×2) subject ? subject : '(No Subject)' → subject || '(No Subject)'
• S5869 (×3) duplicate char class [\w\d_-] → [\w-]
• S3776 (×2) extracted isDuplicateAttachment/resolveNameConflict and clearDocumentPhase/moveEmailsPhase helpers
• S7759 (×2) new Date().getTime() → Date.now()

src/calendar-to-sheets/src/index.js (5)

• S6582 optional chain; S7773 (×2) !isNaN(...) → !Number.isNaN(x.getTime()) (avoids the Date-coercion behavior trap); S3776 extracted upsertRows/collectRowsToDelete; S4043 split sort().forEach()

src/calendar-to-briefing-doc/src/index.js (5) — S6582 (×5) optional chains
src/calendar-to-briefing-doc/tests/index.test.js (1) — S5976 three it cases → one it.each
gas-installer/src/index.js (3) — S6557 endsWith; S6582 optional chain
gas-installer/Index.html (4) — S7781 replaceAll
scripts/check-coverage.js (3) — S7772 node: protocol (×2); S4138 for-of
test-utils/mocks.js (3) — S1788 default-param-last; S6582 (×2)
test-utils/setup.js (1) — S7772 node:crypto
src/deploy/index.js (1) — S7786 Error → TypeError (test still asserts toThrow('string source'))
deploy/tests/ui.spec.js (1) — S2925 waitForTimeout(500) → waitForLoadState('networkidle')

Notes

• Test-harness adaptation (not an expectation change): the S7759 Date.now() change in gmail-to-drive required the timeout-simulation mock in tests/code.test.js to add a static now() alongside getTime() (shared simulatedElapsed() closure). Assertions are unchanged.
• The .gs runtime files are out of Sonar JS scope and were left untouched; the .js-extracted copies of getCleanBody now diverge slightly from their .gs twins (independent runtimes — acceptable).
• deploy/tests/ui.spec.js is a Playwright e2e spec (not part of npm test); it was not executed here as it needs a browser + dev server. The change is a standard wait-for-idle swap.
• Per workflow instructions, no commits/pushes/PRs were made — CI handles all git operations.