[PATCH] D61749: [clang-tidy] initial version of readability-static-const-method

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added subscribers: JonasToth, Eugene.Zelenko.
Eugene.Zelenko added a comment.

@JonasToth tried to implement const check, so probably const and static part should be split. It's hard to tell about state of existing implementation, but you could continue it or at least borrow ideas and tests.



================
Comment at: clang-tools-extra/clang-tidy/readability/StaticConstMethodCheck.cpp:33
+  template <class T> const T *getParent(const Expr *E) {
+    auto Parents = Ctxt.getParents(*E);
+    if (Parents.size() != 1)
----------------
Please don't use auto when type is not spelled explicitly in same statement or it's iterator.


================
Comment at: clang-tools-extra/clang-tidy/readability/StaticConstMethodCheck.cpp:101
+                                    SourceRange Range) {
+  if (SourceMgr.getFileID(Range.getBegin()) !=
+      SourceMgr.getFileID(Range.getEnd())) {
----------------
Please elide braces.


================
Comment at: clang-tools-extra/clang-tidy/readability/StaticConstMethodCheck.cpp:103
+      SourceMgr.getFileID(Range.getEnd())) {
+    return StringRef(); // Empty string.
+  }
----------------
You could return {}


================
Comment at: clang-tools-extra/clang-tidy/readability/StaticConstMethodCheck.cpp:124
+  // Find the exact position of "const".
+  auto Text = getStringFromRange(SourceMgr, LangOpts, Range);
+  auto Offset = Text.find("const");
----------------
Please don't use auto when type is not spelled explicitly in same statement or it's iterator.


================
Comment at: clang-tools-extra/clang-tidy/readability/StaticConstMethodCheck.cpp:129
+
+  auto Start = Range.getBegin().getLocWithOffset(Offset);
+  return {Start, Start.getLocWithOffset(strlen("const") - 1)};
----------------
Please don't use auto when type is not spelled explicitly in same statement or it's iterator.


================
Comment at: clang-tools-extra/clang-tidy/readability/StaticConstMethodCheck.cpp:134
+static SourceLocation getConstInsertionPoint(const CXXMethodDecl *M) {
+  auto TSI = M->getTypeSourceInfo();
+  if (!TSI)
----------------
Please don't use auto when type is not spelled explicitly in same statement or it's iterator.


================
Comment at: clang-tools-extra/clang-tidy/readability/StaticConstMethodCheck.cpp:183
+
+  auto Declaration = Definition->getCanonicalDecl();
+
----------------
Please don't use auto when type is not spelled explicitly in same statement or it's iterator.


================
Comment at: clang-tools-extra/clang-tidy/readability/StaticConstMethodCheck.cpp:199
+      !isa<CXXConversionDecl>(Definition)) {
+    auto Diag = diag(Definition->getLocation(), "method %0 can be made static")
+                << Definition;
----------------
Please don't use auto when type is not spelled explicitly in same statement or it's iterator.


================
Comment at: clang-tools-extra/clang-tidy/readability/StaticConstMethodCheck.cpp:213
+      if (Declaration != Definition) {
+        auto DeclConst = getLocationOfConst(Declaration->getTypeSourceInfo(),
+                                            *Result.SourceManager,
----------------
Please don't use auto when type is not spelled explicitly in same statement or it's iterator.


================
Comment at: clang-tools-extra/clang-tidy/readability/StaticConstMethodCheck.cpp:226
+    Diag << FixItHint::CreateInsertion(Declaration->getBeginLoc(), "static ");
+
+  } else if (UsageOfThis.Usage <= Const && !Definition->isConst()) {
----------------
Unnecessary empty line.


================
Comment at: clang-tools-extra/clang-tidy/readability/StaticConstMethodCheck.cpp:228
+  } else if (UsageOfThis.Usage <= Const && !Definition->isConst()) {
+    auto Diag = diag(Definition->getLocation(), "method %0 can be made const")
+                << Definition
----------------
Please don't use auto when type is not spelled explicitly in same statement or it's iterator.


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:72
 
+- New :doc:`readability-static-const-method
+  <clang-tidy/checks/readability-static-const-method>` check.
----------------
Please rebase from trunk.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61749/new/

https://reviews.llvm.org/D61749