From e1e1dbfe97bd981d70240684cbd8da45a3432508 Mon Sep 17 00:00:00 2001
From: Jakub Melka <Jakub.Melka@seznam.cz>
Date: Tue, 10 Mar 2015 19:35:12 +0100
Subject: [PATCH] Ticket #695: Fixed VS 2010 issue, switched to "style"
 severity, changed error messages and updated unit tests

---
 lib/check.h        |  2 +-
 lib/checkclass.cpp | 10 +++++++---
 lib/checkclass.h   |  2 +-
 test/testclass.cpp |  8 ++++----
 4 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/lib/check.h b/lib/check.h
index 64c5bc1e9..9aa4418ce 100644
--- a/lib/check.h
+++ b/lib/check.h
@@ -129,7 +129,7 @@ private:
 
     /** disabled assignment operator and copy constructor */
     void operator=(const Check &);
-    Check(const Check &) = delete;
+    explicit Check(const Check &);
 };
 
 /// @}
diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp
index b9dc6a9d7..b6febc9e3 100644
--- a/lib/checkclass.cpp
+++ b/lib/checkclass.cpp
@@ -219,7 +219,7 @@ void CheckClass::constructors()
 
 void CheckClass::checkExplicitConstructors()
 {
-    if (!_settings->isEnabled("performance"))
+    if (!_settings->isEnabled("style"))
         return;
 
     const std::size_t classes = symbolDatabase->classAndStructScopes.size();
@@ -771,12 +771,16 @@ void CheckClass::noConstructorError(const Token *tok, const std::string &classna
 
 void CheckClass::noExplicitConstructorError(const Token *tok, const std::string &classname, bool isStruct)
 {
-    reportError(tok, Severity::performance, "noExplicitConstructor", "The constructor of " + std::string(isStruct ? "struct" : "class") + " '" + classname + "' should be marked as explicit. ");
+    const std::string message(std::string(isStruct ? "Struct" : "Class") + " '" + classname + "' has a constructor with 1 argument that is not explicit.");
+    const std::string verbose(message + " Such constructors should in general be explicit for type safety reasons. Using the explicit keyword in the constructor means some mistakes when using the class can be avoided.");
+    reportError(tok, Severity::style, "noExplicitConstructor", message + "\n" + verbose);
 }
 
 void CheckClass::noExplicitCopyMoveConstructorError(const Token *tok, const std::string &classname, bool isStruct)
 {
-    reportError(tok, Severity::performance, "noExplicitCopyMoveConstructor", "The copy/move constructor of abstract " + std::string(isStruct ? "struct" : "class") + " '" + classname + "' should be marked as explicit. ");
+    const std::string message(std::string(isStruct ? "Abstract struct" : "Abstract class") + " '" + classname + "' has a copy/move constructor that is not explicit.");
+    const std::string verbose(message + " For abstract classes, even copy/move constructors may be declared explicit, as, by definition, abstract classes cannot be instantiated, and so objects of such type should never be passed by value.");
+    reportError(tok, Severity::style, "noExplicitCopyMoveConstructor", message + "\n" + verbose);
 }
 
 void CheckClass::uninitVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive)
diff --git a/lib/checkclass.h b/lib/checkclass.h
index 490f98431..d259c7788 100644
--- a/lib/checkclass.h
+++ b/lib/checkclass.h
@@ -208,7 +208,7 @@ private:
         return "Check the code for each class.\n"
                "- Missing constructors and copy constructors\n"
                //"- Missing allocation of memory in copy constructor\n"
-               "- Constructors which should be explicit are explicit.\n"
+               "- Constructors which should be explicit are explicit\n"
                "- Are all variables initialized by the constructors?\n"
                "- Are all variables assigned by 'operator='?\n"
                "- Warn if memset, memcpy etc are used on a class\n"
diff --git a/test/testclass.cpp b/test/testclass.cpp
index f9dd91f82..48c435e2d 100644
--- a/test/testclass.cpp
+++ b/test/testclass.cpp
@@ -188,7 +188,7 @@ private:
         // Clear the error log
         errout.str("");
         Settings settings;
-        settings.addEnabled("performance");
+        settings.addEnabled("style");
 
         // Tokenize..
         Tokenizer tokenizer(&settings, this);
@@ -235,21 +235,21 @@ private:
                                   "{ \n"
                                   "    Class(int i) { } \n"
                                   "};");
-        ASSERT_EQUALS("[test.cpp:3]: (performance) The constructor of class 'Class' should be marked as explicit. \n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (style) Class 'Class' has a constructor with 1 argument that is not explicit.\n", errout.str());
 
         checkExplicitConstructors("class Class \n"
                                   "{ \n"
                                   "    Class(const Class& other) { } \n"
                                   "    virtual int i() = 0; \n"
                                   "};");
-        ASSERT_EQUALS("[test.cpp:3]: (performance) The copy/move constructor of abstract class 'Class' should be marked as explicit. \n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (style) Abstract class 'Class' has a copy/move constructor that is not explicit.\n", errout.str());
 
         checkExplicitConstructors("class Class \n"
                                   "{ \n"
                                   "    Class(Class&& other) { } \n"
                                   "    virtual int i() = 0; \n"
                                   "};");
-        ASSERT_EQUALS("[test.cpp:3]: (performance) The copy/move constructor of abstract class 'Class' should be marked as explicit. \n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (style) Abstract class 'Class' has a copy/move constructor that is not explicit.\n", errout.str());
     }
 
     void checkDuplInheritedMembers(const char code[]) {