From 6f2671b1eeccb5ede9328c1aa89db3e2bf232f8f Mon Sep 17 00:00:00 2001 From: Alena Laskavaia Date: Mon, 22 Aug 2011 22:46:24 -0400 Subject: [PATCH] Bug 352221 - Quick Fix for "case break" checker --- .../META-INF/MANIFEST.MF | 3 +- .../plugin.xml | 9 ++ .../ui/quickfix/CaseBreakQuickFixBreak.java | 112 ++++++++++++++++++ .../ui/quickfix/CaseBreakQuickFixComment.java | 55 +++++++++ .../checkers/ui/quickfix/Messages.java | 2 + .../checkers/ui/quickfix/messages.properties | 2 + .../internal/checkers/CaseBreakChecker.java | 65 ++++++---- .../checkers/CheckersMessages.properties | 2 +- .../checkers/CaseBreakCheckerTest.java | 70 +++++------ .../core/test/AutomatedIntegrationSuite.java | 2 + .../cdt/codan/core/test/CheckerTestCase.java | 11 +- .../cdt/codan/core/test/CodanTestCase.java | 28 +++-- .../ui/quickfix/CaseBreakQuickFixTest.java | 52 ++++++++ 13 files changed, 345 insertions(+), 68 deletions(-) create mode 100644 codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CaseBreakQuickFixBreak.java create mode 100644 codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CaseBreakQuickFixComment.java create mode 100644 codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CaseBreakQuickFixTest.java diff --git a/codan/org.eclipse.cdt.codan.checkers.ui/META-INF/MANIFEST.MF b/codan/org.eclipse.cdt.codan.checkers.ui/META-INF/MANIFEST.MF index d08c9b9ea83..01e93429945 100644 --- a/codan/org.eclipse.cdt.codan.checkers.ui/META-INF/MANIFEST.MF +++ b/codan/org.eclipse.cdt.codan.checkers.ui/META-INF/MANIFEST.MF @@ -12,7 +12,8 @@ Require-Bundle: org.eclipse.ui, org.eclipse.cdt.core, org.eclipse.cdt.codan.core.cxx, org.eclipse.cdt.codan.ui.cxx, - org.eclipse.cdt.codan.core + org.eclipse.cdt.codan.core, + org.eclipse.cdt.codan.checkers Bundle-ActivationPolicy: lazy Bundle-RequiredExecutionEnvironment: J2SE-1.5 Bundle-Vendor: %Bundle-Vendor diff --git a/codan/org.eclipse.cdt.codan.checkers.ui/plugin.xml b/codan/org.eclipse.cdt.codan.checkers.ui/plugin.xml index 839147f898d..0d9a0bfa5c2 100644 --- a/codan/org.eclipse.cdt.codan.checkers.ui/plugin.xml +++ b/codan/org.eclipse.cdt.codan.checkers.ui/plugin.xml @@ -37,6 +37,15 @@ class="org.eclipse.cdt.codan.internal.checkers.ui.quickfix.QuickFixCreateParameter" problemId="org.eclipse.cdt.codan.internal.checkers.VariableResolutionProblem"> + + + + + diff --git a/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CaseBreakQuickFixBreak.java b/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CaseBreakQuickFixBreak.java new file mode 100644 index 00000000000..f00a37497c0 --- /dev/null +++ b/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CaseBreakQuickFixBreak.java @@ -0,0 +1,112 @@ +/******************************************************************************* + * Copyright (c) 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 + *******************************************************************************/ +package org.eclipse.cdt.codan.internal.checkers.ui.quickfix; + +import org.eclipse.cdt.codan.core.cxx.CxxAstUtils; +import org.eclipse.cdt.codan.internal.checkers.ui.CheckersUiActivator; +import org.eclipse.cdt.codan.ui.AbstractAstRewriteQuickFix; +import org.eclipse.cdt.core.dom.ast.IASTBreakStatement; +import org.eclipse.cdt.core.dom.ast.IASTCompoundStatement; +import org.eclipse.cdt.core.dom.ast.IASTNode; +import org.eclipse.cdt.core.dom.ast.IASTNodeSelector; +import org.eclipse.cdt.core.dom.ast.IASTStatement; +import org.eclipse.cdt.core.dom.ast.IASTSwitchStatement; +import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; +import org.eclipse.cdt.core.dom.rewrite.ASTRewrite; +import org.eclipse.cdt.core.index.IIndex; +import org.eclipse.cdt.core.model.ITranslationUnit; +import org.eclipse.core.resources.IMarker; +import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.NullProgressMonitor; +import org.eclipse.jface.text.BadLocationException; +import org.eclipse.jface.text.IRegion; +import org.eclipse.ltk.core.refactoring.Change; + +public class CaseBreakQuickFixBreak extends AbstractAstRewriteQuickFix { + @Override + public boolean isApplicable(IMarker marker) { + int line = marker.getAttribute(IMarker.LINE_NUMBER, -1) - 1; + if (line < 0) + return false; + return true; + } + + public String getLabel() { + return Messages.CaseBreakQuickFixBreak_Label; + } + + protected IASTStatement getStmtBeforeBreak(IMarker marker, IASTTranslationUnit ast) throws BadLocationException { + int line = marker.getAttribute(IMarker.LINE_NUMBER, -1) - 1; + if (line < 0) + return null; + IRegion lineInformation = getDocument().getLineInformation(line); + IASTNodeSelector nodeSelector = ast.getNodeSelector(null); + IASTNode containedNode = nodeSelector.findFirstContainedNode(lineInformation.getOffset(), lineInformation.getLength()); + IASTNode beforeBreakNode = null; + if (containedNode != null) + beforeBreakNode = CxxAstUtils.getInstance().getEnclosingStatement(containedNode); + else + beforeBreakNode = nodeSelector.findEnclosingNode(lineInformation.getOffset(), lineInformation.getLength()); + if (beforeBreakNode instanceof IASTCompoundStatement) { + while (beforeBreakNode != null) { + if (beforeBreakNode.getParent() instanceof IASTCompoundStatement + && beforeBreakNode.getParent().getParent() instanceof IASTSwitchStatement) { + if (beforeBreakNode instanceof IASTCompoundStatement) { + IASTStatement[] statements = ((IASTCompoundStatement) beforeBreakNode).getStatements(); + return statements[statements.length - 1]; // return last one + } + return (IASTStatement) beforeBreakNode; + } + beforeBreakNode = beforeBreakNode.getParent(); + } + } + if (beforeBreakNode instanceof IASTStatement) + return (IASTStatement) beforeBreakNode; + return null; + } + + @Override + public void modifyAST(IIndex index, IMarker marker) { + try { + IASTTranslationUnit ast = getTranslationUnitViaEditor(marker).getAST(index, ITranslationUnit.AST_SKIP_INDEXED_HEADERS); + IASTStatement beforeBreak = getStmtBeforeBreak(marker, ast); + if (beforeBreak.getParent() instanceof IASTCompoundStatement) { + IASTCompoundStatement enclosingStatement = (IASTCompoundStatement) beforeBreak.getParent(); + IASTStatement after = getAfterStatement(beforeBreak); + ASTRewrite r = ASTRewrite.create(enclosingStatement.getTranslationUnit()); + IASTBreakStatement breakStatement = ast.getASTNodeFactory().newBreakStatement(); + r.insertBefore(enclosingStatement, after, breakStatement, null); + Change c = r.rewriteAST(); + c.perform(new NullProgressMonitor()); + } + } catch (CoreException e) { + CheckersUiActivator.log(e); + } catch (BadLocationException e) { + CheckersUiActivator.log(e); + } + } + + private IASTStatement getAfterStatement(IASTStatement beforeBreak) { + IASTCompoundStatement enclosingStatement = (IASTCompoundStatement) beforeBreak.getParent(); + IASTStatement after = null; + IASTStatement[] statements = enclosingStatement.getStatements(); + for (int i = 0; i < statements.length; i++) { + IASTStatement st = statements[i]; + if (st == beforeBreak) { + if (i < statements.length - 1) { + after = statements[i + 1]; + break; + } + } + } + return after; + } +} diff --git a/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CaseBreakQuickFixComment.java b/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CaseBreakQuickFixComment.java new file mode 100644 index 00000000000..27b95adaf87 --- /dev/null +++ b/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CaseBreakQuickFixComment.java @@ -0,0 +1,55 @@ +package org.eclipse.cdt.codan.internal.checkers.ui.quickfix; + +import org.eclipse.cdt.codan.core.model.IProblem; +import org.eclipse.cdt.codan.core.param.RootProblemPreference; +import org.eclipse.cdt.codan.internal.checkers.CaseBreakChecker; +import org.eclipse.cdt.codan.ui.AbstractCodanCMarkerResolution; +import org.eclipse.core.resources.IMarker; +import org.eclipse.jface.text.BadLocationException; +import org.eclipse.jface.text.IDocument; +import org.eclipse.jface.text.IRegion; +import org.eclipse.text.edits.InsertEdit; +import org.eclipse.text.edits.MalformedTreeException; + +public class CaseBreakQuickFixComment extends AbstractCodanCMarkerResolution { + public String getLabel() { + return Messages.CaseBreakQuickFixComment_Label; + } + + @Override + public void apply(IMarker marker, IDocument document) { + try { + int line = marker.getAttribute(IMarker.LINE_NUMBER, -1); + if (line < 0) + return; + int offset = document.getLineOffset(line); + String indent = getIndentationStr(document, line); + String comment = getNoBreakComment(marker); + String editStr = String.format("%s/* %s */\n", indent, comment);//$NON-NLS-1$ + InsertEdit edit = new InsertEdit(offset, editStr); + edit.apply(document); + } catch (MalformedTreeException e) { + return; + } catch (BadLocationException e) { + return; + } + } + + private String getIndentationStr(IDocument document, int line) throws BadLocationException { + int prevLine = line - 1; + IRegion lineInformation = document.getLineInformation(prevLine); + String prevLineStr = document.get(lineInformation.getOffset(), lineInformation.getLength()); + int nonSpace = prevLineStr.indexOf(prevLineStr.trim()); + String indent = prevLineStr.substring(0, nonSpace); + return indent; + } + + private String getNoBreakComment(IMarker marker) { + IProblem problem = getProblem(marker); + RootProblemPreference map = (RootProblemPreference) problem.getPreference(); + String comment = (String) map.getChildValue(CaseBreakChecker.PARAM_NO_BREAK_COMMENT); + if (comment == null || comment.trim().length() == 0) + comment = "no break"; //$NON-NLS-1$ + return comment; + } +} diff --git a/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/Messages.java b/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/Messages.java index dee3330d91b..0a7a6864aa0 100644 --- a/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/Messages.java +++ b/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/Messages.java @@ -4,6 +4,8 @@ import org.eclipse.osgi.util.NLS; public class Messages extends NLS { private static final String BUNDLE_NAME = "org.eclipse.cdt.codan.internal.checkers.ui.quickfix.messages"; //$NON-NLS-1$ + public static String CaseBreakQuickFixBreak_Label; + public static String CaseBreakQuickFixComment_Label; public static String QuickFixCreateField_0; public static String QuickFixCreateLocalVariable_0; public static String QuickFixCreateParameter_0; diff --git a/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/messages.properties b/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/messages.properties index da3f433fce7..0afc2cc4215 100644 --- a/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/messages.properties +++ b/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/messages.properties @@ -1,3 +1,5 @@ +CaseBreakQuickFixBreak_Label=Add break statement +CaseBreakQuickFixComment_Label=Add supressing comment QuickFixCreateField_0=Create field QuickFixCreateLocalVariable_0=Create local variable QuickFixCreateParameter_0=Create parameter \ No newline at end of file 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 a56b94379f5..dfd564bbb40 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,10 +1,10 @@ /******************************************************************************* - * 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 @@ -14,6 +14,8 @@ package org.eclipse.cdt.codan.internal.checkers; import org.eclipse.cdt.codan.core.cxx.CxxAstUtils; import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker; import org.eclipse.cdt.codan.core.model.ICheckerWithPreferences; +import org.eclipse.cdt.codan.core.model.IProblemLocation; +import org.eclipse.cdt.codan.core.model.IProblemLocationFactory; import org.eclipse.cdt.codan.core.model.IProblemWorkingCopy; import org.eclipse.cdt.core.dom.ast.ASTVisitor; import org.eclipse.cdt.core.dom.ast.IASTBreakStatement; @@ -23,9 +25,11 @@ import org.eclipse.cdt.core.dom.ast.IASTCompoundStatement; import org.eclipse.cdt.core.dom.ast.IASTContinueStatement; import org.eclipse.cdt.core.dom.ast.IASTDefaultStatement; import org.eclipse.cdt.core.dom.ast.IASTExpressionStatement; +import org.eclipse.cdt.core.dom.ast.IASTFileLocation; import org.eclipse.cdt.core.dom.ast.IASTGotoStatement; import org.eclipse.cdt.core.dom.ast.IASTIfStatement; import org.eclipse.cdt.core.dom.ast.IASTMacroExpansionLocation; +import org.eclipse.cdt.core.dom.ast.IASTNode; import org.eclipse.cdt.core.dom.ast.IASTNodeLocation; import org.eclipse.cdt.core.dom.ast.IASTReturnStatement; import org.eclipse.cdt.core.dom.ast.IASTStatement; @@ -40,7 +44,10 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements IChecke public static final String DEFAULT_NO_BREAK_COMMENT = "no break"; //$NON-NLS-1$ private Boolean _checkLastCase; // Should we check the last case in the switch? private Boolean _checkEmptyCase; // Should we check an empty case (a case without any statements within it) - private String _noBreakComment; // The comment suppressing this warning + private String _noBreakComment; // The comment suppressing this warning + + public CaseBreakChecker() { + } /** * This visitor looks for "switch" statements and invokes "SwitchVisitor" on @@ -72,7 +79,7 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements IChecke @Override public int visit(IASTStatement statement) { - if (statement instanceof IASTSwitchStatement && !isProducedMyMacroExpansion(statement)) { + if (statement instanceof IASTSwitchStatement && !isProducedByMacroExpansion(statement)) { IASTSwitchStatement switchStmt = (IASTSwitchStatement) statement; IASTStatement body = switchStmt.getBody(); if (body instanceof IASTCompoundStatement) { @@ -110,10 +117,10 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements IChecke } if (comment != null) { String str = getTrimmedComment(comment); - if (str.equalsIgnoreCase(_noBreakComment)) + if (str.toLowerCase().contains(_noBreakComment.toLowerCase())) continue; } - reportProblem(ER_ID, prevCase, (Object) null); + reportProblem(curr, prevCase); } } } @@ -136,13 +143,13 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements IChecke * @return */ public boolean isFallThroughStamement(IASTStatement body) { - if (body == null) return true; + if (body == null) + return true; if (body instanceof IASTCompoundStatement) { IASTStatement[] statements = ((IASTCompoundStatement) body).getStatements(); if (statements.length > 0) { - return isFallThroughStamement(statements[statements.length - 1]); + return isFallThroughStamement(statements[statements.length - 1]); } - return true; } else if (isBreakOrExitStatement(body)) { return false; @@ -150,26 +157,36 @@ public class CaseBreakChecker extends AbstractIndexAstChecker implements IChecke return true; } else if (body instanceof IASTIfStatement) { IASTIfStatement ifs = (IASTIfStatement) body; - return isFallThroughStamement(ifs.getThenClause()) || isFallThroughStamement(ifs.getElseClause()) ; + return isFallThroughStamement(ifs.getThenClause()) || isFallThroughStamement(ifs.getElseClause()); } return true; // TODO } - - /** - * 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. - * @return true if the statement is a result of macro expansion - */ - private boolean isProducedMyMacroExpansion(IASTStatement statement) { - IASTNodeLocation[] locations = statement.getNodeLocations(); - return locations.length > 0 && locations[0] instanceof IASTMacroExpansionLocation && - (locations.length == 1 || locations.length == 2 && locations[1].getNodeLength() == 1); - } } - public CaseBreakChecker() { + public void reportProblem(IASTStatement curr, IASTStatement prevCase) { + reportProblem(ER_ID, getProblemLocationAtEndOfNode(curr)); + } + + private IProblemLocation getProblemLocationAtEndOfNode(IASTNode astNode) { + IASTFileLocation astLocation = astNode.getFileLocation(); + int line = astLocation.getEndingLineNumber(); + IProblemLocationFactory locFactory = getRuntime().getProblemLocationFactory(); + return locFactory.createProblemLocation(getFile(), -1, + -1, line); + } + + /** + * 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. + * @return true if the statement is a result of macro expansion + */ + private boolean isProducedByMacroExpansion(IASTStatement statement) { + IASTNodeLocation[] locations = statement.getNodeLocations(); + return locations.length > 0 && locations[0] instanceof IASTMacroExpansionLocation + && (locations.length == 1 || locations.length == 2 && locations[1].getNodeLength() == 1); } /** diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.properties b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.properties index b219b977cff..e7f79019b3f 100644 --- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.properties +++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/CheckersMessages.properties @@ -8,7 +8,7 @@ # Contributors: # Alena Laskavaia - initial API and implementation ############################################################################### -CaseBreakChecker_DefaultNoBreakCommentDescription=Comment text to suppress the problem (regular expression): +CaseBreakChecker_DefaultNoBreakCommentDescription=Comment text to suppress the problem: CaseBreakChecker_EmptyCaseDescription=Check also empty case statement (except if last) CaseBreakChecker_LastCaseDescription=Check also the last case statement ClassMembersInitializationChecker_SkipConstructorsWithFCalls=Skip constructors with initialization function calls 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 3872ba617e9..083aff6960c 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 @@ -62,7 +62,7 @@ public class CaseBreakCheckerTest extends CheckerTestCase { // } public void testLastCaseBad() { loadCodeAndRun(getAboveComment()); - checkErrorLines(4); + checkErrorLines(5); } // void foo(void) { @@ -181,7 +181,7 @@ public class CaseBreakCheckerTest extends CheckerTestCase { // } public void testEmptyLastCaseTwoSwitches() { loadCodeAndRun(getAboveComment()); - checkErrorLines(3); + checkErrorLines(7); } // void foo(void) { @@ -222,7 +222,7 @@ public class CaseBreakCheckerTest extends CheckerTestCase { // } public void testLastCaseBadCommentNotLast() { loadCodeAndRun(getAboveComment()); - checkErrorLines(4); + checkErrorLines(7); } // void foo(void) { @@ -246,70 +246,74 @@ public class CaseBreakCheckerTest extends CheckerTestCase { // case 6: // b = 2; // /* no break1 */ + // case 7: + // b = 2; + // /* fallthrough */ // } // } public void testDifferentComments() { loadCodeAndRun(getAboveComment()); - checkErrorLines(16, 19); + checkErrorLines(17,23); } // void foo(void) { // int a, b; // switch( a ) { - // case 1: + // case 1: //err // // lolo - // case 2: - // case 3: + // case 2: //err + // case 3://err // } // // switch( a ) { // case 1: - // b = 2; + // b = 2; // err // // lolo // case 2: - // b = 2; - // case 3: + // b = 2; // err + // case 3: // err // case 4: // break; - // case 5: - // case 6: + // case 5: // err + // case 6: // err // } // // switch( a ) { // case 1: - // b = 2; + // b = 2; // err // // lolo // case 2: - // b = 2; + // b = 2; //err // case 3: // b = 2; // /* no break */ // case 4: - // b = 2; + // b = 2; // err // case 5: // b = 2; // break; // case 6: // b = 2; // /* no break */ - // b = 2; + // b = 2; //err // case 7: - // b = 2; + // b = 2;//err // } // // switch( a ) { // case 1: - // b = 2; + // b = 2; // err // // lolo // case 2: - // b = 2; - // default: + // b = 2; // err + // default: //err // } // } public void testGeneral1() { setEmpty(true); + setLast(true); loadCodeAndRun(getAboveComment()); - checkErrorLines(4, 6, 7, 11, 14, 16, 19, 20, 24, 27, 32, 37, 41, 46, 49, 51); + checkErrorComments(); } // void foo(void) { @@ -339,7 +343,7 @@ public class CaseBreakCheckerTest extends CheckerTestCase { // } public void testGeneralComments1() { loadCodeAndRun(getAboveComment()); - checkErrorLines(8, 12); + checkErrorLines(9, 14); } // void foo(void) { @@ -347,14 +351,14 @@ public class CaseBreakCheckerTest extends CheckerTestCase { // switch( a ) { // case 0: // switch( b ) { - // case 2: - // } + // case 2: // err + // } // err // // case 1: // switch( b ) { // case 2: // break; - // } + // } // err // case 3: // switch( b ) { // case 2: @@ -365,17 +369,17 @@ public class CaseBreakCheckerTest extends CheckerTestCase { // switch( b ) { // case 2: // /* no break */ - // } + // } // err // case 5: // switch( b ) { - // case 2: + // case 2: // err // } // /* no break */ // } // } public void testNestedSwitches() { loadCodeAndRun(getAboveComment()); - checkErrorLines(4, 20, 6, 9, 27); + checkErrorComments(); } // void foo(void) { @@ -441,16 +445,16 @@ public class CaseBreakCheckerTest extends CheckerTestCase { // switch( a ) { // case 1: // while (a--) - // break; + // break; // err // case 2: // while (a--) { // break; - // } + // } // err // } // } public void testEmptyCaseWithLoopBreak() { loadCodeAndRun(getAboveComment()); - checkErrorLines(3, 6); + checkErrorComments(); } // void foo(int a) { @@ -518,7 +522,7 @@ public class CaseBreakCheckerTest extends CheckerTestCase { public void testIfErr() { String code = getAboveComment(); loadCodeAndRun(code); - checkErrorLine(3); + checkErrorLine(7); } // #define DEFINE_BREAK {break;} @@ -568,6 +572,6 @@ public class CaseBreakCheckerTest extends CheckerTestCase { public void testEmptyCompoundStatement() { String code = getAboveComment(); loadCodeAndRun(code); - checkErrorLine(4); + checkErrorLine(6); } } diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/AutomatedIntegrationSuite.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/AutomatedIntegrationSuite.java index b832bcd40fe..7f700eaea05 100644 --- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/AutomatedIntegrationSuite.java +++ b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/AutomatedIntegrationSuite.java @@ -30,6 +30,7 @@ import org.eclipse.cdt.codan.core.internal.checkers.StatementHasNoEffectCheckerT import org.eclipse.cdt.codan.core.internal.checkers.SuggestedParenthesisCheckerTest; import org.eclipse.cdt.codan.core.internal.checkers.SuspiciousSemicolonCheckerTest; import org.eclipse.cdt.codan.core.internal.checkers.UnusedSymbolInFileScopeCheckerTest; +import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.CaseBreakQuickFixTest; import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.CatchByReferenceQuickFixTest; import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.CreateLocalVariableQuickFixTest; import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.SuggestedParenthesisQuickFixTest; @@ -74,6 +75,7 @@ public class AutomatedIntegrationSuite extends TestSuite { suite.addTestSuite(CreateLocalVariableQuickFixTest.class); suite.addTestSuite(SuggestedParenthesisQuickFixTest.class); suite.addTestSuite(CatchByReferenceQuickFixTest.class); + suite.addTestSuite(CaseBreakQuickFixTest.class); return suite; } } diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/CheckerTestCase.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/CheckerTestCase.java index e135de07b71..391cc8c6591 100644 --- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/CheckerTestCase.java +++ b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/CheckerTestCase.java @@ -33,7 +33,7 @@ import org.eclipse.core.runtime.NullProgressMonitor; * method to get source directory for the tests, * default is "src". To make it read comment from java class, you need to * include this source directory (with test java files) into the build bundle. - * + * */ @SuppressWarnings("nls") public class CheckerTestCase extends CodanTestCase { @@ -50,6 +50,13 @@ public class CheckerTestCase extends CodanTestCase { assertEquals(args.length, markers.length); } + public void checkErrorComments() { + for (Object i : errLines) { + checkErrorLine((Integer) i); + } + assertEquals("Expected number of errors "+errLines.size(),errLines.size(), markers.length); + } + public IMarker checkErrorLine(int i, String problemId) { return checkErrorLine(currentFile, i, problemId); } @@ -82,7 +89,7 @@ public class CheckerTestCase extends CodanTestCase { break; } } - assertEquals(Integer.valueOf(expectedLine), line); + assertEquals("Error on line "+expectedLine+" is not found",Integer.valueOf(expectedLine), line); if (file != null) assertEquals(file.getName(), mfile); assertTrue(found); diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/CodanTestCase.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/CodanTestCase.java index 76b3397fd6e..5ce5343e19d 100644 --- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/CodanTestCase.java +++ b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/CodanTestCase.java @@ -10,6 +10,12 @@ *******************************************************************************/ package org.eclipse.cdt.codan.core.test; +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.util.ArrayList; + import org.eclipse.cdt.core.CCorePlugin; import org.eclipse.cdt.core.dom.IPDOMManager; import org.eclipse.cdt.core.model.CModelException; @@ -30,12 +36,6 @@ import org.eclipse.core.runtime.NullProgressMonitor; import org.eclipse.core.runtime.Path; import org.eclipse.core.runtime.Plugin; -import java.io.ByteArrayInputStream; -import java.io.File; -import java.io.FileInputStream; -import java.io.IOException; -import java.util.ArrayList; - /** * TODO: add description */ @@ -47,6 +47,7 @@ public class CodanTestCase extends BaseTestCase { protected File currentFile; protected ICElement currentCElem; protected IFile currentIFile; + protected ArrayList errLines= new ArrayList(); /** * @@ -220,7 +221,9 @@ public class CodanTestCase extends BaseTestCase { private File loadcode(String code, File testFile) { try { tempFiles.add(testFile); - TestUtils.saveFile(new ByteArrayInputStream(code.trim().getBytes()), testFile); + String trim = code.trim(); + loadErrorComments(trim); + TestUtils.saveFile(new ByteArrayInputStream(trim.getBytes()), testFile); currentFile = testFile; try { cproject.getProject().refreshLocal(1, null); @@ -240,6 +243,17 @@ public class CodanTestCase extends BaseTestCase { } } + private void loadErrorComments(String trim) { + String[] lines = trim.split("\n"); + for (int i = 0; i < lines.length; i++) { + String string = lines[i]; + if (string.matches(".*//\\s*err\\s*")) { + + errLines.add(i+1); + } + } + } + public File loadcode_c(String code) { return loadcode(code, true); } diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CaseBreakQuickFixTest.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CaseBreakQuickFixTest.java new file mode 100644 index 00000000000..3c057af950e --- /dev/null +++ b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CaseBreakQuickFixTest.java @@ -0,0 +1,52 @@ +package org.eclipse.cdt.codan.internal.checkers.ui.quickfix; + +import org.eclipse.cdt.codan.ui.AbstractCodanCMarkerResolution; + +@SuppressWarnings("nls") +public class CaseBreakQuickFixTest extends QuickFixTestCase { + @SuppressWarnings("restriction") + @Override + protected AbstractCodanCMarkerResolution createQuickFix() { + return new CaseBreakQuickFixBreak(); + } + + // void func() { + // int a; + // switch(a) { + // case 1: + // hello(); + // case 2: + // } + // } + public void testMiddleCase() { + loadcode(getAboveComment()); + String result = runQuickFixOneFile(); + assertContainedIn("break; case 2:", result); + } + + // void func() { + // int a; + // switch(a) { + // case 1: + // hello(); + // } + // } + public void testLastCase() { + loadcode(getAboveComment()); + String result = runQuickFixOneFile(); + assertContainedIn("break; }", result); + } + // void func() { + // int a; + // switch(a) { + // case 1: { + // hello(); + // } + // } + // } + public void testLastCaseComp() { + loadcode(getAboveComment()); + String result = runQuickFixOneFile(); + assertContainedIn("hello(); break;", result); + } +}