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.