SonarCloud: JavaScript/TypeScript code quality

[don-petry]

SonarCloud: JavaScript/TypeScript code quality

17 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:S8754	14	Rename this test title to make it unique within the suite.
javascript:S8786	3	Simplify this regular expression to reduce its runtime, as it has super-linear performance

Affected files

• src/calendar-to-sheets/tests/index.test.js (14)
• src/gas-utils.js (3)

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]

Dev-Lead Implementation Plan

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

Scope

Resolve 17 SonarCloud findings: 14 non-unique test titles (javascript:S8754) in the calendar-to-sheets test suite, and 3 super-linear regexes (javascript:S8786) in src/gas-utils.js — with no behavior change.

Investigation summary

• S8754 (14): All 14 non-unique titles are redundant copy-paste duplicates within the same suite (verified via diff): a 7-test block (fullResync…, tail merge…, …logs warning…) duplicated verbatim, a 3-test getConfigs/empty-SYNC_CONFIGS block (one test differs only in assertion style, e.g. toHaveLength(1) vs .length).toBe(1) — functionally identical), plus 4 standalone duplicated tests. Fix: remove the redundant second occurrences (real fix, not cosmetic renaming). Coverage is provably unchanged since each removed test is identical/equivalent to a retained one.
• S8786 (3): The three /^\s*…/m patterns are super-linear because \s matches newlines, so under /m the leading \s* backtracks across every line anchor → O(n²). Fix: scope the leading whitespace to horizontal-only (^[^\S\r\n]*). Internal \s connectors are left intact so the cross-line From:/Sent: match is preserved.

Implementation Checklist

• src/calendar-to-sheets/tests/index.test.js — remove 14 duplicate tests (5 contiguous blocks)
• src/gas-utils.js — rewrite 3 leading ^\s* anchors as ^[^\S\r\n]*

Tests to Write

• No new tests — behavior-preserving cleanup/refactor. Existing suites (index.test.js, gas-utils.test.js) are the regression guard. ReDoS fix additionally verified with a timing check (O(n²) → O(n)).

Test Command

npx jest --runInBand (+ npx jest --coverage and node scripts/check-coverage.js)

[don-petry]

Dev-Lead: Implementation Complete

Plan Execution

• Removed 14 redundant duplicate tests (javascript:S8754) from src/calendar-to-sheets/tests/index.test.js
• Fixed 3 super-linear regexes (javascript:S8786) in src/gas-utils.js
• Full suite + coverage thresholds green; repo lint (dev-lead-lint.sh), prettier, and eslint clean

What changed & why

S8754 — non-unique test titles (14): every flagged title was a verbatim (or assertion-style-only) copy-paste duplicate within the same Jest suite. Removed the redundant second occurrences rather than renaming, since keeping two identical tests under different names is not a real fix. Each removed test is identical/equivalent to a retained one, so coverage is unchanged.

S8786 — super-linear regexes (3): the three /^\s*…/m header patterns backtracked in O(n²) because \s matches newlines, so under /m the leading \s* scans across every line anchor. Scoped the leading anchor to horizontal whitespace (^[^\S\r\n]*). Internal \s connectors were left untouched so the cross-line From:/Sent: reply-header match still works. Verified O(n²) → O(n): old pattern 3.8→14→56 ms at n=2k/4k/8k; new pattern flat ~0.04 ms.

Test Results

Test Suites: 14 passed, 14 total
Tests:       378 passed, 378 total
Snapshots:   0 total
Time:        3.287 s, estimated 6 s
Ran all test suites.

Coverage

✅ statements     : 98.71% (1146/1161) [required: 95%]
✅ branches       : 90.19% (625/693) [required: 85%]
✅ functions      : 97.50% (117/120) [required: 95%]
✅ lines          : 100% (0/0) [required: 100%]

✅ All coverage metrics meet requirements.

Files Changed

• src/calendar-to-sheets/tests/index.test.js: removed 14 duplicate tests (434 lines) across 5 contiguous blocks; all titles now unique per suite.
• src/gas-utils.js: 3 regex leading anchors ^\s* → ^[^\S\r\n]*.

Notes

• Behavior-preserving change, so Phase 2 (test-first) was skipped per the documented exception; the existing index.test.js and gas-utils.test.js suites act as the regression guard. Test count went 392 → 378 (the 14 removed duplicates); coverage unchanged (lines 100%, all thresholds met).
• The 22 eslint warnings from npm run check are pre-existing unused-var warnings in unrelated gmail-to-drive-by-labels test files — not touched by this change.