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

[sammccall]

revert debugging noise

[hokein]

clang-tools-extra/clangd/tool/ClangdMain.cpp

695

EmptyDefaults.Checks is not none, but it is an empty string I think , we could even assign our default checks to EmptyDefaults.Checks (instead of setting it in GetCalngTidyOptions).

724

bugprone-suspicious-include is a fairly new check, and hasn't been run internally, so I'd be conservative, not enable it.

[sammccall]

clang-tools-extra/clangd/tool/ClangdMain.cpp

695

That doesn't work :-(
The clang tidy options config is a tangled mess.
Ultimately it produces a vector of optionses that are merged in order to produce the final options.
In our case this vector is [EmptyDefaults, options from .clang-tidy, OverrideClangTidyOptions].

For the "Checks" field, the merge just joins the comma-separated lists together.
So the checks from EmptyDefaults would be enabled unless .clang-tidy *specifically* disables them, which is not the behavior I want - the existence of .clang-tidy should override the defaults.

This is what the complainy comment below is supposed to indicate...

clang-tools-extra/clang-tidy/ClangTidyOptions.cpp

 //===--- ClangTidyOptions.cpp - clang-tidy ----------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
static void overrideValue(Optional<T> &Dest, const Optional<T> &Src) {
   if (Src)
     Dest = Src;
 }
 
 ClangTidyOptions
 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);
   overrideValue(Result.SystemHeaders, Other.SystemHeaders);
   overrideValue(Result.FormatStyle, Other.FormatStyle);
   overrideValue(Result.User, Other.User);
   mergeVectors(Result.ExtraArgs, Other.ExtraArgs);
   mergeVectors(Result.ExtraArgsBefore, Other.ExtraArgsBefore);
const char ClangTidyOptionsProvider::OptionsSourceTypeCheckCommandLineOption[] =
     "command-line option '-checks'";
 const char
     ClangTidyOptionsProvider::OptionsSourceTypeConfigCommandLineOption[] =
         "command-line option '-config'";
 
 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;
 }
 
 std::vector<OptionsSource>
 DefaultOptionsProvider::getRawOptions(llvm::StringRef FileName) {
   std::vector<OptionsSource> Result;
   Result.emplace_back(DefaultOptions, OptionsSourceTypeDefaultBinary);
   return Result;

clang-tools-extra/clangd/tool/ClangdMain.cpp

 //===--- ClangdMain.cpp - clangd server loop ------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
if (!PathMappingsArg.empty()) {
     TransportLayer = createPathMappingTransport(std::move(TransportLayer),
                                                 std::move(*Mappings));
   }
   // Create an empty clang-tidy option.
   std::mutex ClangTidyOptMu;
   std::unique_ptr<tidy::ClangTidyOptionsProvider>
       ClangTidyOptProvider; /*GUARDED_BY(ClangTidyOptMu)*/
   if (EnableClangTidy) {
-    auto OverrideClangTidyOptions = tidy::ClangTidyOptions::getDefaults();
+    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.
+      tidy::ClangTidyOptions Opts;
+      {
         std::lock_guard<std::mutex> Lock(ClangTidyOptMu);
         // FIXME: use the FS provided to the function.
-      return ClangTidyOptProvider->getOptions(File);
+        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;
   Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
 
   Opts.TweakFilter = [&](const Tweak &T) {
     if (T.hidden() && !HiddenFeatures)
       return false;