From f8316e315ba5737fa3134b343f30625062a75519 Mon Sep 17 00:00:00 2001 From: Marco Stornelli Date: Sun, 31 Mar 2019 12:22:55 +0200 Subject: [PATCH] Bug 545977 - Added checker for variable initialization Checker checks for multiple declarations on one line and static variables defined in header files. Change-Id: Ibc9670ee129e9bdd3ea58ac5409493fd99c4a234 Signed-off-by: Marco Stornelli --- .../OSGI-INF/l10n/bundle.properties | 9 ++ .../org.eclipse.cdt.codan.checkers/plugin.xml | 25 +++++ .../VariableInitializationChecker.java | 83 ++++++++++++++ .../checkers/VariablesCheckerTest.java | 105 ++++++++++++++++++ .../core/tests/AutomatedIntegrationSuite.java | 2 + 5 files changed, 224 insertions(+) create mode 100644 codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/VariableInitializationChecker.java create mode 100644 codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/VariablesCheckerTest.java 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 153d25c41ff..2d79a703881 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 @@ -184,3 +184,12 @@ checker.name.BlacklistChecker = Function or method in blacklist checker problem.name.BlacklistProblem = Function or method is blacklisted problem.messagePattern.BlacklistProblem = Function or method ''{0}'' is blacklisted problem.description.BlacklistProblem = This rule will flag the use of functions or methods in blacklist + +checker.name.VariablesChecker = Variable checker +problem.name.StaticVariableInHeaderProblem = Static variable in header file +problem.messagePattern.StaticVariableInHeaderProblem = Variable ''{0}'' is duplicated in each translation unit +problem.description.StaticVariableInHeaderProblem = This rule will flag static variable declarations in header files +problem.name.MultipleDeclarationsProblem = Multiple variable declaration +problem.messagePattern.MultipleDeclarationsProblem = Variables should be declared one per line to improve readability +problem.description.MultipleDeclarationsProblem = This rule will flag multiple declarations on one line + diff --git a/codan/org.eclipse.cdt.codan.checkers/plugin.xml b/codan/org.eclipse.cdt.codan.checkers/plugin.xml index 8a5c6f25084..04c37c179a4 100644 --- a/codan/org.eclipse.cdt.codan.checkers/plugin.xml +++ b/codan/org.eclipse.cdt.codan.checkers/plugin.xml @@ -588,5 +588,30 @@ name="%problem.name.BlacklistProblem"> + + + + + + diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/VariableInitializationChecker.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/VariableInitializationChecker.java new file mode 100644 index 00000000000..5e06cf6875a --- /dev/null +++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/VariableInitializationChecker.java @@ -0,0 +1,83 @@ +/******************************************************************************* + * Copyright (c) 2019 Marco Stornelli + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.cdt.codan.internal.checkers; + +import org.eclipse.cdt.codan.checkers.CodanCheckersActivator; +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.EScopeKind; +import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier; +import org.eclipse.cdt.core.dom.ast.IASTDeclaration; +import org.eclipse.cdt.core.dom.ast.IASTDeclarationStatement; +import org.eclipse.cdt.core.dom.ast.IASTDeclarator; +import org.eclipse.cdt.core.dom.ast.IASTForStatement; +import org.eclipse.cdt.core.dom.ast.IASTIfStatement; +import org.eclipse.cdt.core.dom.ast.IASTNode; +import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; +import org.eclipse.cdt.core.dom.ast.IASTSwitchStatement; +import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; +import org.eclipse.cdt.core.dom.ast.IBinding; +import org.eclipse.cdt.core.dom.ast.IScope; +import org.eclipse.cdt.core.dom.ast.IVariable; + +public class VariableInitializationChecker extends AbstractIndexAstChecker { + public static final String STATIC_VAR_ID = "org.eclipse.cdt.codan.internal.checkers.StaticVariableInHeaderProblem"; //$NON-NLS-1$ + public static final String VAR_MULTI_DEC_ID = "org.eclipse.cdt.codan.internal.checkers.MultipleDeclarationsProblem"; //$NON-NLS-1$ + + @Override + public void processAst(IASTTranslationUnit ast) { + ast.accept(new ASTVisitor() { + { + shouldVisitDeclarations = true; + } + + @Override + public int visit(IASTDeclaration declaration) { + if (declaration instanceof IASTSimpleDeclaration) { + IASTSimpleDeclaration simple = (IASTSimpleDeclaration) declaration; + IASTDeclarator decls[] = ((IASTSimpleDeclaration) declaration).getDeclarators(); + if (simple.getDeclSpecifier().getStorageClass() == IASTDeclSpecifier.sc_static + && declaration.getTranslationUnit().isHeaderUnit()) { + for (IASTDeclarator d : decls) { + IBinding binding = d.getName().resolveBinding(); + if (binding != null && binding instanceof IVariable) { + try { + IScope scope = binding.getScope(); + if (scope == null) + return PROCESS_CONTINUE; + if (scope.getKind() == EScopeKind.eGlobal + || scope.getKind() == EScopeKind.eNamespace) + reportProblem(STATIC_VAR_ID, declaration, d.getName()); + } catch (DOMException e) { + CodanCheckersActivator.log(e); + } + } + } + } + if (declaration.getParent() instanceof IASTDeclarationStatement) { + IASTNode node = declaration.getParent().getParent(); + if (node instanceof IASTForStatement || node instanceof IASTIfStatement + || node instanceof IASTSwitchStatement) { + //we allow multiple declarations for if/switch initializers and for statements + return PROCESS_CONTINUE; + } + } + if (decls.length == 0) + return PROCESS_CONTINUE; + if (decls.length != 1) + reportProblem(VAR_MULTI_DEC_ID, declaration); + } + return PROCESS_CONTINUE; + } + }); + } +} diff --git a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/VariablesCheckerTest.java b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/VariablesCheckerTest.java new file mode 100644 index 00000000000..168e14c288d --- /dev/null +++ b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/VariablesCheckerTest.java @@ -0,0 +1,105 @@ +/******************************************************************************* + * Copyright (c) 2019 Marco Stornelli + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.cdt.codan.core.internal.checkers; + +import org.eclipse.cdt.codan.core.tests.CheckerTestCase; +import org.eclipse.cdt.codan.internal.checkers.VariableInitializationChecker; + +/** + * Test for {@link VariableInitializationChecker} class + */ +public class VariablesCheckerTest extends CheckerTestCase { + + public static final String STATIC_ID = VariableInitializationChecker.STATIC_VAR_ID; + public static final String MULTI_ID = VariableInitializationChecker.VAR_MULTI_DEC_ID; + + private boolean header; + + @Override + public void setUp() throws Exception { + super.setUp(); + header = true; + enableProblems(STATIC_ID, MULTI_ID); + } + + @Override + public boolean isCpp() { + return true; + } + + @Override + public boolean isHeader() { + return header; + } + + //static int a = 0; + public void testStaticVarInHeader() throws Exception { + loadCodeAndRun(getAboveComment()); + checkErrorLine(1, STATIC_ID); + } + + //static int a = 0; + public void testStaticVarInCpp() throws Exception { + header = false; + loadCodeAndRun(getAboveComment()); + checkNoErrorsOfKind(STATIC_ID); + } + + //void foo() { + // int p = 0, u = 0; + //} + public void testMultiInitVar() throws Exception { + loadCodeAndRun(getAboveComment()); + checkErrorLine(2, MULTI_ID); + } + + //void foo() { + // int p = 0; + // int u = 0; + //} + public void testCleanVar() throws Exception { + loadCodeAndRun(getAboveComment()); + checkNoErrorsOfKind(MULTI_ID); + checkNoErrorsOfKind(STATIC_ID); + } + + //void foo() { + // for (int i = 0, j = 1; i < 10; ++i, ++j) { + // } + //} + public void testFor() throws Exception { + loadCodeAndRun(getAboveComment()); + checkNoErrorsOfKind(MULTI_ID); + checkNoErrorsOfKind(STATIC_ID); + } + + //void foo() { + // if (int i = 0, j = 0; i + j < 0) { + // } + //} + public void testIf() throws Exception { + loadCodeAndRun(getAboveComment()); + checkNoErrorsOfKind(MULTI_ID); + checkNoErrorsOfKind(STATIC_ID); + } + + //void foo() { + // switch (int i = 0, j = 0; i + j) { + // default: + // break; + // } + //} + public void testSwitch() throws Exception { + loadCodeAndRun(getAboveComment()); + checkNoErrorsOfKind(MULTI_ID); + checkNoErrorsOfKind(STATIC_ID); + } +} diff --git a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java index b280c1ce2f0..7a4b4b25e04 100644 --- a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java +++ b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java @@ -37,6 +37,7 @@ import org.eclipse.cdt.codan.core.internal.checkers.SuggestedParenthesisCheckerT import org.eclipse.cdt.codan.core.internal.checkers.SuspiciousSemicolonCheckerTest; import org.eclipse.cdt.codan.core.internal.checkers.SwitchCaseCheckerTest; import org.eclipse.cdt.codan.core.internal.checkers.UnusedSymbolInFileScopeCheckerTest; +import org.eclipse.cdt.codan.core.internal.checkers.VariablesCheckerTest; import org.eclipse.cdt.codan.core.internal.checkers.VirtualMethodCallCheckerTest; import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.AssignmentInConditionQuickFixTest; import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.CaseBreakQuickFixBreakTest; @@ -97,6 +98,7 @@ public class AutomatedIntegrationSuite extends TestSuite { suite.addTestSuite(SwitchCaseCheckerTest.class); suite.addTestSuite(VirtualMethodCallCheckerTest.class); suite.addTestSuite(AssignmentOperatorCheckerTest.class); + suite.addTestSuite(VariablesCheckerTest.class); // framework suite.addTest(CodanFastTestSuite.suite()); // quick fixes