Skip to content

fix: guard against null ITextEditor document provider #585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
dungdong-aws wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: guard against null ITextEditor document provider #585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions plugin/src/software/aws/toolkits/eclipse/amazonq/util/AutoTriggerPartListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,10 @@ private boolean isCustomizedEditorType(final IWorkbenchPart part) {
}

private void attachDocumentListenerAndUpdateActiveDocument(final ITextEditor editor) {
var document = editor.getDocumentProvider().getDocument(editor.getEditorInput());
var document = resolveDocument(editor);
if (document == null) {
return;
}
attachDocumentListenerAndUpdateActiveDocument(document);
}

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

/**
* Resolves the document backing the given editor, or {@code null} when it
* cannot be resolved.
*
* <p>
* {@link ITextEditor#getDocumentProvider()} is an optional part of the Eclipse
* API and third party editors may return {@code null} from it. Dereferencing it
* unconditionally throws a {@link NullPointerException} out of the
* {@code IPartListener2} callback, which the workbench surfaces as a generic
* error dialog on every editor activation.
* </p>
*/
private static IDocument resolveDocument(final ITextEditor editor) {
if (editor == null) {
return null;
}
var documentProvider = editor.getDocumentProvider();
if (documentProvider == null) {
return null;
}
return documentProvider.getDocument(editor.getEditorInput());
}

private void detachDocumentListenerFromLastActiveDocument() {
if (activeDocument != null) {
activeDocument.removeDocumentListener(docListener);
Expand All @@ -107,7 +133,7 @@ public void onStart() {
public void run() {
var editor = getActiveTextEditor();
if (editor != null) {
var document = editor.getDocumentProvider().getDocument(editor.getEditorInput());
var document = resolveDocument(editor);
if (document == null) {
Display.getDefault().timerExec(1000, this);
return;
Expand Down
31 changes: 29 additions & 2 deletions plugin/src/software/aws/toolkits/eclipse/amazonq/util/QEclipseEditorUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,27 @@ private static ISelection getSelection(final ITextEditor textEditor) {
return textEditor.getSelectionProvider().getSelection();
}

/**
* Resolves the document backing the given editor, or {@code null} when it
* cannot be resolved.
*
* <p>
* {@link ITextEditor#getDocumentProvider()} is an optional part of the Eclipse
* API: third party editors may return {@code null} from it. Callers must
* therefore treat both the provider and the document it returns as nullable.
* </p>
*/
private static IDocument getDocument(final ITextEditor editor) {
if (editor == null) {
return null;
}
var documentProvider = editor.getDocumentProvider();
if (documentProvider == null) {
return null;
}
return documentProvider.getDocument(editor.getEditorInput());
}

private static ITextEditor asTextEditor(final IEditorPart editorPart) {
if (editorPart instanceof ITextEditor) {
return (ITextEditor) editorPart;
Expand Down Expand Up @@ -194,7 +215,10 @@ public static Optional<Range> getSelectionRange(final ITextEditor editor) {
return Optional.empty();
}
ISelection selection = getSelection(editor);
var document = editor.getDocumentProvider().getDocument(editor.getEditorInput());
var document = getDocument(editor);
if (document == null) {
return Optional.empty();
}
if (selection instanceof ITextSelection textSelection) {
try {
int startOffset = textSelection.getOffset();
Expand Down Expand Up @@ -232,7 +256,10 @@ public static Optional<Range> insertAtCursor(final String text) {
}

var selection = editor.getSelectionProvider().getSelection();
var document = editor.getDocumentProvider().getDocument(editor.getEditorInput());
var document = getDocument(editor);
if (document == null) {
return Optional.empty();
}
if (selection instanceof ITextSelection textSelection) {
try {
int startOffset = textSelection.getOffset();
Expand Down
48 changes: 48 additions & 0 deletions plugin/tst/software/aws/toolkits/eclipse/amazonq/util/AutoTriggerPartListenerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

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

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.atLeast;
Expand All @@ -27,6 +28,7 @@
import org.eclipse.ui.IEditorInput;
import org.eclipse.ui.IWorkbenchPart;
import org.eclipse.ui.IWorkbenchPartReference;
import org.eclipse.ui.texteditor.IDocumentProvider;
import org.eclipse.ui.texteditor.ITextEditor;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
Expand Down Expand Up @@ -151,6 +153,52 @@ void testShutdownRoutine() {
verify(docMock, times(removeDocListenerInvocNumber + 1)).removeDocumentListener(docListenerMock);
}

@SuppressWarnings({ "rawtypes", "unchecked" })
@Test
void testPartActivateWithNullDocumentProvider() {
// Test case: an ITextEditor that returns null from getDocumentProvider() must
// be ignored instead of throwing out of the part listener callback
AutoTriggerPartListener partListener = new AutoTriggerPartListener(docListenerMock);
ITextEditor editorWithoutProvider = mock(ITextEditor.class);
when(editorWithoutProvider.getDocumentProvider()).thenReturn(null);
IWorkbenchPartReference partRefMock = mock(IWorkbenchPartReference.class);
when(partRefMock.getPart(any(Boolean.class))).thenReturn(editorWithoutProvider);

int addDocListenerInvocNumber = countDocInvocations("addDocumentListener");
int removeDocListenerInvocNumber = countDocInvocations("removeDocumentListener");

assertDoesNotThrow(() -> partListener.partActivated(partRefMock));
verify(docMock, times(addDocListenerInvocNumber)).addDocumentListener(docListenerMock);

// Test case: no active document was recorded, so deactivation is also a no-op
assertDoesNotThrow(() -> partListener.partDeactivated(partRefMock));
verify(docMock, times(removeDocListenerInvocNumber)).removeDocumentListener(docListenerMock);
}

@SuppressWarnings({ "rawtypes", "unchecked" })
@Test
void testPartActivateWithNullDocument() {
// Test case: a provider that returns a null document must be ignored rather
// than dereferenced when attaching the document listener
AutoTriggerPartListener partListener = new AutoTriggerPartListener(docListenerMock);
IDocumentProvider providerWithoutDocument = mock(IDocumentProvider.class);
when(providerWithoutDocument.getDocument(any())).thenReturn(null);
ITextEditor editorWithoutDocument = mock(ITextEditor.class);
when(editorWithoutDocument.getDocumentProvider()).thenReturn(providerWithoutDocument);
IWorkbenchPartReference partRefMock = mock(IWorkbenchPartReference.class);
when(partRefMock.getPart(any(Boolean.class))).thenReturn(editorWithoutDocument);

int addDocListenerInvocNumber = countDocInvocations("addDocumentListener");

assertDoesNotThrow(() -> partListener.partActivated(partRefMock));
verify(docMock, times(addDocListenerInvocNumber)).addDocumentListener(docListenerMock);
}

private static int countDocInvocations(final String methodName) {
return (int) mockingDetails(docMock).getInvocations().stream()
.filter(inv -> inv.getMethod().getName().equals(methodName)).count();
}

static void mockEditor() {
when(editorMock.getDocumentProvider().getDocument(any(IEditorInput.class))).thenReturn(docMock);
}
Expand Down
Loading