refactor-enforced-Number-namespace

[kart-u]

Proposed change

Resolves #2529

Enforced use of Number namespace for numeric globals

For each instance:

• Replaced NaN with Number.NaN
• Replaced isNaN() with Number.isNaN()
• Replaced parseInt() with Number.parseInt()
• Review logic to ensure behavior remains unchanged after replacement

Checklist

• I've read and followed the contributing guidelines.
• I've run make check-test locally; all checks and tests passed.

[coderabbitai]

Summary by CodeRabbit

• Refactor

  • Improved code quality by standardizing numeric operations to follow JavaScript best practices.

• Chores

  • Added new ESLint rules to enforce consistent use of Number object methods throughout the codebase.

Walkthrough

Refactors frontend code to replace global numeric globals with Number‑scoped equivalents (e.g., parseInt → Number.parseInt, isNaN → Number.isNaN, NaN → Number.NaN) and adds ESLint rules to enforce these patterns; updates tests and utilities accordingly. (46 words)

Changes

Cohort / File(s)	Summary
Tests frontend/__tests__/unit/components/AnimatedCounter.test.tsx, frontend/__tests__/unit/components/SearchPageLayout.test.tsx	Replaced global parseInt usage with Number.parseInt and replaced NaN literals with Number.NaN in test code.
Pages & Hooks frontend/src/app/my/mentorship/page.tsx, frontend/src/hooks/useSearchPage.ts	Replaced parseInt with Number.parseInt when deriving page values from query params / initial state.
Components frontend/src/components/ModuleCard.tsx	Replaced isNaN(...) checks with Number.isNaN(...) in duration calculation logic.
Utilities frontend/src/utils/dateFormatter.ts, frontend/src/utils/round.ts	Replaced isNaN(...) with Number.isNaN(...) across date formatting and rounding utilities.
ESLint rules frontend/eslint-rules/no-global-isfinite.mjs, frontend/eslint-rules/no-global-isnan.mjs, frontend/eslint-rules/no-global-nan.mjs, frontend/eslint-rules/no-global-parsefloat.mjs, frontend/eslint-rules/no-global-parseint.mjs, frontend/eslint.config.mjs	Added five custom ESLint rule modules to disallow global numeric globals (isFinite, isNaN, NaN, parseFloat, parseInt) and wired them into ESLint config with auto-fixers suggesting Number namespace replacements.
Manifest package.json	(Referenced in manifest listing) no code changes described, included as manifest file reference.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Changes are consistent but span tests, runtime code, utilities, and new lint rules (heterogeneous).
• Pay attention to:
  • ESLint rule implementations and their fixers for correct AST handling and edge cases (MemberExpression parents, shadowed identifiers).
  • Date parsing/NaN semantics differences between global isNaN and Number.isNaN where non-number coercion could change outcomes.
  • Query-param parsing locations to ensure radix/whitespace behavior remains intended.

Possibly related PRs

• implemented AnimatedCounter tests #1994 — touches the same test file frontend/__tests__/unit/components/AnimatedCounter.test.tsx; likely overlapping edits.
• refactor: replace window with globalThis #2507 — modifies same test area and global API usage patterns; may conflict or complement this change.

Suggested reviewers

• kasya

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "refactor-enforced-Number-namespace" clearly and concisely describes the primary change in the PR: a refactoring effort to enforce use of the Number namespace for numeric globals. The title accurately summarizes the main objective without being vague or including unnecessary noise, making it easy for someone reviewing the git history to understand the nature of this change at a glance.
Linked Issues Check	✅ Passed	The PR comprehensively addresses the requirements of issue #2529. Global numeric references have been replaced with Number namespace equivalents across multiple files: NaN → Number.NaN in test files [SearchPageLayout.test.tsx], isNaN() → Number.isNaN() in utility files [ModuleCard.tsx, dateFormatter.ts, round.ts], and parseInt() → Number.parseInt() in page and hook files [my/mentorship/page.tsx, useSearchPage.ts, AnimatedCounter.test.tsx]. Additionally, five new ESLint rules have been created and integrated to enforce these patterns going forward, preventing future violations. The PR description confirms that local checks and tests were run and passed.
Out of Scope Changes Check	✅ Passed	All changes in the PR are directly related to the linked issue #2529 objectives. The modifications include replacing global numeric references with Number namespace equivalents in test files, source utilities, page components, and hooks; creating new ESLint rule modules to enforce this pattern; and updating the ESLint configuration to enable these rules. No changes to unrelated functionality, business logic, or features are present in the PR.
Description Check	✅ Passed	The PR description is directly related to the changeset, clearly explaining that it resolves issue #2529 and outlining the specific replacements made (NaN → Number.NaN, isNaN() → Number.isNaN(), parseInt() → Number.parseInt()). It also mentions that logic was reviewed to ensure behavior remains unchanged and that the contributor ran local checks with all tests passing, providing meaningful context about the scope and verification of the changes.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[arkid15r]

LGTM

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

frontend/src/hooks/useSearchPage.ts (1)

40-42: LGTM! Refactor correctly implements Number namespace usage.

The change from parseInt to Number.parseInt is correct and maintains identical behavior. This aligns with the PR objective to enforce the Number namespace for numeric globals.

Optional improvement: Consider explicitly specifying the radix parameter for clarity:

  const [currentPage, setCurrentPage] = useState<number>(
-   Number.parseInt(searchParams.get('page') || '1')
+   Number.parseInt(searchParams.get('page') || '1', 10)
  )

This makes the decimal parsing intent explicit and is considered a best practice, though not strictly required since base-10 is the default for numeric strings.

frontend/eslint-rules/no-global-parsefloat.mjs (1)

1-29: LGTM! Safe replacement for parseFloat.

Like parseInt, Number.parseFloat is functionally identical to the global parseFloat. This replacement is semantically safe.

Consider adding dedicated unit tests for all five custom ESLint rules to verify:

• Correct detection of violations
• Auto-fixer behavior
• Edge cases (e.g., shadowed identifiers, already-namespaced calls)

Example test structure using ESLint's RuleTester:

import { RuleTester } from 'eslint'
import noGlobalParseFloatRule from './no-global-parsefloat.mjs'

const ruleTester = new RuleTester()

ruleTester.run('no-global-parsefloat', noGlobalParseFloatRule, {
  valid: [
    'Number.parseFloat("3.14")',
  ],
  invalid: [
    {
      code: 'parseFloat("3.14")',
      errors: [{ messageId: 'useNumberParseFloat' }],
      output: 'Number.parseFloat("3.14")',
    },
  ],
})

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6cfebaf and 3b8f129.

📒 Files selected for processing (7)

• frontend/eslint-rules/no-global-isfinite.mjs (1 hunks)
• frontend/eslint-rules/no-global-isnan.mjs (1 hunks)
• frontend/eslint-rules/no-global-nan.mjs (1 hunks)
• frontend/eslint-rules/no-global-parsefloat.mjs (1 hunks)
• frontend/eslint-rules/no-global-parseint.mjs (1 hunks)
• frontend/eslint.config.mjs (3 hunks)
• frontend/src/hooks/useSearchPage.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

frontend/eslint.config.mjs (5)

frontend/eslint-rules/no-global-isfinite.mjs (1)

• noGlobalIsFiniteRule (1-29)

frontend/eslint-rules/no-global-isnan.mjs (1)

• noGlobalIsNaNRule (1-29)

frontend/eslint-rules/no-global-nan.mjs (1)

• noGlobalNaNRule (1-42)

frontend/eslint-rules/no-global-parsefloat.mjs (1)

• noGlobalParseFloatRule (1-29)

frontend/eslint-rules/no-global-parseint.mjs (1)

• noGlobalParseIntRule (1-29)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: CodeQL (python)
• GitHub Check: CodeQL (javascript-typescript)
• GitHub Check: Run frontend checks
• GitHub Check: Run pre-commit checks

🔇 Additional comments (5)

frontend/eslint-rules/no-global-nan.mjs (1)

16-29: LGTM! Parent check correctly prevents duplicate reports.

The logic properly skips NaN when it's already part of Number.NaN by checking the parent MemberExpression. The NaN replacement is semantically safe since both refer to the same value.

frontend/eslint.config.mjs (1)

18-22: LGTM! ESLint configuration properly wires the new rules.

The custom rules are correctly imported, registered under the nest plugin namespace, and enabled at error level. This enforces the Number namespace pattern as intended.

Note: Given the semantic differences between global isNaN/isFinite and their Number.* counterparts (see comments on those rule files), ensure the auto-fixers have been tested against the actual codebase before merging.

Also applies to: 69-77, 166-170

frontend/eslint-rules/no-global-parseint.mjs (1)

1-29: LGTM! Safe replacement for parseInt.

Unlike isNaN/isFinite, Number.parseInt is functionally identical to the global parseInt (they reference the same function). This replacement is semantically safe and improves code clarity.

frontend/eslint-rules/no-global-isnan.mjs (1)

1-29: No semantic risk from this rule in the current codebase.

The verification shows the codebase already uses Number.isNaN() exclusively (except in third-party minified libraries). The rule correctly targets only global isNaN() calls via the condition node.callee.name === 'isNaN', so existing code using Number.isNaN() is unaffected. The auto-fixer will only replace problematic patterns where they appear, making the theoretical semantic concern inapplicable to this project's current state.

frontend/eslint-rules/no-global-isfinite.mjs (1)

1-29: The rule is preventive but raises a valid semantic concern worthy of verification before activation.

The codebase contains zero isFinite() calls currently, so the original concern about breaking existing code is moot. However, if this rule is intended to be enabled, the semantic difference between global isFinite() and Number.isFinite() remains important:

isFinite("123")        // true (coerces to number)
Number.isFinite("123") // false (no coercion)

Before enabling this rule, verify:

• Whether auto-fixing to Number.isFinite() aligns with intended behavior
• Whether this should be a suggestion or a stricter enforcement level
• If any other similar rules in the suite (e.g., no-global-isnan) should be enabled together