mgartmann updated this revision to Diff 341155.
mgartmann added a comment.
Replaced string comparison to check if a character is a space with
`std::isspace()`.
Added test case for this scenario.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100972/new/
https://reviews.llvm.org/D100972
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
@@ -2,20 +2,33 @@
int nonConstInt = 0;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstInt = 0;
int &nonConstIntReference = nonConstInt;
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: variable 'nonConstIntReference' provides global access to a non-const object; consider making the referenced data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int &nonConstIntReference = nonConstInt;
int *pointerToNonConstInt = &nonConstInt;
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: variable 'pointerToNonConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
// CHECK-MESSAGES: :[[@LINE-2]]:6: warning: variable 'pointerToNonConstInt' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const pointerToNonConstInt = &nonConstInt;
+
+// clang-format off
+// because the space after * is intended.
+int * pointerToNonConstIntWithSpace = &nonConstInt;
+// clang-format on
+// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: variable 'pointerToNonConstIntWithSpace' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-MESSAGES: :[[@LINE-3]]:7: warning: variable 'pointerToNonConstIntWithSpace' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const pointerToNonConstIntWithSpace = &nonConstInt;
int *const constPointerToNonConstInt = &nonConstInt;
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'constPointerToNonConstInt' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const constPointerToNonConstInt = &nonConstInt;
namespace namespace_name {
int nonConstNamespaceInt = 0;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstNamespaceInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstNamespaceInt = 0;
const int constNamespaceInt = 0;
} // namespace namespace_name
@@ -24,6 +37,7 @@
const int *pointerToConstInt = &constInt;
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'pointerToConstInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const pointerToConstInt = &constInt;
const int *const constPointerToConstInt = &constInt;
@@ -39,6 +53,7 @@
namespace {
int nonConstAnonymousNamespaceInt = 0;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'nonConstAnonymousNamespaceInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int nonConstAnonymousNamespaceInt = 0;
} // namespace
class DummyClass {
@@ -53,27 +68,35 @@
DummyClass nonConstClassInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'nonConstClassInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyClass nonConstClassInstance;
DummyClass *pointerToNonConstDummyClass = &nonConstClassInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'pointerToNonConstDummyClass' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: variable 'pointerToNonConstDummyClass' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyClass *const pointerToNonConstDummyClass = &nonConstClassInstance;
DummyClass &referenceToNonConstDummyClass = nonConstClassInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'referenceToNonConstDummyClass' provides global access to a non-const object; consider making the referenced data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyClass &referenceToNonConstDummyClass = nonConstClassInstance;
int *nonConstPointerToMember = &nonConstClassInstance.nonConstPublicMemberVariable;
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: variable 'nonConstPointerToMember' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
// CHECK-MESSAGES: :[[@LINE-2]]:6: warning: variable 'nonConstPointerToMember' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const nonConstPointerToMember = &nonConstClassInstance.nonConstPublicMemberVariable;
+
int *const constPointerToNonConstMember = &nonConstClassInstance.nonConstPublicMemberVariable;
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'constPointerToNonConstMember' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const int *const constPointerToNonConstMember = &nonConstClassInstance.nonConstPublicMemberVariable;
const DummyClass constClassInstance;
DummyClass *const constPointerToNonConstDummyClass = &nonConstClassInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: variable 'constPointerToNonConstDummyClass' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyClass *const constPointerToNonConstDummyClass = &nonConstClassInstance;
const DummyClass *nonConstPointerToConstDummyClass = &constClassInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: variable 'nonConstPointerToConstDummyClass' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyClass *const nonConstPointerToConstDummyClass = &constClassInstance;
const DummyClass *const constPointerToConstDummyClass = &constClassInstance;
@@ -84,6 +107,7 @@
namespace namespace_name {
DummyClass nonConstNamespaceClassInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'nonConstNamespaceClassInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyClass nonConstNamespaceClassInstance;
const DummyClass constDummyClassInstance;
} // namespace namespace_name
@@ -96,21 +120,26 @@
DummyEnum nonConstDummyEnumInstance = DummyEnum::first;
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: variable 'nonConstDummyEnumInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyEnum nonConstDummyEnumInstance = DummyEnum::first;
DummyEnum *pointerToNonConstDummyEnum = &nonConstDummyEnumInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'pointerToNonConstDummyEnum' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: variable 'pointerToNonConstDummyEnum' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyEnum *const pointerToNonConstDummyEnum = &nonConstDummyEnumInstance;
DummyEnum &referenceToNonConstDummyEnum = nonConstDummyEnumInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'referenceToNonConstDummyEnum' provides global access to a non-const object; consider making the referenced data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyEnum &referenceToNonConstDummyEnum = nonConstDummyEnumInstance;
DummyEnum *const constPointerToNonConstDummyEnum = &nonConstDummyEnumInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: variable 'constPointerToNonConstDummyEnum' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyEnum *const constPointerToNonConstDummyEnum = &nonConstDummyEnumInstance;
const DummyEnum constDummyEnumInstance = DummyEnum::first;
const DummyEnum *nonConstPointerToConstDummyEnum = &constDummyEnumInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: variable 'nonConstPointerToConstDummyEnum' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyEnum *const nonConstPointerToConstDummyEnum = &constDummyEnumInstance;
const DummyEnum *const constPointerToConstDummyEnum = &constDummyEnumInstance;
@@ -119,6 +148,7 @@
namespace namespace_name {
DummyEnum nonConstNamespaceEnumInstance = DummyEnum::first;
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: variable 'nonConstNamespaceEnumInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyEnum nonConstNamespaceEnumInstance = DummyEnum::first;
const DummyEnum constNamespaceEnumInstance = DummyEnum::first;
} // namespace namespace_name
@@ -127,33 +157,53 @@
DummyEnum nonConstAnonymousNamespaceEnumInstance = DummyEnum::first;
}
// CHECK-MESSAGES: :[[@LINE-2]]:11: warning: variable 'nonConstAnonymousNamespaceEnumInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyEnum nonConstAnonymousNamespaceEnumInstance = DummyEnum::first;
// CHECKING FOR NON-CONST GLOBAL STRUCT ///////////////////////////////////////
+struct {
+ // CHECK-FIXES: const struct {
+public:
+ int structIntElement = 0;
+ const int constStructIntElement = 0;
+
+private:
+ int privateStructIntElement = 0;
+} nonConstUnnamedStructInstance{};
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: variable 'nonConstUnnamedStructInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+
struct DummyStruct {
+ // CHECK-FIXES: const struct DummyStruct {
public:
int structIntElement = 0;
const int constStructIntElement = 0;
private:
int privateStructIntElement = 0;
-};
+} directNonConstDummyStructInstance{};
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: variable 'directNonConstDummyStructInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
DummyStruct nonConstDummyStructInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'nonConstDummyStructInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyStruct nonConstDummyStructInstance;
DummyStruct *pointerToNonConstDummyStruct = &nonConstDummyStructInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: variable 'pointerToNonConstDummyStruct' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
// CHECK-MESSAGES: :[[@LINE-2]]:14: warning: variable 'pointerToNonConstDummyStruct' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyStruct *const pointerToNonConstDummyStruct = &nonConstDummyStructInstance;
DummyStruct &referenceToNonConstDummyStruct = nonConstDummyStructInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: variable 'referenceToNonConstDummyStruct' provides global access to a non-const object; consider making the referenced data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyStruct &referenceToNonConstDummyStruct = nonConstDummyStructInstance;
+
DummyStruct *const constPointerToNonConstDummyStruct = &nonConstDummyStructInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: variable 'constPointerToNonConstDummyStruct' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyStruct *const constPointerToNonConstDummyStruct = &nonConstDummyStructInstance;
const DummyStruct constDummyStructInstance;
const DummyStruct *nonConstPointerToConstDummyStruct = &constDummyStructInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: variable 'nonConstPointerToConstDummyStruct' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyStruct *const nonConstPointerToConstDummyStruct = &constDummyStructInstance;
const DummyStruct *const constPointerToConstDummyStruct = &constDummyStructInstance;
@@ -162,6 +212,7 @@
namespace namespace_name {
DummyStruct nonConstNamespaceDummyStructInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'nonConstNamespaceDummyStructInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyStruct nonConstNamespaceDummyStructInstance;
const DummyStruct constNamespaceDummyStructInstance;
} // namespace namespace_name
@@ -170,6 +221,7 @@
DummyStruct nonConstAnonymousNamespaceStructInstance;
}
// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: variable 'nonConstAnonymousNamespaceStructInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyStruct nonConstAnonymousNamespaceStructInstance;
// CHECKING FOR NON-CONST GLOBAL UNION ////////////////////////////////////////
union DummyUnion {
@@ -179,21 +231,26 @@
DummyUnion nonConstUnionIntInstance = {0x0};
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'nonConstUnionIntInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyUnion nonConstUnionIntInstance = {0x0};
DummyUnion *nonConstPointerToNonConstUnionInt = &nonConstUnionIntInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'nonConstPointerToNonConstUnionInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: variable 'nonConstPointerToNonConstUnionInt' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyUnion *const nonConstPointerToNonConstUnionInt = &nonConstUnionIntInstance;
DummyUnion *const constPointerToNonConstUnionInt = &nonConstUnionIntInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: variable 'constPointerToNonConstUnionInt' provides global access to a non-const object; consider making the pointed-to data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyUnion *const constPointerToNonConstUnionInt = &nonConstUnionIntInstance;
DummyUnion &referenceToNonConstUnionInt = nonConstUnionIntInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'referenceToNonConstUnionInt' provides global access to a non-const object; consider making the referenced data 'const' [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyUnion &referenceToNonConstUnionInt = nonConstUnionIntInstance;
const DummyUnion constUnionIntInstance = {0x0};
const DummyUnion *nonConstPointerToConstUnionInt = &constUnionIntInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: variable 'nonConstPointerToConstUnionInt' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyUnion *const nonConstPointerToConstUnionInt = &constUnionIntInstance;
const DummyUnion *const constPointerToConstUnionInt = &constUnionIntInstance;
@@ -202,6 +259,7 @@
namespace namespace_name {
DummyUnion nonConstNamespaceDummyUnionInstance;
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'nonConstNamespaceDummyUnionInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyUnion nonConstNamespaceDummyUnionInstance;
const DummyUnion constNamespaceDummyUnionInstance = {0x0};
} // namespace namespace_name
@@ -210,6 +268,7 @@
DummyUnion nonConstAnonymousNamespaceUnionInstance = {0x0};
}
// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: variable 'nonConstAnonymousNamespaceUnionInstance' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const DummyUnion nonConstAnonymousNamespaceUnionInstance = {0x0};
// CHECKING FOR NON-CONST GLOBAL FUNCTION POINTER /////////////////////////////
int dummyFunction() {
@@ -219,10 +278,12 @@
typedef int (*functionPointer)();
functionPointer fp1 = &dummyFunction;
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: variable 'fp1' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const functionPointer fp1 = &dummyFunction;
typedef int (*const functionConstPointer)();
functionPointer fp2 = &dummyFunction;
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: variable 'fp2' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: const functionPointer fp2 = &dummyFunction;
// CHECKING FOR NON-CONST GLOBAL TEMPLATE VARIABLE ////////////////////////////
template <class T>
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst
@@ -36,3 +36,6 @@
``c_reference``, will all generate warnings since they are either:
a globally accessible variable and non-const, a pointer or reference providing
global access to non-const data or both.
+
+The check also provides fixes that make the problematic variables, pointers and
+references constant.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -118,6 +118,12 @@
function or assignment to ``nullptr``.
Added support for pointers to ``std::unique_ptr``.
+- Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
+ <clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables` check.
+
+ Added ``FixItHints`` to the checks which make the problematic variables, pointers
+ and references constant.
+
Removed checks
^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h
@@ -10,6 +10,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDNONCONSTGLOBALVARIABLESCHECK_H
#include "../ClangTidyCheck.h"
+#include <string>
namespace clang {
namespace tidy {
@@ -21,6 +22,18 @@
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.html
class AvoidNonConstGlobalVariablesCheck : public ClangTidyCheck {
+ std::string generateReplacementString(
+ const ast_matchers::MatchFinder::MatchResult &Result,
+ const VarDecl &Variablee) const;
+
+ bool hasSpaceAfterType(const ast_matchers::MatchFinder::MatchResult &Result,
+ const VarDecl &Variable,
+ const std::string &NonConstType) const;
+
+ CharSourceRange generateReplacementRange(const VarDecl &Variable) const;
+
+ std::string cleanType(const QualType &Type) const;
+
public:
AvoidNonConstGlobalVariablesCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
@@ -10,6 +10,8 @@
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+#include <locale>
using namespace clang::ast_matchers;
@@ -49,9 +51,15 @@
if (const auto *Variable =
Result.Nodes.getNodeAs<VarDecl>("non-const_variable")) {
+
+ auto ReplacementRange = generateReplacementRange(*Variable);
+ auto Replacement = generateReplacementString(Result, *Variable);
+
diag(Variable->getLocation(), "variable %0 is non-const and globally "
"accessible, consider making it const")
- << Variable; // FIXME: Add fix-it hint to Variable
+ << Variable
+ << FixItHint::CreateReplacement(ReplacementRange, Replacement);
+
// Don't return early, a non-const variable may also be a pointer or
// reference to non-const data.
}
@@ -61,11 +69,89 @@
diag(VD->getLocation(),
"variable %0 provides global access to a non-const object; consider "
"making the %select{referenced|pointed-to}1 data 'const'")
- << VD
- << VD->getType()->isPointerType(); // FIXME: Add fix-it hint to Variable
+ << VD << VD->getType()->isPointerType()
+ << FixItHint::CreateInsertion(VD->getBeginLoc(), "const ");
}
}
+/// Makes the provided type constant and converts it to a string.
+/// If the current type sticks to the variable name as in the example below, a
+/// space is inserted:
+//
+/// \code
+/// int *y = &x;
+/// ^^
+/// \endcode
+///
+/// \returns string representation of the constant type of \p Variable.
+std::string AvoidNonConstGlobalVariablesCheck::generateReplacementString(
+ const MatchFinder::MatchResult &Result, const VarDecl &Variable) const {
+
+ auto Type = Variable.getType();
+ bool HasSpace = hasSpaceAfterType(Result, Variable, cleanType(Type));
+ Type.addConst();
+ return cleanType(Type) + (HasSpace ? "" : " ");
+}
+
+bool AvoidNonConstGlobalVariablesCheck::hasSpaceAfterType(
+ const MatchFinder::MatchResult &Result, const VarDecl &Variable,
+ const std::string &NonConstType) const {
+
+ StringRef VariableText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Variable.getSourceRange()),
+ *Result.SourceManager, getLangOpts());
+
+ /// Check to make the function error-robust in case \c NonConstType.length()
+ /// exceeds \c VariableText length. This would occure if the \c PrintingPolicy
+ /// used in \c printCleanedType did not remove all superfluous type
+ /// information. As a fallback, it is assumed that the type is not followed by
+ /// a space in the source code.
+ if (NonConstType.length() > VariableText.str().length()) {
+ llvm::errs() << "Checking for space failed: the type's effective "
+ "length is greater than the variable declaration.";
+ return false;
+ }
+
+ return std::isspace(VariableText.str().at(NonConstType.length()));
+}
+
+CharSourceRange AvoidNonConstGlobalVariablesCheck::generateReplacementRange(
+ const VarDecl &Variable) const {
+
+ auto TypeBeginLoc = Variable.getBeginLoc();
+
+ auto TypeEndLoc =
+ TypeBeginLoc.getLocWithOffset(cleanType(Variable.getType()).length());
+
+ return CharSourceRange::getCharRange(TypeBeginLoc, TypeEndLoc);
+}
+
+/// Creates a string representation of \p Type and suppresses the \c class
+/// keyword in a class instance's type. Also, on unnamed/anonymous types, the
+/// tag location and the \c unnamed or \c anonymous keyword are removed from the
+/// type description. If this would not be done, those keywords would be
+/// inserted into the source code as part of the \c FixItHint replacement.
+std::string
+AvoidNonConstGlobalVariablesCheck::cleanType(const QualType &Type) const {
+
+ /// \c PrintingPolicy suppresses the "class" keyword in a class
+ /// instance's type and locations of anonymous tags.
+ PrintingPolicy PrintingPolicy{getLangOpts()};
+ PrintingPolicy.AnonymousTagLocations = false;
+ std::string PolicyCleanedType = Type.getAsString(PrintingPolicy);
+
+ auto StringsToErase = SmallVector<std::string>{" (unnamed)", " (anonymous)"};
+
+ for (std::string StringToErase : StringsToErase) {
+ size_t StartPositionToErase = PolicyCleanedType.find(StringToErase);
+ if (StartPositionToErase == std::string::npos)
+ continue;
+ PolicyCleanedType.erase(StartPositionToErase, StringToErase.length());
+ }
+
+ return PolicyCleanedType;
+}
+
} // namespace cppcoreguidelines
} // namespace tidy
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
