[clang-tidy] Add check 'readability-pointer-type-star-placement'.

[balazske]

Summary

This check should warn if '*' character at pointer declarations is not aligned to right side.
I know that clang-format can do these changes but if the goal is not to reformat the whole code
this check can be useful.

[Eugene.Zelenko]

I think it'll be much more reasonable to use clang-format and filter its output to select only changes with stars in build rule. clang-format-diff is another method to introduce formatting changes gradually.

clang-tools-extra/clang-tidy/readability/PointerTypeStarPlacementCheck.cpp

1

Two lines must be joined.

clang-tools-extra/clang-tidy/readability/PointerTypeStarPlacementCheck.h

19

Some description should be provided, or empty comment removed.

clang-tools-extra/docs/ReleaseNotes.rst

70

Merge artifact?

clang-tools-extra/docs/clang-tidy/checks/readability-pointer-type-star-placement.rst

10

Please remove empty line.

28

Copy-paste error?

[njames93]

I don't see the use case you suggest as a strong enough motivation for this, clang-format does this job very well and we shouldn't be reinventing the wheel here.

[whisperity]

In D96607#2561287, @njames93 wrote:

I don't see the use case you suggest as a strong enough motivation for this, clang-format does this job very well and we shouldn't be reinventing the wheel here.

Can you turn all mechanisms of clang-format off, except the pointer placement rule?

---

Several coding guidelines and projects put the pointer to the left, not to the right. So even if this check goes in, there should definitely be an option for which side you want to enforce and fix for...

[balazske]

I think now that not checkers are the best solution for this problem. There is already a separate tool for formatting. And we do not want to have a checker for every other formatting rule too. The clang-format could be improved to support selective formatting (apply only a part of the formatting rules). (And it has weaknesses that may prevent users from using it.) It seems difficult to filter out changes related to pointer declarations (these rules are not line-based and multiplications should be excluded) after the formatting.

[balazske]

Checks about code formatting are likely not to be accepted. Formatting can be done in external tool and is not subject for checks. The difference is that it is not possible to format separately for one rule only (with clang-format). But the better option should be to format the whole code instead of parts of it.

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

add_clang_library(clangTidyReadabilityModule
   IsolateDeclarationCheck.cpp
   MagicNumbersCheck.cpp
   MakeMemberFunctionConstCheck.cpp
   MisleadingIndentationCheck.cpp
   MisplacedArrayIndexCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   NonConstParameterCheck.cpp
+  PointerTypeStarPlacementCheck.cpp
   QualifiedAutoCheck.cpp
   ReadabilityTidyModule.cpp
   RedundantAccessSpecifiersCheck.cpp
   RedundantControlFlowCheck.cpp
   RedundantDeclarationCheck.cpp
   RedundantFunctionPtrDereferenceCheck.cpp
   RedundantMemberInitCheck.cpp
   RedundantPreprocessorCheck.cpp

clang-tools-extra/clang-tidy/readability/PointerTypeStarPlacementCheck.h

+//===--- PointerTypeStarPlacementCheck.h - clang-tidy -----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_POINTERTYPESTARPLACEMENTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_POINTERTYPESTARPLACEMENTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+///
+///
+///
+///
+class PointerTypeStarPlacementCheck : public ClangTidyCheck {
+public:
+  PointerTypeStarPlacementCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  void checkStarLoc(const PointerTypeLoc &FirstPtrLoc,
+                    const PointerTypeLoc &LastPtrLoc, const SourceManager &SM,
+                    const LangOptions &LO);
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_POINTERTYPESTARPLACEMENTCHECK_H

clang-tools-extra/clang-tidy/readability/PointerTypeStarPlacementCheck.cpp

+//===--- PointerTypeStarPlacementCheck.cpp - clang-tidy
+//---------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "PointerTypeStarPlacementCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void PointerTypeStarPlacementCheck::checkStarLoc(
+    const PointerTypeLoc &FirstPtrLoc, const PointerTypeLoc &LastPtrLoc,
+    const SourceManager &SM, const LangOptions &LO) {
+  // FirstPtrLoc:
+  // int *** X;
+  //       ^
+  // LastPtrLoc:
+  // int *** X;
+  //     ^
+
+  bool DiagLastPtrLoc = false;
+  for (PointerTypeLoc PtrLoc = FirstPtrLoc;;
+       PtrLoc = PtrLoc.getNextTypeLoc().castAs<PointerTypeLoc>()) {
+    SourceLocation StarLoc = PtrLoc.getStarLoc();
+    StarLoc.dump(SM);
+    Token StarTok;
+    bool Invalid = Lexer::getRawToken(StarLoc, StarTok, SM, LO);
+    assert(!Invalid && "Expected a token at star location");
+    assert(StarTok.is(tok::star) && "Expected star token at star location");
+
+    Optional<Token> AfterStarTok = Lexer::findNextToken(StarLoc, SM, LO);
+    if (!AfterStarTok)
+      continue;
+
+    if (AfterStarTok->hasLeadingSpace()) {
+      diag(StarLoc, "star character at pointer declaration should be aligned "
+                    "to the right");
+      DiagLastPtrLoc = true;
+    }
+
+    if (PtrLoc == LastPtrLoc)
+      break;
+
+    DiagLastPtrLoc = false;
+  }
+
+  SourceLocation LastStarLoc = LastPtrLoc.getStarLoc();
+  SourceLocation BeforeLastStarLoc = LastStarLoc.getLocWithOffset(-1);
+  CharSourceRange R{SourceRange{BeforeLastStarLoc, LastStarLoc}, false};
+  bool Invalid = false;
+  StringRef TextAtLastStar = Lexer::getSourceText(R, SM, LO, &Invalid);
+  if (Invalid)
+    return;
+  if (isIdentifierBody(TextAtLastStar[0]) && !DiagLastPtrLoc)
+    diag(
+        LastStarLoc,
+        "star character at pointer declaration should be aligned to the right");
+}
+
+PointerTypeStarPlacementCheck::PointerTypeStarPlacementCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context) {}
+
+void PointerTypeStarPlacementCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      varDecl(hasDescendant(pointerType()), unless(isImplicit())).bind("var"),
+      this);
+}
+
+void PointerTypeStarPlacementCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const SourceManager &SM = *Result.SourceManager;
+  const ASTContext *Context = Result.Context;
+
+  const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var");
+  TypeLoc Loc = Var->getTypeSourceInfo()->getTypeLoc();
+
+  PointerTypeLoc FirstPtrLoc, LastPtrLoc;
+  while (Loc) {
+    // Handle PointerTypeLoc only.
+    // This excludes pointer-to-member types which are not to be checked.
+    if (auto PointerLoc = Loc.getAs<clang::PointerTypeLoc>()) {
+      // Find start of a new consecutive pointer type loc sequence.
+      if (!FirstPtrLoc)
+        FirstPtrLoc = PointerLoc;
+      // Save the last part of the sequence.
+      LastPtrLoc = PointerLoc;
+    } else {
+      if (FirstPtrLoc) {
+        checkStarLoc(FirstPtrLoc, LastPtrLoc, SM, Context->getLangOpts());
+        FirstPtrLoc = PointerTypeLoc{};
+      }
+    }
+    Loc = Loc.getNextTypeLoc();
+  }
+  if (FirstPtrLoc)
+    checkStarLoc(FirstPtrLoc, LastPtrLoc, SM, Context->getLangOpts());
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "InconsistentDeclarationParameterNameCheck.h"
 #include "IsolateDeclarationCheck.h"
 #include "MagicNumbersCheck.h"
 #include "MakeMemberFunctionConstCheck.h"
 #include "MisleadingIndentationCheck.h"
 #include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
 #include "NonConstParameterCheck.h"
+#include "PointerTypeStarPlacementCheck.h"
 #include "QualifiedAutoCheck.h"
 #include "RedundantAccessSpecifiersCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
 #include "RedundantFunctionPtrDereferenceCheck.h"
 #include "RedundantMemberInitCheck.h"
 #include "RedundantPreprocessorCheck.h"
 #include "RedundantSmartptrGetCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<MagicNumbersCheck>(
         "readability-magic-numbers");
     CheckFactories.registerCheck<MakeMemberFunctionConstCheck>(
         "readability-make-member-function-const");
     CheckFactories.registerCheck<MisleadingIndentationCheck>(
         "readability-misleading-indentation");
     CheckFactories.registerCheck<MisplacedArrayIndexCheck>(
         "readability-misplaced-array-index");
+    CheckFactories.registerCheck<PointerTypeStarPlacementCheck>(
+        "readability-pointer-type-star-placement");
     CheckFactories.registerCheck<QualifiedAutoCheck>(
         "readability-qualified-auto");
     CheckFactories.registerCheck<RedundantAccessSpecifiersCheck>(
         "readability-redundant-access-specifiers");
     CheckFactories.registerCheck<RedundantFunctionPtrDereferenceCheck>(
         "readability-redundant-function-ptr-dereference");
     CheckFactories.registerCheck<RedundantMemberInitCheck>(
         "readability-redundant-member-init");

clang-tools-extra/docs/ReleaseNotes.rst

 Improvements to clang-rename
 ----------------------------
 
 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
-The improvements are...
+- Checks that allow configuring names of headers to include now support wrapping
+  the include in angle brackets to create a system include. For example,
+  :doc:`cppcoreguidelines-init-variables
+  <clang-tidy/checks/cppcoreguidelines-init-variables>` and
+  :doc:`modernize-make-unique <clang-tidy/checks/modernize-make-unique>`.
+
+New modules
+^^^^^^^^^^^
+
+- New ``altera`` module.
+
+  Includes checks related to OpenCL for FPGA coding guidelines, based on the
+  `Altera SDK for OpenCL: Best Practices Guide
+  <https://www.altera.com/en_US/pdfs/literature/hb/opencl-sdk/aocl_optimization_guide.pdf>`_.
+
+New checks
+^^^^^^^^^^
+
+- New :doc:`altera-struct-pack-align
+  <clang-tidy/checks/altera-struct-pack-align>` check.
+
+  Finds structs that are inefficiently packed or aligned, and recommends
+  packing and/or aligning of said structs as needed.
+
+- New :doc:`cppcoreguidelines-prefer-member-initializer
+  <clang-tidy/checks/cppcoreguidelines-prefer-member-initializer>` check.
+
+  Finds member initializations in the constructor body which can be placed into
+  the initialization list instead.
+
+- New :doc:`bugprone-misplaced-pointer-arithmetic-in-alloc
+  <clang-tidy/checks/bugprone-misplaced-pointer-arithmetic-in-alloc>` check.
+
+- New :doc:`bugprone-redundant-branch-condition
+  <clang-tidy/checks/bugprone-redundant-branch-condition>` check.
+
+  Finds condition variables in nested ``if`` statements that were also checked
+  in the outer ``if`` statement and were not changed.
+
+- New :doc:`readability-pointer-type-star-placement
+  <clang-tidy/checks/readability-pointer-type-star-placement>` check.
+
+  Finds ``*`` characters in pointer declarations that are not aligned to
+  the right side.
+
+  - New :doc:`readability-function-cognitive-complexity
+  <clang-tidy/checks/readability-function-cognitive-complexity>` check.
+
+  Flags functions with Cognitive Complexity metric exceeding the configured limit.
+
+Changes in existing checks
+^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+- Improved :doc:`readability-identifier-naming
+  <clang-tidy/checks/readability-identifier-naming>` check.
+
+  Added an option `GetConfigPerFile` to support including files which use
+  different naming styles.
+
+- Removed `google-runtime-references` check because the rule it checks does
+  not exist in the Google Style Guide anymore.
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 
 Improvements to clang-include-fixer
 -----------------------------------

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

.. csv-table::
    `readability-inconsistent-declaration-parameter-name <readability-inconsistent-declaration-parameter-name.html>`_, "Yes"
    `readability-isolate-declaration <readability-isolate-declaration.html>`_, "Yes"
    `readability-magic-numbers <readability-magic-numbers.html>`_,
    `readability-make-member-function-const <readability-make-member-function-const.html>`_, "Yes"
    `readability-misleading-indentation <readability-misleading-indentation.html>`_,
    `readability-misplaced-array-index <readability-misplaced-array-index.html>`_, "Yes"
    `readability-named-parameter <readability-named-parameter.html>`_, "Yes"
    `readability-non-const-parameter <readability-non-const-parameter.html>`_, "Yes"
+   `readability-pointer-type-star-placement <readability-pointer-type-star-placement.html>`_, "Yes"
    `readability-qualified-auto <readability-qualified-auto.html>`_, "Yes"
    `readability-redundant-access-specifiers <readability-redundant-access-specifiers.html>`_, "Yes"
    `readability-redundant-control-flow <readability-redundant-control-flow.html>`_, "Yes"
    `readability-redundant-declaration <readability-redundant-declaration.html>`_, "Yes"
    `readability-redundant-function-ptr-dereference <readability-redundant-function-ptr-dereference.html>`_, "Yes"
    `readability-redundant-member-init <readability-redundant-member-init.html>`_, "Yes"
    `readability-redundant-preprocessor <readability-redundant-preprocessor.html>`_,
    `readability-redundant-smartptr-get <readability-redundant-smartptr-get.html>`_, "Yes"

clang-tools-extra/docs/clang-tidy/checks/readability-pointer-type-star-placement.rst

+.. title:: clang-tidy - readability-pointer-type-star-placement
+
+readability-pointer-type-star-placement
+=======================================
+
+Finds ``*`` characters in pointer declarations that are not aligned to the
+right side. The check works for pointer-to-pointers and arrays too.
+Return type of functions, pointer-to-members and template arguments are not
+checked.
+
+
+Before:
+
+.. code-block:: c++
+
+  int* p1;
+  int * *p2;
+  int*const*p2;
+
+After:
+
+.. code-block:: c++
+
+  int *p;
+  int **p2;
+  int *const *p2;
+
+Options
+-------
+
+.. option:: ShortStatementLines

clang-tools-extra/test/clang-tidy/checkers/readability-pointer-type-star-placement.cpp

+// RUN: %check_clang_tidy %s readability-pointer-type-star-placement %t
+
+// clang-format off
+
+int *P1;
+
+int *P2;
+// CHECK-MESSAGES: :[[@LINE-1]]:4: warning: star character at pointer declaration should be aligned to the right
+int *P3;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: star character at pointer declaration should be aligned to the right
+int *P4;
+// CHECK-MESSAGES: :[[@LINE-1]]:4: warning: star character at pointer declaration should be aligned to the right
+
+int **P5;
+// CHECK-MESSAGES: :[[@LINE-1]]:4: warning: star character at pointer declaration should be aligned to the right
+
+int **P6;
+int **P7;
+// CHECK-MESSAGES: :[[@LINE-1]]:4: warning: star character at pointer declaration should be aligned to the right
+int **P8;
+// CHECK-MESSAGES: :[[@LINE-1]]:4: warning: star character at pointer declaration should be aligned to the right
+// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: star character at pointer declaration should be aligned to the right
+
+int **P9;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: star character at pointer declaration should be aligned to the right
+int **P10;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: star character at pointer declaration should be aligned to the right
+int **P11;
+// CHECK-MESSAGES: :[[@LINE-1]]:4: warning: star character at pointer declaration should be aligned to the right
+// CHECK-MESSAGES: :[[@LINE-2]]:6: warning: star character at pointer declaration should be aligned to the right
+
+int **P12;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: star character at pointer declaration should be aligned to the right
+// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: star character at pointer declaration should be aligned to the right
+
+int *P13, *P14, *P15;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: star character at pointer declaration should be aligned to the right
+
+int *PArr1[2];
+// CHECK-MESSAGES: :[[@LINE-1]]:4: warning: star character at pointer declaration should be aligned to the right
+int *(*PArr2)[2];
+int *(*PArr3)[2];
+// CHECK-MESSAGES: :[[@LINE-1]]:4: warning: star character at pointer declaration should be aligned to the right
+// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: star character at pointer declaration should be aligned to the right
+
+void f1(int *P1, int *P2, int *P3, int *P4);
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: star character at pointer declaration should be aligned to the right
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: star character at pointer declaration should be aligned to the right
+// CHECK-MESSAGES: :[[@LINE-3]]:39: warning: star character at pointer declaration should be aligned to the right
+void f2(int *, int *);
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: star character at pointer declaration should be aligned to the right
+
+struct S {
+  void f1(int *, int *);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: star character at pointer declaration should be aligned to the right
+};
+int S::*PM1;
+void (S::*PM2)(int *, int *);
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: star character at pointer declaration should be aligned to the right
+
+int *const *PC1;
+int *const *PC2;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: star character at pointer declaration should be aligned to the right
+int *const *PC3;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: star character at pointer declaration should be aligned to the right
+int *const *PC4;
+// CHECK-MESSAGES: :[[@LINE-1]]:4: warning: star character at pointer declaration should be aligned to the right
+int *const *PC5;
+// CHECK-MESSAGES: :[[@LINE-1]]:4: warning: star character at pointer declaration should be aligned to the right
+
+void (*PF1)(int);
+void (*PF2)(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: star character at pointer declaration should be aligned to the right
+void (*PF3)(int *);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: star character at pointer declaration should be aligned to the right