summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSam McCall <sam.mccall@gmail.com>2018-07-13 11:41:56 +0000
committerSam McCall <sam.mccall@gmail.com>2018-07-13 11:41:56 +0000
commitc7373d4e35ecc1b55d67c1411bc78a3ac0366dfd (patch)
tree7ee186eaa66fafb7580d6c1d76022081b60dbcd5
parente44dd1e4637868d931334cb5a971ed25856811ae (diff)
[clang-tidy] readability-inconsistent-declaration-parameter-name: accept approximate name matches.
Summary: The goal is to reduce false positives when the difference is intentional, like: foo(StringRef name); foo(StringRef name_ref) { string name = cleanup(name_ref); ... } Or semantically unimportant, like: foo(StringRef full_name); foo(StringRef name) { ... } There are other matching names we won't recognise (e.g. syns vs synonyms) but this catches many that we see in practice, and gives people a systematic workaround. The old behavior is available as a 'Strict' option. Subscribers: xazax.hun, cfe-commits Differential Revision: https://reviews.llvm.org/D49285 git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@336992 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r--clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp28
-rw-r--r--clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h4
-rw-r--r--docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst14
-rw-r--r--test/clang-tidy/readability-inconsistent-declaration-parameter-name-strict.cpp11
-rw-r--r--test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp2
5 files changed, 49 insertions, 10 deletions
diff --git a/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp b/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
index 254ef960..f1d0036c 100644
--- a/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
+++ b/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
@@ -90,10 +90,20 @@ bool checkIfFixItHintIsApplicable(
return true;
}
+bool nameMatch(StringRef L, StringRef R, bool Strict) {
+ if (Strict)
+ return L.empty() || R.empty() || L == R;
+ // We allow two names if one is a prefix/suffix of the other, ignoring case.
+ // Important special case: this is true if either parameter has no name!
+ return L.startswith_lower(R) || R.startswith_lower(L) ||
+ L.endswith_lower(R) || R.endswith_lower(L);
+}
+
DifferingParamsContainer
findDifferingParamsInDeclaration(const FunctionDecl *ParameterSourceDeclaration,
const FunctionDecl *OtherDeclaration,
- const FunctionDecl *OriginalDeclaration) {
+ const FunctionDecl *OriginalDeclaration,
+ bool Strict) {
DifferingParamsContainer DifferingParams;
auto SourceParamIt = ParameterSourceDeclaration->param_begin();
@@ -106,8 +116,7 @@ findDifferingParamsInDeclaration(const FunctionDecl *ParameterSourceDeclaration,
// FIXME: Provide a way to extract commented out parameter name from comment
// next to it.
- if (!SourceParamName.empty() && !OtherParamName.empty() &&
- SourceParamName != OtherParamName) {
+ if (!nameMatch(SourceParamName, OtherParamName, Strict)) {
SourceRange OtherParamNameRange =
DeclarationNameInfo((*OtherParamIt)->getDeclName(),
(*OtherParamIt)->getLocation())
@@ -128,9 +137,9 @@ findDifferingParamsInDeclaration(const FunctionDecl *ParameterSourceDeclaration,
}
InconsistentDeclarationsContainer
-findInconsitentDeclarations(const FunctionDecl *OriginalDeclaration,
+findInconsistentDeclarations(const FunctionDecl *OriginalDeclaration,
const FunctionDecl *ParameterSourceDeclaration,
- SourceManager &SM) {
+ SourceManager &SM, bool Strict) {
InconsistentDeclarationsContainer InconsistentDeclarations;
SourceLocation ParameterSourceLocation =
ParameterSourceDeclaration->getLocation();
@@ -141,7 +150,7 @@ findInconsitentDeclarations(const FunctionDecl *OriginalDeclaration,
DifferingParamsContainer DifferingParams =
findDifferingParamsInDeclaration(ParameterSourceDeclaration,
OtherDeclaration,
- OriginalDeclaration);
+ OriginalDeclaration, Strict);
if (!DifferingParams.empty()) {
InconsistentDeclarations.emplace_back(OtherDeclaration->getLocation(),
std::move(DifferingParams));
@@ -284,6 +293,7 @@ void formatDiagnostics(
void InconsistentDeclarationParameterNameCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+ Options.store(Opts, "Strict", Strict);
}
void InconsistentDeclarationParameterNameCheck::registerMatchers(
@@ -305,9 +315,9 @@ void InconsistentDeclarationParameterNameCheck::check(
getParameterSourceDeclaration(OriginalDeclaration);
InconsistentDeclarationsContainer InconsistentDeclarations =
- findInconsitentDeclarations(OriginalDeclaration,
- ParameterSourceDeclaration,
- *Result.SourceManager);
+ findInconsistentDeclarations(OriginalDeclaration,
+ ParameterSourceDeclaration,
+ *Result.SourceManager, Strict);
if (InconsistentDeclarations.empty()) {
// Avoid unnecessary further visits.
markRedeclarationsAsVisited(OriginalDeclaration);
diff --git a/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h b/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
index a6e0dfc4..602856fa 100644
--- a/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
+++ b/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
@@ -28,7 +28,8 @@ public:
InconsistentDeclarationParameterNameCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
+ IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0),
+ Strict(Options.getLocalOrGlobal("Strict", 0) != 0) {}
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
@@ -39,6 +40,7 @@ private:
llvm::DenseSet<const FunctionDecl *> VisitedDeclarations;
const bool IgnoreMacros;
+ const bool Strict;
};
} // namespace readability
diff --git a/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst b/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
index cd262e2d..a47bb41b 100644
--- a/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
+++ b/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
@@ -29,6 +29,15 @@ function declarations, for example:
void foo(int a);
void foo(int); // no warning
+One name is also allowed to be a case-insensitive prefix/suffix of the other:
+
+.. code-block:: c++
+
+ void foo(int count);
+ void foo(int count_input) { // no warning
+ int count = adjustCount(count_input);
+ }
+
To help with refactoring, in some cases fix-it hints are generated to align
parameter names to a single naming convention. This works with the assumption
that the function definition is the most up-to-date version, as it directly
@@ -47,3 +56,8 @@ the definition or the first declaration seen in a translation unit.
If this option is set to non-zero (default is `1`), the check will not warn
about names declared inside macros.
+
+.. option:: Strict
+
+ If this option is set to non-zero (default is `0`), then names must match
+ exactly (or be absent).
diff --git a/test/clang-tidy/readability-inconsistent-declaration-parameter-name-strict.cpp b/test/clang-tidy/readability-inconsistent-declaration-parameter-name-strict.cpp
new file mode 100644
index 00000000..90ac45fd
--- /dev/null
+++ b/test/clang-tidy/readability-inconsistent-declaration-parameter-name-strict.cpp
@@ -0,0 +1,11 @@
+// RUN: %check_clang_tidy %s readability-inconsistent-declaration-parameter-name %t -- \
+// RUN: -config="{CheckOptions: [{key: readability-inconsistent-declaration-parameter-name.Strict, value: 1}]}" \
+// RUN: -- -std=c++11
+
+void inconsistentFunction(int a, int b, int c);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'inconsistentFunction' has 1 other declaration with different parameter names
+void inconsistentFunction(int prefixA, int b, int cSuffix);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: note: the 1st inconsistent declaration seen here
+// CHECK-MESSAGES: :[[@LINE-2]]:6: note: differing parameters are named here: ('prefixA', 'cSuffix'), in the other declaration: ('a', 'c')
+void inconsistentFunction(int a, int b, int c);
+void inconsistentFunction(int /*c*/, int /*c*/, int /*c*/);
diff --git a/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp b/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
index 43412f52..c98258bd 100644
--- a/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
+++ b/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
@@ -2,6 +2,8 @@
void consistentFunction(int a, int b, int c);
void consistentFunction(int a, int b, int c);
+void consistentFunction(int prefixA, int b, int cSuffix);
+void consistentFunction(int a, int b, int c);
void consistentFunction(int a, int b, int /*c*/);
void consistentFunction(int /*c*/, int /*c*/, int /*c*/);