Skip to content

fix: prefer optional chaining over logical AND guards (S6582)#3485

Merged
kasya merged 2 commits intoOWASP:mainfrom
anukalp2804:fix/sonar-s6582-optional-chaining
Jan 23, 2026
Merged

fix: prefer optional chaining over logical AND guards (S6582)#3485
kasya merged 2 commits intoOWASP:mainfrom
anukalp2804:fix/sonar-s6582-optional-chaining

Conversation

@anukalp2804
Copy link
Contributor

Proposed change

Resolves #3472

This PR addresses SonarCloud rule typescript:S6582 by replacing logical AND guards with optional chaining in multiple components.

The change improves readability and maintainability while preserving existing behavior.

Checklist

  • I followed the contributing workflow
  • I verified that the code works as intended
  • I ran the relevant local checks (lint)
  • I used AI for code, documentation, tests, or communication related to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 23, 2026

Summary by CodeRabbit

  • Refactor
    • Improved stability in the chapters view by adding safer guards when rendering potentially missing list data, reducing the chance of runtime errors.
    • Improved stability in the committees view with similar rendering guards so missing or undefined lists no longer cause display issues.

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

Walkthrough

Replaces logical AND guards (&&) with optional chaining (?.) in two React page components to conditionally render lists; no exported signatures or control-flow branches were added.

Changes

Cohort / File(s) Change Summary
Optional chaining refactor
frontend/src/app/chapters/page.tsx, frontend/src/app/committees/page.tsx
Replaced chapters && chapters.filter(...).map(...) and committees && committees.map(...) with chapters?.filter(...).map(...) and committees?.map(...) respectively; no signature or behavioral changes beyond the guard style.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • arkid15r
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: replacing logical AND guards with optional chaining to address SonarCloud rule S6582.
Description check ✅ Passed The description relates to the changeset by referencing issue #3472, the SonarCloud rule being addressed, and the pattern being fixed across multiple components.
Linked Issues check ✅ Passed The PR successfully addresses both code patterns specified in issue #3472: replacing logical AND guards with optional chaining in chapters and committees components.
Out of Scope Changes check ✅ Passed All changes are scoped to the linked issue #3472: modifications only update the two affected components (chapters and committees pages) to replace AND guards with optional chaining.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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

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

@anukalp2804
Copy link
Contributor Author

@arkid15r , @kasya
This PR addresses the assigned SonarCloud S6582 issue. Please let me know if any changes are needed.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/app/chapters/page.tsx (1)

91-107: Inconsistent null-safety: chapters.length on line 91 vs optional chaining on line 105.

If chapters can be undefined or null, line 91 (chapters.length > 0) will throw a TypeError before the optional chaining on line 105 is ever reached. Conversely, if useSearchPage always returns an array, the optional chaining is unnecessary (though harmless).

For consistency, consider one of these approaches:

  • If chapters can be nullish: {(chapters?.length ?? 0) > 0 && (...)}
  • If chapters is always an array: the ?. on line 105 is safe but redundant

Also, the indentation on lines 106-107 appears slightly off.

Suggested fix if chapters can be nullish
-      {chapters.length > 0 && (
+      {(chapters?.length ?? 0) > 0 && (
         <ChapterMapWrapper
           ...
         />
       )}
-      {chapters
-      ?.filter((chapter) => chapter.isActive)
-          .map(renderChapterCard)}
+      {chapters?.filter((chapter) => chapter.isActive).map(renderChapterCard)}

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 23, 2026
@sonarqubecloud
Copy link

@codecov
Copy link

codecov bot commented Jan 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.60%. Comparing base (7484604) to head (2ec2776).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3485      +/-   ##
==========================================
- Coverage   85.60%   85.60%   -0.01%     
==========================================
  Files         461      461              
  Lines       14222    14221       -1     
  Branches     1894     1892       -2     
==========================================
- Hits        12175    12174       -1     
  Misses       1678     1678              
  Partials      369      369              
Flag Coverage Δ
backend 84.47% <ø> (ø)
frontend 88.74% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
frontend/src/app/chapters/page.tsx 96.87% <100.00%> (ø)
frontend/src/app/committees/page.tsx 100.00% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7484604...2ec2776. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@kasya kasya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anukalp2804 thanks for updating this!
Please run make check-test next time before pushing changes 👍🏼

@kasya kasya enabled auto-merge January 23, 2026 04:11
@kasya kasya added this pull request to the merge queue Jan 23, 2026
Merged via the queue into OWASP:main with commit 41e700a Jan 23, 2026
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix Sonar S6582: Prefer optional chaining over logical AND guards

2 participants

Comments