From da074afa86e08b9ea953e14def164ab612c043e2 Mon Sep 17 00:00:00 2001 From: Alena Laskavaia Date: Fri, 27 Aug 2010 00:47:28 +0000 Subject: [PATCH] Bug 323602: fixed f.p. in constructors/destructors for return checker --- .../internal/checkers/ReturnChecker.java | 27 ++++++++++++--- .../internal/checkers/ReturnCheckerTest.java | 33 +++++++++++++++++++ .../cdt/codan/core/test/CheckerTestCase.java | 15 +++++++-- 3 files changed, 68 insertions(+), 7 deletions(-) diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java index 77e963f73b4..3ebc6131945 100644 --- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java +++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java @@ -34,6 +34,9 @@ import org.eclipse.cdt.core.dom.ast.IBasicType; import org.eclipse.cdt.core.dom.ast.IBinding; import org.eclipse.cdt.core.dom.ast.IType; import org.eclipse.cdt.core.dom.ast.c.ICASTSimpleDeclSpecifier; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDefinition; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPConstructor; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPMethod; /** * The checker suppose to find issue related to mismatched return value/function @@ -44,10 +47,10 @@ import org.eclipse.cdt.core.dom.ast.c.ICASTSimpleDeclSpecifier; * flow graph) */ public class ReturnChecker extends AbstractAstFunctionChecker { - private static final String PARAM_IMPLICIT = "implicit"; //$NON-NLS-1$ - public final String RET_NO_VALUE_ID = "org.eclipse.cdt.codan.checkers.noreturn"; //$NON-NLS-1$ - public final String RET_ERR_VALUE_ID = "org.eclipse.cdt.codan.checkers.errreturnvalue"; //$NON-NLS-1$ - public final String RET_NORET_ID = "org.eclipse.cdt.codan.checkers.errnoreturn"; //$NON-NLS-1$ + public static final String PARAM_IMPLICIT = "implicit"; //$NON-NLS-1$ + public static final String RET_NO_VALUE_ID = "org.eclipse.cdt.codan.checkers.noreturn"; //$NON-NLS-1$ + public static final String RET_ERR_VALUE_ID = "org.eclipse.cdt.codan.checkers.errreturnvalue"; //$NON-NLS-1$ + public static final String RET_NORET_ID = "org.eclipse.cdt.codan.checkers.errnoreturn"; //$NON-NLS-1$ class ReturnStmpVisitor extends ASTVisitor { private IASTFunctionDefinition func; @@ -68,9 +71,10 @@ public class ReturnChecker extends AbstractAstFunctionChecker { if (stmt instanceof IASTReturnStatement) { hasret = true; IASTReturnStatement ret = (IASTReturnStatement) stmt; - if (!isVoid(func)) { + if (!isVoid(func) && !isConstructorDestructor()) { if (checkImplicitReturn(RET_NO_VALUE_ID) || isExplicitReturn(func)) { + if (ret.getReturnValue() == null) reportProblem(RET_NO_VALUE_ID, ret); } @@ -87,6 +91,19 @@ public class ReturnChecker extends AbstractAstFunctionChecker { } return PROCESS_CONTINUE; } + /** + * @return + * + */ + public boolean isConstructorDestructor() { + if (func instanceof ICPPASTFunctionDefinition) { + IBinding method = func.getDeclarator().getName().resolveBinding(); + if (method instanceof ICPPConstructor || method instanceof ICPPMethod && ((ICPPMethod)method).isDestructor()) { + return true; + } + } + return false; + } } diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java index ce43f168cda..bdb3ccd76bc 100644 --- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java +++ b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java @@ -11,7 +11,9 @@ *******************************************************************************/ package org.eclipse.cdt.codan.core.internal.checkers; +import org.eclipse.cdt.codan.core.param.IProblemPreference; import org.eclipse.cdt.codan.core.test.CheckerTestCase; +import org.eclipse.cdt.codan.internal.checkers.ReturnChecker; /** * Test for {@see ReturnCheckerTest} class @@ -105,4 +107,35 @@ public class ReturnCheckerTest extends CheckerTestCase { loadCodeAndRunCpp(getAboveComment()); checkErrorLine(5); } + + // class c { + // c() { + // return 0; + // } + // + // ~c() { + // return; + // } + // }; + public void testContructorRetValue() { + loadCodeAndRunCpp(getAboveComment()); + checkErrorLine(3, ReturnChecker.RET_ERR_VALUE_ID); + } + + // class c { + // c() { + // return; + // } + // + // ~c() { + // return; + // } + // }; + public void testContructor_Bug323602() { + IProblemPreference macro = getPreference(ReturnChecker.RET_NO_VALUE_ID, + ReturnChecker.PARAM_IMPLICIT); + macro.setValue(Boolean.TRUE); + loadCodeAndRunCpp(getAboveComment()); + checkNoErrors(); + } } \ No newline at end of file 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 d9ecb70eab1..5c4bca22a13 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 @@ -20,6 +20,7 @@ import org.eclipse.cdt.codan.core.model.IProblemReporter; import org.eclipse.cdt.codan.core.param.IProblemPreference; import org.eclipse.cdt.codan.core.param.MapProblemPreference; import org.eclipse.cdt.codan.internal.core.model.CodanProblem; +import org.eclipse.cdt.codan.internal.core.model.CodanProblemMarker; import org.eclipse.core.resources.IMarker; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.NullProgressMonitor; @@ -35,12 +36,20 @@ public class CheckerTestCase extends CodanTestCase { return checkErrorLine(currentFile, i); } + public IMarker checkErrorLine(int i, String problemId) { + return checkErrorLine(currentFile, i, problemId); + } + + public IMarker checkErrorLine(File file, int expectedLine) { + return checkErrorLine(file, expectedLine, null); + } + /** * @param expectedLine * - line * @return */ - public IMarker checkErrorLine(File file, int expectedLine) { + public IMarker checkErrorLine(File file, int expectedLine, String problemId) { assertTrue(markers != null); assertTrue("No problems found but should", markers.length > 0); //$NON-NLS-1$ boolean found = false; @@ -62,7 +71,9 @@ public class CheckerTestCase extends CodanTestCase { fail(e.getMessage()); } mfile = m.getResource().getName(); - if (line.equals(expectedLine)) { + if (line.equals(expectedLine) + && (problemId == null || problemId + .equals(CodanProblemMarker.getProblemId(m)))) { found = true; if (file != null && !file.getName().equals(mfile)) found = false;