[clang-tools-extra] r213075 - [clang-tidy] Add a checker that flags unnamed parameters.
Benjamin Kramer
benny.kra at googlemail.com
Tue Jul 15 09:47:12 PDT 2014
Author: d0k
Date: Tue Jul 15 11:47:09 2014
New Revision: 213075
URL: http://llvm.org/viewvc/llvm-project?rev=213075&view=rev
Log:
[clang-tidy] Add a checker that flags unnamed parameters.
Summary:
We still allow the escape hatch foo(int /*x*/) and also suggest this
in a fixit. This is more powerful than the corresponding cpplint.py check
it also flags functions with multiple arguments as naming all arguments is
recommended by the google style guide.
Reviewers: alexfh, djasper
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D4518
Added:
clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.cpp
clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.h
clang-tools-extra/trunk/test/clang-tidy/google-readability-function.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp
Modified: clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt?rev=213075&r1=213074&r2=213075&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt Tue Jul 15 11:47:09 2014
@@ -5,6 +5,7 @@ add_clang_library(clangTidyGoogleModule
ExplicitConstructorCheck.cpp
ExplicitMakePairCheck.cpp
GoogleTidyModule.cpp
+ NamedParameterCheck.cpp
OverloadedUnaryAndCheck.cpp
LINK_LIBS
Modified: clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp?rev=213075&r1=213074&r2=213075&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp Tue Jul 15 11:47:09 2014
@@ -13,6 +13,7 @@
#include "AvoidCStyleCastsCheck.h"
#include "ExplicitConstructorCheck.h"
#include "ExplicitMakePairCheck.h"
+#include "NamedParameterCheck.h"
#include "OverloadedUnaryAndCheck.h"
using namespace clang::ast_matchers;
@@ -35,6 +36,9 @@ public:
CheckFactories.addCheckFactory(
"google-readability-casting",
new ClangTidyCheckFactory<readability::AvoidCStyleCastsCheck>());
+ CheckFactories.addCheckFactory(
+ "google-readability-function",
+ new ClangTidyCheckFactory<readability::NamedParameterCheck>());
}
};
Added: clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.cpp?rev=213075&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.cpp Tue Jul 15 11:47:09 2014
@@ -0,0 +1,98 @@
+//===--- NamedParameterCheck.cpp - clang-tidy -------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "NamedParameterCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/AST/ASTContext.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void NamedParameterCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
+ Finder->addMatcher(
+ functionDecl(
+ unless(hasAncestor(decl(
+ anyOf(recordDecl(ast_matchers::isTemplateInstantiation()),
+ functionDecl(ast_matchers::isTemplateInstantiation()))))))
+ .bind("decl"),
+ this);
+}
+
+void NamedParameterCheck::check(const MatchFinder::MatchResult &Result) {
+ const SourceManager &SM = *Result.SourceManager;
+ const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("decl");
+ SmallVector<std::pair<const FunctionDecl *, unsigned>, 4> UnnamedParams;
+
+ // Ignore implicitly generated members.
+ if (Function->isImplicit())
+ return;
+
+ // TODO: Handle overloads.
+ // TODO: We could check that all redeclarations use the same name for
+ // arguments in the same position.
+ for (unsigned I = 0, E = Function->getNumParams(); I != E; ++I) {
+ const ParmVarDecl *Parm = Function->getParamDecl(I);
+ // Look for unnamed parameters.
+ if (!Parm->getName().empty())
+ continue;
+
+ // Sanity check the source locations.
+ if (!Parm->getLocation().isValid() || Parm->getLocation().isMacroID() ||
+ !SM.isWrittenInSameFile(Parm->getLocStart(), Parm->getLocation()))
+ continue;
+
+ // Look for comments. We explicitly want to allow idioms like
+ // void foo(int /*unused*/)
+ const char *Begin = SM.getCharacterData(Parm->getLocStart());
+ const char *End = SM.getCharacterData(Parm->getLocation());
+ StringRef Data(Begin, End - Begin);
+ if (Data.find("/*") != StringRef::npos)
+ continue;
+
+ UnnamedParams.push_back(std::make_pair(Function, I));
+ }
+
+ // Emit only one warning per function but fixits for all unnamed parameters.
+ if (!UnnamedParams.empty()) {
+ const ParmVarDecl *FirstParm =
+ UnnamedParams.front().first->getParamDecl(UnnamedParams.front().second);
+ auto D = diag(FirstParm->getLocation(),
+ "all parameters should be named in a function");
+
+ for (auto P : UnnamedParams) {
+ // If the method is overridden, try to copy the name from the base method
+ // into the overrider.
+ const ParmVarDecl *Parm = P.first->getParamDecl(P.second);
+ const auto *M = dyn_cast<CXXMethodDecl>(P.first);
+ if (M && M->size_overridden_methods() > 0) {
+ const ParmVarDecl *OtherParm =
+ (*M->begin_overridden_methods())->getParamDecl(P.second);
+ std::string Name = OtherParm->getNameAsString();
+ if (!Name.empty()) {
+ D << FixItHint::CreateInsertion(Parm->getLocation(),
+ " /*" + Name + "*/");
+ continue;
+ }
+ }
+
+ // Otherwise just insert an unused marker. Note that getLocation() points
+ // to the place where the name would be, this allows us to also get
+ // complex cases like function pointers right.
+ D << FixItHint::CreateInsertion(Parm->getLocation(), " /*unused*/");
+ }
+ }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.h?rev=213075&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.h Tue Jul 15 11:47:09 2014
@@ -0,0 +1,32 @@
+//===--- NamedParameterCheck.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_GOOGLE_NAMED_PARAMETER_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NAMED_PARAMETER_CHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// \brief Find functions with unnamed arguments.
+///
+/// Corresponding cpplint.py check name: 'readability/function'.
+class NamedParameterCheck : public ClangTidyCheck {
+public:
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NAMED_PARAMETER_CHECK_H
Added: clang-tools-extra/trunk/test/clang-tidy/google-readability-function.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-function.cpp?rev=213075&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/google-readability-function.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/google-readability-function.cpp Tue Jul 15 11:47:09 2014
@@ -0,0 +1,72 @@
+// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s google-readability-function %t
+// REQUIRES: shell
+
+void Method(char *) { /* */ }
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: all parameters should be named in a function
+// CHECK-FIXES: void Method(char * /*unused*/) { /* */ }
+void Method2(char *);
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in a function
+// CHECK-FIXES: void Method2(char * /*unused*/);
+void Method3(char *, void *);
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in a function
+// CHECK-FIXES: void Method3(char * /*unused*/, void * /*unused*/);
+void Method4(char *, int /*unused*/);
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in a function
+// CHECK-FIXES: void Method4(char * /*unused*/, int /*unused*/);
+void operator delete[](void *) throw();
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: all parameters should be named in a function
+// CHECK-FIXES: void operator delete[](void * /*unused*/) throw();
+int Method5(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: all parameters should be named in a function
+// CHECK-FIXES: int Method5(int /*unused*/);
+void Method6(void (*)(void *)) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: all parameters should be named in a function
+// CHECK-FIXES: void Method6(void (* /*unused*/)(void *)) {}
+template <typename T> void Method7(T);
+// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: all parameters should be named in a function
+// CHECK-FIXES: template <typename T> void Method7(T /*unused*/);
+
+// Don't warn in macros.
+#define M void MethodM(int);
+M
+
+void operator delete(void *x) throw();
+void Method7(char * /*x*/) {}
+void Method8(char *x);
+typedef void (*TypeM)(int x);
+void operator delete[](void *x) throw();
+void operator delete[](void * /*x*/) throw();
+
+struct X {
+ X operator++(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: all parameters should be named in a function
+// CHECK-FIXES: X operator++(int /*unused*/);
+ X operator--(int /*unused*/);
+
+ const int &i;
+};
+
+void (*Func1)(void *);
+void Func2(void (*func)(void *)) {}
+template <void Func(void *)> void Func3();
+
+template <typename T>
+struct Y {
+ void foo(T);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: all parameters should be named in a function
+// CHECK-FIXES: void foo(T /*unused*/);
+};
+
+Y<int> y;
+Y<float> z;
+
+struct Base {
+ virtual void foo(bool notThisOne);
+ virtual void foo(int argname);
+};
+
+struct Derived : public Base {
+ void foo(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: all parameters should be named in a function
+// CHECK-FIXES: void foo(int /*argname*/);
+};
More information about the cfe-commits
mailing list