From dce3efe73399ecb1c1c2bd810800aa7e9c5f453a Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Sun, 28 Aug 2011 19:35:40 -0700 Subject: [PATCH] Bug 356037 - "No break at the end of case" warning for a case statement produced by macro expansion. --- .../OSGI-INF/l10n/bundle.properties | 6 +- .../internal/checkers/CaseBreakChecker.java | 32 +++-- .../checkers/CaseBreakCheckerTest.java | 116 ++++++++---------- 3 files changed, 72 insertions(+), 82 deletions(-) diff --git a/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties b/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties index a59ba9ebebb..e25a9205617 100644 --- a/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties +++ b/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties @@ -52,7 +52,7 @@ problem.name.FormatString = Format String Vulnerability checker.name.AssignmentToItself = Assignment to itself problem.messagePattern.AssignmentToItself = Assignment to itself ''{0}'' problem.name.AssignmentToItself = Assignment to itself -problem.description.AssignmentToItself = Finds expression where left and right side of the assignment is the same, i.e. 'var = var' +problem.description.AssignmentToItself = Finds expression where left and right sides of the assignment are the same, i.e. 'var = var' checker.name.ReturnStyle = Return with parenthesis problem.name.ReturnStyle = Return with parenthesis problem.messagePattern.ReturnStyle = Return statement has invalid style. Return value should be surrounded by parenthesis @@ -60,10 +60,10 @@ problem.description.ReturnStyle = Checks for return statements that do no return checker.name.SuspiciousSemicolon = Suspicious semicolon problem.name.SuspiciousSemicolon = Suspicious semicolon problem.messagePattern.SuspiciousSemicolon = Suspicious semicolon -problem.description.SuspiciousSemicolon = A semicolon is used as a null statement in a condition. For example, 'if(expression);' +problem.description.SuspiciousSemicolon = A semicolon is used as a null statement in a condition. For example, 'if (expression);' checker.name.CaseBreak = No break at end of case problem.description.CaseBreak = Looks for "case" statements which end without a "break" statement -problem.messagePattern.CaseBreak = No break at the end of this case +problem.messagePattern.CaseBreak = No break at the end of case binding.checker.name = Problem Binding Checker problem.description.G = Name resolution problem found by the indexer problem.messagePattern.G = Symbol ''{0}'' could not be resolved diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CaseBreakChecker.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CaseBreakChecker.java index dfd564bbb40..20da627cbc6 100644 --- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CaseBreakChecker.java +++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CaseBreakChecker.java @@ -1,13 +1,14 @@ /******************************************************************************* - * Copyright (c) 2010,2011 Gil Barash + * Copyright (c) 2010, 2011 Gil Barash * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Gil Barash - Initial implementation - * Elena laskavaia - Rewrote checker to reduce false positives in complex cases + * Gil Barash - Initial implementation + * Elena laskavaia - Rewrote checker to reduce false positives in complex cases + * Sergey Prigogin (Google) *******************************************************************************/ package org.eclipse.cdt.codan.internal.checkers; @@ -50,8 +51,7 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements IChecke } /** - * This visitor looks for "switch" statements and invokes "SwitchVisitor" on - * them. + * This visitor looks for "switch" statements and invokes "SwitchVisitor" on them. */ class SwitchFindingVisitor extends ASTVisitor { SwitchFindingVisitor() { @@ -67,7 +67,7 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements IChecke * - "continue" * - "goto" (does not check that the goto actually exists the * switch) - * - "thorw" + * - "throw" * - "exit" */ protected boolean isBreakOrExitStatement(IASTStatement statement) { @@ -83,7 +83,7 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements IChecke IASTSwitchStatement switchStmt = (IASTSwitchStatement) statement; IASTStatement body = switchStmt.getBody(); if (body instanceof IASTCompoundStatement) { - // if not it is not really a switch + // If not it is not really a switch IASTStatement[] statements = ((IASTCompoundStatement) body).getStatements(); IASTStatement prevCase = null; for (int i = 0; i < statements.length; i++) { @@ -97,16 +97,16 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements IChecke if (isCaseStatement(curr)) { prevCase = curr; } - // next is case or end of switch - means this one is the last + // Next is case or end of switch - means this one is the last if (prevCase != null && (isCaseStatement(next) || next == null)) { - // check that current statement end with break or any other exit statement + // Check that current statement end with break or any other exit statement if (!_checkEmptyCase && isCaseStatement(curr) && next != null) { - continue; // empty case & we don't care + continue; // Empty case and we don't care } if (!_checkLastCase && next == null) { - continue; // last case and we don't care + continue; // Last case and we don't care } - if (isFallThroughStamement(curr)) { + if (!isProducedByMacroExpansion(prevCase) && isFallThroughStamement(curr)) { IASTComment comment = null; if (next != null) { comment = getLeadingComment(next); @@ -139,7 +139,7 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements IChecke } /** - * @param nextstatement + * @param body * @return */ public boolean isFallThroughStamement(IASTStatement body) { @@ -171,13 +171,11 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements IChecke IASTFileLocation astLocation = astNode.getFileLocation(); int line = astLocation.getEndingLineNumber(); IProblemLocationFactory locFactory = getRuntime().getProblemLocationFactory(); - return locFactory.createProblemLocation(getFile(), -1, - -1, line); + return locFactory.createProblemLocation(getFile(), -1, -1, line); } /** - * Checks if the given statement is a result of macro expansion with a - * possible + * Checks if the given statement is a result of macro expansion with a possible * exception for the trailing semicolon. * * @param statement the statement to check. diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/CaseBreakCheckerTest.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/CaseBreakCheckerTest.java index 083aff6960c..92799531245 100644 --- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/CaseBreakCheckerTest.java +++ b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/CaseBreakCheckerTest.java @@ -6,7 +6,8 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Gil Barash - Initial implementation + * Gil Barash - Initial implementation + * Sergey Prigogin (Google) *******************************************************************************/ package org.eclipse.cdt.codan.core.internal.checkers; @@ -383,11 +384,11 @@ public class CaseBreakCheckerTest extends CheckerTestCase { } // void foo(void) { - // int a, b; - // switch( a ) { - // case 1: - // b = 2; - // } + // int a, b; + // switch( a ) { + // case 1: + // b = 2; + // } // } public void testLastCaseIgnore() { setLast(false); @@ -474,12 +475,12 @@ public class CaseBreakCheckerTest extends CheckerTestCase { } // void foo(void) { - // int a; - // switch( a ) { - // case 2: + // int a; + // switch( a ) { + // case 2: // break; - // case 1: - // } + // case 1: + // } // } public void testEmptyLastCaseError() { String code = getAboveComment(); @@ -493,31 +494,32 @@ public class CaseBreakCheckerTest extends CheckerTestCase { } // void foo(int a) { - // switch( a ) { - // case 2: + // switch( a ) { + // case 2: // if (a*2<10) - // return; + // return; // else - // break; - // case 1: - // break; - // } + // break; + // case 1: + // break; + // } // } public void testIf() { String code = getAboveComment(); loadCodeAndRun(code); checkNoErrors(); } + // void foo(int a) { - // switch( a ) { - // case 2: + // switch(a) { + // case 2: // if (a*2<10) - // return; + // return; // else - // a++; - // case 1: - // break; - // } + // a++; + // case 1: + // break; + // } // } public void testIfErr() { String code = getAboveComment(); @@ -525,53 +527,43 @@ public class CaseBreakCheckerTest extends CheckerTestCase { checkErrorLine(7); } -// #define DEFINE_BREAK {break;} -// void foo ( int a ) -// { -// switch ( a ) -// { -// case 1: -// DEFINE_BREAK // <-- Warning: No break at the end of this case -// } -// } + // #define DEFINE_BREAK {break;} + // void foo(int a) { + // switch (a) { + // case 1: + // DEFINE_BREAK // No warning here + // } + // } public void testBreakInBraces() { String code = getAboveComment(); loadCodeAndRun(code); checkNoErrors(); } - -// #define MY_MACRO(i) \ -// case i: \ -// { \ -// break; \ -// } -// -// void f() -// { -// int x; -// switch (x) -// { -// MY_MACRO(1) // WARNING HERE -// } -// } - + // #define MY_MACRO(i) \ + // case i: { \ + // } + // + // void f() { + // int x; + // switch (x) { + // MY_MACRO(1) // No warning here + // } + // } public void testInMacro() { String code = getAboveComment(); loadCodeAndRun(code); checkNoErrors(); } - //void foo() - //{ - //switch(0) - //default: - //{ - //} - //} - public void testEmptyCompoundStatement() { - String code = getAboveComment(); - loadCodeAndRun(code); - checkErrorLine(6); - } + // void foo() { + // switch (0) + // default: { + // } + // } + public void testEmptyCompoundStatement() { + String code = getAboveComment(); + loadCodeAndRun(code); + checkErrorLine(4); + } }