Hi alexfh, djasper,

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.

http://reviews.llvm.org/D4518

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/GoogleTidyModule.cpp
  clang-tidy/google/NamedParameterCheck.cpp
  clang-tidy/google/NamedParameterCheck.h
  test/clang-tidy/google-readability-function.cpp
Index: clang-tidy/google/CMakeLists.txt
===================================================================
--- clang-tidy/google/CMakeLists.txt
+++ clang-tidy/google/CMakeLists.txt
@@ -5,6 +5,7 @@
   ExplicitConstructorCheck.cpp
   ExplicitMakePairCheck.cpp
   GoogleTidyModule.cpp
+  NamedParameterCheck.cpp
   OverloadedUnaryAndCheck.cpp
 
   LINK_LIBS
Index: clang-tidy/google/GoogleTidyModule.cpp
===================================================================
--- clang-tidy/google/GoogleTidyModule.cpp
+++ clang-tidy/google/GoogleTidyModule.cpp
@@ -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 @@
     CheckFactories.addCheckFactory(
         "google-readability-casting",
         new ClangTidyCheckFactory<readability::AvoidCStyleCastsCheck>());
+    CheckFactories.addCheckFactory(
+        "google-readability-function",
+        new ClangTidyCheckFactory<readability::NamedParameterCheck>());
   }
 };
 
Index: clang-tidy/google/NamedParameterCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/google/NamedParameterCheck.cpp
@@ -0,0 +1,69 @@
+//===--- 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().bind("decl"), this);
+}
+
+void NamedParameterCheck::check(const MatchFinder::MatchResult &Result) {
+  const SourceManager &SM = *Result.SourceManager;
+  const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("decl");
+  SmallVector<SourceLocation, 4> UnnamedParamLocations;
+
+  // Ignore implicitly generated members.
+  if (Function->isDefaulted() && !Function->isExplicitlyDefaulted())
+    return;
+
+  // TODO: Handle overloads.
+  // TODO: We could check that all redeclarations use the same name for
+  //       arguments in the same position.
+  for (const ParmVarDecl *Parm : Function->params()) {
+    // 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;
+
+    UnnamedParamLocations.push_back(Parm->getLocation());
+  }
+
+  // Emit only one warning per function but fixits for all unnamed parameters.
+  if (!UnnamedParamLocations.empty()) {
+    auto D = diag(UnnamedParamLocations.front(),
+                  "all parameters should be named in a function");
+    for (SourceLocation Loc : UnnamedParamLocations)
+      D << FixItHint::CreateInsertion(Loc, " /*unused*/");
+  }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/google/NamedParameterCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/google/NamedParameterCheck.h
@@ -0,0 +1,31 @@
+//===--- 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
Index: test/clang-tidy/google-readability-function.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/google-readability-function.cpp
@@ -0,0 +1,51 @@
+// 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();
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to