[clang-tidy] Add new modernize use unary assert check

[barancsuk]

Summary

The check finds static_assert declarations with empty messages, and
replaces them with an unary static_assert declaration.

The check is only applicable for C++17 code.

The following code:

void f_textless(int a){
  static_assert(sizeof(a) <= 10, "");
}

is replaced by:

void f_textless(int a){
  static_assert(sizeof(a) <= 10 );
}

[Eugene.Zelenko]

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).

clang-tidy/modernize/UnaryStaticAssertCheck.cpp

28 ↗

(On Diff #106020)

Please remove empty line.

docs/clang-tidy/checks/modernize-unary-static-assert.rst

15 ↗

(On Diff #106020)

Please Clang-format snippets. Same below.

26 ↗

(On Diff #106020)

Please remove empty line.

[barancsuk]

• Do not rewrite macro expansions

[aaron.ballman]

clang-tidy/modernize/UnaryStaticAssertCheck.cpp

28 ↗

(On Diff #106020)

Can remove the empty line.

36 ↗

(On Diff #106020)

I would hoist this up to the previous if statement and use !AssertMessage->getLength(). This eliminates the possibility of an assert failing because the string literal is not an ASCII string.

You should add a test with L"", which I would expect to also be replaced.

40 ↗

(On Diff #106020)

How about:

use unary 'static_assert' when the string literal is an empty string

clang-tidy/modernize/UnaryStaticAssertCheck.h

19 ↗

(On Diff #106020)

static assert -> a static_assert
with empty -> with an empty

20 ↗

(On Diff #106020)

with the unary version.

docs/clang-tidy/checks/modernize-unary-static-assert.rst

6–7 ↗

(On Diff #106020)

This mixes singular and plural a bit oddly. How about:

The check diagnoses any static_assert declaration with an empty string literal and provides a fix-it to replace the declaration with a single-argument static_assert declaration.

9 ↗

(On Diff #106020)

C++17 and later

[barancsuk]

• Address review comments

[barancsuk]

• Missed some fixes last time

[aaron.ballman]

clang-tidy/modernize/UnaryStaticAssertCheck.cpp

32 ↗

(On Diff #106026)

I think this should be !AssertMessage->getLength()

test/clang-tidy/modernize-unary-static-assert.cpp

18 ↗

(On Diff #106026)

typo: Sie

23 ↗

(On Diff #106026)

I think you also need to handle the case where the string literal is empty but produces from a macro expansion. e.g.,

#define MSG ""

static_assert(condition, MSG);

This should not suggest removing the empty string literal.

[barancsuk]

• Further reviews addressed

[barancsuk]

clang-tidy/modernize/UnaryStaticAssertCheck.cpp

32 ↗

(On Diff #106026)

It works correctly without the negation, otherwise the tests fail.

[aaron.ballman]

clang-tidy/modernize/UnaryStaticAssertCheck.cpp

32 ↗

(On Diff #106026)

Sorry about the brain hiccup, you're correct.

test/clang-tidy/modernize-unary-static-assert.cpp

16 ↗

(On Diff #106148)

This probably should not diagnose, but definitely should not provide a fixit -- just because the macro is empty doesn't mean it will *always* be empty. Consider:

#if FOO
#define MSG ""
#else
#define MSG "Not empty"
#endif

I think diagnosing this case is just as likely to be a false-positive as not diagnosing, so my preference is to be silent instead of chatty. However, maybe there are other opinions.

[barancsuk]

• No longer warn when the message is from a macro expansion

[barancsuk]

test/clang-tidy/modernize-unary-static-assert.cpp

16 ↗

(On Diff #106148)

You are right. Unfortunately, the message is also from a macro expansion when the whole static assert is from a macro expansion. Maybe it is possible to check whether they are from the same macro, but I think it might better be addressed in a separate patch if there is a need to support that case.

[aaron.ballman]

LGTM!

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

add_clang_library(clangTidyModernizeModule
   ModernizeTidyModule.cpp
   PassByValueCheck.cpp
   RawStringLiteralCheck.cpp
   RedundantVoidArgCheck.cpp
   ReplaceAutoPtrCheck.cpp
   ReplaceRandomShuffleCheck.cpp
   ReturnBracedInitListCheck.cpp
   ShrinkToFitCheck.cpp
+  UnaryStaticAssertCheck.cpp
   UseAutoCheck.cpp
   UseBoolLiteralsCheck.cpp
   UseDefaultMemberInitCheck.cpp
   UseEmplaceCheck.cpp
   UseEqualsDefaultCheck.cpp
   UseEqualsDeleteCheck.cpp
   UseNoexceptCheck.cpp
   UseNullptrCheck.cpp

clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp

 #include "MakeUniqueCheck.h"
 #include "PassByValueCheck.h"
 #include "RawStringLiteralCheck.h"
 #include "RedundantVoidArgCheck.h"
 #include "ReplaceAutoPtrCheck.h"
 #include "ReplaceRandomShuffleCheck.h"
 #include "ReturnBracedInitListCheck.h"
 #include "ShrinkToFitCheck.h"
+#include "UnaryStaticAssertCheck.h"
 #include "UseAutoCheck.h"
 #include "UseBoolLiteralsCheck.h"
 #include "UseDefaultMemberInitCheck.h"
 #include "UseEmplaceCheck.h"
 #include "UseEqualsDefaultCheck.h"
 #include "UseEqualsDeleteCheck.h"
 #include "UseNoexceptCheck.h"
 #include "UseNullptrCheck.h"
CheckFactories.registerCheck<RedundantVoidArgCheck>(
         "modernize-redundant-void-arg");
     CheckFactories.registerCheck<ReplaceAutoPtrCheck>(
         "modernize-replace-auto-ptr");
     CheckFactories.registerCheck<ReplaceRandomShuffleCheck>(
         "modernize-replace-random-shuffle");
     CheckFactories.registerCheck<ReturnBracedInitListCheck>(
         "modernize-return-braced-init-list");
     CheckFactories.registerCheck<ShrinkToFitCheck>("modernize-shrink-to-fit");
+    CheckFactories.registerCheck<UnaryStaticAssertCheck>(
+        "modernize-unary-static-assert");
     CheckFactories.registerCheck<UseAutoCheck>("modernize-use-auto");
     CheckFactories.registerCheck<UseBoolLiteralsCheck>(
         "modernize-use-bool-literals");
     CheckFactories.registerCheck<UseDefaultMemberInitCheck>(
         "modernize-use-default-member-init");
     CheckFactories.registerCheck<UseEmplaceCheck>("modernize-use-emplace");
     CheckFactories.registerCheck<UseEqualsDefaultCheck>("modernize-use-equals-default");
     CheckFactories.registerCheck<UseEqualsDeleteCheck>(

clang-tools-extra/trunk/clang-tidy/modernize/UnaryStaticAssertCheck.h

+//===--- UnaryStaticAssertCheck.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_MODERNIZE_UNARY_STATIC_ASSERT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_UNARY_STATIC_ASSERT_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replaces a static_assert declaration with an empty message
+/// with the unary version.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-unary-static-assert.html
+class UnaryStaticAssertCheck : public ClangTidyCheck {
+public:
+  UnaryStaticAssertCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_UNARY_STATIC_ASSERT_H

clang-tools-extra/trunk/clang-tidy/modernize/UnaryStaticAssertCheck.cpp

+//===--- UnaryStaticAssertCheck.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 "UnaryStaticAssertCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+void UnaryStaticAssertCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus1z)
+    return;
+
+  Finder->addMatcher(staticAssertDecl().bind("static_assert"), this);
+}
+
+void UnaryStaticAssertCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl =
+      Result.Nodes.getNodeAs<StaticAssertDecl>("static_assert");
+  const StringLiteral *AssertMessage = MatchedDecl->getMessage();
+
+  SourceLocation Loc = MatchedDecl->getLocation();
+
+  if (!AssertMessage || AssertMessage->getLength() ||
+      AssertMessage->getLocStart().isMacroID() || Loc.isMacroID())
+    return;
+
+  diag(Loc,
+       "use unary 'static_assert' when the string literal is an empty string")
+      << FixItHint::CreateRemoval(AssertMessage->getSourceRange());
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang

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

- New `modernize-replace-random-shuffle
   Finds and fixes usage of ``std::random_shuffle`` as the function has been removed from C++17.
 
 - New `modernize-return-braced-init-list
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-return-braced-init-list.html>`_ check
 
   Finds and replaces explicit calls to the constructor in a return statement by
   a braced initializer list so that the return type is not needlessly repeated.
 
+- New `modernize-unary-static-assert-check
+  <http://clang.llvm.org/extra/clang-tidy/checks/modernize-unary-static-assert.html>`_ check
+
+  The check diagnoses any ``static_assert`` declaration with an empty string literal
+  and provides a fix-it to replace the declaration with a single-argument ``static_assert`` declaration.
+
 - Improved `modernize-use-emplace
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html>`_ check
 
   Removes unnecessary ``std::make_pair`` and ``std::make_tuple`` calls in
   push_back calls and turns them into emplace_back. The check now also is able
   to remove user-defined make functions from ``push_back`` calls on containers
   of custom tuple-like types by providing `TupleTypes` and `TupleMakeFunctions`.
 

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

.. toctree::
    modernize-make-unique
    modernize-pass-by-value
    modernize-raw-string-literal
    modernize-redundant-void-arg
    modernize-replace-auto-ptr
    modernize-replace-random-shuffle
    modernize-return-braced-init-list
    modernize-shrink-to-fit
+   modernize-unary-static-assert
    modernize-use-auto
    modernize-use-bool-literals
    modernize-use-default-member-init
    modernize-use-emplace
    modernize-use-equals-default
    modernize-use-equals-delete
    modernize-use-noexcept
    modernize-use-nullptr

clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-unary-static-assert.rst

+.. title:: clang-tidy - modernize-unary-static-assert
+
+modernize-unary-static-assert
+=============================
+
+The check diagnoses any ``static_assert`` declaration with an empty string literal
+and provides a fix-it to replace the declaration with a single-argument ``static_assert`` declaration.
+
+The check is only applicable for C++17 and later code.
+
+The following code:
+
+.. code-block:: c++
+
+  void f_textless(int a) {
+    static_assert(sizeof(a) <= 10, "");
+  }
+
+is replaced by:
+
+.. code-block:: c++
+
+  void f_textless(int a) {
+    static_assert(sizeof(a) <= 10);
+  }

clang-tools-extra/trunk/test/clang-tidy/modernize-unary-static-assert.cpp

+// RUN: %check_clang_tidy %s modernize-unary-static-assert %t -- -- -std=c++1z
+
+#define FOO static_assert(sizeof(a) <= 15, "");
+#define MSG ""
+
+void f_textless(int a) {
+  static_assert(sizeof(a) <= 10, "");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use unary 'static_assert' when the string literal is an empty string [modernize-unary-static-assert]
+  // CHECK-FIXES: {{^}}  static_assert(sizeof(a) <= 10 );{{$}}
+  static_assert(sizeof(a) <= 12, L"");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use unary 'static_assert' when
+  // CHECK-FIXES: {{^}}  static_assert(sizeof(a) <= 12 );{{$}}
+  FOO
+  // CHECK-FIXES: {{^}}  FOO{{$}}
+  static_assert(sizeof(a) <= 17, MSG);
+  // CHECK-FIXES: {{^}}  static_assert(sizeof(a) <= 17, MSG);{{$}}
+}
+
+void f_with_tex(int a) {
+  static_assert(sizeof(a) <= 10, "Size of variable a is out of range!");
+}
+
+void f_unary(int a) { static_assert(sizeof(a) <= 10); }
+
+void f_incorrect_assert() { static_assert(""); }