Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 499980

Summary: [content assist] Potential NPE below CompletionProposalPopup.computeProposals if no proposals possible
Product: [Eclipse Project] Platform Reporter: Andreas Sewe <sewe>
Component: TextAssignee: Dani Megert <daniel_megert>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: bruno.do.medeiros, daniel_megert, noopur_gupta
Version: 4.7Flags: noopur_gupta: review+
Target Milestone: 4.6.1   
Hardware: All   
OS: All   
See Also: https://bugs.eclipse.org/bugs/show_bug.cgi?id=499653
Whiteboard:

Description Andreas Sewe CLA 2016-08-19 07:17:37 EDT 
Hi,

the implementation of ContentAssistant#computeCompletionProposals, it returns null to indicate that no proposals are possible (even if the Javadoc says otherwise; see Bug 499654).

This can lead to a NPE further down the road in Arrays#sort, as the proposals arrays is null:

    at java.util.Arrays.sort(null:-1)
    at org.eclipse.jface.text.contentassist.CompletionProposalPopup.sortProposals(CompletionProposalPopup.java:2007)
    at org.eclipse.jface.text.contentassist.CompletionProposalPopup.computeProposals(CompletionProposalPopup.java:568)
    at org.eclipse.jface.text.contentassist.CompletionProposalPopup.access$16(CompletionProposalPopup.java:560)
    at org.eclipse.jface.text.contentassist.CompletionProposalPopup$13.run(CompletionProposalPopup.java:1637)
    at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70)
    at org.eclipse.jface.text.contentassist.CompletionProposalPopup.incrementalComplete(CompletionProposalPopup.java:1630)
    at org.eclipse.jface.text.contentassist.ContentAssistant.showPossibleCompletions(ContentAssistant.java:1746)

IMHO, CompletionProposalPopup#computeProposals should be prepared for this case, i.e., that proposals is null:

	private ICompletionProposal[] computeProposals(int offset) {
		ICompletionProposal[] proposals;
		if (fContentAssistSubjectControl != null) {
			proposals= fContentAssistant.computeCompletionProposals(fContentAssistSubjectControl, offset);
		} else {
			proposals= fContentAssistant.computeCompletionProposals(fViewer, offset);
		}
		if (fSorter != null) {
			sortProposals(proposals);
			fIsInitialSort= true;
		}
		return proposals;
	}

Alternatively, ContentAssistant#computeCompletionProposals could return an empty array as Null Object, but this might be considered an incompatible API change.

What do you think?
Comment 1 Andreas Sewe CLA 2016-08-19 07:42:34 EDT 
FWIW, this problem does occur in the wild: <https://dev.eclipse.org/recommenders/committers/aeri/v2/#!/problems/56cc3d63e4b0d4f494e6a3c3>
Comment 2 Andreas Sewe CLA 2016-08-19 07:49:23 EDT 
(In reply to Andreas Sewe from comment #1)
> FWIW, this problem does occur in the wild:
> <https://dev.eclipse.org/recommenders/committers/aeri/v2/#!/problems/
> 56cc3d63e4b0d4f494e6a3c3>

There are also other problems recorded by the automated error reporting that are a probably duplicates [1] of the aforementioned problem [2], in case anyone wants more information about the possible calling contexts in which this NPE is thrown.

Hope this helps.

[1] <https://dev.eclipse.org/recommenders/committers/aeri/v2/#!/problems/?c:!((t:root-exception,val:java.lang.NullPointerException),(t:stacktrace-class,val:java.util.Arrays.sort,sc:ANY),(t:stacktrace-class,val:org.eclipse.jface.text.contentassist.CompletionProposalPopup.sortProposals,sc:ANY)),sort:!((prop:modifiedOn,dir:DESCENDING))>
[2] <https://dev.eclipse.org/recommenders/committers/aeri/v2/#!/problems/56cc3d63e4b0d4f494e6a3c3>
Comment 5 Andreas Sewe CLA 2016-08-19 09:20:13 EDT 
(In reply to Dani Megert from comment #3)
> Fixed with
> http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/
> ?id=23e01242dfe21800d196e8d805735cd4a23f2f96

(In reply to Dani Megert from comment #4)
> Fixed part 2 with
> http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/
> ?id=44d03aad38b002ebb13e47f0c67e74e2090ca598

Thanks for the quick response, Dani.
Comment 6 Dani Megert CLA 2016-08-19 09:46:25 EDT 
We should backport this fix. It's a regression due to the sorting code we added.

Noopur, please review my fix in master.
Comment 7 Noopur Gupta CLA 2016-08-19 10:01:32 EDT 
Looks good. +1 for 4.6.1.
Comment 9 Bruno Medeiros CLA 2016-08-23 14:01:46 EDT 
*** Bug 500154 has been marked as a duplicate of this bug. ***
Comment 10 Dani Megert CLA 2016-08-25 01:52:26 EDT 
Verified in M20160824-0059 through code inspection.