Skip to content

Commit 40b4d12

Browse files
authored
fix: guard against null ITextEditor document provider (#585)
ITextEditor.getDocumentProvider() is optional in the Eclipse API and may return null for third party editors. AutoTriggerPartListener dereferenced it unconditionally on every editor activation, so the NPE escaped the IPartListener2 callback and the workbench showed a generic error dialog. Resolve the provider and document defensively in AutoTriggerPartListener (partActivated and onStart) and in QEclipseEditorUtils.getSelectionRange and insertAtCursor, degrading to a no-op instead of throwing.
1 parent 73d6fad commit 40b4d12

3 files changed

Lines changed: 105 additions & 4 deletions

File tree

‎plugin/src/software/aws/toolkits/eclipse/amazonq/util/AutoTriggerPartListener.java‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,10 @@ private boolean isCustomizedEditorType(final IWorkbenchPart part) {
8181
}
8282

8383
private void attachDocumentListenerAndUpdateActiveDocument(final ITextEditor editor) {
84-
var document = editor.getDocumentProvider().getDocument(editor.getEditorInput());
84+
var document = resolveDocument(editor);
85+
if (document == null) {
86+
return;
87+
}
8588
attachDocumentListenerAndUpdateActiveDocument(document);
8689
}
8790

@@ -90,6 +93,29 @@ private void attachDocumentListenerAndUpdateActiveDocument(final IDocument docum
9093
setActiveDocument(document);
9194
}
9295

96+
/**
97+
* Resolves the document backing the given editor, or {@code null} when it
98+
* cannot be resolved.
99+
*
100+
* <p>
101+
* {@link ITextEditor#getDocumentProvider()} is an optional part of the Eclipse
102+
* API and third party editors may return {@code null} from it. Dereferencing it
103+
* unconditionally throws a {@link NullPointerException} out of the
104+
* {@code IPartListener2} callback, which the workbench surfaces as a generic
105+
* error dialog on every editor activation.
106+
* </p>
107+
*/
108+
private static IDocument resolveDocument(final ITextEditor editor) {
109+
if (editor == null) {
110+
return null;
111+
}
112+
var documentProvider = editor.getDocumentProvider();
113+
if (documentProvider == null) {
114+
return null;
115+
}
116+
return documentProvider.getDocument(editor.getEditorInput());
117+
}
118+
93119
private void detachDocumentListenerFromLastActiveDocument() {
94120
if (activeDocument != null) {
95121
activeDocument.removeDocumentListener(docListener);
@@ -107,7 +133,7 @@ public void onStart() {
107133
public void run() {
108134
var editor = getActiveTextEditor();
109135
if (editor != null) {
110-
var document = editor.getDocumentProvider().getDocument(editor.getEditorInput());
136+
var document = resolveDocument(editor);
111137
if (document == null) {
112138
Display.getDefault().timerExec(1000, this);
113139
return;

‎plugin/src/software/aws/toolkits/eclipse/amazonq/util/QEclipseEditorUtils.java‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,27 @@ private static ISelection getSelection(final ITextEditor textEditor) {
7676
return textEditor.getSelectionProvider().getSelection();
7777
}
7878

79+
/**
80+
* Resolves the document backing the given editor, or {@code null} when it
81+
* cannot be resolved.
82+
*
83+
* <p>
84+
* {@link ITextEditor#getDocumentProvider()} is an optional part of the Eclipse
85+
* API: third party editors may return {@code null} from it. Callers must
86+
* therefore treat both the provider and the document it returns as nullable.
87+
* </p>
88+
*/
89+
private static IDocument getDocument(final ITextEditor editor) {
90+
if (editor == null) {
91+
return null;
92+
}
93+
var documentProvider = editor.getDocumentProvider();
94+
if (documentProvider == null) {
95+
return null;
96+
}
97+
return documentProvider.getDocument(editor.getEditorInput());
98+
}
99+
79100
private static ITextEditor asTextEditor(final IEditorPart editorPart) {
80101
if (editorPart instanceof ITextEditor) {
81102
return (ITextEditor) editorPart;
@@ -194,7 +215,10 @@ public static Optional<Range> getSelectionRange(final ITextEditor editor) {
194215
return Optional.empty();
195216
}
196217
ISelection selection = getSelection(editor);
197-
var document = editor.getDocumentProvider().getDocument(editor.getEditorInput());
218+
var document = getDocument(editor);
219+
if (document == null) {
220+
return Optional.empty();
221+
}
198222
if (selection instanceof ITextSelection textSelection) {
199223
try {
200224
int startOffset = textSelection.getOffset();
@@ -232,7 +256,10 @@ public static Optional<Range> insertAtCursor(final String text) {
232256
}
233257

234258
var selection = editor.getSelectionProvider().getSelection();
235-
var document = editor.getDocumentProvider().getDocument(editor.getEditorInput());
259+
var document = getDocument(editor);
260+
if (document == null) {
261+
return Optional.empty();
262+
}
236263
if (selection instanceof ITextSelection textSelection) {
237264
try {
238265
int startOffset = textSelection.getOffset();

‎plugin/tst/software/aws/toolkits/eclipse/amazonq/util/AutoTriggerPartListenerTest.java‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
package software.aws.toolkits.eclipse.amazonq.util;
55

6+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
67
import static org.mockito.ArgumentMatchers.any;
78
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
89
import static org.mockito.Mockito.atLeast;
@@ -27,6 +28,7 @@
2728
import org.eclipse.ui.IEditorInput;
2829
import org.eclipse.ui.IWorkbenchPart;
2930
import org.eclipse.ui.IWorkbenchPartReference;
31+
import org.eclipse.ui.texteditor.IDocumentProvider;
3032
import org.eclipse.ui.texteditor.ITextEditor;
3133
import org.junit.jupiter.api.AfterAll;
3234
import org.junit.jupiter.api.BeforeAll;
@@ -151,6 +153,52 @@ void testShutdownRoutine() {
151153
verify(docMock, times(removeDocListenerInvocNumber + 1)).removeDocumentListener(docListenerMock);
152154
}
153155

156+
@SuppressWarnings({ "rawtypes", "unchecked" })
157+
@Test
158+
void testPartActivateWithNullDocumentProvider() {
159+
// Test case: an ITextEditor that returns null from getDocumentProvider() must
160+
// be ignored instead of throwing out of the part listener callback
161+
AutoTriggerPartListener partListener = new AutoTriggerPartListener(docListenerMock);
162+
ITextEditor editorWithoutProvider = mock(ITextEditor.class);
163+
when(editorWithoutProvider.getDocumentProvider()).thenReturn(null);
164+
IWorkbenchPartReference partRefMock = mock(IWorkbenchPartReference.class);
165+
when(partRefMock.getPart(any(Boolean.class))).thenReturn(editorWithoutProvider);
166+
167+
int addDocListenerInvocNumber = countDocInvocations("addDocumentListener");
168+
int removeDocListenerInvocNumber = countDocInvocations("removeDocumentListener");
169+
170+
assertDoesNotThrow(() -> partListener.partActivated(partRefMock));
171+
verify(docMock, times(addDocListenerInvocNumber)).addDocumentListener(docListenerMock);
172+
173+
// Test case: no active document was recorded, so deactivation is also a no-op
174+
assertDoesNotThrow(() -> partListener.partDeactivated(partRefMock));
175+
verify(docMock, times(removeDocListenerInvocNumber)).removeDocumentListener(docListenerMock);
176+
}
177+
178+
@SuppressWarnings({ "rawtypes", "unchecked" })
179+
@Test
180+
void testPartActivateWithNullDocument() {
181+
// Test case: a provider that returns a null document must be ignored rather
182+
// than dereferenced when attaching the document listener
183+
AutoTriggerPartListener partListener = new AutoTriggerPartListener(docListenerMock);
184+
IDocumentProvider providerWithoutDocument = mock(IDocumentProvider.class);
185+
when(providerWithoutDocument.getDocument(any())).thenReturn(null);
186+
ITextEditor editorWithoutDocument = mock(ITextEditor.class);
187+
when(editorWithoutDocument.getDocumentProvider()).thenReturn(providerWithoutDocument);
188+
IWorkbenchPartReference partRefMock = mock(IWorkbenchPartReference.class);
189+
when(partRefMock.getPart(any(Boolean.class))).thenReturn(editorWithoutDocument);
190+
191+
int addDocListenerInvocNumber = countDocInvocations("addDocumentListener");
192+
193+
assertDoesNotThrow(() -> partListener.partActivated(partRefMock));
194+
verify(docMock, times(addDocListenerInvocNumber)).addDocumentListener(docListenerMock);
195+
}
196+
197+
private static int countDocInvocations(final String methodName) {
198+
return (int) mockingDetails(docMock).getInvocations().stream()
199+
.filter(inv -> inv.getMethod().getName().equals(methodName)).count();
200+
}
201+
154202
static void mockEditor() {
155203
when(editorMock.getDocumentProvider().getDocument(any(IEditorInput.class))).thenReturn(docMock);
156204
}

0 commit comments

Comments
 (0)