[PATCH] D77348: [clangd] Enable some nice clang-tidy checks by default.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77348

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -691,18 +691,42 @@
   std::unique_ptr<tidy::ClangTidyOptionsProvider>
       ClangTidyOptProvider; /*GUARDED_BY(ClangTidyOptMu)*/
   if (EnableClangTidy) {
-    auto OverrideClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
-    OverrideClangTidyOptions.Checks = ClangTidyChecks;
+    auto EmptyDefaults = tidy::ClangTidyOptions::getDefaults();
+    EmptyDefaults.Checks.reset(); // So we can tell if checks were ever set.
+    tidy::ClangTidyOptions OverrideClangTidyOptions;
+    if (!ClangTidyChecks.empty())
+      OverrideClangTidyOptions.Checks = ClangTidyChecks;
     ClangTidyOptProvider = std::make_unique<tidy::FileOptionsProvider>(
         tidy::ClangTidyGlobalOptions(),
-        /* Default */ tidy::ClangTidyOptions::getDefaults(),
+        /* Default */ EmptyDefaults,
         /* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem());
     Opts.GetClangTidyOptions = [&](llvm::vfs::FileSystem &,
                                    llvm::StringRef File) {
       // This function must be thread-safe and tidy option providers are not.
-      std::lock_guard<std::mutex> Lock(ClangTidyOptMu);
-      // FIXME: use the FS provided to the function.
-      return ClangTidyOptProvider->getOptions(File);
+      tidy::ClangTidyOptions Opts;
+      {
+        std::lock_guard<std::mutex> Lock(ClangTidyOptMu);
+        // FIXME: use the FS provided to the function.
+        Opts = ClangTidyOptProvider->getOptions(File);
+      }
+      if (!Opts.Checks) {
+        // If the user hasn't configured clang-tidy checks at all, including
+        // via .clang-tidy, give them a nice set of checks.
+        // (This should be what the "default" options does, but it isn't...)
+        //
+        // These default checks are chosen for:
+        //  - low false-positive rate
+        //  - providing a lot of value
+        //  - being reasonably efficient
+        Opts.Checks = llvm::join_items(
+            ",", "readability-misleading-indentation",
+            "readability-deleted-default", "bugprone-integer-division",
+            "bugprone-sizeof-expression", "bugprone-suspicious-include",
+            "bugprone-suspicious-missing-comma", "bugprone-unused-raii",
+            "bugprone-unused-return-value", "misc-unused-using-decls",
+            "misc-unused-alias-decls", "misc-definitions-in-headers");
+      }
+      return Opts;
     };
   }
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
Index: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -142,6 +142,9 @@
 ClangTidyOptions::mergeWith(const ClangTidyOptions &Other) const {
   ClangTidyOptions Result = *this;
 
+  llvm::errs() << "merging " << bool(Other.Checks) << "\n";
+  if (Other.Checks)
+    llvm::errs() << "value: " << *Other.Checks << "\n";
   mergeCommaSeparatedLists(Result.Checks, Other.Checks);
   mergeCommaSeparatedLists(Result.WarningsAsErrors, Other.WarningsAsErrors);
   overrideValue(Result.HeaderFilterRegex, Other.HeaderFilterRegex);
@@ -168,8 +171,10 @@
 ClangTidyOptions
 ClangTidyOptionsProvider::getOptions(llvm::StringRef FileName) {
   ClangTidyOptions Result;
-  for (const auto &Source : getRawOptions(FileName))
+  for (const auto &Source : getRawOptions(FileName)) {
+    llvm::errs() << Source.second << "\n";
     Result = Result.mergeWith(Source.first);
+  }
   return Result;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D77348.254644.patch
Type: text/x-patch
Size: 3733 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200402/9506f5b9/attachment-0001.bin>