Skip to main content
aboutsummaryrefslogtreecommitdiffstats
path: root/codan
diff options
context:
space:
mode:
authorMarco Stornelli2019-03-30 16:45:11 +0000
committerMarco Stornelli2019-07-23 04:47:39 +0000
commit4c0e7d9f68c8d2b1ccbb1f9f9c30058bff59cebb (patch)
treeefb519d2594e5964660251222016f0b25caa1df8 /codan
parent04350fec0eb5063051144fa63c116fa7e83adb09 (diff)
downloadorg.eclipse.cdt-4c0e7d9f68c8d2b1ccbb1f9f9c30058bff59cebb.tar.gz
org.eclipse.cdt-4c0e7d9f68c8d2b1ccbb1f9f9c30058bff59cebb.tar.xz
org.eclipse.cdt-4c0e7d9f68c8d2b1ccbb1f9f9c30058bff59cebb.zip
Bug 545959 - Added checker for assignment operator
Change-Id: Ib48742cbc04679ab9e48349f4d68aea5657d38c9 Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
Diffstat (limited to 'codan')
-rw-r--r--codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties8
-rw-r--r--codan/org.eclipse.cdt.codan.checkers/plugin.xml25
-rw-r--r--codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/AssignmentOperatorChecker.java181
-rw-r--r--codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/AssignmentOperatorCheckerTest.java191
-rw-r--r--codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java2
5 files changed, 407 insertions, 0 deletions
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 735b445d5e..5e13f9864c 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
@@ -168,3 +168,11 @@ checker.name.VirtualMethodCallChecker = Virtual method call checker
problem.name.VirtualMethodCallProblem = Virtual method call in constructor/destructor
problem.messagePattern.VirtualMethodCallProblem = Calling a virtual method in constructor or destructor can cause crashes and unexpected behavior
problem.description.VirtualMethodCallProblem = This rule will flag constructor and destructor with virtual method calls
+
+checker.name.AssignmentOperatorChecker = Assignment operator checker
+problem.name.MissSelfCheckProblem = Missing self check in assignment operator
+problem.messagePattern.MissSelfCheckProblem = Self assignment checks improves performance and avoid to unexpected behavior
+problem.description.MissSelfCheckProblem = This rule will flag assignment operators without a self assignment check
+problem.name.MissReferenceProblem = Missing reference return value in assignment operator
+problem.messagePattern.MissReferenceProblem = Assignment operators should return by reference
+problem.description.MissReferenceProblem = This rule will flag assignment operators not returning by reference
diff --git a/codan/org.eclipse.cdt.codan.checkers/plugin.xml b/codan/org.eclipse.cdt.codan.checkers/plugin.xml
index 470dbc9bd9..99fae9b9f4 100644
--- a/codan/org.eclipse.cdt.codan.checkers/plugin.xml
+++ b/codan/org.eclipse.cdt.codan.checkers/plugin.xml
@@ -539,5 +539,30 @@
name="%problem.name.VirtualMethodCallProblem">
</problem>
</checker>
+ <checker
+ class="org.eclipse.cdt.codan.internal.checkers.AssignmentOperatorChecker"
+ id="org.eclipse.cdt.codan.internal.checkers.AssignmentOperatorChecker"
+ name="%checker.name.AssignmentOperatorChecker">
+ <problem
+ category="org.eclipse.cdt.codan.core.categories.ProgrammingProblems"
+ defaultEnabled="false"
+ defaultSeverity="Warning"
+ description="%problem.description.MissSelfCheckProblem"
+ id="org.eclipse.cdt.codan.internal.checkers.MissSelfCheckProblem"
+ markerType="org.eclipse.cdt.codan.core.codanProblem"
+ messagePattern="%problem.messagePattern.MissSelfCheckProblem"
+ name="%problem.name.MissSelfCheckProblem">
+ </problem>
+ <problem
+ category="org.eclipse.cdt.codan.core.categories.CodeStyle"
+ defaultEnabled="false"
+ defaultSeverity="Warning"
+ description="%problem.description.MissReferenceProblem"
+ id="org.eclipse.cdt.codan.internal.checkers.MissReferenceProblem"
+ markerType="org.eclipse.cdt.codan.core.codanProblem"
+ messagePattern="%problem.messagePattern.MissReferenceProblem"
+ name="%problem.name.MissReferenceProblem">
+ </problem>
+ </checker>
</extension>
</plugin>
diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/AssignmentOperatorChecker.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/AssignmentOperatorChecker.java
new file mode 100644
index 0000000000..c3196b940f
--- /dev/null
+++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/AssignmentOperatorChecker.java
@@ -0,0 +1,181 @@
+/*******************************************************************************
+ * 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 java.util.Arrays;
+import java.util.Stack;
+
+import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker;
+import org.eclipse.cdt.core.dom.ast.ASTVisitor;
+import org.eclipse.cdt.core.dom.ast.IASTBinaryExpression;
+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.IASTLiteralExpression;
+import org.eclipse.cdt.core.dom.ast.IASTNode;
+import org.eclipse.cdt.core.dom.ast.IASTPointerOperator;
+import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
+import org.eclipse.cdt.core.dom.ast.IASTUnaryExpression;
+import org.eclipse.cdt.core.dom.ast.IBinding;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDeclarator;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDefinition;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTParameterDeclaration;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTReferenceOperator;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTUnaryExpression;
+import org.eclipse.cdt.core.dom.ast.cpp.ICPPMethod;
+import org.eclipse.cdt.core.dom.ast.cpp.SemanticQueries;
+
+public class AssignmentOperatorChecker extends AbstractIndexAstChecker {
+ public static final String MISS_REF_ID = "org.eclipse.cdt.codan.internal.checkers.MissReferenceProblem"; //$NON-NLS-1$
+ public static final String MISS_SELF_CHECK_ID = "org.eclipse.cdt.codan.internal.checkers.MissSelfCheckProblem"; //$NON-NLS-1$
+ private static final String OPERATOR_EQ = "operator ="; //$NON-NLS-1$
+
+ @Override
+ public void processAst(IASTTranslationUnit ast) {
+ ast.accept(new OnEachClass());
+ }
+
+ private static class OperatorEqInfo {
+ IASTDeclarator decl;
+ ICPPASTParameterDeclaration[] parameters;
+ }
+
+ class OnEachClass extends ASTVisitor {
+ private final Stack<OperatorEqInfo> opInfo = new Stack<>();
+
+ OnEachClass() {
+ shouldVisitDeclarations = true;
+ shouldVisitExpressions = true;
+ }
+
+ @Override
+ public int visit(IASTDeclaration declaration) {
+ ICPPMethod method = getOperatorEq(declaration);
+ if (method != null) {
+ OperatorEqInfo info = opInfo.push(new OperatorEqInfo());
+ IASTDeclarator declMethod = ((ICPPASTFunctionDefinition) declaration).getDeclarator();
+ if (!(declMethod instanceof ICPPASTFunctionDeclarator))
+ return PROCESS_CONTINUE;
+ if (((ICPPASTFunctionDefinition) declaration).isDefaulted()
+ || ((ICPPASTFunctionDefinition) declaration).isDeleted())
+ return PROCESS_CONTINUE;
+ IASTPointerOperator[] pointers = declMethod.getPointerOperators();
+ info.parameters = ((ICPPASTFunctionDeclarator) declMethod).getParameters();
+ if (pointers.length != 1 || !(pointers[0] instanceof ICPPASTReferenceOperator))
+ reportProblem(MISS_REF_ID, declaration);
+
+ if (!SemanticQueries.isCopyOrMoveAssignmentOperator(method))
+ return PROCESS_CONTINUE;
+
+ info.decl = declMethod;
+ }
+ return PROCESS_CONTINUE;
+ }
+
+ @Override
+ public int leave(IASTDeclaration declaration) {
+ if (getOperatorEq(declaration) != null && !opInfo.isEmpty()) {
+ opInfo.pop();
+ }
+ return PROCESS_CONTINUE;
+ }
+
+ @Override
+ public int visit(IASTExpression expression) {
+ if (!opInfo.isEmpty()) {
+ OperatorEqInfo info = opInfo.peek();
+ if (info.decl == null)
+ return PROCESS_CONTINUE;
+ if (expression instanceof IASTBinaryExpression) {
+ IASTBinaryExpression binary = (IASTBinaryExpression) expression;
+ if ((binary.getOperator() == IASTBinaryExpression.op_equals
+ || binary.getOperator() == IASTBinaryExpression.op_notequals)) {
+ if (referencesThis(binary.getOperand1()) && referencesParameter(info, binary.getOperand2())) {
+ info.decl = null;
+ } else if (referencesThis(binary.getOperand2())
+ && referencesParameter(info, binary.getOperand1())) {
+ info.decl = null;
+ } else {
+ reportProblem(MISS_SELF_CHECK_ID, info.decl);
+ }
+ } else {
+ reportProblem(MISS_SELF_CHECK_ID, info.decl);
+ }
+ info.decl = null;
+ } else {
+ reportProblem(MISS_SELF_CHECK_ID, info.decl);
+ info.decl = null;
+ }
+ }
+ return PROCESS_CONTINUE;
+ }
+
+ /**
+ * Checks whether expression references the parameter (directly, by pointer or by reference)
+ */
+ public boolean referencesParameter(OperatorEqInfo info, IASTNode expr) {
+ if (expr instanceof IASTIdExpression) {
+ IASTIdExpression id = (IASTIdExpression) expr;
+ if (Arrays.equals(id.getName().getSimpleID(),
+ info.parameters[0].getDeclarator().getName().getSimpleID())) {
+ return true;
+ }
+ } else if (expr instanceof ICPPASTUnaryExpression) {
+ ICPPASTUnaryExpression unExpr = (ICPPASTUnaryExpression) expr;
+ switch (unExpr.getOperator()) {
+ case IASTUnaryExpression.op_amper:
+ case IASTUnaryExpression.op_star:
+ case IASTUnaryExpression.op_bracketedPrimary:
+ return referencesParameter(info, unExpr.getOperand());
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Checks whether expression references this (directly, by pointer or by reference)
+ */
+ public boolean referencesThis(IASTNode expr) {
+ if (expr instanceof IASTLiteralExpression) {
+ IASTLiteralExpression litArg = (IASTLiteralExpression) expr;
+ if (litArg.getKind() == IASTLiteralExpression.lk_this) {
+ return true;
+ }
+ } else if (expr instanceof ICPPASTUnaryExpression) {
+ ICPPASTUnaryExpression unExpr = (ICPPASTUnaryExpression) expr;
+ switch (unExpr.getOperator()) {
+ case IASTUnaryExpression.op_amper:
+ case IASTUnaryExpression.op_star:
+ case IASTUnaryExpression.op_bracketedPrimary:
+ return referencesThis(unExpr.getOperand());
+ }
+ }
+ return false;
+ }
+
+ private ICPPMethod getOperatorEq(IASTDeclaration decl) {
+ if (decl instanceof ICPPASTFunctionDefinition) {
+ ICPPASTFunctionDefinition functionDefinition = (ICPPASTFunctionDefinition) decl;
+ if (functionDefinition.isDeleted())
+ return null;
+ IBinding binding = functionDefinition.getDeclarator().getName().resolveBinding();
+ if (binding instanceof ICPPMethod) {
+ ICPPMethod method = (ICPPMethod) binding;
+ if (OPERATOR_EQ.equals(method.getName()))
+ return method;
+ }
+ }
+ return null;
+ }
+ }
+}
diff --git a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/AssignmentOperatorCheckerTest.java b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/AssignmentOperatorCheckerTest.java
new file mode 100644
index 0000000000..3d687bd8c8
--- /dev/null
+++ b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/AssignmentOperatorCheckerTest.java
@@ -0,0 +1,191 @@
+/*******************************************************************************
+ * 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.AssignmentOperatorChecker;
+
+/**
+ * Test for {@link AssignmentOperatorChecker} class
+ */
+public class AssignmentOperatorCheckerTest extends CheckerTestCase {
+
+ public static final String MISS_REF_ID = AssignmentOperatorChecker.MISS_REF_ID;
+ public static final String MISS_SELF_ID = AssignmentOperatorChecker.MISS_SELF_CHECK_ID;
+
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+ enableProblems(MISS_REF_ID, MISS_SELF_ID);
+ }
+
+ @Override
+ public boolean isCpp() {
+ return true;
+ }
+
+ //class Foo {
+ //public:
+ //Foo& operator=(const Foo& f);
+ //};
+ //Foo& Foo::operator=(const Foo& f) {
+ // if (this != &f) {
+ // return *this;
+ // }
+ //}
+ public void testWithNoError() throws Exception {
+ loadCodeAndRun(getAboveComment());
+ checkNoErrorsOfKind(MISS_REF_ID);
+ checkNoErrorsOfKind(MISS_SELF_ID);
+ }
+
+ //class Foo {
+ //public:
+ //Foo operator=(const Foo& f);
+ //};
+ //Foo Foo::operator=(const Foo& f) {
+ // if (this != &f) {
+ // return *this;
+ // }
+ // return *this;
+ //}
+ public void testWithReturnByCopyPassByRef() throws Exception {
+ loadCodeAndRun(getAboveComment());
+ checkErrorLine(5, MISS_REF_ID);
+ checkNoErrorsOfKind(MISS_SELF_ID);
+ }
+
+ //class Foo {
+ //public:
+ //Foo operator=(Foo f);
+ //};
+ //Foo Foo::operator=(Foo f) {
+ // if (this != &f) {
+ // return *this;
+ // }
+ // return *this;
+ //}
+ public void testWithReturnByCopyPassByValue() throws Exception {
+ loadCodeAndRun(getAboveComment());
+ checkErrorLine(5, MISS_REF_ID);
+ checkNoErrorsOfKind(MISS_SELF_ID);
+ }
+
+ //class Foo {
+ //public:
+ //Foo& operator=(const Foo& f);
+ //};
+ //Foo& Foo::operator=(const Foo& f) {
+ // return *this;
+ //}
+ public void testWithNoSelfCheckPassByRef() throws Exception {
+ loadCodeAndRun(getAboveComment());
+ checkNoErrorsOfKind(MISS_REF_ID);
+ checkErrorLine(5, MISS_SELF_ID);
+ }
+
+ //class Foo {
+ //public:
+ //Foo& operator=(Foo f);
+ //};
+ //Foo& Foo::operator=(Foo f) {
+ // return *this;
+ //}
+ public void testWithNoSelfCheckPassByValue() throws Exception {
+ loadCodeAndRun(getAboveComment());
+ checkNoErrorsOfKind(MISS_REF_ID);
+ checkErrorLine(5, MISS_SELF_ID);
+ }
+
+ //class Foo {
+ //private:
+ //int p;
+ //public:
+ //Foo& operator=(const Foo& f);
+ //};
+ //Foo& Foo::operator=(const Foo& f) {
+ // if (this == &f) {
+ // return *this;
+ // }
+ // p = f.p;
+ // return *this;
+ //}
+ public void testWithEqSelfCheck() throws Exception {
+ loadCodeAndRun(getAboveComment());
+ checkNoErrorsOfKind(MISS_REF_ID);
+ checkNoErrorsOfKind(MISS_SELF_ID);
+ }
+
+ //class Foo {
+ //private:
+ //int p;
+ //public:
+ //void operator=(const int& f);
+ //};
+ //void Foo::operator=(const int& f) {
+ // p = f.p;
+ //}
+ public void testWithOpEqNoAssignment() throws Exception {
+ loadCodeAndRun(getAboveComment());
+ checkErrorLine(7, MISS_REF_ID);
+ checkNoErrorsOfKind(MISS_SELF_ID);
+ }
+
+ //class Foo {
+ // Foo& operator=(Foo&) {
+ // return *this;
+ // }
+ //};
+ public void testWithInlineOpEq() throws Exception {
+ loadCodeAndRun(getAboveComment());
+ checkErrorLine(2, MISS_SELF_ID);
+ checkNoErrorsOfKind(MISS_REF_ID);
+ }
+
+ //class Foo {
+ // class Bar {
+ // Bar& operator=(Bar&) {
+ // return *this;
+ // }
+ // }
+ //};
+ public void testWithNestedClasses() throws Exception {
+ loadCodeAndRun(getAboveComment());
+ checkErrorLine(3, MISS_SELF_ID);
+ checkNoErrorsOfKind(MISS_REF_ID);
+ }
+
+ //class Foo {
+ // Foo& operator=(Foo& a) {
+ // class Local {
+ // Local& operator=(Local& a) {
+ // return *this;
+ // }
+ // };
+ // return *this;
+ // }
+ //};
+ public void testWithLocalClass() throws Exception {
+ loadCodeAndRun(getAboveComment());
+ checkErrorLine(2, MISS_SELF_ID);
+ checkErrorLine(4, MISS_SELF_ID);
+ checkNoErrorsOfKind(MISS_REF_ID);
+ }
+
+ //class Foo {
+ // Foo& operator=(Foo&) = delete;
+ //};
+ public void testWithDeletedOpEq() throws Exception {
+ loadCodeAndRun(getAboveComment());
+ checkNoErrorsOfKind(MISS_SELF_ID);
+ checkNoErrorsOfKind(MISS_REF_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 40120ed32d..b280c1ce2f 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
@@ -16,6 +16,7 @@ package org.eclipse.cdt.codan.core.tests;
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.AssignmentOperatorCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.AssignmentToItselfCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.CStyleCastCheckerTest;
import org.eclipse.cdt.codan.core.internal.checkers.CaseBreakCheckerTest;
@@ -95,6 +96,7 @@ public class AutomatedIntegrationSuite extends TestSuite {
suite.addTestSuite(CopyrightCheckerTest.class);
suite.addTestSuite(SwitchCaseCheckerTest.class);
suite.addTestSuite(VirtualMethodCallCheckerTest.class);
+ suite.addTestSuite(AssignmentOperatorCheckerTest.class);
// framework
suite.addTest(CodanFastTestSuite.suite());
// quick fixes

Back to the top