JonasToth updated this revision to Diff 166044.
JonasToth added a comment.

- minor adjustments, add fixmes


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclCheck.cpp
  clang-tidy/readability/IsolateDeclCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-decl.rst
  test/clang-tidy/readability-isolate-decl-cxx17.cpp
  test/clang-tidy/readability-isolate-decl.cpp

Index: test/clang-tidy/readability-isolate-decl.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl.cpp
@@ -0,0 +1,354 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+  //
+  int /* here is a comment */ v1,
+      // another comment
+      v2 = 42 // Ok, more comments
+      ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^      }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^      }};
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = &i;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = &i;
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 5 variables
+  // CHECK-FIXES: double d = 42. /* foo */;
+  // CHECK-FIXES: {{^  }}double z = 43.;
+  // CHECK-FIXES: {{^  }}double /* hi */ y;
+  // CHECK-FIXES: {{^  }}double c /* */ /*  */;
+  // CHECK-FIXES: {{^  }}double l = 2.;
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+void f5() {
+  SomeClass v1, v2(42), v3{42}, v4(42.5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: SomeClass v1;
+  // CHECK-FIXES: {{^  }}SomeClass v2(42);
+  // CHECK-FIXES: {{^  }}SomeClass v3{42};
+  // CHECK-FIXES: {{^  }}SomeClass v4(42.5);
+
+  SomeClass v5 = 42, *p1 = nullptr;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: SomeClass v5 = 42;
+  // CHECK-FIXES: {{^  }}SomeClass *p1 = nullptr;
+}
+
+void f6() {
+  int array1[] = {1, 2, 3, 4}, array2[] = {1, 2, 3}, value1, value2 = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // CHECK-FIXES: int array1[] = {1, 2, 3, 4};
+  // CHECK-FIXES: {{^  }}int array2[] = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}int value1;
+  // CHECK-FIXES: {{^  }}int value2 = 42;
+}
+
+template <typename T>
+struct TemplatedType {
+  TemplatedType() = default;
+  TemplatedType(T value);
+};
+
+void f7() {
+  TemplatedType<int> TT1(42), TT2{42}, TT3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+  // CHECK-FIXES: TemplatedType<int> TT1(42);
+  // CHECK-FIXES: {{^  }}TemplatedType<int> TT2{42};
+  // CHECK-FIXES: {{^  }}TemplatedType<int> TT3;
+  //
+  TemplatedType<int *> *TT4(nullptr), TT5, **TT6 = nullptr, *const *const TT7{nullptr};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 4 variables
+  // TODO Fix this
+  // CHECK FIXES: TemplatedType<int *> *TT4(nullptr);
+  // CHECK FIXES: {{^  }}TemplatedType<int *> TT5;
+  // CHECK FIXES: {{^  }}TemplatedType<int *> **TT6 = nullptr;
+  // CHECK FIXES: {{^  }}TemplatedType<int *> *const *const TT7{nullptr};
+
+  TemplatedType<int &> **TT8(nullptr), TT9;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // TODO Fix this
+}
+
+void forbidden_transformations() {
+  for (int i = 0, j = 42; i < j; ++i)
+    ;
+}
+
+#define NULL 0
+#define MY_NICE_TYPE int **
+#define VAR_NAME(name) name##__LINE__
+#define A_BUNCH_OF_VARIABLES int m1 = 42, m2 = 43, m3 = 44;
+
+void macros() {
+  int *p1 = NULL, *p2 = NULL;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: int *p1 = NULL;
+  // CHECK-FIXES: {{^  }}int *p2 = NULL;
+
+  // Macros are involved, so there will be no transformation
+  MY_NICE_TYPE p3, v1, v2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+
+  int VAR_NAME(v3),
+      VAR_NAME(v4),
+      VAR_NAME(v5);
+  // CHECK-MESSAGES: [[@LINE-3]]:3: warning: this statement declares 3 variables
+
+  A_BUNCH_OF_VARIABLES
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+}
+
+void dontTouchParameter(int param1, int param2) {}
+
+struct StructOne {
+  StructOne() {}
+  StructOne(int b) {}
+
+  int member1, member2;
+  // TODO Transform FieldDecl's as well
+};
+
+using PointerType = int;
+
+struct {
+  int i;
+} AS1, AS2;
+struct TemT {
+  template <typename T>
+  T *getAs() {
+    return nullptr;
+  }
+} TT1, TT2;
+
+void complex() {
+  typedef int *IntPtr;
+  typedef int ArrayType[2];
+  typedef int FunType(void);
+
+  IntPtr intptr1, intptr2 = nullptr, intptr3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+  // TODO SourceText is invalid error
+  // CHECK FIXES: IntPtr intptr1;
+  // CHECK FIXES: {{^  }}IntPtr intptr2 = nullptr;
+  // CHECK FIXES: {{^  }}IntPtr intptr3;
+
+  ArrayType arraytype1, arraytype2 = {1}, arraytype3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+  // CHECK-FIXES: ArrayType arraytype1;
+  // CHECK-FIXES: {{^  }}ArrayType arraytype2 = {1};
+  // CHECK-FIXES: {{^  }}ArrayType arraytype3;
+
+  // Don't touch function declarations.
+  FunType funtype1, funtype2, functype3;
+
+  for (int index1 = 0, index2 = 0;;) {
+    int localFor1 = 1, localFor2 = 2;
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+    // CHECK-FIXES: int localFor1 = 1;
+    // CHECK-FIXES: {{^    }}int localFor2 = 2;
+  }
+
+  StructOne s1, s2(23), s3, s4(3), *sptr = new StructOne(2);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 5 variables
+  // CHECK-FIXES: StructOne s1;
+  // CHECK-FIXES: {{^  }}StructOne s2(23);
+  // CHECK-FIXES: {{^  }}StructOne s3;
+  // CHECK-FIXES: {{^  }}StructOne s4(3);
+  // CHECK-FIXES: {{^  }}StructOne *sptr = new StructOne(2);
+
+  struct StructOne cs1, cs2(42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: struct StructOne cs1;
+  // CHECK-FIXES: {{^  }}struct StructOne cs2(42);
+
+  int *ptrArray[3], dummy, **ptrArray2[5], twoDim[2][3], *twoDimPtr[2][3];
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 5 variables
+  // CHECK-FIXES: int *ptrArray[3];
+  // CHECK-FIXES: {{^  }}int dummy;
+  // CHECK-FIXES: {{^  }}int **ptrArray2[5];
+  // CHECK-FIXES: {{^  }}int twoDim[2][3];
+  // CHECK-FIXES: {{^  }}int *twoDimPtr[2][3];
+
+  {
+    void f1(int), g1(int, float);
+  }
+
+  {
+    void gg(int, float);
+
+    void (*f2)(int), (*g2)(int, float) = gg;
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+    // CHECK-FIXES: void (*f2)(int);
+    // CHECK-FIXES: {{^    }}void (*g2)(int, float) = gg;
+
+    void /*(*/ (/*(*/ *f3)(int), (*g3)(int, float);
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+    // TODO Why is this incorrect?
+    // CHECK FIXES: void /*(*/ (/*(*/ *f3)(int);
+    // CHECK FIXES: {{^    }}void /*(*/ (*g3)(int, float);
+  }
+
+  struct S {
+    int a;
+    const int b;
+    void f() {}
+  };
+
+  int S::*p = &S::a, S::*const q = &S::a;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK FIXES: int S::*p = &S::a;
+  // CHECK FIXES: {{^  }}int S::*const q = &S::a;
+
+  int /* :: */ S::*pp2 = &S::a, var1 = 0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK FIXES: int /* :: */ S::*pp2 = &S::a;
+  // CHECK FIXES: {{^  }}int var1 = 0;
+
+  const int S::*r = &S::b, S::*t;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK FIXES: const int S::*r = &S::b;
+  // CHECK FIXES: {{^  }}const int S::*t;
+
+  {
+    int S::*mdpa1[2] = {&S::a, &S::a}, var1 = 0;
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+    // CHECK FIXES: int S::*mdpa1[2] = {&S::a, &S::a};
+    // CHECK FIXES: {{^    }}int var1 = 0;
+
+    int S ::**mdpa2[2] = {&p, &pp2}, var2 = 0;
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+    // CHECK FIXES: int S ::**mdpa2[2] = {&p, &pp2};
+    // CHECK FIXES: {{^    }}int var2 = 0;
+
+    void (S::*mdfp1)() = &S::f, (S::*mdfp2)() = &S::f;
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+    // CHECK FIXES: void (S::*mdfp1)() = &S::f;
+    // CHECK FIXES: {{^    }}void (S::*mdfp2)() = &S::f;
+
+    void (S::*mdfpa1[2])() = {&S::f, &S::f}, (S::*mdfpa2)() = &S::f;
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+    // CHECK FIXES: void (S::*mdfpa1[2])() = {&S::f, &S::f};
+    // CHECK FIXES: {{^    }}void (S::*mdfpa2)() = &S::f;
+
+    void (S::* * mdfpa3[2])() = {&mdfpa1[0], &mdfpa1[1]}, (S::*mdfpa4)() = &S::f;
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 2 variables
+    // CHECK FIXES: void (S::* * mdfpa3[2])() = {&mdfpa1[0], &mdfpa1[1]};
+    // CHECK FIXES: {{^    }}void (S::*mdfpa4)() = &S::f;
+  }
+
+  typedef const int S::*MemPtr;
+  MemPtr aaa = &S::a, bbb = &S::b;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK FIXES: MemPtr aaa = &S::a;
+  // CHECK FIXES: {{^  }}MemPtr bbb = &S::b;
+
+  class CS {
+  public:
+    int a;
+    const int b;
+  };
+  int const CS ::*pp = &CS::a, CS::*const qq = &CS::a;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK FIXES: int const CS ::*pp = &CS::a;
+  // CHECK FIXES: {{^  }}int const CS::*const qq = &CS::a;
+
+  // clang-format off
+  auto returner = []() { return int(32); };
+  int intfunction = returner(), intarray[] =
+          {
+                  1,
+                  2,
+                  3,
+                  4
+          }, bb = 4;
+  // CHECK-MESSAGES: [[@LINE-7]]:3: warning: this statement declares 3 variables
+  // CHECK-FIXES: int intfunction = returner();
+  // CHECK-FIXES: {{^  }}int intarray[] =
+  // CHECK-FIXES: {{^          }}{
+  // CHECK-FIXES: {{^                  }}1,
+  // CHECK-FIXES: {{^                  }}2,
+  // CHECK-FIXES: {{^                  }}3,
+  // CHECK-FIXES: {{^                  }}4
+  // CHECK-FIXES: {{^          }}};
+  // CHECK-FIXES: {{^  }}int bb = 4;
+  // clang-format on
+
+  TemT *T1 = &TT1, *T2 = &TT2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: TemT *T1 = &TT1;
+  // CHECK-FIXES: {{^  }}TemT *T2 = &TT2;
+
+  const PointerType *PT1 = T1->getAs<PointerType>(),
+                    *PT2 = T2->getAs<PointerType>();
+  // CHECK-MESSAGES: [[@LINE-2]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: const PointerType *PT1 = T1->getAs<PointerType>();
+  // CHECK-FIXES: {{^  }}const PointerType *PT2 = T2->getAs<PointerType>();
+
+  const int *p1 = nullptr;
+  const int *p2 = nullptr;
+
+  const int *&pref1 = p1, *&pref2 = p2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: const int *&pref1 = p1;
+  // CHECK-FIXES: {{^  }}const int *&pref2 = p2;
+
+  // clang-format off
+  const char *literal1 = "clang"   "test"\
+                         "one",
+             *literal2 = "empty", literal3[] = "three";
+  // CHECK-MESSAGES: [[@LINE-3]]:3: warning: this statement declares 3 variables
+  // CHECK-FIXES: const char *literal1 = "clang"   "test"\
+  // CHECK-FIXES: {{^                         }}"one";
+  // CHECK-FIXES: {{^  }}const char *literal2 = "empty";
+  // CHECK-FIXES: {{^  }}const char literal3[] = "three";
+  // clang-format on
+}
+
+typedef int *tptr, tbt;
+typedef int (&tfp)(int, long), tarr[10];
+typedef int tarr2[10], tct;
+
+template <typename A, typename B>
+void should_not_be_touched(A, B);
+
+int variable, function(void);
+
+int call_func_with_sideeffect();
+void bad_if_decl() {
+  if (true)
+    int i, j, k = call_func_with_sideeffect();
+}
Index: test/clang-tidy/readability-isolate-decl-cxx17.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/readability-isolate-decl-cxx17.cpp
@@ -0,0 +1,105 @@
+// RUN: %check_clang_tidy %s readability-isolate-decl %t -- -- -std=c++17
+
+template <typename T1, typename T2>
+struct pair {
+  T1 first;
+  T2 second;
+  pair(T1 v1, T2 v2) : first(v1), second(v2) {}
+
+  template <int N>
+  decltype(auto) get() const {
+    if constexpr (N == 0)
+      return first;
+    else if constexpr (N == 1)
+      return second;
+  }
+};
+
+void forbidden_transformations() {
+  if (int i = 42, j = i; i == j)
+    ;
+
+  auto [i, j] = pair<int, int>(42, 42);
+}
+
+struct SomeClass {
+  SomeClass() = default;
+  SomeClass(int value);
+};
+
+namespace std {
+template <typename T>
+class initializer_list {};
+
+template <typename T>
+class vector {
+public:
+  vector() = default;
+  vector(initializer_list<T> init) {}
+};
+
+class string {
+public:
+  string() = default;
+  string(const char *) {}
+};
+
+namespace string_literals {
+string operator""s(const char *, decltype(sizeof(int))) {
+  return string();
+}
+} // namespace string_literals
+} // namespace std
+
+namespace Types {
+typedef int MyType;
+} // namespace Types
+
+int touch1, touch2;
+// TODO This is not a 'declStmt', why?
+// CHECK MESSAGES: [[@LINE-1]]:1: warning: this statement declares 2 variables
+// CHECK FIXES: {{^}}int touch1;
+// CHECK FIXES: {{^}}int touch2;
+
+void modern() {
+  auto autoInt1 = 3, autoInt2 = 4;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: auto autoInt1 = 3;
+  // CHECK-FIXES: {{^  }}auto autoInt2 = 4;
+
+  decltype(int()) declnottouch = 4;
+  decltype(int()) declint1 = 5, declint2 = 3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: decltype(int()) declint1 = 5;
+  // CHECK-FIXES: {{^  }}decltype(int()) declint2 = 3;
+
+  std::vector<int> vectorA = {1, 2}, vectorB = {1, 2, 3}, vectorC({1, 1, 1});
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+  // CHECK-FIXES: std::vector<int> vectorA = {1, 2};
+  // CHECK-FIXES: {{^  }}std::vector<int> vectorB = {1, 2, 3};
+  // CHECK-FIXES: {{^  }}std::vector<int> vectorC({1, 1, 1});
+
+  using uType = int;
+  uType utype1, utype2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 2 variables
+  // CHECK-FIXES: uType utype1;
+  // CHECK-FIXES: {{^  }}uType utype2;
+
+  Types::MyType mytype1, mytype2, mytype3 = 3;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: this statement declares 3 variables
+  // CHECK-FIXES: Types::MyType mytype1;
+  // CHECK-FIXES: {{^  }}Types::MyType mytype2;
+  // CHECK-FIXES: {{^  }}Types::MyType mytype3 = 3;
+
+  {
+    using namespace std::string_literals;
+
+    std::vector<std::string> s{"foo"s, "bar"s}, t{"foo"s}, u, a({"hey", "you"}), bb = {"h", "a"};
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: this statement declares 5 variables
+    // CHECK-FIXES: std::vector<std::string> s{"foo"s, "bar"s};
+    // CHECK-FIXES: {{^    }}std::vector<std::string> t{"foo"s};
+    // CHECK-FIXES: {{^    }}std::vector<std::string> u;
+    // CHECK-FIXES: {{^    }}std::vector<std::string> a({"hey", "you"});
+    // CHECK-FIXES: {{^    }}std::vector<std::string> bb = {"h", "a"};
+  }
+}
Index: docs/clang-tidy/checks/readability-isolate-decl.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/readability-isolate-decl.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - readability-isolate-decl
+
+readability-isolate-decl
+========================
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -225,6 +225,7 @@
    readability-identifier-naming
    readability-implicit-bool-conversion
    readability-inconsistent-declaration-parameter-name
+   readability-isolate-decl
    readability-magic-numbers
    readability-misleading-indentation
    readability-misplaced-array-index
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -93,6 +93,11 @@
   Flags uses of ``absl::StrCat()`` to append to a ``std::string``. Suggests 
   ``absl::StrAppend()`` should be used instead.
 
+- New :doc:`readability-isolate-decl
+  <clang-tidy/checks/readability-isolate-decl>` check.
+
+  FIXME: add release notes.
+
 - New :doc:`readability-magic-numbers
   <clang-tidy/checks/readability-magic-numbers>` check.
 
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -20,6 +20,7 @@
 #include "IdentifierNamingCheck.h"
 #include "ImplicitBoolConversionCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
+#include "IsolateDeclCheck.h"
 #include "MagicNumbersCheck.h"
 #include "MisleadingIndentationCheck.h"
 #include "MisplacedArrayIndexCheck.h"
@@ -66,6 +67,8 @@
         "readability-implicit-bool-conversion");
     CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
         "readability-inconsistent-declaration-parameter-name");
+    CheckFactories.registerCheck<IsolateDeclCheck>(
+        "readability-isolate-decl");
     CheckFactories.registerCheck<MagicNumbersCheck>(
         "readability-magic-numbers");
     CheckFactories.registerCheck<MisleadingIndentationCheck>(
Index: clang-tidy/readability/IsolateDeclCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/readability/IsolateDeclCheck.h
@@ -0,0 +1,35 @@
+//===--- IsolateDeclCheck.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_ISOLATEDECLCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_ISOLATEDECLCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-isolate-decl.html
+class IsolateDeclCheck : public ClangTidyCheck {
+public:
+  IsolateDeclCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  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_READABILITY_ISOLATEDECLCHECK_H
Index: clang-tidy/readability/IsolateDeclCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/readability/IsolateDeclCheck.cpp
@@ -0,0 +1,375 @@
+//===--- IsolateDeclCheck.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 "IsolateDeclCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/Support/Error.h"
+
+#include <iostream>
+#include <numeric>
+
+using namespace clang::ast_matchers;
+
+#define PRINT_DEBUG 1
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+namespace {
+AST_MATCHER(DeclStmt, isSingleDecl) { return Node.isSingleDecl(); }
+AST_MATCHER(DeclStmt, isVariablesOnly) {
+  return std::all_of(Node.decl_begin(), Node.decl_end(),
+                     [](Decl *D) { return isa<VarDecl>(D); });
+}
+
+AST_MATCHER_P(IfStmt, hasInit, ast_matchers::internal::Matcher<Stmt>,
+              InnerMatcher) {
+  const Stmt *const InitStmt = Node.getInit();
+  if (InitStmt)
+    return InnerMatcher.matches(*InitStmt, Finder, Builder);
+  return false;
+}
+} // namespace
+
+void IsolateDeclCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      stmt(allOf(
+          declStmt(allOf(isVariablesOnly(), unless(isSingleDecl()),
+                         hasParent(compoundStmt())))
+              .bind("decl_stmt"),
+          unless(
+              hasAncestor(forStmt(hasLoopInit(equalsBoundNode("decl_stmt"))))),
+          unless(hasAncestor(ifStmt(hasInit(equalsBoundNode("decl_stmt"))))))),
+      this);
+}
+
+template <typename TokenKind, typename... TokenKinds>
+static SourceLocation findNextAnyTokenKind(SourceLocation Start,
+                                           const SourceManager &SM,
+                                           const LangOptions &LangOpts,
+                                           TokenKind TK, TokenKinds... TKs) {
+  while (true) {
+    Optional<Token> CurrentToken = Lexer::findNextToken(Start, SM, LangOpts);
+
+    if (!CurrentToken.hasValue())
+      return SourceLocation();
+
+    Token PotentialMatch = *CurrentToken;
+    if (PotentialMatch.isOneOf(TK, TKs...))
+      return PotentialMatch.getLocation();
+
+    Start =
+        Lexer::getLocForEndOfToken(Start, 0, SM, LangOpts).getLocWithOffset(1);
+  }
+}
+
+static SourceLocation findNextTerminator(SourceLocation Start,
+                                         const SourceManager &SM,
+                                         const LangOptions &LangOpts) {
+  return findNextAnyTokenKind(Start, SM, LangOpts, tok::comma, tok::semi);
+}
+
+SourceLocation findPreviousTokenStart(SourceLocation Start,
+                                      const SourceManager &SM,
+                                      const LangOptions &LangOpts) {
+  if (Start.isInvalid() || Start.isMacroID())
+    return SourceLocation();
+
+  SourceLocation BeforeStart = Start.getLocWithOffset(-1);
+  if (BeforeStart.isInvalid() || BeforeStart.isMacroID())
+    return SourceLocation();
+
+  return Lexer::GetBeginningOfToken(BeforeStart, SM, LangOpts);
+}
+
+static SourceLocation findPreviousTokenKind(SourceLocation Start,
+                                            const SourceManager &SM,
+                                            const LangOptions &LangOpts,
+                                            tok::TokenKind TK) {
+  while (true) {
+    SourceLocation L = findPreviousTokenStart(Start, SM, LangOpts);
+    if (L.isInvalid() || L.isMacroID())
+      return SourceLocation();
+
+    Token T;
+    bool FailedRetrievingToken = Lexer::getRawToken(L, T, SM, LangOpts);
+
+    if (FailedRetrievingToken)
+      return SourceLocation();
+
+    if (T.is(TK))
+      return T.getLocation();
+
+    Start = L;
+  }
+}
+template <typename TokenKind, typename... TokenKinds>
+static SourceLocation
+findPreviousAnyTokenKind(SourceLocation Start, const SourceManager &SM,
+                         const LangOptions &LangOpts, TokenKind TK,
+                         TokenKinds... TKs) {
+  while (true) {
+    SourceLocation L = findPreviousTokenStart(Start, SM, LangOpts);
+    if (L.isInvalid() || L.isMacroID())
+      return SourceLocation();
+
+    Token T;
+    bool FailedRetrievingToken = Lexer::getRawToken(L, T, SM, LangOpts);
+
+    if (FailedRetrievingToken)
+      return SourceLocation();
+
+    if (T.isOneOf(TK, TKs...))
+      return T.getLocation();
+
+    Start = L;
+  }
+}
+
+static SourceLocation findStartOfIndirection(SourceLocation Start,
+                                             int Indirections,
+                                             const SourceManager &SM,
+                                             const LangOptions &LangOpts) {
+  assert(Indirections >= 0 && "Indirections must be non-negative");
+  if (Indirections == 0)
+    return Start;
+
+  for (int i = Indirections; i > 0; --i) {
+    Start = findPreviousAnyTokenKind(Start, SM, LangOpts, tok::star, tok::amp);
+    if (Start.isInvalid() || Start.isMacroID())
+      return SourceLocation();
+  }
+  return Start;
+}
+
+static bool isMacroID(SourceRange R) {
+  return R.getBegin().isMacroID() || R.getEnd().isMacroID();
+}
+
+static int countIndirections(const Type *T, int Indirections = 0) {
+  assert(T && "Require non-null");
+  if (T->isPointerType() || T->isReferenceType())
+    return countIndirections(T->getPointeeType().getTypePtr(), ++Indirections);
+
+  if (T->isArrayType()) {
+    const auto *AT = dyn_cast<ArrayType>(T->getCanonicalTypeInternal());
+    // Note: Do not increment the 'Indirections' because it is not yet clear
+    // if there is an indirection added in the source code of the array
+    // declaration.
+    return countIndirections(AT->getElementType().getTypePtr(), Indirections);
+  }
+  return Indirections;
+}
+
+/// This function assumes, that \p DS only contains VarDecl elements.
+static Optional<std::vector<SourceRange>>
+DeclSlice(const DeclStmt *DS, const SourceManager &SM,
+          const LangOptions &LangOpts) {
+  std::size_t DeclCount = std::distance(DS->decl_begin(), DS->decl_end());
+  if (DeclCount < 2) {
+#if PRINT_DEBUG
+    std::cerr << "Not enough Declarations (" << DeclCount
+              << ") in statement to create isolated declarations\n";
+#endif
+    return None;
+  }
+
+  // The initial type of the declaration and each declaration has it's slice.
+  std::vector<SourceRange> Slices;
+  Slices.reserve(DeclCount + 1);
+
+#if PRINT_DEBUG
+  std::cerr << "Number of Slices to create: " << Slices.capacity() << "\n";
+#endif
+
+  // Special case for the type where the SourceRange is computed
+  // differently.
+  VarDecl *FirstDecl = dyn_cast<VarDecl>(*DS->decl_begin());
+  assert(FirstDecl && "Expect only VarDecls");
+
+  if (FirstDecl->getType()->isMemberPointerType() ||
+      FirstDecl->getType()->isMemberDataPointerType() ||
+      FirstDecl->getType()->isMemberFunctionPointerType())
+    return None;
+
+  SourceLocation Start = findStartOfIndirection(
+      FirstDecl->getLocation(),
+      countIndirections(FirstDecl->getType().getTypePtr()), SM, LangOpts);
+
+  // Fix function-pointer declarations that have a '(' in front of the
+  // pointer.
+  if (FirstDecl->getType()->isFunctionPointerType())
+    Start = findPreviousTokenKind(Start, SM, LangOpts, tok::l_paren);
+
+#if PRINT_DEBUG
+  std::cerr << "Start of Indirection: " << Start.printToString(SM) << "\n";
+#endif
+
+  SourceRange DeclRange(DS->getBeginLoc(), Start);
+  if (DeclRange.isInvalid()) {
+#if PRINT_DEBUG
+    std::cerr << "DeclRange is invalid\n";
+#endif
+    return None;
+  }
+  if (isMacroID(DeclRange)) {
+#if PRINT_DEBUG
+    std::cerr << "DeclRange is in macro\n";
+#endif
+    return None;
+  }
+
+  Slices.emplace_back(DeclRange);
+
+  SourceLocation DeclBegin = Start;
+  for (auto It = DS->decl_begin(), End = DS->decl_end(); It != End; ++It) {
+    VarDecl *CurrentDecl = dyn_cast<VarDecl>(*It);
+    assert(CurrentDecl && "Expect only VarDecls here");
+
+    if (CurrentDecl->getType()->isMemberPointerType() ||
+        CurrentDecl->getType()->isMemberDataPointerType() ||
+        CurrentDecl->getType()->isMemberFunctionPointerType())
+      return None;
+
+    SourceLocation DeclEnd;
+    if (CurrentDecl->hasInit())
+      DeclEnd =
+          findNextTerminator(CurrentDecl->getInit()->getEndLoc(), SM, LangOpts);
+    else
+      DeclEnd = findNextTerminator(CurrentDecl->getEndLoc(), SM, LangOpts);
+
+#if PRINT_DEBUG
+    std::cerr << "Start of Slice: " << DeclBegin.printToString(SM) << "\n";
+    std::cerr << "End of Slice: " << DeclEnd.printToString(SM) << "\n";
+#endif
+
+    SourceRange VarNameRange(DeclBegin, DeclEnd);
+    if (VarNameRange.isInvalid()) {
+#if PRINT_DEBUG
+      std::cerr << "VarNameRange is invalid\n";
+#endif
+      return None;
+    }
+    if (isMacroID(VarNameRange)) {
+#if PRINT_DEBUG
+      std::cerr << "VarNameRange is in macro\n";
+#endif
+      return None;
+    }
+
+    Slices.emplace_back(VarNameRange);
+    DeclBegin = DeclEnd.getLocWithOffset(1);
+  }
+
+  return Slices;
+}
+
+static Optional<std::vector<StringRef>>
+SourceFromRanges(const std::vector<SourceRange> &Ranges,
+                 const SourceManager &SM, const LangOptions &LangOpts) {
+  std::vector<StringRef> Snippets;
+  Snippets.reserve(Ranges.size());
+
+  for (const auto &Range : Ranges) {
+    CharSourceRange CR = Lexer::getAsCharRange(
+        CharSourceRange::getCharRange(Range.getBegin(), Range.getEnd()), SM,
+        LangOpts);
+
+    if (CR.isInvalid()) {
+#if PRINT_DEBUG
+      std::cerr << "CharSourceRange is invalid\n";
+#endif
+      return None;
+    }
+
+    bool InvalidText = false;
+    StringRef Snippet = Lexer::getSourceText(CR, SM, LangOpts, &InvalidText);
+
+    if (InvalidText) {
+#if PRINT_DEBUG
+      std::cerr << "SourcetText in invalid\n";
+#endif
+      return None;
+    }
+
+    Snippets.emplace_back(Snippet);
+  }
+
+  return Snippets;
+}
+
+/// Excepts a vector {TypeSnippet, Firstdecl, SecondDecl, ...}.
+static std::vector<std::string>
+CreateIsolatedDecls(const std::vector<StringRef> &Snippets) {
+  // The first section is the type snippet, which does not make a decl itself.
+  assert(Snippets.size() >= 2 && "No enough snippets to create isolated decls");
+  std::vector<std::string> Decls(Snippets.size() - 1);
+
+  for (std::size_t i = 1, End = Snippets.size(); i < End; ++i)
+    Decls[i - 1] = Twine(Snippets[0]).concat(Snippets[i].ltrim()).str();
+
+  return Decls;
+}
+
+static std::string &TrimRight(std::string &str) {
+  auto it1 = std::find_if(str.rbegin(), str.rend(), [](char ch) {
+    return !std::isspace<char>(ch, std::locale::classic());
+  });
+  str.erase(it1.base(), str.end());
+  return str;
+}
+
+static std::string Concat(const std::vector<std::string> &Decls,
+                          StringRef Indent) {
+  std::string R;
+  for (const StringRef &D : Decls)
+    R += Twine(D).concat(";\n").concat(Indent).str();
+  return TrimRight(R);
+}
+
+void IsolateDeclCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *WholeDecl = Result.Nodes.getNodeAs<DeclStmt>("decl_stmt");
+
+  auto Diag =
+      diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+      << static_cast<unsigned int>(
+             std::distance(WholeDecl->decl_begin(), WholeDecl->decl_end()));
+
+  Optional<std::vector<SourceRange>> PotentialSlices =
+      DeclSlice(WholeDecl, *Result.SourceManager, getLangOpts());
+  if (!PotentialSlices)
+    return;
+
+  const std::vector<SourceRange> &Slices = *PotentialSlices;
+
+  Optional<std::vector<StringRef>> PotentialSnippets =
+      SourceFromRanges(Slices, *Result.SourceManager, getLangOpts());
+
+  if (!PotentialSnippets)
+    return;
+
+  std::vector<StringRef> Snippets = *PotentialSnippets;
+
+  std::vector<std::string> NewDecls = CreateIsolatedDecls(Snippets);
+  std::string Replacement =
+      Concat(NewDecls, Lexer::getIndentationForLine(WholeDecl->getBeginLoc(),
+                                                    *Result.SourceManager));
+#if PRINT_DEBUG
+  std::cerr << Replacement << "\n";
+#endif
+
+  Diag << FixItHint::CreateReplacement(WholeDecl->getSourceRange(),
+                                       Replacement);
+}
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tidy/readability/CMakeLists.txt
+++ clang-tidy/readability/CMakeLists.txt
@@ -11,6 +11,7 @@
   IdentifierNamingCheck.cpp
   ImplicitBoolConversionCheck.cpp
   InconsistentDeclarationParameterNameCheck.cpp
+  IsolateDeclCheck.cpp
   MagicNumbersCheck.cpp
   MisleadingIndentationCheck.cpp
   MisplacedArrayIndexCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to