Repository navigation
SonarCloud: JavaScript code-quality modernization (×68) #421
Description
JavaScript code-quality modernization
SonarCloud open findings in petry-projects/google-app-scripts — 68 issue(s) grouped as one workstream. Part of the org SonarCloud tuning initiative combsmith/.github#593.
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:S7764 | 8 | Prefer globalThis over window. |
| 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:S2486 | 3 | Handle this exception or don't catch it at all. |
| javascript:S5869 | 3 | Remove duplicates in this character class. |
| javascript:S3358 | 2 | Extract this nested ternary operation into an independent statement. |
| javascript:S7735 | 2 | Unexpected negated condition. |
| css:S7924 | 2 | Text does not meet the minimal contrast requirement with its background. |
| 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:S4624 | 1 | Refactor this code to not use nested template literals. |
| javascript:S1788 | 1 | Default parameters should be last. |
| javascript:S1481 | 1 | Remove the declaration of the unused 'safeId' variable. |
| javascript:S1854 | 1 | Remove this useless assignment to variable "safeId". |
| javascript:S6353 | 1 | Use concise character class syntax '\w' instead of '[a-zA-Z0-9_]'. |
| javascript:S7744 | 1 | The empty object is useless. |
| javascript:S7786 | 1 | new Error() is too unspecific for a type check. Use new TypeError() instead. |
| javascript:S4138 | 1 | Expected a for-of loop instead of a for loop with this simple iteration. |
| javascript:S4043 | 1 | Move this array "sort" operation to a separate statement or replace it with "toSorted". |
| javascript:S7765 | 1 | Use .includes(), rather than .lastIndexOf(), when checking for existence. |
Affected files
deploy/index.html(20)src/gmail-to-drive-by-labels/src/index.js(9)src/gas-utils.js(8)src/calendar-to-briefing-doc/src/index.js(6)test-utils/setup.js(6)gas-installer/Index.html(5)src/calendar-to-sheets/src/index.js(4)test-utils/mocks.js(3)gas-installer/src/index.js(3)scripts/check-coverage.js(3)src/deploy/index.js(1)
Approach
Apply the flagged idiom fixes: optional chaining (S6582), replaceAll (S7781), globalThis over window (S7764), Number.parseInt (S7773), and the other per-line suggestions. Each carries SonarCloud's exact recommendation. Mechanical; behavior identical.
Acceptance
- All listed SonarCloud findings for this repo/workstream resolved to zero
- No behavior change; existing tests/CI green
- Fixes are real (no blanket
NOSONARunless a confirmed false positive, noted inline)
Filed from the 2026-07-03 org SonarCloud review.
Activity
don-petry commentedon Jul 3, 2026
Dev-Lead Implementation Plan
Issue: #421 — SonarCloud: JavaScript code-quality modernization (×68)
Scope
Apply SonarCloud's exact per-rule idiom modernizations across the 11 affected files — all behavior-preserving refactors (optional chaining,
replaceAll,globalThis,Number.parseInt,node:builtins,TypeError,Date.now,for-of,toSorted, regex simplification, nested-ternary extraction, unused-var removal, conditional-default simplification,endsWith, CSS contrast).Implementation Checklist
src/gas-utils.js—node:crypto,replaceAll,TypeError, regex super-linear simplification (S8786)src/gmail-to-drive-by-labels/src/index.js—Date.now(), char-class dedupe[\w-], conditional-default||,toSortedsrc/calendar-to-briefing-doc/src/index.js— optional chaining / idiom fixessrc/calendar-to-sheets/src/index.js—toSorted, char-class / idiom fixessrc/deploy/index.js— idiom fixtest-utils/setup.js—globalThis,node:cryptotest-utils/mocks.js— optional chaining fixesscripts/check-coverage.js—node:fs,node:path,for-ofgas-installer/src/index.js— idiom fixesgas-installer/Index.html—replaceAll, optional chainingdeploy/index.html—globalThis,Number.parseInt,replaceAll, optional chaining, nested-ternary extraction, remove unusedsafeId, CSS contrastTests to Write
Test Command
npm test(Jest, includes coverage) andnpm run check(prettier + eslint)don-petry commentedon Jul 3, 2026
Dev-Lead: issue #421 implementation failed — will retry
The engine failed (
engine-error— e.g. a timeout or transient engine error). This was attempt 1 of 3; dev-lead will retry automatically. You can also re-apply thedev-leadlabel to retry now.engine-errordon-petry commentedon Jul 3, 2026
Dev-Lead: issue #421 implementation failed — will retry
The engine failed (
engine-error— e.g. a timeout or transient engine error). This was attempt 2 of 3; dev-lead will retry automatically. You can also re-apply thedev-leadlabel to retry now.engine-errorLast 40 lines of engine output (redacted)
don-petry commentedon Jul 4, 2026
Dev-Lead Implementation Plan
Issue: #421 — SonarCloud: JavaScript code-quality modernization (×68)
Scope
Apply the flagged SonarCloud idiom modernizations (optional chaining,
String#replaceAll,globalThis,Number.parseInt,Date.now(),endsWith, concise/dedup char classes,TypeError, no-nested-ternary/template, unused-var removal, etc.) across the 11 affected files. Mechanical, behavior-preserving.Implementation Checklist
src/gas-utils.js—replaceAll, optional chaining,TypeErrorsrc/gmail-to-drive-by-labels/src/index.js— default-assignment,Date.now(), dedup char classsrc/calendar-to-briefing-doc/src/index.js— optional chaining, misc idiomssrc/calendar-to-sheets/src/index.js—toSorted, misc idiomssrc/deploy/index.js— idiom fixgas-installer/src/index.js—endsWith, optional chainingscripts/check-coverage.js—node:fs,for-oftest-utils/setup.js,test-utils/mocks.js— optional chaining, miscdeploy/index.html—replaceAll, optional chaining,globalThis,Number.parseInt, nested ternary/template, unusedsafeIdgas-installer/Index.html—replaceAll, optional chainingTests to Write
.gsfiles are intentionally untouched (outside the affected-file scope and not scanned).Test Command
npm test(Jest),npm run check(prettier + eslint),bash .dev-lead/scripts/dev-lead-lint.shdon-petry commentedon Jul 4, 2026
Dev-Lead: cannot implement issue #421 — needs human attention
Automatic retries are exhausted (attempt 3 of 3). The most recent failure was
engine-error.Please review the failures above. To retry anyway, remove the
dev-lead:needs-humanlabel and re-apply thedev-leadlabel.don-petry commentedon Jul 15, 2026
Re-scoped as a parent tracker. The single ×68 issue exhausted dev-lead's 3 retries on 2026-07-04 (mislabeled `engine-error` — the pre-fix deep-impl timeout, since resolved by #1018/#1021 + follow-ups now live on `dev-lead/v1-stable`). Per the split+rescope guidance for oversized issues, this is now broken into three single-concern stories routed to dev-lead individually:
src/app codetest-utils/& build scriptsgas-installer/& deploy HTMLRouting #442 first to validate the split on the stabilized engine; #443/#444 follow once it lands.
dev-lead/dev-lead:needs-humancleared from this tracker so it isn't picked up directly.