[clang-tidy] Lift parsing of sequence of names functions to utils.

[etienneb]

Summary

Lift some common code used by multiple checkers.

This function is also used by checkers that are coming.
It is quite common for a checker to parse a list of names.

[alexfh]

clang-tidy/misc/SuspiciousStringCompareCheck.cpp

97

Please add a comment that the KnownStringCompareFunctions string should end with a semicolon.

clang-tidy/utils/OptionUtils.h

18 ↗

(On Diff #55929)

"options" would convey the idea slightly better, imo.

21 ↗

(On Diff #55929)

I don't think this code is specific to "names". Maybe parseStrings, parseList or parseSemicolonSeparatedList?

23 ↗

(On Diff #55929)

1. s/parse/parsed/
2. Please enclose parseName in backquotes.
3. It's more common to use ArrayRef from llvm/ADT/ArrayRef.h to pass a reference to a number of sequential elements.

[etienneb]

address alexfh comments

[alexfh]

Looks good with one nit. Thank you!

clang-tidy/utils/OptionsUtils.h

21

On the second thought, parseStrings is too vague. Maybe makes sense to rename to parseStringList (and serializeStringList).

[etienneb]

fix alexfh comments (renaming)

[etienneb]

renaming done, landing.

clang-tidy/misc/DanglingHandleCheck.cpp

 //===--- DanglingHandleCheck.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 "DanglingHandleCheck.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
 namespace {
 
-static const char HandleClassesDelimiter[] = ";";
-
-std::vector<std::string> parseClasses(StringRef Option) {
-  SmallVector<StringRef, 4> Classes;
-  Option.split(Classes, HandleClassesDelimiter);
-  std::vector<std::string> Result;
-  for (StringRef &Class : Classes) {
-    Class = Class.trim();
-    if (!Class.empty())
-      Result.push_back(Class);
-  }
-  return Result;
-}
-
 ast_matchers::internal::BindableMatcher<Stmt>
 handleFrom(ast_matchers::internal::Matcher<RecordDecl> IsAHandle,
            ast_matchers::internal::Matcher<Expr> Arg) {
   return cxxConstructExpr(hasDeclaration(cxxMethodDecl(ofClass(IsAHandle))),
                           hasArgument(0, Arg));
 }
 
 ast_matchers::internal::Matcher<Stmt> handleFromTemporaryValue(
return callExpr(
                               hasOverloadedOperatorName("[]"))));
 }
 
 } // anonymous namespace
 
 DanglingHandleCheck::DanglingHandleCheck(StringRef Name,
                                          ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      HandleClasses(parseClasses(Options.get(
+      HandleClasses(utils::options::parseStrings(Options.get(
           "HandleClasses",
           "std::basic_string_view;std::experimental::basic_string_view"))),
       IsAHandle(cxxRecordDecl(hasAnyName(std::vector<StringRef>(
                                   HandleClasses.begin(), HandleClasses.end())))
                     .bind("handle")) {}
 
 void DanglingHandleCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "HandleClasses",
-                llvm::join(HandleClasses.begin(), HandleClasses.end(),
-                           HandleClassesDelimiter));
+                utils::options::serializeStrings(HandleClasses));
 }
 
 void DanglingHandleCheck::registerMatchersForVariables(MatchFinder *Finder) {
   const auto ConvertedHandle = handleFromTemporaryValue(IsAHandle);
 
   // Find 'Handle foo(ReturnsAValue());'
   Finder->addMatcher(
       varDecl(hasType(cxxRecordDecl(IsAHandle)),

clang-tidy/misc/SuspiciousStringCompareCheck.cpp

 //===--- SuspiciousStringCompareCheck.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 "SuspiciousStringCompareCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
-#include "../utils/Matchers.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
+
+// Semicolon separated list of known string compare-like functions. The list
+// must ends with a semicolon.
 static const char KnownStringCompareFunctions[] = "__builtin_memcmp;"
                                                   "__builtin_strcasecmp;"
                                                   "__builtin_strcmp;"
                                                   "__builtin_strncasecmp;"
                                                   "__builtin_strncmp;"
                                                   "_mbscmp;"
                                                   "_mbscmp_l;"
                                                   "_mbsicmp;"
static const char KnownStringCompareFunctions[] = "__builtin_memcmp;"
                                                   "strnicmp;"
                                                   "wcscasecmp;"
                                                   "wcscmp;"
                                                   "wcsicmp;"
                                                   "wcsncmp;"
                                                   "wcsnicmp;"
                                                   "wmemcmp;";
 
-static const char StringCompareLikeFunctionsDelimiter[] = ";";
-
-static void ParseFunctionNames(StringRef Option,
-                               std::vector<std::string> *Result) {
-  SmallVector<StringRef, 4> Functions;
-  Option.split(Functions, StringCompareLikeFunctionsDelimiter);
-  for (StringRef &Function : Functions) {
-    Function = Function.trim();
-    if (!Function.empty())
-      Result->push_back(Function);
-  }
-}
-
 SuspiciousStringCompareCheck::SuspiciousStringCompareCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       WarnOnImplicitComparison(Options.get("WarnOnImplicitComparison", 1)),
       WarnOnLogicalNotComparison(Options.get("WarnOnLogicalNotComparison", 0)),
       StringCompareLikeFunctions(
           Options.get("StringCompareLikeFunctions", "")) {}
 
void SuspiciousStringCompareCheck::registerMatchers(MatchFinder *Finder) {
   const auto ComparisonUnaryOperator = unaryOperator(hasOperatorName("!"));
   const auto ComparisonBinaryOperator =
       binaryOperator(matchers::isComparisonOperator());
   const auto ComparisonOperator =
       expr(anyOf(ComparisonUnaryOperator, ComparisonBinaryOperator));
 
   // Add the list of known string compare-like functions and add user-defined
   // functions.
-  std::vector<std::string> FunctionNames;
-  ParseFunctionNames(KnownStringCompareFunctions, &FunctionNames);
-  ParseFunctionNames(StringCompareLikeFunctions, &FunctionNames);
+  std::vector<std::string> FunctionNames = utils::options::parseStrings(
+      (llvm::Twine(KnownStringCompareFunctions) + StringCompareLikeFunctions)
+          .str());
 
   // Match a call to a string compare functions.
   const auto FunctionCompareDecl =
       functionDecl(hasAnyName(std::vector<StringRef>(FunctionNames.begin(),
                                                      FunctionNames.end())))
           .bind("decl");
   const auto DirectStringCompareCallExpr =
       callExpr(hasDeclaration(FunctionCompareDecl)).bind("call");

clang-tidy/performance/FasterStringFindCheck.cpp

 //===--- FasterStringFindCheck.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 "FasterStringFindCheck.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace performance {
 
 namespace {
 
-static const char StringLikeClassesDelimiter[] = ";";
-
-std::vector<std::string> ParseClasses(StringRef Option) {
-  SmallVector<StringRef, 4> Classes;
-  Option.split(Classes, StringLikeClassesDelimiter);
-  std::vector<std::string> Result;
-  for (StringRef &Class : Classes) {
-    Class = Class.trim();
-    if (!Class.empty())
-      Result.push_back(Class);
-  }
-  return Result;
-}
-
 llvm::Optional<std::string> MakeCharacterLiteral(const StringLiteral *Literal) {
   std::string Result;
   {
     llvm::raw_string_ostream OS(Result);
     Literal->outputString(OS);
   }
   // Now replace the " with '.
   auto pos = Result.find_first_of('"');
return hasType(qualType(anyOf(substTemplateTypeParmType(),
                                 hasDescendant(substTemplateTypeParmType()))));
 }
 
 } // namespace
 
 FasterStringFindCheck::FasterStringFindCheck(StringRef Name,
                                              ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      StringLikeClasses(
-          ParseClasses(Options.get("StringLikeClasses", "std::basic_string"))) {
+      StringLikeClasses(utils::options::parseStrings(
+          Options.get("StringLikeClasses", "std::basic_string"))) {
 }
 
 void FasterStringFindCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "StringLikeClasses",
-                llvm::join(StringLikeClasses.begin(), StringLikeClasses.end(),
-                           StringLikeClassesDelimiter));
+                utils::options::serializeStrings(StringLikeClasses));
 }
 
 void FasterStringFindCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
     return;
 
   const auto SingleChar =
       expr(ignoringParenCasts(stringLiteral(lengthIsOne()).bind("literal")));

clang-tidy/utils/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyUtils
   DeclRefExprUtils.cpp
   FixItHintUtils.cpp
-  HeaderGuard.cpp
   HeaderFileExtensionsUtils.cpp
+  HeaderGuard.cpp
   IncludeInserter.cpp
   IncludeSorter.cpp
   LexerUtils.cpp
+  OptionsUtils.cpp
   TypeTraits.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   )

clang-tidy/utils/OptionsUtils.h

+//===--- DanglingHandleCheck.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_MISC_OPTIONUTILS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_OPTIONUTILS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace utils {
+namespace options {
+
+/// \brief Parse a semicolon separated list of strings.
+std::vector<std::string> parseStrings(StringRef Option);
+
+/// \brief Serialize a sequence of names that can be parsed by 'parseStrings'.
+std::string serializeStrings(ArrayRef<std::string> Strings);
+
+} // namespace options
+} // namespace utils
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_OPTIONUTILS_H

clang-tidy/utils/OptionsUtils.cpp

+//===--- DanglingHandleCheck.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 "OptionsUtils.h"
+
+namespace clang {
+namespace tidy {
+namespace utils {
+namespace options {
+
+static const char StringsDelimiter[] = ";";
+
+std::vector<std::string> parseStrings(StringRef Option) {
+  SmallVector<StringRef, 4> Names;
+  Option.split(Names, StringsDelimiter);
+  std::vector<std::string> Result;
+  for (StringRef &Name : Names) {
+    Name = Name.trim();
+    if (!Name.empty())
+      Result.push_back(Name);
+  }
+  return Result;
+}
+
+std::string serializeStrings(ArrayRef<std::string> Strings) {
+  return llvm::join(Strings.begin(), Strings.end(), StringsDelimiter);
+}
+
+} // namespace options
+} // namespace utils
+} // namespace tidy
+} // namespace clang