[PATCH] D145138: [clang-tidy] Implement FixIts for C arrays

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp:588
+    : ClangTidyCheck(Name, Context),
+      IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+                                               utils::IncludeSorter::IS_LLVM),
----------------
PiotrZSL wrote:
> ccotter wrote:
> > Eugene.Zelenko wrote:
> > > ccotter wrote:
> > > > Eugene.Zelenko wrote:
> > > > > Should be global option, because it's used in other checks.
> > > > Could you clarify this a bit? This is how most other tests consume IncludeStyle (`Options.getLocalOrGlobal("IncludeStyle", utils::IncludeSorter::IS_LLVM)`.
> > > @carlosgalvezp is best person to answer because he recently introduced global option for source files and headers.
> > bump @carlosgalvezp 
> ```
> Local:
>   - key: modernize-avoid-c-arrays.IncludeStyle
>      value: google
> 
> Global:
>   - key: IncludeStyle
>      value: google
> ```
> 
> Your code is correct, simply if there is local defined, it will be used, if there is global defined, then global will be used.
> 
Yep, this is similar to `HeaderFileExtensions`, current state is that all checks duplicate the same code when it should instead be just a global option. Since the global option is not available yet, let's leave it as it is and I will put up a patch to deprecate the local options.

Luckily this time it will be much easier since the value is just a string!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145138/new/

https://reviews.llvm.org/D145138