Skip to content

fix(no-unnecessary-condition): handle IndexedAccess types in ?? and ??= paths#784

Merged
camc314 merged 2 commits intooxc-project:mainfrom
younggglcy:hollow-radon
Mar 10, 2026
Merged

fix(no-unnecessary-condition): handle IndexedAccess types in ?? and ??= paths#784
camc314 merged 2 commits intooxc-project:mainfrom
younggglcy:hollow-radon

Conversation

@younggglcy
Copy link
Copy Markdown
Contributor

@younggglcy younggglcy commented Mar 10, 2026

Summary

  • Fix false positive in no-unnecessary-condition when the left side of ?? or ??= is an unresolved indexed access type (e.g., Partial<Record<R, T[]>>[R] where R is a generic type parameter)
  • Resolve the type via getBaseConstraintOfType; if the constraint is still unresolvable or includes nullish, skip the report conservatively
  • Extract shared logic into a checkIndexedAccessNullish closure to avoid duplication between ?? and ??= paths

Related Issue in oxc: oxc-project/oxc#20149

Test plan

  • Added two valid test cases for Partial<Record<R, T[]>> (with and without noUncheckedIndexedAccess)
  • Verified existing invalid-49 test (T[K] with known non-nullish constraint) still correctly reports
  • All existing tests pass: go test ./internal/rules/no_unnecessary_condition/ -count=1

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a false positive in the no-unnecessary-condition rule when analyzing nullish coalescing (??) and nullish coalescing assignment (??=) where the LHS type is an unresolved indexed access involving generics (e.g., Partial<Record<R, T[]>>[R]).

Changes:

  • Add a shared checkIndexedAccessNullish helper to resolve indexed access types via getBaseConstraintOfType and skip/report conservatively.
  • Apply the indexed-access nullish handling to both ?? and ??= code paths.
  • Add valid test coverage for Partial<Record<R, T[]>> + generic index under both default and noUncheckedIndexedAccess configs (for ??).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
internal/rules/no_unnecessary_condition/no_unnecessary_condition.go Adds constraint-based handling for unresolved indexed access types in ??/??= to avoid false positives.
internal/rules/no_unnecessary_condition/no_unnecessary_condition_test.go Adds new valid cases exercising the unresolved generic indexed-access scenario (currently for ??).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread internal/rules/no_unnecessary_condition/no_unnecessary_condition.go Outdated
Comment thread internal/rules/no_unnecessary_condition/no_unnecessary_condition.go Outdated
…?= paths

Indexed access types like `Partial<Record<R, T[]>>[R]` with generic type
parameters cannot be fully resolved at compile time. The ?? and ??= handlers
were missing this case, causing false positives when the unresolved type
appeared non-nullish.

Resolve the type via `getBaseConstraintOfType`; if the constraint is still
unresolvable or includes nullish, skip the report conservatively.

Fixes oxc-project/oxc#20149
Copy link
Copy Markdown
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

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

thank you!

@camc314 camc314 enabled auto-merge (squash) March 10, 2026 19:34
@camc314 camc314 merged commit e3b67cc into oxc-project:main Mar 10, 2026
7 checks passed
@younggglcy younggglcy deleted the hollow-radon branch March 10, 2026 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants