Implement the readability-const-return-type check.

This check flags function top-level const-qualified return types and suggests removing the mostly-superfluous const qualifier where possible.

Patch by Yitzhak Mandelbaum.

git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@345764 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 167 insertions, 0 deletions

diff --git a/clang-tidy/readability/CMakeLists.txt b/clang-tidy/readability/CMakeLists.txt
index dace226a..c429b812 100644
--- a/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tidy/readability/CMakeLists.txt
@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support)
 add_clang_library(clangTidyReadabilityModule
   AvoidConstParamsInDecls.cpp
   BracesAroundStatementsCheck.cpp
+  ConstReturnTypeCheck.cpp
   ContainerSizeEmptyCheck.cpp
   DeleteNullPointerCheck.cpp
   DeletedDefaultCheck.cpp
diff --git a/clang-tidy/readability/ConstReturnTypeCheck.cpp b/clang-tidy/readability/ConstReturnTypeCheck.cpp
new file mode 100644
index 00000000..d9654df4
--- /dev/null
+++ b/clang-tidy/readability/ConstReturnTypeCheck.cpp
@@ -0,0 +1,128 @@
+//===--- ConstReturnTypeCheck.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 "ConstReturnTypeCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/Optional.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+// Finds the location of the qualifying `const` token in the `FunctionDecl`'s
+// return type. Returns `None` when the return type is not `const`-qualified or
+// `const` does not appear in `Def`'s source like when the type is an alias or a
+// macro.
+static llvm::Optional<Token>
+findConstToRemove(const FunctionDecl *Def,
+                  const MatchFinder::MatchResult &Result) {
+  if (!Def->getReturnType().isLocalConstQualified())
+    return None;
+
+  // Get the begin location for the function name, including any qualifiers
+  // written in the source (for out-of-line declarations). A FunctionDecl's
+  // "location" is the start of its name, so, when the name is unqualified, we
+  // use `getLocation()`.
+  SourceLocation NameBeginLoc = Def->getQualifier()
+                                    ? Def->getQualifierLoc().getBeginLoc()
+                                    : Def->getLocation();
+  // Since either of the locs can be in a macro, use `makeFileCharRange` to be
+  // sure that we have a consistent `CharSourceRange`, located entirely in the
+  // source file.
+  CharSourceRange FileRange = Lexer::makeFileCharRange(
+      CharSourceRange::getCharRange(Def->getBeginLoc(), NameBeginLoc),
+      *Result.SourceManager, Result.Context->getLangOpts());
+
+  if (FileRange.isInvalid())
+    return None;
+
+  return utils::lexer::getConstQualifyingToken(FileRange, *Result.Context,
+                                               *Result.SourceManager);
+}
+
+namespace {
+
+struct CheckResult {
+  // Source range of the relevant `const` token in the definition being checked.
+  CharSourceRange ConstRange;
+
+  // FixItHints associated with the definition being checked.
+  llvm::SmallVector<clang::FixItHint, 4> Hints;
+
+  // Locations of any declarations that could not be fixed.
+  llvm::SmallVector<clang::SourceLocation, 4> DeclLocs;
+};
+
+} // namespace
+
+// Does the actual work of the check.
+static CheckResult checkDef(const clang::FunctionDecl *Def,
+                            const MatchFinder::MatchResult &MatchResult) {
+  CheckResult Result;
+  llvm::Optional<Token> Tok = findConstToRemove(Def, MatchResult);
+  if (!Tok)
+    return Result;
+
+  Result.ConstRange =
+      CharSourceRange::getCharRange(Tok->getLocation(), Tok->getEndLoc());
+  Result.Hints.push_back(FixItHint::CreateRemoval(Result.ConstRange));
+
+  // Fix the definition and any visible declarations, but don't warn
+  // seperately for each declaration. Instead, associate all fixes with the
+  // single warning at the definition.
+  for (const FunctionDecl *Decl = Def->getPreviousDecl(); Decl != nullptr;
+       Decl = Decl->getPreviousDecl()) {
+    if (llvm::Optional<Token> T = findConstToRemove(Decl, MatchResult))
+      Result.Hints.push_back(FixItHint::CreateRemoval(
+          CharSourceRange::getCharRange(T->getLocation(), T->getEndLoc())));
+    else
+      // `getInnerLocStart` gives the start of the return type.
+      Result.DeclLocs.push_back(Decl->getInnerLocStart());
+  }
+  return Result;
+}
+
+void ConstReturnTypeCheck::registerMatchers(MatchFinder *Finder) {
+  // Find all function definitions for which the return types are `const`
+  // qualified.
+  Finder->addMatcher(
+      functionDecl(returns(isConstQualified()), isDefinition()).bind("func"),
+      this);
+}
+
+void ConstReturnTypeCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Def = Result.Nodes.getNodeAs<FunctionDecl>("func");
+  CheckResult CR = checkDef(Def, Result);
+  {
+    // Clang only supports one in-flight diagnostic at a time. So, delimit the
+    // scope of `Diagnostic` to allow further diagnostics after the scope.  We
+    // use `getInnerLocStart` to get the start of the return type.
+    DiagnosticBuilder Diagnostic =
+        diag(Def->getInnerLocStart(),
+             "return type %0 is 'const'-qualified at the top level, which may "
+             "reduce code readability without improving const correctness")
+        << Def->getReturnType();
+    if (CR.ConstRange.isValid())
+      Diagnostic << CR.ConstRange;
+    for (auto &Hint : CR.Hints)
+      Diagnostic << Hint;
+  }
+  for (auto Loc : CR.DeclLocs)
+    diag(Loc, "could not transform this declaration", DiagnosticIDs::Note);
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tidy/readability/ConstReturnTypeCheck.h b/clang-tidy/readability/ConstReturnTypeCheck.h
new file mode 100644
index 00000000..2c7e94de
--- /dev/null
+++ b/clang-tidy/readability/ConstReturnTypeCheck.h
@@ -0,0 +1,35 @@
+//===--- ConstReturnTypeCheck.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_CONSTRETURNTYPECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONSTRETURNTYPECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// For any function whose return type is const-qualified, suggests removal of
+/// the `const` qualifier from that return type.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-const-return-type.html
+class ConstReturnTypeCheck : public ClangTidyCheck {
+ public:
+  using ClangTidyCheck::ClangTidyCheck;
+  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_CONSTRETURNTYPECHECK_H
diff --git a/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tidy/readability/ReadabilityTidyModule.cpp
index 5855bb07..8dab2968 100644
--- a/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "AvoidConstParamsInDecls.h"
 #include "BracesAroundStatementsCheck.h"
+#include "ConstReturnTypeCheck.h"
 #include "ContainerSizeEmptyCheck.h"
 #include "DeleteNullPointerCheck.h"
 #include "DeletedDefaultCheck.h"
@@ -52,6 +53,8 @@ public:
         "readability-avoid-const-params-in-decls");
     CheckFactories.registerCheck<BracesAroundStatementsCheck>(
         "readability-braces-around-statements");
+    CheckFactories.registerCheck<ConstReturnTypeCheck>(
+        "readability-const-return-type");
     CheckFactories.registerCheck<ContainerSizeEmptyCheck>(
         "readability-container-size-empty");
     CheckFactories.registerCheck<DeleteNullPointerCheck>(