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 8bc6efd6284..f94ed6f9e75 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 @@ -106,4 +106,8 @@ problem.messagePattern.11 = Method ''{0}'' could not be resolved problem.name.11 = Method cannot be resolved problem.description.12 = Name resolution problem found by the indexer problem.messagePattern.12 = Field ''{0}'' could not be resolved -problem.name.12 = Field cannot be resolved \ No newline at end of file +problem.name.12 = Field cannot be resolved +checker.name.AbstractClassCreation = Abstract class cannot be instantiated +problem.name.AbstractClassCreation = Abstract class cannot be instantiated +problem.messagePattern.AbstractClassCreation = The type ''{0}'' must implement the inherited pure virtual method ''{1}'' +problem.description.AbstractClassCreation = All inherited pure virtual methods must be implemented to allow instantiation of the class diff --git a/codan/org.eclipse.cdt.codan.checkers/plugin.xml b/codan/org.eclipse.cdt.codan.checkers/plugin.xml index d1cffe7a94c..0d09d0c6563 100644 --- a/codan/org.eclipse.cdt.codan.checkers/plugin.xml +++ b/codan/org.eclipse.cdt.codan.checkers/plugin.xml @@ -335,5 +335,19 @@ name="%problem.name.FormatString"> + + + + diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/AbstractClassInstantiationChecker.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/AbstractClassInstantiationChecker.java new file mode 100644 index 00000000000..b0b82c084b5 --- /dev/null +++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/AbstractClassInstantiationChecker.java @@ -0,0 +1,192 @@ +/******************************************************************************* + * Copyright (c) 2011 Anton Gorenkov + * 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: + * Anton Gorenkov - initial implementation + *******************************************************************************/ +package org.eclipse.cdt.codan.internal.checkers; + +import org.eclipse.cdt.codan.checkers.CodanCheckersActivator; +import org.eclipse.cdt.codan.core.cxx.CxxAstUtils; +import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker; +import org.eclipse.cdt.core.dom.ast.ASTVisitor; +import org.eclipse.cdt.core.dom.ast.DOMException; +import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier; +import org.eclipse.cdt.core.dom.ast.IASTDeclaration; +import org.eclipse.cdt.core.dom.ast.IASTDeclarator; +import org.eclipse.cdt.core.dom.ast.IASTExpression; +import org.eclipse.cdt.core.dom.ast.IASTIdExpression; +import org.eclipse.cdt.core.dom.ast.IASTName; +import org.eclipse.cdt.core.dom.ast.IASTNode; +import org.eclipse.cdt.core.dom.ast.IASTParameterDeclaration; +import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; +import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; +import org.eclipse.cdt.core.dom.ast.IBinding; +import org.eclipse.cdt.core.dom.ast.IType; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionCallExpression; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNamedTypeSpecifier; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNewExpression; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPBinding; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPClassType; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPConstructor; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPMethod; +import org.eclipse.cdt.internal.core.dom.parser.cpp.ClassTypeHelper; + +/** + * Reports a problem if object of a class cannot be created because + * class is abstract (it self or its bases have one or more pure virtual + * functions). + * + * @author Anton Gorenkov + * + */ +public class AbstractClassInstantiationChecker extends AbstractIndexAstChecker { + public static final String ER_ID = "org.eclipse.cdt.codan.internal.checkers.AbstractClassCreation"; //$NON-NLS-1$ + + public void processAst(IASTTranslationUnit ast) { + ast.accept(new OnEachClass()); + } + + class OnEachClass extends ASTVisitor { + + OnEachClass() { + shouldVisitDeclarations = true; + shouldVisitExpressions = true; + shouldVisitParameterDeclarations = true; + } + + public int visit(IASTDeclaration declaration) { + // Looking for the variables declarations + if (declaration instanceof IASTSimpleDeclaration) { + // If there is at least one non-pointer and non-reference type... + IASTSimpleDeclaration simpleDecl = (IASTSimpleDeclaration)declaration; + IASTDeclSpecifier declSpec = simpleDecl.getDeclSpecifier(); + if (declSpec.getStorageClass() != IASTDeclSpecifier.sc_typedef) { + for (IASTDeclarator declarator : simpleDecl.getDeclarators()) { + if (!hasPointerOrReference(declarator)) { + // ... check whether type is an abstract class + checkClass(declSpec); + break; + } + } + } + } + return PROCESS_CONTINUE; + } + + public int visit(IASTParameterDeclaration parameterDecl) { + // Looking for parameters declaration. Skip references & pointers. + if (!hasPointerOrReference(parameterDecl.getDeclarator())) { + checkClass(parameterDecl.getDeclSpecifier()); + } + return PROCESS_CONTINUE; + } + + /** + * Checks whether declarator contains a pinter or reference + */ + private boolean hasPointerOrReference(IASTDeclarator declarator) { + return declarator.getPointerOperators().length != 0; + } + + private void checkClass(IASTDeclSpecifier declSpec) { + if (declSpec instanceof ICPPASTNamedTypeSpecifier) { + IASTName className = ((ICPPASTNamedTypeSpecifier)declSpec).getName(); + IBinding binding = getOrResolveBinding(className); + if (binding instanceof IType) { + // Resolve class and check whether it is abstract + reportProblemsIfAbstract((IType)binding, className); + } + } + } + + public int visit(IASTExpression expression) { + // Looking for the new expression + if (expression instanceof ICPPASTNewExpression) { + ICPPASTNewExpression newExpression = (ICPPASTNewExpression)expression; + if (!hasPointerOrReference(newExpression.getTypeId().getAbstractDeclarator())) { + // Try to resolve its implicit constructor + IASTDeclSpecifier declSpecifier = newExpression.getTypeId().getDeclSpecifier(); + if (declSpecifier instanceof ICPPASTNamedTypeSpecifier) { + IASTName constructorName = ((ICPPASTNamedTypeSpecifier)declSpecifier).getName(); + checkClassConstructor(constructorName); + } + } + } + // Looking for direct class constructor call and check it + else if (expression instanceof ICPPASTFunctionCallExpression) { + ICPPASTFunctionCallExpression functionCall = (ICPPASTFunctionCallExpression)expression; + IASTExpression functionName = functionCall.getFunctionNameExpression(); + if (functionName instanceof IASTIdExpression) { + IASTName constructorName = ((IASTIdExpression)functionName).getName(); + checkClassConstructor(constructorName); + } + } + return PROCESS_CONTINUE; + } + + /** + * Resolves constructor by AST Name, then get its owner class + * and check whether it is abstract. If it is - report problems + */ + private void checkClassConstructor(IASTName constructorName) { + IBinding binding = getOrResolveBinding(constructorName); + if (binding instanceof ICPPConstructor) { + // Resolve class and check whether it is abstract + reportProblemsIfAbstract(((ICPPConstructor)binding).getClassOwner(), constructorName); + } + else if (binding instanceof IType) { + reportProblemsIfAbstract((IType)binding, constructorName); + } + } + + /** + * Tries to get binding by AST Name. If it is not available - tries to resolve it + */ + private IBinding getOrResolveBinding(IASTName name) { + IBinding binding = name.getBinding(); + if (binding == null) { + binding = name.resolveBinding(); + } + return binding; + } + + /** + * Tries to resolve qualified name. If it is not available returns simple name. + */ + private String resolveName(ICPPBinding binding) { + try { + if (binding.isGloballyQualified()) { + StringBuilder buf = new StringBuilder(); + for (String item : binding.getQualifiedName()) { + if (buf.length() != 0) + buf.append("::"); //$NON-NLS-1$ + buf.append(item); + } + return buf.toString(); + } + } catch (DOMException e) { + CodanCheckersActivator.log(e); + } + return binding.getName(); + } + + /** + * Checks whether specified type (class or typedef to the class) is abstract class. + * If it is - reports violations on each pure virtual method + */ + private void reportProblemsIfAbstract(IType typeToCheck, IASTNode problemNode ) { + IType unwindedType = CxxAstUtils.getInstance().unwindTypedef(typeToCheck); + if (unwindedType instanceof ICPPClassType) { + ICPPClassType classType = (ICPPClassType)unwindedType; + for (ICPPMethod method : ClassTypeHelper.getPureVirtualMethods(classType)) { + reportProblem(ER_ID, problemNode, resolveName(classType), resolveName(method)); + } + } + } + } +} diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/AbstractClassInstantiationCheckerTest.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/AbstractClassInstantiationCheckerTest.java new file mode 100644 index 00000000000..a5f9fef290b --- /dev/null +++ b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/AbstractClassInstantiationCheckerTest.java @@ -0,0 +1,231 @@ +/******************************************************************************* + * Copyright (c) 2011 Anton Gorenkov + * 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: + * Anton Gorenkov - initial implementation + *******************************************************************************/ +package org.eclipse.cdt.codan.core.internal.checkers; + +import org.eclipse.cdt.codan.core.test.CheckerTestCase; +import org.eclipse.cdt.codan.internal.checkers.AbstractClassInstantiationChecker; + +/** + * Test for {@see AbstractClassInstantiationChecker} class + * + */ +public class AbstractClassInstantiationCheckerTest extends CheckerTestCase { + @Override + public boolean isCpp() { + return true; + } + + @Override + public void setUp() throws Exception { + super.setUp(); + enableProblems(AbstractClassInstantiationChecker.ER_ID); + } + + // class C { + // virtual void f() {} + // }; + // void scope () { + // C c; // No errors. + // } + public void testNotAbstractClassCreationOnStack() { + loadCodeAndRun(getAboveComment()); + checkNoErrors(); + } + + // class C { + // virtual void f() {} + // }; + // void scope () { + // C* c = new C(); // No errors. + // } + public void testNotAbstractClassCreationWithNew() { + loadCodeAndRun(getAboveComment()); + checkNoErrors(); + } + + // class C { + // virtual void f() {} + // }; + // void scope () { + // C::C(); // No errors. + // } + public void testNotAbstractClassCreationWithDirectConstructorCall() { + loadCodeAndRun(getAboveComment()); + checkNoErrors(); + } + + // class C { + // virtual void f() = 0; + // }; + // void scope () { + // C* c1; // No errors. + // C& c2; // No errors. + // } + public void testAbstractClassPointerOrReverenceDeclaration() { + loadCodeAndRun(getAboveComment()); + checkNoErrors(); + } + + // class C { + // virtual void f() = 0; + // }; + // typedef C typedefC; + // void scope () { + // C c; // 1 error for: C::f(). + // typedefC tc; // 1 error for: C::f(). + // } + public void testAbstractClassCreationOnStack() { + loadCodeAndRun(getAboveComment()); + checkErrorLines(6, 7); + } + + // class C { + // virtual void f() = 0; + // }; + // typedef C typedefC; + // void scope () { + // C *c1, c2, &c3; // 1 error for: C::f(). + // typedefC *tc1, tc2, &tc3; // 1 error for: C::f(). + // } + public void testAbstractClassCreationOnStackWithRefAndPtr() { + loadCodeAndRun(getAboveComment()); + checkErrorLines(6, 7); + } + + // class C { + // virtual void f() = 0; + // }; + // typedef C typedefC; + // void test ( C _c ) {} // 1 error for: C::f(). + // void test2 ( typedefC _c ) {} // 1 error for: C::f(). + // void test3 ( C _c, typedefC _c ) {} // 2 errors for: C::f(), C::f(). + // void test4 ( C ) {} // 1 error for: C::f(). + // void test5 ( C* _c ) {} // No errors. + // void test6 ( typedefC& _c ) {} // No errors. + public void testAbstractClassCreationAsFunctionParameter() { + loadCodeAndRun(getAboveComment()); + checkErrorLines(5, 6, 7, 7, 8); + } + + // class C { + // virtual void f() = 0; + // }; + // template // No errors. + // void test () {} + public void testAbstractClassCreationAsFunctionTemplateParameter() { + loadCodeAndRun(getAboveComment()); + checkNoErrors(); + } + + // class C { + // virtual void f() = 0; + // }; + // typedef C typedefC; + // void scope () { + // C* c1 = new C(); // 1 error for: C::f(). + // C* c2 = new C[10]; // 1 error for: C::f(). + // C* c3 = new typedefC(); // 1 error for: C::f(). + // C* c4 = new typedefC; // 1 error for: C::f(). + // C* c5 (new C()); // 1 error for: C::f(). + // C* c6 (new typedefC()); // 1 error for: C::f(). + // C* c7 = new typedefC[10]; // 1 error for: C::f(). + // C** x1 = new C*(); // No errors. + // typedefC** x2 = new typedefC*(); // No errors. + // } + public void testAbstractClassCreationWithNew() { + loadCodeAndRun(getAboveComment()); + checkErrorLines(6, 7, 8, 9, 10, 11, 12); + } + + // class C { + // virtual void f() = 0; + // }; + // typedef C typedefC; + // void scope () { + // C::C(); // 1 error for: C::f(). + // typedefC::C(); // 1 error for: C::f(). + // } + public void testAbstractClassCreationWithDirectConstructorCall() { + loadCodeAndRun(getAboveComment()); + checkErrorLines(6, 7); + } + + // namespace N { + // class C { + // virtual void f() = 0; + // }; + // } + // void scope () { + // N::C* c = new N::C(); // 1 error for: N::C::f(). + // } + public void testAbstractClassFromNamespace() { + loadCodeAndRun(getAboveComment()); + checkErrorLines(7); + } + + // class C { + // virtual void f() = 0; + // virtual int g() const = 0; + // }; + // void scope () { + // C* c = new C(); // 2 errors for: C::f(), C::g(). + // } + public void testAbstractClassWithAFewVirtualMethods() { + loadCodeAndRun(getAboveComment()); + checkErrorLines(6, 6); + } + + // class Base { + // virtual void f() = 0; + // }; + // class Derived : public Base { + // }; + // void scope () { + // Derived* d = new Derived(); // 1 error for: Base::f(). + // } + public void testAbstractClassBecauseOfBaseClass() { + loadCodeAndRun(getAboveComment()); + checkErrorLines(7); + } + + // class Base { + // virtual void f() = 0; + // virtual int g() const = 0; + // }; + // class Derived : public Base { + // virtual int g() const = 0; + // }; + // void scope () { + // Derived* c = new Derived(); // 2 errors for: Base::f(), Derived::g(). + // } + public void testAbstractClassWithVirtualRedefinition() { + loadCodeAndRun(getAboveComment()); + checkErrorLines(9, 9); + } + + // class C { + // virtual void f() = 0; + // }; + // typedef C typedefC; // No errors. + public void testAbstractClassTypedef() { + loadCodeAndRun(getAboveComment()); + checkNoErrors(); + } + + // class C { + // virtual void f() = 0; + // }; + // extern C typedefC; // 1 error for: C::f(). + public void testExternAbstractClassDeclaration() { + loadCodeAndRun(getAboveComment()); + checkErrorLines(4); + } +} 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 3ed319422d7..0bf302e7d43 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 @@ -15,6 +15,7 @@ import junit.framework.Test; import junit.framework.TestCase; import junit.framework.TestSuite; +import org.eclipse.cdt.codan.core.internal.checkers.AbstractClassInstantiationCheckerTest; import org.eclipse.cdt.codan.core.internal.checkers.AssignmentInConditionCheckerTest; import org.eclipse.cdt.codan.core.internal.checkers.AssignmentToItselfCheckerTest; import org.eclipse.cdt.codan.core.internal.checkers.CaseBreakCheckerTest; @@ -48,22 +49,23 @@ public class AutomatedIntegrationSuite extends TestSuite { public static Test suite() { final AutomatedIntegrationSuite suite = new AutomatedIntegrationSuite(); // checkers - suite.addTestSuite(StatementHasNoEffectCheckerTest.class); - suite.addTestSuite(SuggestedParenthesisCheckerTest.class); - suite.addTestSuite(ReturnCheckerTest.class); - suite.addTestSuite(CatchByReferenceTest.class); + suite.addTestSuite(AbstractClassInstantiationCheckerTest.class); suite.addTestSuite(AssignmentInConditionCheckerTest.class); suite.addTestSuite(AssignmentToItselfCheckerTest.class); - suite.addTestSuite(ReturnStyleCheckerTest.class); - suite.addTestSuite(SuspiciousSemicolonCheckerTest.class); suite.addTestSuite(CaseBreakCheckerTest.class); + suite.addTestSuite(CatchByReferenceTest.class); suite.addTestSuite(FormatStringCheckerTest.class); suite.addTestSuite(ProblemBindingCheckerTest.class); + suite.addTestSuite(ReturnCheckerTest.class); + suite.addTestSuite(ReturnStyleCheckerTest.class); + suite.addTestSuite(StatementHasNoEffectCheckerTest.class); + suite.addTestSuite(SuggestedParenthesisCheckerTest.class); + suite.addTestSuite(SuspiciousSemicolonCheckerTest.class); // framework suite.addTest(CodanFastTestSuite.suite()); // quick fixes - suite.addTestSuite(SuggestedParenthesisQuickFixTest.class); suite.addTestSuite(CreateLocalVariableQuickFixTest.class); + suite.addTestSuite(SuggestedParenthesisQuickFixTest.class); return suite; } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/ClassTypeHelper.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/ClassTypeHelper.java index d2f2dbcbf70..37968bb0638 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/ClassTypeHelper.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/ClassTypeHelper.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2004, 2010 IBM Corporation and others. + * Copyright (c) 2004, 2011 IBM Corporation and others. * 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 @@ -11,15 +11,20 @@ * Bryan Wilkinson (QNX) * Sergey Prigogin (Google) * Andrew Ferguson (Symbian) + * Anton Gorenkov *******************************************************************************/ package org.eclipse.cdt.internal.core.dom.parser.cpp; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.Map; +import java.util.Set; import org.eclipse.cdt.core.dom.ast.ASTTypeUtil; import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier; @@ -791,4 +796,83 @@ public class ClassTypeHelper { } return true; } + + /** + * Checks whether class is abstract, i.e. has pure virtual functions that were + * not implemented in base after declaration. + * + * NOTE: The method produces complete results for template instantiations + * but doesn't take into account base classes and methods dependent on unspecified + * template parameters. + */ + public static ICPPMethod[] getPureVirtualMethods(ICPPClassType classTarget) { + Collection> result = collectPureVirtualMethods(classTarget).values(); + int resultArraySize = 0; + for (Set set : result) { + resultArraySize += set.size(); + } + ICPPMethod[] resultArray = new ICPPMethod[resultArraySize]; + int resultArrayIdx = 0; + for (Set methodsSet : result) { + for (ICPPMethod method : methodsSet) { + resultArray[resultArrayIdx] = method; + ++resultArrayIdx; + } + } + return resultArray; + } + + /** + * Returns pure virtual methods of the given class grouped by their names. + * + * @param classTarget The class to obtain the pure virtual method for. + * @return pure virtual methods grouped by their names. + */ + private static Map > collectPureVirtualMethods(ICPPClassType classTarget) { + // Collect pure virtual functions from base classes + Map> pureVirtualMethods = new HashMap>(); + for (ICPPBase base : classTarget.getBases()) { + if (base.getBaseClass() instanceof ICPPClassType) { + ICPPClassType baseClass = (ICPPClassType) base.getBaseClass(); + Map > derivedPureVirtualMethods = collectPureVirtualMethods(baseClass); + // Merge derived pure virtual methods + for (Map.Entry > currMethodEntry : derivedPureVirtualMethods.entrySet()) { + Set methodsSet = pureVirtualMethods.get(currMethodEntry.getKey()); + if (methodsSet == null) { + pureVirtualMethods.put(currMethodEntry.getKey(), currMethodEntry.getValue()); + } + else { + methodsSet.addAll(currMethodEntry.getValue()); + } + } + } + } + // Remove overridden methods (even if they are pure virtual) + for (ICPPMethod declaredMethod : classTarget.getDeclaredMethods()) { + Set methodsSet = pureVirtualMethods.get(declaredMethod.getName()); + if (methodsSet != null) { + for (Iterator methodIt = methodsSet.iterator(); methodIt.hasNext();) { + ICPPMethod method = methodIt.next(); + if (functionTypesAllowOverride(declaredMethod.getType(), method.getType())) { + methodIt.remove(); + } + } + if (methodsSet.isEmpty()) { + pureVirtualMethods.remove(declaredMethod.getName()); + } + } + } + // Add pure virtual methods of current class + for (ICPPMethod method : classTarget.getDeclaredMethods()) { + if (method.isPureVirtual()) { + Set methodsSet = pureVirtualMethods.get(method.getName()); + if (methodsSet == null) { + methodsSet = new HashSet(); + pureVirtualMethods.put(method.getName(), methodsSet); + } + methodsSet.add(method); + } + } + return pureVirtualMethods; + } }