SonarCloud: JavaScript/TypeScript code quality

[don-petry]

SonarCloud: JavaScript/TypeScript code quality

8 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:S7773	2	Prefer Number.isNaN over isNaN.
javascript:S7784	2	Prefer structuredClone(…) over JSON.parse(JSON.stringify(…)) to create a deep clone.
javascript:S3800	1	Refactor this function to always return the same type.
javascript:S6582	1	Prefer using an optional chain expression instead, as it's more concise and easier to read
javascript:S7765	1	Use .includes(), rather than .indexOf(), when checking for existence.
javascript:S7781	1	Prefer String#replaceAll() over String#replace().

Affected files

• src/gmail-ai-classifier/src/github-sync.js (8)

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: #577 — SonarCloud: JavaScript/TypeScript code quality

Scope

Resolve all 8 open SonarCloud findings in src/gmail-ai-classifier/src/github-sync.js via real, behavior-preserving fixes (no blanket suppressions), keeping the Node/Jest test suite green.

Finding → fix mapping

• S7773 ×2 (Number.isNaN): isNaN(remoteTs) / isNaN(localTs) → Number.isNaN(...) (inputs are Date#getTime() numbers, so identical).
• S7784 ×2 (deep clone): JSON.parse(JSON.stringify(x)) → shallow spread copy { ...x } before delete _sha. Avoids structuredClone (not available in the Apps Script V8 runtime) while removing the flagged pattern; behavior identical (only top-level _sha removed, output JSON unchanged).
• S3800 ×1 (consistent return type): executeGitHubCommit returns true | false | 'IDEMPOTENT_SKIP'. Unify to boolean — idempotent skip returns true. Caller appendMarkdownEntryToGitHubRepo already collapses both success cases to true, so its external behavior is unchanged; the now-dead || result === 'IDEMPOTENT_SKIP' guard is simplified.
• S6582 ×1 (optional chain): refreshed && refreshed.updatedAt && ... → refreshed?.updatedAt && ....
• S7765 ×1 (.includes): rawContent.indexOf(entryMd.trim()) !== -1 → rawContent.includes(entryMd.trim()).
• S7781 ×1 (replaceAll): topic.replace(/-/g, ' ') → topic.replaceAll('-', ' ').

Implementation Checklist

• src/gmail-ai-classifier/src/github-sync.js — apply the 6 fixes above

Tests to Write/Update

• src/gmail-ai-classifier/tests/gitHubSync.test.js — update the "handles 200 with idempotent skip" assertion to reflect the unified boolean return (true); all other tests are behavior-preserving and must stay green.

Notes on approach

Most changes are behavior-preserving refactors validated by the existing suite (TDD Phase-2 exception). The only observable-contract change is S3800's return-type unification, covered by the updated idempotent-skip test.

Test Command

npm test (Jest + coverage); lint via npm run check.

[don-petry]

Dev-Lead: Progress (in progress — not yet durable)

Provisional. Nothing is pushed yet; the automation posts the authoritative
completion record (with PR number + head SHA) after the push. Treat the notes
below as intent, not delivered results.

Plan Execution (intended)

All 8 SonarCloud findings in src/gmail-ai-classifier/src/github-sync.js addressed with real, behavior-preserving fixes — no blanket NOSONAR, no structuredClone (unavailable in the Apps Script V8 runtime):

• S7773 ×2 → Number.isNaN(...)
• S7784 ×2 → shallow spread copy { ...obj } + delete _sha (removes the JSON.parse(JSON.stringify()) pattern without structuredClone; identical output)
• S3800 ×1 → executeGitHubCommit now returns a consistent boolean (idempotent skip returns true); caller appendMarkdownEntryToGitHubRepo already collapsed both success cases, so external behavior is unchanged
• S6582 ×1 → optional chain refreshed?.updatedAt && ...
• S7765 ×1 → rawContent.includes(...)
• S7781 ×1 → topic.replaceAll('-', ' ')

Files touched (this workspace, not yet pushed)

• src/gmail-ai-classifier/src/github-sync.js: the 6 fixes above
• src/gmail-ai-classifier/tests/gitHubSync.test.js: updated the idempotent-skip test to assert the unified boolean return (the only observable-contract change)

Local check output (provisional — not the pushed run)

npm test            → all suites green
npm run check       → prettier OK; eslint 0 errors (pre-existing warnings only, unrelated files)
.dev-lead lint      → all checks passed
github-sync.js cov  → 100% stmts / 100% branch / 100% funcs / 100% lines

Notes

• Only github-sync.js (the SonarCloud-scanned .js) was changed; gitHubSync.gs is out of scope and left untouched.
• structuredClone was deliberately avoided as it is a host-provided global not present in the GAS V8 runtime; the spread-copy fix preserves behavior in both Node and GAS.
• Most changes are behavior-preserving refactors (TDD Phase-2 exception); the S3800 return-type unification is covered by the updated idempotent-skip test.

[don-petry]

Dev-Lead: Implementation Complete — PR #578

The implementation for issue #577 is durable: commits are pushed and a pull request is open. This record is backed by verifiable artifacts you can confirm in one click.

• Pull request: feat: implement issue #577 — SonarCloud: JavaScript/TypeScript code quality #578
• Head commit: cb53ffb262a8fc3c550e05c74236819441d8168f
• Run: https://github.com/petry-projects/google-app-scripts/actions/runs/34271831951

Review happens on the PR. Acceptance criteria and test results are evidenced by the PR's diff and CI checks — not asserted here. If a later stage fails, this claim is retracted in place.