[clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

Summary:
Detects when the integral literal or floating point (decimal or hexadecimal)
literal has non-uppercase suffix, and suggests to make the suffix uppercase,
with fix-it.

All valid combinations of suffixes are supported.

```
  auto x = 1;  // OK, no suffix.

  auto x = 1u; // warning: integer literal suffix 'u' is not upper-case

  auto x = 1U; // OK, suffix is uppercase.

  ...
```

References:
* [[ https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152241 | CERT DCL16-C ]]
* MISRA C:2012, 7.3 - The lowercase character "l" shall not be used in a literal suffix
* MISRA C++:2008, 2-13-4 - Literal suffixes shall be upper case

Reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun

Reviewed By: aaron.ballman

Subscribers: Eugene.Zelenko, mgorny, rnkovacs, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D52670

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

10 files changed, 338 insertions, 19 deletions

diff --git a/clang-tidy/cert/CERTTidyModule.cpp b/clang-tidy/cert/CERTTidyModule.cpp
index da09932e..b6a0e7b6 100644
--- a/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tidy/cert/CERTTidyModule.cpp
@@ -16,6 +16,7 @@
 #include "../misc/StaticAssertCheck.h"
 #include "../misc/ThrowByValueCatchByReferenceCheck.h"
 #include "../performance/MoveConstructorInitCheck.h"
+#include "../readability/UppercaseLiteralSuffixCheck.h"
 #include "CommandProcessorCheck.h"
 #include "DontModifyStdNamespaceCheck.h"
 #include "FloatLoopCounter.h"
@@ -65,6 +66,8 @@ public:
     // C checkers
     // DCL
     CheckFactories.registerCheck<misc::StaticAssertCheck>("cert-dcl03-c");
+    CheckFactories.registerCheck<readability::UppercaseLiteralSuffixCheck>(
+        "cert-dcl16-c");
     // ENV
     CheckFactories.registerCheck<CommandProcessorCheck>("cert-env33-c");
     // FLP
@@ -78,6 +81,13 @@ public:
     CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>(
         "cert-msc32-c");
   }
+
+  ClangTidyOptions getModuleOptions() override {
+    ClangTidyOptions Options;
+    ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
+    Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU";
+    return Options;
+  }
 };
 
 } // namespace cert
diff --git a/clang-tidy/cert/CMakeLists.txt b/clang-tidy/cert/CMakeLists.txt
index edc93c8e..aa05cc47 100644
--- a/clang-tidy/cert/CMakeLists.txt
+++ b/clang-tidy/cert/CMakeLists.txt
@@ -24,5 +24,6 @@ add_clang_library(clangTidyCERTModule
   clangTidyGoogleModule
   clangTidyMiscModule
   clangTidyPerformanceModule
+  clangTidyReadabilityModule
   clangTidyUtils
   )
diff --git a/clang-tidy/hicpp/HICPPTidyModule.cpp b/clang-tidy/hicpp/HICPPTidyModule.cpp
index d2b9fc6d..7b25cdaa 100644
--- a/clang-tidy/hicpp/HICPPTidyModule.cpp
+++ b/clang-tidy/hicpp/HICPPTidyModule.cpp
@@ -35,6 +35,7 @@
 #include "../readability/BracesAroundStatementsCheck.h"
 #include "../readability/FunctionSizeCheck.h"
 #include "../readability/IdentifierNamingCheck.h"
+#include "../readability/UppercaseLiteralSuffixCheck.h"
 #include "ExceptionBaseclassCheck.h"
 #include "MultiwayPathsCoveredCheck.h"
 #include "NoAssemblerCheck.h"
@@ -100,6 +101,8 @@ public:
         "hicpp-use-nullptr");
     CheckFactories.registerCheck<modernize::UseOverrideCheck>(
         "hicpp-use-override");
+    CheckFactories.registerCheck<readability::UppercaseLiteralSuffixCheck>(
+        "hicpp-uppercase-literal-suffix");
     CheckFactories.registerCheck<cppcoreguidelines::ProTypeVarargCheck>(
         "hicpp-vararg");
   }
diff --git a/clang-tidy/readability/CMakeLists.txt b/clang-tidy/readability/CMakeLists.txt
index da2320ea..bd010af9 100644
--- a/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tidy/readability/CMakeLists.txt
@@ -31,6 +31,7 @@ add_clang_library(clangTidyReadabilityModule
   StaticDefinitionInAnonymousNamespaceCheck.cpp
   StringCompareCheck.cpp
   UniqueptrDeleteReleaseCheck.cpp
+  UppercaseLiteralSuffixCheck.cpp
 
   LINK_LIBS
   clangAST
diff --git a/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tidy/readability/IdentifierNamingCheck.cpp
index 1d116817..fb3c02e1 100644
--- a/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -9,6 +9,7 @@
 
 #include "IdentifierNamingCheck.h"
 
+#include "../utils/ASTUtils.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/PPCallbacks.h"
@@ -681,25 +682,7 @@ static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
   if (!Failure.ShouldFix)
     return;
 
-  // Check if the range is entirely contained within a macro argument.
-  SourceLocation MacroArgExpansionStartForRangeBegin;
-  SourceLocation MacroArgExpansionStartForRangeEnd;
-  bool RangeIsEntirelyWithinMacroArgument =
-      SourceMgr &&
-      SourceMgr->isMacroArgExpansion(Range.getBegin(),
-                                     &MacroArgExpansionStartForRangeBegin) &&
-      SourceMgr->isMacroArgExpansion(Range.getEnd(),
-                                     &MacroArgExpansionStartForRangeEnd) &&
-      MacroArgExpansionStartForRangeBegin == MacroArgExpansionStartForRangeEnd;
-
-  // Check if the range contains any locations from a macro expansion.
-  bool RangeContainsMacroExpansion = RangeIsEntirelyWithinMacroArgument ||
-                                     Range.getBegin().isMacroID() ||
-                                     Range.getEnd().isMacroID();
-
-  bool RangeCanBeFixed =
-      RangeIsEntirelyWithinMacroArgument || !RangeContainsMacroExpansion;
-  Failure.ShouldFix = RangeCanBeFixed;
+  Failure.ShouldFix = utils::rangeCanBeFixed(Range, SourceMgr);
 }
 
 /// Convenience method when the usage to be added is a NamedDecl
diff --git a/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tidy/readability/ReadabilityTidyModule.cpp
index a5b91544..ca49f6f1 100644
--- a/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -38,6 +38,7 @@
 #include "StaticDefinitionInAnonymousNamespaceCheck.h"
 #include "StringCompareCheck.h"
 #include "UniqueptrDeleteReleaseCheck.h"
+#include "UppercaseLiteralSuffixCheck.h"
 
 namespace clang {
 namespace tidy {
@@ -102,6 +103,8 @@ public:
         "readability-simplify-boolean-expr");
     CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>(
         "readability-uniqueptr-delete-release");
+    CheckFactories.registerCheck<UppercaseLiteralSuffixCheck>(
+        "readability-uppercase-literal-suffix");
   }
 };
 
diff --git a/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp b/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
new file mode 100644
index 00000000..3feeaf99
--- /dev/null
+++ b/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
@@ -0,0 +1,236 @@
+//===--- UppercaseLiteralSuffixCheck.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 "UppercaseLiteralSuffixCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+namespace {
+
+struct IntegerLiteralCheck {
+  using type = clang::IntegerLiteral;
+  static constexpr llvm::StringLiteral Name = llvm::StringLiteral("integer");
+  // What should be skipped before looking for the Suffixes? (Nothing here.)
+  static constexpr llvm::StringLiteral SkipFirst = llvm::StringLiteral("");
+  // Suffix can only consist of 'u' and 'l' chars, and can be a complex number
+  // ('i', 'j'). In MS compatibility mode, suffixes like i32 are supported.
+  static constexpr llvm::StringLiteral Suffixes =
+      llvm::StringLiteral("uUlLiIjJ");
+};
+constexpr llvm::StringLiteral IntegerLiteralCheck::Name;
+constexpr llvm::StringLiteral IntegerLiteralCheck::SkipFirst;
+constexpr llvm::StringLiteral IntegerLiteralCheck::Suffixes;
+
+struct FloatingLiteralCheck {
+  using type = clang::FloatingLiteral;
+  static constexpr llvm::StringLiteral Name =
+      llvm::StringLiteral("floating point");
+  // C++17 introduced hexadecimal floating-point literals, and 'f' is both a
+  // valid hexadecimal digit in a hex float literal and a valid floating-point
+  // literal suffix.
+  // So we can't just "skip to the chars that can be in the suffix".
+  // Since the exponent ('p'/'P') is mandatory for hexadecimal floating-point
+  // literals, we first skip everything before the exponent.
+  static constexpr llvm::StringLiteral SkipFirst = llvm::StringLiteral("pP");
+  // Suffix can only consist of 'f', 'l', "f16", 'h', 'q' chars,
+  // and can be a complex number ('i', 'j').
+  static constexpr llvm::StringLiteral Suffixes =
+      llvm::StringLiteral("fFlLhHqQiIjJ");
+};
+constexpr llvm::StringLiteral FloatingLiteralCheck::Name;
+constexpr llvm::StringLiteral FloatingLiteralCheck::SkipFirst;
+constexpr llvm::StringLiteral FloatingLiteralCheck::Suffixes;
+
+struct NewSuffix {
+  SourceLocation LiteralLocation;
+  StringRef OldSuffix;
+  llvm::Optional<FixItHint> FixIt;
+};
+
+llvm::Optional<SourceLocation> GetMacroAwareLocation(SourceLocation Loc,
+                                                     const SourceManager &SM) {
+  // Do nothing if the provided location is invalid.
+  if (Loc.isInvalid())
+    return llvm::None;
+  // Look where the location was *actually* written.
+  SourceLocation SpellingLoc = SM.getSpellingLoc(Loc);
+  if (SpellingLoc.isInvalid())
+    return llvm::None;
+  return SpellingLoc;
+}
+
+llvm::Optional<SourceRange> GetMacroAwareSourceRange(SourceRange Loc,
+                                                     const SourceManager &SM) {
+  llvm::Optional<SourceLocation> Begin =
+      GetMacroAwareLocation(Loc.getBegin(), SM);
+  llvm::Optional<SourceLocation> End = GetMacroAwareLocation(Loc.getEnd(), SM);
+  if (!Begin || !End)
+    return llvm::None;
+  return SourceRange(*Begin, *End);
+}
+
+llvm::Optional<std::string>
+getNewSuffix(llvm::StringRef OldSuffix,
+             const std::vector<std::string> &NewSuffixes) {
+  // If there is no config, just uppercase the entirety of the suffix.
+  if (NewSuffixes.empty())
+    return OldSuffix.upper();
+  // Else, find matching suffix, case-*insensitive*ly.
+  auto NewSuffix = llvm::find_if(
+      NewSuffixes, [OldSuffix](const std::string &PotentialNewSuffix) {
+        return OldSuffix.equals_lower(PotentialNewSuffix);
+      });
+  // Have a match, return it.
+  if (NewSuffix != NewSuffixes.end())
+    return *NewSuffix;
+  // Nope, I guess we have to keep it as-is.
+  return llvm::None;
+}
+
+template <typename LiteralType>
+llvm::Optional<NewSuffix>
+shouldReplaceLiteralSuffix(const Expr &Literal,
+                           const std::vector<std::string> &NewSuffixes,
+                           const SourceManager &SM, const LangOptions &LO) {
+  NewSuffix ReplacementDsc;
+
+  const auto &L = cast<typename LiteralType::type>(Literal);
+
+  // The naive location of the literal. Is always valid.
+  ReplacementDsc.LiteralLocation = L.getLocation();
+
+  // Was this literal fully spelled or is it a product of macro expansion?
+  bool RangeCanBeFixed =
+      utils::rangeCanBeFixed(ReplacementDsc.LiteralLocation, &SM);
+
+  // The literal may have macro expansion, we need the final expanded src range.
+  llvm::Optional<SourceRange> Range =
+      GetMacroAwareSourceRange(ReplacementDsc.LiteralLocation, SM);
+  if (!Range)
+    return llvm::None;
+
+  if (RangeCanBeFixed)
+    ReplacementDsc.LiteralLocation = Range->getBegin();
+  // Else keep the naive literal location!
+
+  // Get the whole literal from the source buffer.
+  bool Invalid;
+  const StringRef LiteralSourceText = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(*Range), SM, LO, &Invalid);
+  assert(!Invalid && "Failed to retrieve the source text.");
+
+  size_t Skip = 0;
+
+  // Do we need to ignore something before actually looking for the suffix?
+  if (!LiteralType::SkipFirst.empty()) {
+    // E.g. we can't look for 'f' suffix in hexadecimal floating-point literals
+    // until after we skip to the exponent (which is mandatory there),
+    // because hex-digit-sequence may contain 'f'.
+    Skip = LiteralSourceText.find_first_of(LiteralType::SkipFirst);
+    // We could be in non-hexadecimal floating-point literal, with no exponent.
+    if (Skip == StringRef::npos)
+      Skip = 0;
+  }
+
+  // Find the beginning of the suffix by looking for the first char that is
+  // one of these chars that can be in the suffix, potentially starting looking
+  // in the exponent, if we are skipping hex-digit-sequence.
+  Skip = LiteralSourceText.find_first_of(LiteralType::Suffixes, /*From=*/Skip);
+
+  // We can't check whether the *Literal has any suffix or not without actually
+  // looking for the suffix. So it is totally possible that there is no suffix.
+  if (Skip == StringRef::npos)
+    return llvm::None;
+
+  // Move the cursor in the source range to the beginning of the suffix.
+  Range->setBegin(Range->getBegin().getLocWithOffset(Skip));
+  // And in our textual representation too.
+  ReplacementDsc.OldSuffix = LiteralSourceText.drop_front(Skip);
+  assert(!ReplacementDsc.OldSuffix.empty() &&
+         "We still should have some chars left.");
+
+  // And get the replacement suffix.
+  llvm::Optional<std::string> NewSuffix =
+      getNewSuffix(ReplacementDsc.OldSuffix, NewSuffixes);
+  if (!NewSuffix || ReplacementDsc.OldSuffix == *NewSuffix)
+    return llvm::None; // The suffix was already the way it should be.
+
+  if (RangeCanBeFixed)
+    ReplacementDsc.FixIt = FixItHint::CreateReplacement(*Range, *NewSuffix);
+
+  return ReplacementDsc;
+}
+
+} // namespace
+
+UppercaseLiteralSuffixCheck::UppercaseLiteralSuffixCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      NewSuffixes(
+          utils::options::parseStringList(Options.get("NewSuffixes", ""))) {}
+
+void UppercaseLiteralSuffixCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "NewSuffixes",
+                utils::options::serializeStringList(NewSuffixes));
+}
+
+void UppercaseLiteralSuffixCheck::registerMatchers(MatchFinder *Finder) {
+  // Sadly, we can't check whether the literal has sufix or not.
+  // E.g. i32 suffix still results in 'BuiltinType::Kind::Int'.
+  // And such an info is not stored in the *Literal itself.
+  Finder->addMatcher(
+      stmt(allOf(eachOf(integerLiteral().bind(IntegerLiteralCheck::Name),
+                        floatLiteral().bind(FloatingLiteralCheck::Name)),
+                 unless(hasParent(userDefinedLiteral())))),
+      this);
+}
+
+template <typename LiteralType>
+bool UppercaseLiteralSuffixCheck::checkBoundMatch(
+    const MatchFinder::MatchResult &Result) {
+  const auto *Literal =
+      Result.Nodes.getNodeAs<typename LiteralType::type>(LiteralType::Name);
+  if (!Literal)
+    return false;
+
+  // We won't *always* want to diagnose.
+  // We might have a suffix that is already uppercase.
+  if (auto Details = shouldReplaceLiteralSuffix<LiteralType>(
+          *Literal, NewSuffixes, *Result.SourceManager, getLangOpts())) {
+    auto Complaint = diag(Details->LiteralLocation,
+                          "%0 literal has suffix '%1', which is not uppercase")
+                     << LiteralType::Name << Details->OldSuffix;
+    if (Details->FixIt) // Similarly, a fix-it is not always possible.
+      Complaint << *(Details->FixIt);
+  }
+
+  return true;
+}
+
+void UppercaseLiteralSuffixCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  if (checkBoundMatch<IntegerLiteralCheck>(Result))
+    return; // If it *was* IntegerLiteral, don't check for FloatingLiteral.
+  checkBoundMatch<FloatingLiteralCheck>(Result);
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tidy/readability/UppercaseLiteralSuffixCheck.h b/clang-tidy/readability/UppercaseLiteralSuffixCheck.h
new file mode 100644
index 00000000..8d26e6c2
--- /dev/null
+++ b/clang-tidy/readability/UppercaseLiteralSuffixCheck.h
@@ -0,0 +1,44 @@
+//===--- UppercaseLiteralSuffixCheck.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_UPPERCASELITERALSUFFIXCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UPPERCASELITERALSUFFIXCHECK_H
+
+#include "../ClangTidy.h"
+#include "../utils/OptionsUtils.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Detects when the integral literal or floating point literal has
+/// non-uppercase suffix, and suggests to make the suffix uppercase.
+/// Alternatively, a list of destination suffixes can be provided.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-uppercase-literal-suffix.html
+class UppercaseLiteralSuffixCheck : public ClangTidyCheck {
+public:
+  UppercaseLiteralSuffixCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  template <typename LiteralType>
+  bool checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result);
+
+  const std::vector<std::string> NewSuffixes;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UPPERCASELITERALSUFFIXCHECK_H
diff --git a/clang-tidy/utils/ASTUtils.cpp b/clang-tidy/utils/ASTUtils.cpp
index 5c6b9843..3c0427fd 100644
--- a/clang-tidy/utils/ASTUtils.cpp
+++ b/clang-tidy/utils/ASTUtils.cpp
@@ -67,6 +67,32 @@ bool exprHasBitFlagWithSpelling(const Expr *Flags, const SourceManager &SM,
   return true;
 }
 
+bool rangeIsEntirelyWithinMacroArgument(SourceRange Range,
+                                        const SourceManager *SM) {
+  // Check if the range is entirely contained within a macro argument.
+  SourceLocation MacroArgExpansionStartForRangeBegin;
+  SourceLocation MacroArgExpansionStartForRangeEnd;
+  bool RangeIsEntirelyWithinMacroArgument =
+      SM &&
+      SM->isMacroArgExpansion(Range.getBegin(),
+                              &MacroArgExpansionStartForRangeBegin) &&
+      SM->isMacroArgExpansion(Range.getEnd(),
+                              &MacroArgExpansionStartForRangeEnd) &&
+      MacroArgExpansionStartForRangeBegin == MacroArgExpansionStartForRangeEnd;
+
+  return RangeIsEntirelyWithinMacroArgument;
+}
+
+bool rangeContainsMacroExpansion(SourceRange Range, const SourceManager *SM) {
+  return rangeIsEntirelyWithinMacroArgument(Range, SM) ||
+         Range.getBegin().isMacroID() || Range.getEnd().isMacroID();
+}
+
+bool rangeCanBeFixed(SourceRange Range, const SourceManager *SM) {
+  return utils::rangeIsEntirelyWithinMacroArgument(Range, SM) ||
+         !utils::rangeContainsMacroExpansion(Range, SM);
+}
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
diff --git a/clang-tidy/utils/ASTUtils.h b/clang-tidy/utils/ASTUtils.h
index ccff000a..4196eeb9 100644
--- a/clang-tidy/utils/ASTUtils.h
+++ b/clang-tidy/utils/ASTUtils.h
@@ -27,6 +27,18 @@ bool IsBinaryOrTernary(const Expr *E);
 bool exprHasBitFlagWithSpelling(const Expr *Flags, const SourceManager &SM,
                                 const LangOptions &LangOpts,
                                 StringRef FlagName);
+
+// Check if the range is entirely contained within a macro argument.
+bool rangeIsEntirelyWithinMacroArgument(SourceRange Range,
+                                        const SourceManager *SM);
+
+// Check if the range contains any locations from a macro expansion.
+bool rangeContainsMacroExpansion(SourceRange Range, const SourceManager *SM);
+
+// Can a fix-it be issued for this whole Range?
+// FIXME: false-negative if the entire range is fully expanded from a macro.
+bool rangeCanBeFixed(SourceRange Range, const SourceManager *SM);
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang