piotrdz removed rL LLVM as the repository for this revision.
piotrdz updated this revision to Diff 33527.
piotrdz added a comment.
I noticed a few things I should have done better:
- use std::set instead of std::unordered_set (it doesn't seem to be used
elsewhere)
- move checking set of reported functions to just before emitting diagnostic
- improved documentation
- changed diff directory to inside clang-tools-extra repository
http://reviews.llvm.org/D12462
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
Index: test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
===================================================================
--- test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
+++ test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
@@ -0,0 +1,20 @@
+// RUN: %python %S/check_clang_tidy.py %s readability-inconsistent-declaration-parameter-name %t
+
+void consistentFunction(int a, int b, int c);
+void consistentFunction(int a, int b, int c);
+void consistentFunction(int a, int b, int /*c*/);
+void consistentFunction(int /*c*/, int /*c*/, int /*c*/);
+
+// CHECK-MESSAGES: :[[@LINE+1]]:6: warning: function 'inconsistentFunction' has other declaration with different parameter name(s) [readability-inconsistent-declaration-parameter-name]
+void inconsistentFunction(int a, int b, int c);
+void inconsistentFunction(int d, int e, int f);
+void inconsistentFunction(int x, int y, int z);
+
+struct SomeStruct
+{
+// CHECK-MESSAGES: :[[@LINE+1]]:10: warning: function 'SomeStruct::inconsistentFunction' has other declaration with different parameter name(s) [readability-inconsistent-declaration-parameter-name]
+ void inconsistentFunction(int a);
+};
+
+void SomeStruct::inconsistentFunction(int b)
+{}
Index: docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
===================================================================
--- docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
+++ docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
@@ -0,0 +1,29 @@
+readability-inconsistent-declaration-parameter-name
+===================================================
+
+
+Find function declarations which differ in parameter names.
+
+Example:
+
+.. code:: c++
+
+ // in foo.hpp:
+ void foo(int a, int b, int c);
+
+ // in foo.cpp:
+ void foo(int d, int e, int f); // warning
+
+This check should help to enforce consistency in large projects, where it often happens
+that a definition of function is refactored, changing the parameter names, but its declaration
+in header file is not updated. With this check, we can easily find and correct such inconsistencies,
+keeping declaration and definition always in sync.
+
+Unnamed parameters are allowed and are not taken into account when comparing function declarations, for example:
+
+.. code:: c++
+
+ void foo(int a);
+ void foo(int); // no warning
+
+If there are multiple declarations of same function, only one warning will be generated.
\ No newline at end of file
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -47,6 +47,7 @@
readability-else-after-return
readability-function-size
readability-identifier-naming
+ readability-inconsistent-declaration-parameter-name
readability-named-parameter
readability-redundant-smartptr-get
readability-redundant-string-cstr
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -15,6 +15,7 @@
#include "ElseAfterReturnCheck.h"
#include "FunctionSizeCheck.h"
#include "IdentifierNamingCheck.h"
+#include "InconsistentDeclarationParameterNameCheck.h"
#include "NamedParameterCheck.h"
#include "RedundantSmartptrGetCheck.h"
#include "RedundantStringCStrCheck.h"
@@ -38,6 +39,8 @@
"readability-function-size");
CheckFactories.registerCheck<IdentifierNamingCheck>(
"readability-identifier-naming");
+ CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
+ "readability-inconsistent-declaration-parameter-name");
CheckFactories.registerCheck<readability::NamedParameterCheck>(
"readability-named-parameter");
CheckFactories.registerCheck<RedundantSmartptrGetCheck>(
Index: clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
===================================================================
--- clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
+++ clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
@@ -0,0 +1,72 @@
+//===--- InconsistentDeclarationParameterNameCheck.h - clang-tidy-------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_INCONSISTENT_DECLARATION_PARAMETER_NAME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_INCONSISTENT_DECLARATION_PARAMETER_NAME_H
+
+#include "../ClangTidy.h"
+
+#include <set>
+#include <string>
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// \brief Checks for declarations of functions which differ in parameter names
+///
+/// This check will find inconsistencies between declarations of functions,
+/// which are syntactically correct, but use different names of parameters,
+/// for example:
+///
+/// \code
+/// // foo.hpp:
+/// void foo(int a, int b, int c);
+/// // foo.cpp:
+/// void foo(int d, int e, int f) // warning
+/// { /* ... */ }
+/// \endcode
+///
+/// This check should help to enforce consistency in large projects, where it often happens
+/// that a definition of function is refactored, changing the parameter names, but its declaration
+/// in header file is not updated. With this check, we can easily find and correct such inconsistencies,
+/// keeping declaration and definition always in sync.
+///
+/// Unnamed parameters are allowed and are not taken into account when comparing function declarations, for example:
+/// \code
+/// void foo(int a);
+/// void foo(int); // no warning
+/// \endcode
+///
+/// FIXME: provide a way to extract name of unnamed parameter from a comment next to its declaration, for example:
+/// \code
+/// void foo(int a);
+/// void foo(int /*b*/); // warning
+/// \endcode
+///
+/// If there are multiple declarations of same function, only one warning will be generated.
+///
+class InconsistentDeclarationParameterNameCheck : public ClangTidyCheck {
+public:
+ InconsistentDeclarationParameterNameCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ std::set<std::string> ReportedFunctions;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_INCONSISTENT_DECLARATION_PARAMETER_NAME_H
+
Index: clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
===================================================================
--- clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
+++ clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
@@ -0,0 +1,98 @@
+//===--- InconsistentDeclarationParameterNameCheck.cpp - clang-tidy---------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "InconsistentDeclarationParameterNameCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+namespace {
+
+struct CheckResult {
+ CheckResult(bool HasInconsistentParams,
+ SourceLocation InconsistentDeclLoc = SourceLocation())
+ : HasInconsistentParams(HasInconsistentParams),
+ InconsistentDeclLoc(InconsistentDeclLoc) {}
+
+ bool HasInconsistentParams;
+ SourceLocation InconsistentDeclLoc;
+};
+
+struct InconsistentParamChecker {
+ static CheckResult checkForInconsitentDeclarationParameters(const FunctionDecl* FunctionDeclaration);
+};
+
+} // anonymous namespace
+
+void InconsistentDeclarationParameterNameCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ functionDecl(
+ unless(anyOf(
+ isExpansionInSystemHeader(),
+ isImplicit())))
+ .bind("functionDecl"), this);
+}
+
+void InconsistentDeclarationParameterNameCheck::check(const MatchFinder::MatchResult &Result) {
+ auto* FunctionDeclaration = Result.Nodes.getNodeAs<FunctionDecl>("functionDecl");
+ if (FunctionDeclaration == nullptr)
+ return;
+
+ auto CheckResult = InconsistentParamChecker::checkForInconsitentDeclarationParameters(FunctionDeclaration);
+ if (CheckResult.HasInconsistentParams) {
+ auto QualName = FunctionDeclaration->getQualifiedNameAsString();
+ if (ReportedFunctions.count(QualName) > 0)
+ return; // avoid multiple warnings
+
+ diag(FunctionDeclaration->getLocation(), "function '%0' has other declaration with different parameter name(s)")
+ << QualName;
+ diag(CheckResult.InconsistentDeclLoc, "other declaration seen here", DiagnosticIDs::Level::Note);
+ ReportedFunctions.insert(QualName);
+ }
+}
+
+CheckResult InconsistentParamChecker::checkForInconsitentDeclarationParameters(const FunctionDecl* FunctionDeclaration) {
+ auto Location = FunctionDeclaration->getLocation();
+
+ for (const auto& OtherDeclaration : FunctionDeclaration->redecls()) {
+ if (OtherDeclaration->getLocation() == Location) // skip self
+ continue;
+
+ auto MyParamIt = FunctionDeclaration->param_begin();
+ auto OtherParamIt = OtherDeclaration->param_begin();
+
+ while (MyParamIt != FunctionDeclaration->param_end() &&
+ OtherParamIt != OtherDeclaration->param_end()) {
+ auto MyParamName = (*MyParamIt)->getName();
+ auto OtherParamName = (*OtherParamIt)->getName();
+
+ if (!MyParamName.empty() &&
+ !OtherParamName.empty() &&
+ MyParamName != OtherParamName) {
+ return CheckResult{true, OtherDeclaration->getLocation()};
+ }
+
+ ++MyParamIt;
+ ++OtherParamIt;
+ }
+ }
+
+ return CheckResult{false};
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
Index: clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tidy/readability/CMakeLists.txt
+++ clang-tidy/readability/CMakeLists.txt
@@ -6,6 +6,7 @@
ElseAfterReturnCheck.cpp
FunctionSizeCheck.cpp
IdentifierNamingCheck.cpp
+ InconsistentDeclarationParameterNameCheck.cpp
NamedParameterCheck.cpp
NamespaceCommentCheck.cpp
ReadabilityTidyModule.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
