[clang-tidy] Optional inheritance of file configs from parent directo…

…ries 

Summary:
Without this patch clang-tidy stops finding file configs on the nearest
.clang-tidy file. In some cases it is not very convenient because it
results in common parts duplication into every child .clang-tidy file.
This diff adds optional config inheritance from the parent directories
config files.

Test Plan:

Added test cases in existing config test.

Reviewers: alexfh, gribozavr2, klimek, hokein

Subscribers: njames93, arphaman, xazax.hun, aheejin, cfe-commits

Tags: #clang, #clang-tools-extra

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

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

@@ -328,7 +328,9 @@ static void setStaticAnalyzerCheckerOpts(const ClangTidyOptions &Opts,
     StringRef OptName(Opt.first);
     if (!OptName.startswith(AnalyzerPrefix))
       continue;
-    AnalyzerOptions->Config[OptName.substr(AnalyzerPrefix.size())] = Opt.second;
+    // Analyzer options are always local options so we can ignore priority.
+    AnalyzerOptions->Config[OptName.substr(AnalyzerPrefix.size())] =
+        Opt.second.Value;
   }
 }
 

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

@@ -72,19 +72,20 @@ llvm::Expected<std::string>
 ClangTidyCheck::OptionsView::get(StringRef LocalName) const {
   const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str());
   if (Iter != CheckOptions.end())
-    return Iter->second;
+    return Iter->second.Value;
   return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str());
 }
 
 llvm::Expected<std::string>
 ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName) const {
-  auto Iter = CheckOptions.find(NamePrefix + LocalName.str());
-  if (Iter != CheckOptions.end())
-    return Iter->second;
-  // Fallback to global setting, if present.
-  Iter = CheckOptions.find(LocalName.str());
-  if (Iter != CheckOptions.end())
-    return Iter->second;
+  auto IterLocal = CheckOptions.find(NamePrefix + LocalName.str());
+  auto IterGlobal = CheckOptions.find(LocalName.str());
+  if (IterLocal != CheckOptions.end() &&
+      (IterGlobal == CheckOptions.end() ||
+       IterLocal->second.Priority >= IterGlobal->second.Priority))
+    return IterLocal->second.Value;
+  if (IterGlobal != CheckOptions.end())
+    return IterGlobal->second.Value;
   return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str());
 }
 
@@ -123,17 +124,15 @@ bool ClangTidyCheck::OptionsView::get<bool>(StringRef LocalName,
 template <>
 llvm::Expected<bool>
 ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName) const {
-  llvm::Expected<std::string> ValueOr = get(LocalName);
-  bool IsGlobal = false;
-  if (!ValueOr) {
-    llvm::consumeError(ValueOr.takeError());
-    ValueOr = getLocalOrGlobal(LocalName);
-    IsGlobal = true;
-  }
-  if (!ValueOr)
-    return ValueOr.takeError();
-  return getAsBool(*ValueOr, IsGlobal ? llvm::Twine(LocalName)
-                                      : (NamePrefix + LocalName));
+  auto IterLocal = CheckOptions.find(NamePrefix + LocalName.str());
+  auto IterGlobal = CheckOptions.find(LocalName.str());
+  if (IterLocal != CheckOptions.end() &&
+      (IterGlobal == CheckOptions.end() ||
+       IterLocal->second.Priority >= IterGlobal->second.Priority))
+    return getAsBool(IterLocal->second.Value, NamePrefix + LocalName);
+  if (IterGlobal != CheckOptions.end())
+    return getAsBool(IterGlobal->second.Value, llvm::Twine(LocalName));
+  return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str());
 }
 
 template <>
@@ -149,7 +148,7 @@ bool ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName,
 void ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap &Options,
                                         StringRef LocalName,
                                         StringRef Value) const {
-  Options[NamePrefix + LocalName.str()] = std::string(Value);
+  Options[NamePrefix + LocalName.str()] = Value;
 }
 
 void ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap &Options,
@@ -167,7 +166,7 @@ llvm::Expected<int64_t> ClangTidyCheck::OptionsView::getEnumInt(
   if (Iter == CheckOptions.end())
     return llvm::make_error<MissingOptionError>((NamePrefix + LocalName).str());
 
-  StringRef Value = Iter->second;
+  StringRef Value = Iter->second.Value;
   StringRef Closest;
   unsigned EditDistance = -1;
   for (const auto &NameAndEnum : Mapping) {
@@ -189,9 +188,9 @@ llvm::Expected<int64_t> ClangTidyCheck::OptionsView::getEnumInt(
   }
   if (EditDistance < 3)
     return llvm::make_error<UnparseableEnumOptionError>(
-        Iter->first, Iter->second, std::string(Closest));
+        Iter->first, Iter->second.Value, std::string(Closest));
   return llvm::make_error<UnparseableEnumOptionError>(Iter->first,
-                                                      Iter->second);
+                                                      Iter->second.Value);
 }
 
 void ClangTidyCheck::OptionsView::logErrToStdErr(llvm::Error &&Err) {

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

@@ -199,7 +199,7 @@ ClangTidyOptions ClangTidyContext::getOptionsForFile(StringRef File) const {
   // Merge options on top of getDefaults() as a safeguard against options with
   // unset values.
   return ClangTidyOptions::getDefaults().mergeWith(
-      OptionsProvider->getOptions(File));
+      OptionsProvider->getOptions(File), 0);
 }
 
 void ClangTidyContext::setEnableProfiling(bool P) { Profile = P; }

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

@@ -67,12 +67,15 @@ template <> struct MappingTraits<ClangTidyOptions::StringPair> {
 
 struct NOptionMap {
   NOptionMap(IO &) {}
-  NOptionMap(IO &, const ClangTidyOptions::OptionMap &OptionMap)
-      : Options(OptionMap.begin(), OptionMap.end()) {}
+  NOptionMap(IO &, const ClangTidyOptions::OptionMap &OptionMap) {
+    Options.reserve(OptionMap.size());
+    for (const auto &KeyValue : OptionMap)
+      Options.emplace_back(KeyValue.first, KeyValue.second.Value);
+  }
   ClangTidyOptions::OptionMap denormalize(IO &) {
     ClangTidyOptions::OptionMap Map;
     for (const auto &KeyValue : Options)
-      Map[KeyValue.first] = KeyValue.second;
+      Map[KeyValue.first] = ClangTidyOptions::ClangTidyValue(KeyValue.second);
     return Map;
   }
   std::vector<ClangTidyOptions::StringPair> Options;
@@ -92,6 +95,7 @@ template <> struct MappingTraits<ClangTidyOptions> {
     IO.mapOptional("CheckOptions", NOpts->Options);
     IO.mapOptional("ExtraArgs", Options.ExtraArgs);
     IO.mapOptional("ExtraArgsBefore", Options.ExtraArgsBefore);
+    IO.mapOptional("InheritParentConfig", Options.InheritParentConfig);
   }
 };
 
@@ -109,10 +113,12 @@ ClangTidyOptions ClangTidyOptions::getDefaults() {
   Options.SystemHeaders = false;
   Options.FormatStyle = "none";
   Options.User = llvm::None;
+  unsigned Priority = 0;
   for (ClangTidyModuleRegistry::iterator I = ClangTidyModuleRegistry::begin(),
                                          E = ClangTidyModuleRegistry::end();
        I != E; ++I)
-    Options = Options.mergeWith(I->instantiate()->getModuleOptions());
+    Options =
+        Options.mergeWith(I->instantiate()->getModuleOptions(), ++Priority);
   return Options;
 }
 
@@ -138,8 +144,8 @@ static void overrideValue(Optional<T> &Dest, const Optional<T> &Src) {
     Dest = Src;
 }
 
-ClangTidyOptions
-ClangTidyOptions::mergeWith(const ClangTidyOptions &Other) const {
+ClangTidyOptions ClangTidyOptions::mergeWith(const ClangTidyOptions &Other,
+                                             unsigned Priority) const {
   ClangTidyOptions Result = *this;
 
   mergeCommaSeparatedLists(Result.Checks, Other.Checks);
@@ -151,8 +157,10 @@ ClangTidyOptions::mergeWith(const ClangTidyOptions &Other) const {
   mergeVectors(Result.ExtraArgs, Other.ExtraArgs);
   mergeVectors(Result.ExtraArgsBefore, Other.ExtraArgsBefore);
 
-  for (const auto &KeyValue : Other.CheckOptions)
-    Result.CheckOptions[KeyValue.first] = KeyValue.second;
+  for (const auto &KeyValue : Other.CheckOptions) {
+    Result.CheckOptions[KeyValue.first] = ClangTidyValue(
+        KeyValue.second.Value, KeyValue.second.Priority + Priority);
+  }
 
   return Result;
 }
@@ -168,8 +176,9 @@ const char
 ClangTidyOptions
 ClangTidyOptionsProvider::getOptions(llvm::StringRef FileName) {
   ClangTidyOptions Result;
+  unsigned Priority = 0;
   for (const auto &Source : getRawOptions(FileName))
-    Result = Result.mergeWith(Source.first);
+    Result = Result.mergeWith(Source.first, ++Priority);
   return Result;
 }
 
@@ -237,6 +246,7 @@ FileOptionsProvider::getRawOptions(StringRef FileName) {
       DefaultOptionsProvider::getRawOptions(AbsoluteFilePath.str());
   OptionsSource CommandLineOptions(OverrideOptions,
                                    OptionsSourceTypeCheckCommandLineOption);
+  size_t FirstFileConfig = RawOptions.size();
   // Look for a suitable configuration file in all parent directories of the
   // file. Start with the immediate parent directory and move up.
   StringRef Path = llvm::sys::path::parent_path(AbsoluteFilePath.str());
@@ -256,15 +266,21 @@ FileOptionsProvider::getRawOptions(StringRef FileName) {
       while (Path != CurrentPath) {
         LLVM_DEBUG(llvm::dbgs()
                    << "Caching configuration for path " << Path << ".\n");
-        CachedOptions[Path] = *Result;
+        if (!CachedOptions.count(Path))
+          CachedOptions[Path] = *Result;
         Path = llvm::sys::path::parent_path(Path);
       }
       CachedOptions[Path] = *Result;
 
       RawOptions.push_back(*Result);
-      break;
+      if (!Result->first.InheritParentConfig ||
+          !*Result->first.InheritParentConfig)
+        break;
     }
   }
+  // Reverse order of file configs because closer configs should have higher
+  // priority.
+  std::reverse(RawOptions.begin() + FirstFileConfig, RawOptions.end());
   RawOptions.push_back(CommandLineOptions);
   return RawOptions;
 }

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

@@ -58,7 +58,9 @@ struct ClangTidyOptions {
 
   /// Creates a new \c ClangTidyOptions instance combined from all fields
   /// of this instance overridden by the fields of \p Other that have a value.
-  ClangTidyOptions mergeWith(const ClangTidyOptions &Other) const;
+  /// \p Order specifies precedence of \p Other option.
+  ClangTidyOptions mergeWith(const ClangTidyOptions &Other,
+                             unsigned Order) const;
 
   /// Checks filter.
   llvm::Optional<std::string> Checks;
@@ -93,8 +95,20 @@ struct ClangTidyOptions {
   /// comments in the relevant check.
   llvm::Optional<std::string> User;
 
+  /// Helper structure for storing option value with priority of the value.
+  struct ClangTidyValue {
+    ClangTidyValue() : Value(), Priority(0) {}
+    ClangTidyValue(const char *Value) : Value(Value), Priority(0) {}
+    ClangTidyValue(llvm::StringRef Value, unsigned Priority = 0)
+        : Value(Value), Priority(Priority) {}
+
+    std::string Value;
+    /// Priority stores relative precedence of the value loaded from config
+    /// files to disambigute local vs global value from different levels.
+    unsigned Priority;
+  };
   typedef std::pair<std::string, std::string> StringPair;
-  typedef std::map<std::string, std::string> OptionMap;
+  typedef std::map<std::string, ClangTidyValue> OptionMap;
 
   /// Key-value mapping used to store check-specific options.
   OptionMap CheckOptions;
@@ -106,6 +120,12 @@ struct ClangTidyOptions {
 
   /// Add extra compilation arguments to the start of the list.
   llvm::Optional<ArgList> ExtraArgsBefore;
+
+  /// Only used in the FileOptionsProvider. If true, FileOptionsProvider will
+  /// take a configuration file in the parent directory (if any exists) and
+  /// apply this config file on top of the parent one. If false or missing,
+  /// only this configuration file will be used.
+  llvm::Optional<bool> InheritParentConfig;
 };
 
 /// Abstract interface for retrieving various ClangTidy options.

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

@@ -36,17 +36,20 @@ static cl::extrahelp ClangTidyHelp(R"(
 Configuration files:
   clang-tidy attempts to read configuration for each source file from a
   .clang-tidy file located in the closest parent directory of the source
-  file. If any configuration options have a corresponding command-line
-  option, command-line option takes precedence. The effective
-  configuration can be inspected using -dump-config:
+  file. If InheritParentConfig is true in a config file, the configuration file
+  in the parent directory (if any exists) will be taken and current config file
+  will be applied on top of the parent one. If any configuration options have
+  a corresponding command-line option, command-line option takes precedence.
+  The effective configuration can be inspected using -dump-config:
 
     $ clang-tidy -dump-config
     ---
-    Checks:          '-*,some-check'
-    WarningsAsErrors: ''
-    HeaderFilterRegex: ''
-    FormatStyle:     none
-    User:            user
+    Checks:              '-*,some-check'
+    WarningsAsErrors:    ''
+    HeaderFilterRegex:   ''
+    FormatStyle:         none
+    InheritParentConfig: true
+    User:                user
     CheckOptions:
       - key:             some-check.SomeOption
         value:           'some value'
@@ -294,7 +297,7 @@ static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider(
             parseConfiguration(Config)) {
       return std::make_unique<ConfigOptionsProvider>(
           GlobalOptions,
-          ClangTidyOptions::getDefaults().mergeWith(DefaultOptions),
+          ClangTidyOptions::getDefaults().mergeWith(DefaultOptions, 0),
           *ParsedConfig, OverrideOptions);
     } else {
       llvm::errs() << "Error: invalid configuration specified.\n"
@@ -406,7 +409,7 @@ int clangTidyMain(int argc, const char **argv) {
         getCheckOptions(EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
     llvm::outs() << configurationAsText(
                         ClangTidyOptions::getDefaults().mergeWith(
-                            EffectiveOptions))
+                            EffectiveOptions, 0))
                  << "\n";
     return 0;
   }

clang-tools-extra/docs/clang-tidy/index.rst

@@ -247,17 +247,20 @@ An overview of all the command-line options:
   Configuration files:
     clang-tidy attempts to read configuration for each source file from a
     .clang-tidy file located in the closest parent directory of the source
-    file. If any configuration options have a corresponding command-line
-    option, command-line option takes precedence. The effective
-    configuration can be inspected using -dump-config:
+    file. If InheritParentConfig is true in a config file, the configuration file
+    in the parent directory (if any exists) will be taken and current config file
+    will be applied on top of the parent one. If any configuration options have
+    a corresponding command-line option, command-line option takes precedence.
+    The effective configuration can be inspected using -dump-config:
 
       $ clang-tidy -dump-config
       ---
-      Checks:          '-*,some-check'
-      WarningsAsErrors: ''
-      HeaderFilterRegex: ''
-      FormatStyle:     none
-      User:            user
+      Checks:              '-*,some-check'
+      WarningsAsErrors:    ''
+      HeaderFilterRegex:   ''
+      FormatStyle:         none
+      InheritParentConfig: true
+      User:                user
       CheckOptions:
         - key:             some-check.SomeOption
           value:           'some value'

clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/3/.clang-tidy

@@ -0,0 +1,3 @@
+InheritParentConfig: true
+Checks: 'from-child3'
+HeaderFilterRegex: 'child3'

clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/.clang-tidy

@@ -0,0 +1,8 @@
+Checks: '-*,modernize-loop-convert,modernize-use-using'
+CheckOptions:
+  - key:             modernize-loop-convert.MaxCopySize
+    value:           '10'
+  - key:             modernize-loop-convert.MinConfidence
+    value:           reasonable
+  - key:             modernize-use-using.IgnoreMacros
+    value:           1

clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/44/.clang-tidy

@@ -0,0 +1,9 @@
+InheritParentConfig: true
+Checks: 'llvm-qualified-auto'
+CheckOptions:
+  - key:             modernize-loop-convert.MaxCopySize
+    value:           '20'
+  - key:             llvm-qualified-auto.AddConstToQualified
+    value:           '1'
+  - key:             IgnoreMacros
+    value:           '0'

clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp

@@ -7,6 +7,26 @@
 // RUN: clang-tidy -dump-config %S/Inputs/config-files/2/- -- | FileCheck %s -check-prefix=CHECK-CHILD2
 // CHECK-CHILD2: Checks: {{.*}}from-parent
 // CHECK-CHILD2: HeaderFilterRegex: parent
+// RUN: clang-tidy -dump-config %S/Inputs/config-files/3/- -- | FileCheck %s -check-prefix=CHECK-CHILD3
+// CHECK-CHILD3: Checks: {{.*}}from-parent,from-child3
+// CHECK-CHILD3: HeaderFilterRegex: child3
 // RUN: clang-tidy -dump-config -checks='from-command-line' -header-filter='from command line' %S/Inputs/config-files/- -- | FileCheck %s -check-prefix=CHECK-COMMAND-LINE
 // CHECK-COMMAND-LINE: Checks: {{.*}}from-parent,from-command-line
 // CHECK-COMMAND-LINE: HeaderFilterRegex: from command line
+
+// For this test we have to use names of the real checks because otherwise values are ignored.
+// RUN: clang-tidy -dump-config %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD4
+// CHECK-CHILD4: Checks: {{.*}}modernize-loop-convert,modernize-use-using,llvm-qualified-auto
+// CHECK-CHILD4: - key: llvm-qualified-auto.AddConstToQualified
+// CHECK-CHILD4-NEXT: value: '1
+// CHECK-CHILD4: - key: modernize-loop-convert.MaxCopySize
+// CHECK-CHILD4-NEXT: value: '20'
+// CHECK-CHILD4: - key: modernize-loop-convert.MinConfidence
+// CHECK-CHILD4-NEXT: value: reasonable
+// CHECK-CHILD4: - key: modernize-use-using.IgnoreMacros
+// CHECK-CHILD4-NEXT: value: '0'
+
+// RUN: clang-tidy --explain-config %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-EXPLAIN
+// CHECK-EXPLAIN: 'llvm-qualified-auto' is enabled in the {{.*}}/Inputs/config-files/4/44/.clang-tidy.
+// CHECK-EXPLAIN: 'modernize-loop-convert' is enabled in the {{.*}}/Inputs/config-files/4/.clang-tidy.
+// CHECK-EXPLAIN: 'modernize-use-using' is enabled in the {{.*}}/Inputs/config-files/4/.clang-tidy.

clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

@@ -87,7 +87,7 @@ TEST(ParseConfiguration, MergeConfigurations) {
       ExtraArgsBefore: ['arg-before3', 'arg-before4']
   )");
   ASSERT_TRUE(!!Options2);
-  ClangTidyOptions Options = Options1->mergeWith(*Options2);
+  ClangTidyOptions Options = Options1->mergeWith(*Options2, 0);
   EXPECT_EQ("check1,check2,check3,check4", *Options.Checks);
   EXPECT_EQ("filter2", *Options.HeaderFilterRegex);
   EXPECT_EQ("user2", *Options.User);