Skip to content

Comments

Fix/remove constant none checks#3261

Closed
hussainjamal760 wants to merge 2 commits intoOWASP:mainfrom
hussainjamal760:fix/remove-constant-none-checks
Closed

Fix/remove constant none checks#3261
hussainjamal760 wants to merge 2 commits intoOWASP:mainfrom
hussainjamal760:fix/remove-constant-none-checks

Conversation

@hussainjamal760
Copy link
Contributor

issue resolved : #3260

This PR removes constant None identity checks from tests where the value is guaranteed to be non-None.

Some tests were asserting is not None on objects created via class constructors (e.g. QuestionDetector()), which always return a valid instance. These assertions were constant expressions that always evaluated to True and were flagged by Sonar as unnecessary.

The fix removes these redundant checks and replaces them with meaningful assertions such as:

type validation using isinstance

attribute existence checks

This improves test clarity, aligns with static analysis rules, and ensures tests validate real behavior rather than guaranteed conditions.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 8, 2026

Caution

Review failed

The pull request is closed.

Summary by CodeRabbit

  • Refactor

    • Improved internal TypeScript type handling and inference across multiple components and utilities for better code quality.
  • Tests

    • Enhanced test assertion specificity for improved validation accuracy.

Note: These changes are internal improvements with no user-facing impact.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

This PR systematically removes unnecessary TypeScript type assertions and non-null assertions across backend tests and frontend components, relying on type inference where feasible and intentionally widening environment variable types to reflect runtime reality.

Changes

Cohort / File(s) Summary
Backend Test Assertion Tightening
backend/tests/apps/slack/common/question_detector_test.py
Replaced truthy check (detector is not None) with explicit type check (isinstance(detector, QuestionDetector)) for stricter initialization validation.
Frontend Components - Type Assertion Removals
frontend/src/components/CardDetailsPage.tsx, frontend/src/components/ChapterMap.tsx, frontend/src/components/MultiSearch.tsx, frontend/src/components/SingleModuleCard.tsx, frontend/src/components/UserMenu.tsx
Removed redundant explicit type casts (as MouseEvent, as ExtendedSession, as Event[], as string) across admin login comparisons, event handling, and session property access, relying on type inference instead.
Frontend Page - Type Assertion Removals
frontend/src/app/members/[memberKey]/page.tsx
Removed explicit string type assertions on memberKey parameter when calling fetchHeatmapData and assigning to username.
Frontend Utilities & Hooks - Type Assertion Removals
frontend/src/hooks/useSearchPage.ts, frontend/src/utils/env.server.ts
Removed number cast from response.totalPages and removed non-null assertions from GITHUB_CLIENT_ID and GITHUB_CLIENT_SECRET, widening their exported types from string to string | undefined.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

frontend, frontend-tests

Suggested reviewers

  • kasya
  • arkid15r
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent 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 3b85a08 and 0d8b025.

📒 Files selected for processing (9)
  • backend/tests/apps/slack/common/question_detector_test.py
  • frontend/src/app/members/[memberKey]/page.tsx
  • frontend/src/components/CardDetailsPage.tsx
  • frontend/src/components/ChapterMap.tsx
  • frontend/src/components/MultiSearch.tsx
  • frontend/src/components/SingleModuleCard.tsx
  • frontend/src/components/UserMenu.tsx
  • frontend/src/hooks/useSearchPage.ts
  • frontend/src/utils/env.server.ts

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

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

@github-actions
Copy link

github-actions bot commented Jan 8, 2026

The linked issue must be assigned to the PR author.

@github-actions github-actions bot closed this Jan 8, 2026
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 8, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove constant None identity checks that are always true

1 participant