[clang-tidy] new google-default-arguments check

[courbet]

Summary

To check the google style guide rule here:
https://google.github.io/styleguide/cppguide.html#Default_Arguments

[hokein]

clang-tidy/google/DefaultArgumentsCheck.cpp

31 ↗

(On Diff #55006)

Usually, clang-tidy's warning message is not a sentence, so remove the . at the end.

How about default arguments on virtual or override methods are prohibited?

clang-tidy/google/GoogleTidyModule.cpp

40 ↗

(On Diff #55006)

Please keep the alphabetical order.

docs/ReleaseNotes.rst

92 ↗

(On Diff #55006)

s/vitual/virtual

docs/clang-tidy/checks/google-default-arguments.rst

6 ↗

(On Diff #55006)

I'm a little confused about the words here. Indeed, the google-default-arguments checks the default parameter given for virtual methods.

test/clang-tidy/google-default-arguments.cpp

5 ↗

(On Diff #55006)

The the first warning message you need check the all message including the check name.

[courbet]

Cosmetics + change error message.

clang-tidy/google/GoogleTidyModule.cpp

40 ↗

(On Diff #55006)

I think the script auto-generated that, but done.

docs/clang-tidy/checks/google-default-arguments.rst

6 ↗

(On Diff #55006)

Right, I got mixed up in the terminology: the default argument is given as an initializer for the parameter declaration.

[hokein]

LGTM with one nit.

test/clang-tidy/google-default-arguments.cpp

10 ↗

(On Diff #56000)

You can remove the [google-default-arguments] in this line and line 15. Usually we only keep it on the first warning message.

[courbet]

Thanks. I don't have write access, could you please submit this for me ?

clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyGoogleModule
   AvoidCStyleCastsCheck.cpp
+  DefaultArgumentsCheck.cpp
   ExplicitConstructorCheck.cpp
   ExplicitMakePairCheck.cpp
   GlobalNamesInHeadersCheck.cpp
   GoogleTidyModule.cpp
   IntegerTypesCheck.cpp
   MemsetZeroLengthCheck.cpp
   NonConstReferences.cpp
   OverloadedUnaryAndCheck.cpp

clang-tools-extra/trunk/clang-tidy/google/DefaultArgumentsCheck.h

+//===--- DefaultArgumentsCheck.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_DEFAULT_ARGUMENTS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_DEFAULT_ARGUMENTS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace google {
+
+/// Checks that default parameters are not given for virtual methods.
+///
+/// See https://google.github.io/styleguide/cppguide.html#Default_Arguments
+class DefaultArgumentsCheck : public ClangTidyCheck {
+public:
+  DefaultArgumentsCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace google
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_DEFAULT_ARGUMENTS_H

clang-tools-extra/trunk/clang-tidy/google/DefaultArgumentsCheck.cpp

+//===--- DefaultArgumentsCheck.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 "DefaultArgumentsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace google {
+
+void DefaultArgumentsCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      cxxMethodDecl(anyOf(isOverride(), isVirtual()),
+                    hasAnyParameter(parmVarDecl(hasInitializer(expr()))))
+          .bind("Decl"),
+      this);
+}
+
+void DefaultArgumentsCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<CXXMethodDecl>("Decl");
+  diag(MatchedDecl->getLocation(),
+       "default arguments on virtual or override methods are prohibited");
+}
+
+} // namespace google
+} // namespace tidy
+} // namespace clang

clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp

 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../readability/BracesAroundStatementsCheck.h"
 #include "../readability/FunctionSizeCheck.h"
 #include "../readability/NamespaceCommentCheck.h"
 #include "../readability/RedundantSmartptrGetCheck.h"
 #include "AvoidCStyleCastsCheck.h"
+#include "DefaultArgumentsCheck.h"
 #include "ExplicitConstructorCheck.h"
 #include "ExplicitMakePairCheck.h"
 #include "GlobalNamesInHeadersCheck.h"
 #include "IntegerTypesCheck.h"
 #include "MemsetZeroLengthCheck.h"
 #include "OverloadedUnaryAndCheck.h"
 #include "NonConstReferences.h"
 #include "StringReferenceMemberCheck.h"

 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<build::ExplicitMakePairCheck>(
         "google-build-explicit-make-pair");
     CheckFactories.registerCheck<build::UnnamedNamespaceInHeaderCheck>(
         "google-build-namespaces");
     CheckFactories.registerCheck<build::UsingNamespaceDirectiveCheck>(
         "google-build-using-namespace");
+    CheckFactories.registerCheck<DefaultArgumentsCheck>(
+        "google-default-arguments");
     CheckFactories.registerCheck<ExplicitConstructorCheck>(
         "google-explicit-constructor");
     CheckFactories.registerCheck<runtime::IntegerTypesCheck>(
         "google-runtime-int");
     CheckFactories.registerCheck<runtime::OverloadedUnaryAndCheck>(
         "google-runtime-operator");
     CheckFactories.registerCheck<runtime::NonConstReferences>(
         "google-runtime-references");

clang-tools-extra/trunk/docs/ReleaseNotes.rst

- New `cppcoreguidelines-interfaces-global-init
   lead to order-of-initialization problems.
 
 - New `cppcoreguidelines-pro-type-member-init
   <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.html>`_ check
 
   Flags user-defined constructor definitions that do not initialize all builtin
   and pointer fields which leaves their memory in an undefined state.
 
+- New `google-default-arguments
+  <http://clang.llvm.org/extra/clang-tidy/checks/google-default-arguments.html>`_ check
+
+  Flags default arguments in virtual methods.
+
 - New `misc-dangling-handle
   <http://clang.llvm.org/extra/clang-tidy/checks/misc-dangling-handle.html>`_ check
 
   Detects dangling references in value handlers like
   ``std::experimental::string_view``.
 
 - New `misc-fold-init-type
   <http://clang.llvm.org/extra/clang-tidy/checks/misc-fold-init-type.html>`_ check

clang-tools-extra/trunk/docs/clang-tidy/checks/google-default-arguments.rst

+.. title:: clang-tidy - google-default-arguments
+
+google-default-arguments
+========================
+
+Checks that default arguments are not given for virtual methods.
+
+See https://google.github.io/styleguide/cppguide.html#Default_Arguments

clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

.. toctree::
    cppcoreguidelines-pro-type-member-init
    cppcoreguidelines-pro-type-reinterpret-cast
    cppcoreguidelines-pro-type-static-cast-downcast
    cppcoreguidelines-pro-type-union-access
    cppcoreguidelines-pro-type-vararg
    google-build-explicit-make-pair
    google-build-namespaces
    google-build-using-namespace
+   google-default-arguments
    google-explicit-constructor
    google-global-names-in-headers
    google-readability-braces-around-statements (redirects to readability-braces-around-statements) <google-readability-braces-around-statements>
    google-readability-casting
    google-readability-function-size (redirects to readability-function-size) <google-readability-function-size>
    google-readability-namespace-comments
    google-readability-redundant-smartptr-get
    google-readability-todo

clang-tools-extra/trunk/test/clang-tidy/google-default-arguments.cpp

+// RUN: %check_clang_tidy %s google-default-arguments %t
+
+struct A {
+  virtual void f(int I, int J = 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: default arguments on virtual or override methods are prohibited [google-default-arguments]
+};
+
+struct B : public A {
+  void f(int I, int J = 5);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: default arguments on virtual or override methods are prohibited
+};
+
+struct C : public B {
+  void f(int I, int J = 5) override;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: default arguments on virtual or override methods are prohibited
+};
+
+// Negatives.
+struct D : public B {
+  void f(int I, int J) override;
+};
+
+struct X {
+  void f(int I, int J = 3);
+};
+
+struct Y : public X {
+  void f(int I, int J = 5);
+};