From 91ab5f5876a57e33baa6ea0f5358d8ba75962c82 Mon Sep 17 00:00:00 2001 From: Andrew Gvozdev Date: Fri, 29 Apr 2011 19:25:26 +0000 Subject: [PATCH] bug 343429: [checker] Checker to pinpoint unused static functions in a file A few false positives corrected --- .../UnusedSymbolInFileScopeChecker.java | 54 +++++++++++++++---- .../UnusedSymbolInFileScopeCheckerTest.java | 52 ++++++++++++++++-- 2 files changed, 93 insertions(+), 13 deletions(-) diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/UnusedSymbolInFileScopeChecker.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/UnusedSymbolInFileScopeChecker.java index 6759b2631b3..8ef51bdcf2b 100644 --- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/UnusedSymbolInFileScopeChecker.java +++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/UnusedSymbolInFileScopeChecker.java @@ -12,8 +12,10 @@ package org.eclipse.cdt.codan.internal.checkers; import java.util.ArrayList; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import org.eclipse.cdt.codan.checkers.CodanCheckersActivator; import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker; @@ -36,9 +38,11 @@ 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.IFunction; +import org.eclipse.cdt.core.dom.ast.IProblemBinding; import org.eclipse.cdt.core.dom.ast.IProblemType; import org.eclipse.cdt.core.dom.ast.IType; import org.eclipse.cdt.core.dom.ast.IVariable; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTConstructorInitializer; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName; import org.eclipse.cdt.core.dom.ast.cpp.ICPPClassType; import org.eclipse.cdt.core.dom.ast.cpp.ICPPMethod; @@ -134,23 +138,34 @@ public class UnusedSymbolInFileScopeChecker extends AbstractIndexAstChecker { } } else if (binding instanceof IVariable) { if (storageClass == IASTDeclSpecifier.sc_extern) { - externVariableDeclarations.put(binding, decl); + IASTInitializer initializer = decl.getInitializer(); + // initializer makes "extern" declaration to become definition do not count these + if (initializer==null) { + externVariableDeclarations.put(binding, decl); + } } else if (storageClass == IASTDeclSpecifier.sc_static) { IType type = ((IVariable) binding).getType(); // account for class constructor and avoid possible false positive if (!(type instanceof ICPPClassType) && !(type instanceof IProblemType)) { + // check if initializer disqualifies it IASTInitializer initializer = decl.getInitializer(); + IASTInitializerClause clause = null; if (initializer instanceof IASTEqualsInitializer) { IASTEqualsInitializer equalsInitializer = (IASTEqualsInitializer) initializer; - IASTInitializerClause clause = equalsInitializer.getInitializerClause(); - if (clause instanceof IASTLiteralExpression) { - IASTLiteralExpression literalExpression = (IASTLiteralExpression) clause; - String literal = literalExpression.toString(); - if (isFilteredOut(literal, unusedVariableProblem, PARAM_EXCEPT_ARG_LIST)) - continue; - } + clause = equalsInitializer.getInitializerClause(); + } else if (initializer instanceof ICPPASTConstructorInitializer) { + ICPPASTConstructorInitializer constructorInitializer = (ICPPASTConstructorInitializer) initializer; + IASTInitializerClause[] args = constructorInitializer.getArguments(); + if (args.length==1) + clause = args[0]; } - + if (clause instanceof IASTLiteralExpression) { + IASTLiteralExpression literalExpression = (IASTLiteralExpression) clause; + String literal = literalExpression.toString(); + if (isFilteredOut(literal, unusedVariableProblem, PARAM_EXCEPT_ARG_LIST)) + continue; + } + staticVariableDeclarations.put(binding, decl); } } @@ -200,6 +215,16 @@ public class UnusedSymbolInFileScopeChecker extends AbstractIndexAstChecker { if (binding instanceof ICPPMethod) return PROCESS_CONTINUE; + if (binding instanceof IProblemBinding) { + // avoid false positives related to unresolved names + String plainName = name.toString(); + filterOutByPlainName(externFunctionDeclarations, plainName); + filterOutByPlainName(staticFunctionDeclarations, plainName); + filterOutByPlainName(staticFunctionDefinitions, plainName); + filterOutByPlainName(externVariableDeclarations, plainName); + filterOutByPlainName(staticVariableDeclarations, plainName); + } + IASTNode parentNode = name.getParent(); if (!(parentNode instanceof IASTFunctionDefinition || parentNode instanceof IASTFunctionDeclarator)) { @@ -218,6 +243,17 @@ public class UnusedSymbolInFileScopeChecker extends AbstractIndexAstChecker { return PROCESS_CONTINUE; } + private void filterOutByPlainName(Map declarators, String id) { + Iterator> iter = declarators.entrySet().iterator(); + while (iter.hasNext()) { + Entry entry = iter.next(); + IASTDeclarator decl = entry.getValue(); + IASTName astName = getAstName(decl); + if (id.equals(astName.toString())) + iter.remove(); + } + } + }); } catch (Exception e) { CodanCheckersActivator.log(e); diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/UnusedSymbolInFileScopeCheckerTest.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/UnusedSymbolInFileScopeCheckerTest.java index 5975ea70d74..7206e8b7852 100644 --- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/UnusedSymbolInFileScopeCheckerTest.java +++ b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/UnusedSymbolInFileScopeCheckerTest.java @@ -161,6 +161,29 @@ public class UnusedSymbolInFileScopeCheckerTest extends CheckerTestCase { checkErrorLine(1); } + // static int test_fun(int i) {} + // int i = test_fun(X); + public void testStaticFunction_Definition_UnknownParameterType() throws IOException { + loadCodeAndRunCpp(getAboveComment()); + checkNoErrors(); + } + + // static void test_fun(void) {} + // void Class::fun() { + // test_fun(); + // } + public void testStaticFunction_Definition_InQualifiedFunction() throws IOException { + loadCodeAndRunCpp(getAboveComment()); + checkNoErrors(); + } + + // static int test_fun(X) {} + // int i = test_fun(X); + public void testStaticFunction_Definition_UnknownArgumentType() throws IOException { + loadCodeAndRunCpp(getAboveComment()); + checkNoErrors(); + } + //////////////////////////////////////////////////////////////////////////// // Extern variables declaration //////////////////////////////////////////////////////////////////////////// @@ -194,6 +217,12 @@ public class UnusedSymbolInFileScopeCheckerTest extends CheckerTestCase { // int test_var; // int test_var2=0; + public void testGlobalVariable_Definition() throws IOException { + loadCodeAndRun(getAboveComment()); + checkNoErrors(); + } + + // extern int test_var=0; // not quite legal but some compilers allow that public void testExternVariable_Definition() throws IOException { loadCodeAndRun(getAboveComment()); checkNoErrors(); @@ -231,6 +260,15 @@ public class UnusedSymbolInFileScopeCheckerTest extends CheckerTestCase { checkNoErrors(); } + // static int test_var; + // void Class::fun() { + // test_var = 0; + // } + public void testStaticVariable_Used_InQualifiedFunction() throws IOException { + loadCodeAndRunCpp(getAboveComment()); + checkNoErrors(); + } + // class Class; // static Class test_var; // constructor is called here public void testStaticVariable_Used_Constructor() throws IOException { @@ -244,16 +282,22 @@ public class UnusedSymbolInFileScopeCheckerTest extends CheckerTestCase { checkNoErrors(); } - // static char* test_var="$Id: file.c,v 1.1 2000/01/01 11:11:11 agvozdev Exp $"; - public void testExternVariable_Declaration_cvs_ident() throws IOException { + // static char* test_var="$Id: UnusedSymbolInFileScopeCheckerTest.java,v 1.2 2011/04/29 11:17:42 agvozdev Exp $"; + public void testExternVariable_Declaration_CvsIdent() throws IOException { loadCodeAndRun(getAboveComment()); checkNoErrors(); } - // static char* test_var="@(#) $Header: /src/file.c,v 1.1 2000/01/01 11:11:11 agvozdev Exp $"; - public void testExternVariable_Declaration_sccs_ident() throws IOException { + // static char* test_var="@(#) $Header: /cvsroot/tools/org.eclipse.cdt/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/UnusedSymbolInFileScopeCheckerTest.java,v 1.2 2011/04/29 11:17:42 agvozdev Exp $"; + public void testExternVariable_Declaration_SccsIdent() throws IOException { loadCodeAndRun(getAboveComment()); checkNoErrors(); } + // static char test_var[]("@(#) $Id: UnusedSymbolInFileScopeCheckerTest.java,v 1.2 2011/04/29 11:17:42 agvozdev Exp $"); + public void testExternVariable_Declaration_CvsIdentInitializer() throws IOException { + loadCodeAndRunCpp(getAboveComment()); + checkNoErrors(); + } + }