Repository navigation
Remove unreachable null guard in ViewerState.updateSelection() - #4506
Conversation
updateSelection() has a single caller, setSelectedRanges(), which always passes an array created by Arrays.stream(...).toArray(). The argument is never null. ViewerState is a private inner class of TextViewer, so no other caller exists.
There was a problem hiding this comment.
🟢 Approval recommended
The sole caller guarantees a non-null array, so the cleanup preserves existing behavior.
0 open findings
What changed in this PR
Removes an unreachable null guard from JFace’s selection tracking, following up on #4426 without changing behavior.
Changes:
- Removes the null-array branch from
ViewerState.updateSelection(). - Preserves handling of empty and populated selection arrays.
| File | Description |
|---|---|
| bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/TextViewer.java | Removes the unreachable selection null guard. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
HeikoKlare
left a comment
HeikoKlare
left a comment
There was a problem hiding this comment.
Thank you for that reasonable cleanup. With only a single caller that already properly ensures a non-null argument, it is not necessary to have explicit handling for it in the called method as well. Otherwise, a similar adaptation for this case like done in #4426 would be necessary here, which is obsolete as the code is never called.
Follow-up to #4426, as discussed in this review comment.
What
Removes the
selections == nullbranch fromTextViewer.ViewerState.updateSelection().Why
The branch is never reached:
updateSelection()has a single caller,setSelectedRanges(), which passesArrays.stream(...).toArray(Position[]::new). That's nevernull, at most an empty array.elsebranch.ViewerStateis a private inner class ofTextViewer. No other code can call this method.No behavior change.