[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

llvm-svn: 336992

5 files changed, 49 insertions, 10 deletions

diff --git a/clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp b/clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
index 254ef960e89..f1d0036c6f5 100644
--- a/clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
+++ b/clang-tools-extra/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-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h b/clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
index a6e0dfc4fe0..602856fa28f 100644
--- a/clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
+++ b/clang-tools-extra/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/clang-tools-extra/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
index cd262e2dd47..a47bb41b16b 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
+++ b/clang-tools-extra/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/clang-tools-extra/test/clang-tidy/readability-inconsistent-declaration-parameter-name-strict.cpp b/clang-tools-extra/test/clang-tidy/readability-inconsistent-declaration-parameter-name-strict.cpp
new file mode 100644
index 00000000000..90ac45fd4b4
--- /dev/null
+++ b/clang-tools-extra/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/clang-tools-extra/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp b/clang-tools-extra/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
index 43412f528fe..c98258bd931 100644
--- a/clang-tools-extra/test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
+++ b/clang-tools-extra/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*/);