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

Bug 329497

Summary: [checker] Suggest adding a checker for "no break at end of case"
Product: [Tools] CDT Reporter: Gil Barash <scobido1984>
Component: cdt-codanAssignee: Elena Laskavaia <elaskavaia.cdt>
Status: RESOLVED FIXED QA Contact: Elena Laskavaia <elaskavaia.cdt>
Severity: enhancement    
Priority: P3 CC: cdtdoug, eclipse.sprigogin, malaperle, scobido1984
Version: 8.0   
Target Milestone: 8.0   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Patch for "case break checker"
elaskavaia.cdt: iplog+
Fixing the false negative... elaskavaia.cdt: iplog+

Description Gil Barash CLA 2010-11-04 17:23:21 EDT 
Build Identifier: Eclipse I20100805-1700. Codan 8

In this example there should be two warnings:

switch {
case 1:
 // <- here
case 2:
 /* no break */ // <- This is OK
case 3:
 // <- here
}

Attached is a patch of my implementation for this checker


Reproducible: Always
Comment 1 Gil Barash CLA 2010-11-04 17:24:25 EDT 
Created attachment 182435 [details]
Patch for "case break checker"
Comment 2 Andrew Gvozdev CLA 2010-11-04 18:00:42 EDT 
(In reply to comment #0)
> case 2:
> /* no break */ // <- This is OK
> case 3:
In Java, there is @SuppressWarnings("fallthrough") annotation, perhaps wording "fallthrough" is better for consistency? Some variants could be @fallthrough, {@codan fallthrough} or even the whole @SuppressWarnings("fallthrough"). Another idea to to specify the disabling tag as a parameter in the checker preferences.
Comment 3 Sergey Prigogin CLA 2010-11-04 18:10:40 EDT 
The checker should probably look for any comment containing word fallthrough in any case.
Comment 4 Gil Barash CLA 2010-11-05 06:43:20 EDT 
I'm way ahead of you :-)
In the patch I supplied, I allow configuring the comment which suppress the warning.
So by default it's "no break" but it can be easily modified, by the user, to any comment ("fallthrough", for example).
Comment 5 Elena Laskavaia CLA 2010-11-30 21:14:49 EST 
This checker would report false problem for this

case 1:
   return;


I tried to fix it but code is very complex so I cannot figure out how...
Comment 6 Elena Laskavaia CLA 2010-11-30 21:21:05 EST 
Same for goto, continue and throw
Comment 7 Elena Laskavaia CLA 2010-11-30 21:41:53 EST 
And this is false negative:
void foo(int a) {
   switch( a ) {
   case 1:
     while (a--)
       break;
   }
}

I committed the patch with the following changes:
I added check for return, etc instead of just check for break. 
Still has false negative with nested constructs as mentioned above.

thanks for the patch
Comment 8 CDT Genie CLA 2010-11-30 22:23:02 EST 
*** cdt cvs genie on behalf of elaskavaia ***
Bug 329497 - checker for no break at the end of case

[*] CxxAstUtils.java 1.10 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/CxxAstUtils.java?root=Tools_Project&r1=1.9&r2=1.10

[*] bundle.properties 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties?root=Tools_Project&r1=1.6&r2=1.7

[*] messages.properties 1.11 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/messages.properties?root=Tools_Project&r1=1.10&r2=1.11
[*] CheckersMessages.java 1.9 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.java?root=Tools_Project&r1=1.8&r2=1.9
[+] CaseBreakChecker.java  http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CaseBreakChecker.java?root=Tools_Project&revision=1.1&view=markup

[*] plugin.xml 1.31 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.checkers/plugin.xml?root=Tools_Project&r1=1.30&r2=1.31

[+] CaseBreakCheckerTest.java  http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/CaseBreakCheckerTest.java?root=Tools_Project&revision=1.1&view=markup

[*] AutomatedIntegrationSuite.java 1.14 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/AutomatedIntegrationSuite.java?root=Tools_Project&r1=1.13&r2=1.14
[*] CheckerTestCase.java 1.18 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/CheckerTestCase.java?root=Tools_Project&r1=1.17&r2=1.18

[*] CaseBreakCheckerTest.java 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/CaseBreakCheckerTest.java?root=Tools_Project&r1=1.1&r2=1.2
Comment 9 Gil Barash CLA 2010-12-04 09:01:46 EST 
Created attachment 184533 [details]
Fixing the false negative...

This patch modifies the code committed by Alena. 
It fixes the "false negative" issue reported (and a small typo).
Comment 10 Gil Barash CLA 2011-01-18 14:08:58 EST 
Attached a path to fix the reported issues
Comment 11 Elena Laskavaia CLA 2011-02-23 21:55:56 EST 
patch partially applied thanks, I think message was fixed earlier on my commit