Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 363884 - NPE using 'Add break statement' quick fix
Summary: NPE using 'Add break statement' quick fix
Status: VERIFIED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-codan (show other bugs)
Version: 8.1.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 8.2.1   Edit
Assignee: Nathan Ridge CLA
QA Contact: Elena Laskavaia CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-15 20:04 EST by Marc-André Laperle CLA
Modified: 2014-01-29 22:58 EST (History)
5 users (show)

See Also:

Attachments
proposed fix (1.08 KB, patch)
2012-11-25 13:46 EST, Stefan Gh CLA 		cdtdoug: iplog+
Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Marc-André Laperle CLA 2011-11-15 20:04:20 EST 
Using CDT 8.1.0.201111131105

int main() {
    int a;
    switch(a)
    {
        case 0:
        {
        } // Use 'Add break statement' quick fix here
        default:
            break;
    }
    return 0;
}

java.lang.NullPointerException
	at org.eclipse.cdt.codan.internal.checkers.ui.quickfix.CaseBreakQuickFixBreak.modifyAST(CaseBreakQuickFixBreak.java:81)
	at org.eclipse.cdt.codan.ui.AbstractAstRewriteQuickFix.apply(AbstractAstRewriteQuickFix.java:52)
	at org.eclipse.cdt.codan.ui.AbstractCodanCMarkerResolution.run(AbstractCodanCMarkerResolution.java:93)
	at org.eclipse.cdt.internal.ui.text.correction.MarkerResolutionProposal.apply(MarkerResolutionProposal.java:43)
	at org.eclipse.jface.text.contentassist.CompletionProposalPopup.insertProposal(CompletionProposalPopup.java:935)
	at org.eclipse.jface.text.contentassist.CompletionProposalPopup.insertSelectedProposalWithMask(CompletionProposalPopup.java:881)
	at org.eclipse.jface.text.contentassist.CompletionProposalPopup.verifyKey(CompletionProposalPopup.java:1307)
	at org.eclipse.jface.text.contentassist.ContentAssistant$InternalListener.verifyKey(ContentAssistant.java:807)
	at org.eclipse.jface.text.TextViewer$VerifyKeyListenersManager.verifyKey(TextViewer.java:491)
	at org.eclipse.swt.custom.StyledTextListener.handleEvent(StyledTextListener.java:65)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
Comment 1 Stefan Gh CLA 2012-11-25 13:46:26 EST 
Created attachment 223936 [details]
proposed fix

It seems to be a small error in the ast parsing.

The proposed patch fixes the described bug, as well this closely related one:

int main() {
    int a;
    switch (a) {
        case 0:
        {
            int b;
        }  // quickfix here
        default:
            break;
    }
    return 0;
}
...
        case 0:
        {
            int b;
            // break statement gets added here
        }
        // instead of here
        default:
            break;
...
Comment 2 Nathan Ridge CLA 2013-07-18 01:14:58 EDT 
(In reply to comment #1)
> Created attachment 223936 [details]
> proposed fix
> 
> It seems to be a small error in the ast parsing.
> 
> The proposed patch fixes the described bug, as well this closely related one:
> 
> int main() {
>     int a;
>     switch (a) {
>         case 0:
>         {
>             int b;
>         }  // quickfix here
>         default:
>             break;
>     }
>     return 0;
> }
> ...
>         case 0:
>         {
>             int b;
>             // break statement gets added here
>         }
>         // instead of here
>         default:
>             break;
> ...

I think it's more conventional to place the break inside the compound-statement.
Comment 3 Nathan Ridge CLA 2013-07-18 01:38:02 EDT 
Patch: https://git.eclipse.org/r/14640
Comment 4 CDT Genie CLA 2013-07-18 14:22:04 EDT 
*** cdt git genie on behalf of Nathan Ridge ***

    Bug 363884 - NPE using 'Add break statement' quick fix
    Change-Id: I240b196d04b5f032bfaa9666db595b788b7a1d02
    Signed-off-by: Nathan Ridge <zeratul976@xxxxxxxxxxx>
    Reviewed-on: <a  href="https://git.eclipse.org/r/14640">https://git.eclipse.org/r/14640</a>
    Reviewed-by: Sergey Prigogin &lt;eclipse.sprigogin@xxxxxxxxx&gt;
    IP-Clean: Sergey Prigogin &lt;eclipse.sprigogin@xxxxxxxxx&gt;
    Tested-by: Sergey Prigogin &lt;eclipse.sprigogin@xxxxxxxxx&gt;

[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=33fcecc5b0666e24f6dcb965bed7f3166c726e03
Comment 5 Marc-André Laperle CLA 2013-07-19 09:07:51 EDT 
Fixed in 8.2.1 and master. Thanks!
Comment 6 Marc-André Laperle CLA 2013-08-31 01:05:04 EDT 
Verified in 20130821-2252 (RC1). However, I did find another problem when I kept the comment in the test code, see bug 416283.