Skip to content

Commit

Permalink
[clang-tidy] Introduce HeaderFileExtensions and ImplementationFileExt…
Browse files Browse the repository at this point in the history
…ensions options

We have a number of checks designed to analyze problems
in header files only, for example:

bugprone-suspicious-include
google-build-namespaces
llvm-header-guard
misc-definitions-in-header
...

All these checks duplicate the same logic and options
to determine whether a location is placed in the main
source file or in the header. More checks are coming
up with similar requirements.

Thus, to remove duplication, let's move this option
to the top-level configuration of clang-tidy (since
it's something all checks should share).

Since the checks fetch the option via getLocalOrGlobal,
the behavior is unchanged.

Add a deprecation notice for all checks that use the
local option, prompting to update to the global option.

The functionality for parsing the option will need to
remain in the checks during the transition period.
Once the local options are fully removed, the goal
is to store the parsed options in the ClangTidyContext,
that checks can easily have access to.

Differential Revision: https://reviews.llvm.org/D141000
  • Loading branch information
carlosgalvezp committed Jan 23, 2023
1 parent c8f5f97 commit 4240c91
Show file tree
Hide file tree
Showing 14 changed files with 161 additions and 23 deletions.
8 changes: 8 additions & 0 deletions clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ template <> struct MappingTraits<ClangTidyOptions> {
bool Ignored = false;
IO.mapOptional("Checks", Options.Checks);
IO.mapOptional("WarningsAsErrors", Options.WarningsAsErrors);
IO.mapOptional("HeaderFileExtensions", Options.HeaderFileExtensions);
IO.mapOptional("ImplementationFileExtensions",
Options.ImplementationFileExtensions);
IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex);
IO.mapOptional("AnalyzeTemporaryDtors", Ignored); // deprecated
IO.mapOptional("FormatStyle", Options.FormatStyle);
Expand All @@ -142,6 +145,8 @@ ClangTidyOptions ClangTidyOptions::getDefaults() {
ClangTidyOptions Options;
Options.Checks = "";
Options.WarningsAsErrors = "";
Options.HeaderFileExtensions = {"", "h", "hh", "hpp", "hxx"};
Options.ImplementationFileExtensions = {"c", "cc", "cpp", "cxx"};
Options.HeaderFilterRegex = "";
Options.SystemHeaders = false;
Options.FormatStyle = "none";
Expand Down Expand Up @@ -178,6 +183,9 @@ ClangTidyOptions &ClangTidyOptions::mergeWith(const ClangTidyOptions &Other,
unsigned Order) {
mergeCommaSeparatedLists(Checks, Other.Checks);
mergeCommaSeparatedLists(WarningsAsErrors, Other.WarningsAsErrors);
overrideValue(HeaderFileExtensions, Other.HeaderFileExtensions);
overrideValue(ImplementationFileExtensions,
Other.ImplementationFileExtensions);
overrideValue(HeaderFilterRegex, Other.HeaderFilterRegex);
overrideValue(SystemHeaders, Other.SystemHeaders);
overrideValue(FormatStyle, Other.FormatStyle);
Expand Down
8 changes: 8 additions & 0 deletions clang-tools-extra/clang-tidy/ClangTidyOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ struct ClangTidyOptions {
/// WarningsAsErrors filter.
std::optional<std::string> WarningsAsErrors;

/// File extensions to consider to determine if a given diagnostic is located
/// in a header file.
std::optional<std::vector<std::string>> HeaderFileExtensions;

/// File extensions to consider to determine if a given diagnostic is located
/// is located in an implementation file.
std::optional<std::vector<std::string>> ImplementationFileExtensions;

/// Output warnings from headers matching this filter. Warnings from
/// main files will always be displayed.
std::optional<std::string> HeaderFilterRegex;
Expand Down
47 changes: 37 additions & 10 deletions clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,14 @@ Configuration files:
$ clang-tidy -dump-config
---
Checks: '-*,some-check'
WarningsAsErrors: ''
HeaderFilterRegex: ''
FormatStyle: none
InheritParentConfig: true
User: user
Checks: '-*,some-check'
WarningsAsErrors: ''
HeaderFileExtensions: ['', 'h','hh','hpp','hxx']
ImplementationFileExtensions: ['c','cc','cpp','cxx']
HeaderFilterRegex: ''
FormatStyle: none
InheritParentConfig: true
User: user
CheckOptions:
some-check.SomeOption: 'some value'
...
Expand Down Expand Up @@ -130,10 +132,10 @@ well.
cl::init(false), cl::cat(ClangTidyCategory));

static cl::opt<bool> FixNotes("fix-notes", cl::desc(R"(
If a warning has no fix, but a single fix can
be found through an associated diagnostic note,
apply the fix.
Specifying this flag will implicitly enable the
If a warning has no fix, but a single fix can
be found through an associated diagnostic note,
apply the fix.
Specifying this flag will implicitly enable the
'--fix' flag.
)"),
cl::init(false), cl::cat(ClangTidyCategory));
Expand Down Expand Up @@ -458,6 +460,26 @@ static bool verifyChecks(const StringSet<> &AllChecks, StringRef CheckGlob,
return AnyInvalid;
}

static bool verifyFileExtensions(
const std::vector<std::string> &HeaderFileExtensions,
const std::vector<std::string> &ImplementationFileExtensions,
StringRef Source) {
bool AnyInvalid = false;
for (const auto &HeaderExtension : HeaderFileExtensions) {
for (const auto &ImplementationExtension : ImplementationFileExtensions) {
if (HeaderExtension == ImplementationExtension) {
AnyInvalid = true;
auto &Output = llvm::WithColor::warning(llvm::errs(), Source)
<< "HeaderFileExtension '" << HeaderExtension << '\''
<< " is the same as ImplementationFileExtension '"
<< ImplementationExtension << '\'';
Output << VerifyConfigWarningEnd;
}
}
}
return AnyInvalid;
}

int clangTidyMain(int argc, const char **argv) {
llvm::InitLLVM X(argc, argv);

Expand Down Expand Up @@ -561,6 +583,11 @@ int clangTidyMain(int argc, const char **argv) {
if (Opts.Checks)
AnyInvalid |= verifyChecks(Valid.Names, *Opts.Checks, Source);

if (Opts.HeaderFileExtensions && Opts.ImplementationFileExtensions)
AnyInvalid |=
verifyFileExtensions(*Opts.HeaderFileExtensions,
*Opts.ImplementationFileExtensions, Source);

for (auto Key : Opts.CheckOptions.keys()) {
if (Valid.Options.contains(Key))
continue;
Expand Down
39 changes: 39 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ Improvements to clang-tidy
which is no longer in use. The option will be fully removed in
:program:`clang-tidy` version 18.

- New global configuration file options `HeaderFileExtensions` and
`ImplementationFileExtensions`, replacing the check-local options of the
same name.

New checks
^^^^^^^^^^

Expand Down Expand Up @@ -157,26 +161,61 @@ Changes in existing checks
<clang-tidy/checks/bugprone/assignment-in-if-condition>` check when there
was an assignement in a lambda found in the condition of an ``if``.

- Deprecated check-local options `HeaderFileExtensions` and `ImplementationFileExtensions`
in :doc:`bugprone-dynamic-static-initializers
<clang-tidy/checks/bugprone/dynamic-static-initializers>` check.
Global options of the same name should be used instead.

- Improved :doc:`bugprone-signal-handler
<clang-tidy/checks/bugprone/signal-handler>` check. Partial
support for C++14 signal handler rules was added. Bug report generation was
improved.

- Deprecated check-local options `HeaderFileExtensions` and `ImplementationFileExtensions`
in :doc:`bugprone-suspicious-include
<clang-tidy/checks/bugprone/suspicious-include>` check.
Global options of the same name should be used instead.

- Fixed a false positive in :doc:`cppcoreguidelines-pro-type-member-init
<clang-tidy/checks/cppcoreguidelines/pro-type-member-init>` when warnings
would be emitted for uninitialized members of an anonymous union despite
there being an initializer for one of the other members.

- Deprecated check-local options `HeaderFileExtensions` and `ImplementationFileExtensions`
in :doc:`google-build-namespaces
<clang-tidy/checks/google/build-namespaces>` check.
Global options of the same name should be used instead.

- Deprecated check-local options `HeaderFileExtensions` and `ImplementationFileExtensions`
in :doc:`google-global-names-in-headers
<clang-tidy/checks/google/global-names-in-headers>` check.
Global options of the same name should be used instead.

- Fixed false positives in :doc:`google-objc-avoid-throwing-exception
<clang-tidy/checks/google/objc-avoid-throwing-exception>` check for exceptions
thrown by code emitted from macros in system headers.

- Deprecated check-local options `HeaderFileExtensions` and `ImplementationFileExtensions`
in :doc:`llvm-header-guard
<clang-tidy/checks/llvm/header-guard>` check.
Global options of the same name should be used instead.

- Deprecated check-local options `HeaderFileExtensions` and `ImplementationFileExtensions`
in :doc:`misc-definitions-in-headers
<clang-tidy/checks/misc/definitions-in-headers>` check.
Global options of the same name should be used instead.

- Improved :doc:`misc-redundant-expression <clang-tidy/checks/misc/redundant-expression>`
check.

The check now skips concept definitions since redundant expressions still make sense
inside them.

- Deprecated check-local options `HeaderFileExtensions` and `ImplementationFileExtensions`
in :doc:`misc-unused-using-decls
<clang-tidy/checks/misc/misc-unused-using-decls>` check.
Global options of the same name should be used instead.

- Improved :doc:`modernize-loop-convert <clang-tidy/checks/modernize/loop-convert>`
to check for container functions ``begin``/``end`` etc on base classes of the container
type, instead of only as direct members of the container type itself.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ Options
-------
.. option:: HeaderFileExtensions

Note: this option is deprecated, it will be removed in :program:`clang-tidy`
version 18. Please use the global configuration option
`HeaderFileExtensions`.

Default value: ``";h;hh;hpp;hxx"``
A semicolon-separated list of filename extensions of header files (the
filename extensions should not contain a "." prefix). For extension-less
Expand All @@ -27,6 +31,10 @@ Options

.. option:: ImplementationFileExtensions

Note: this option is deprecated, it will be removed in :program:`clang-tidy`.
version 18. Please use the global configuration option
`ImplementationFileExtensions`.

Default value: ``"c;cc;cpp;cxx"``
Likewise, a semicolon-separated list of filename extensions of
implementation files.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ Options

.. option:: HeaderFileExtensions

Note: this option is deprecated, it will be removed in :program:`clang-tidy`
version 18. Please use the global configuration option
`HeaderFileExtensions`.

A comma-separated list of filename extensions of header files (the filename
extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
For header files without an extension, use an empty string (if there are no
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ Options

.. option:: HeaderFileExtensions

Note: this option is deprecated, it will be removed in :program:`clang-tidy`
version 18. Please use the global configuration option
`HeaderFileExtensions`.

A comma-separated list of filename extensions of header files (the filename
extensions should not contain "." prefix). Default is "h".
For header files without an extension, use an empty string (if there are no
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ Options

.. option:: HeaderFileExtensions

Note: this option is deprecated, it will be removed in :program:`clang-tidy`
version 18. Please use the global configuration option
`HeaderFileExtensions`.

A comma-separated list of filename extensions of header files (the filename
extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
For header files without an extension, use an empty string (if there are no
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ Options

.. option:: HeaderFileExtensions

Note: this option is deprecated, it will be removed in :program:`clang-tidy`
version 18. Please use the global configuration option
`HeaderFileExtensions`.

A comma-separated list of filename extensions of header files (the filename
extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
For header files without an extension, use an empty string (if there are no
Expand All @@ -100,5 +104,9 @@ Options

.. option:: UseHeaderFileExtension

Note: this option is deprecated, it will be removed in :program:`clang-tidy`
version 18. The check will unconditionally use the global option
`HeaderFileExtensions`.

When `true`, the check will use the file extension to distinguish header
files. Default is `true`.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ Options

.. option:: HeaderFileExtensions

Note: this option is deprecated, it will be removed in :program:`clang-tidy`
version 18. Please use the global configuration option
`HeaderFileExtensions`.

A semicolon-separated list of filename extensions of header files (the filename
extensions should not include "." prefix). Default is "h,hh,hpp,hxx".
For extension-less header files, use an empty string or leave an
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ Options

.. option:: HeaderFileExtensions

Note: this option is deprecated, it will be removed in :program:`clang-tidy`
version 18. Please use the global configuration option
`HeaderFileExtensions`.

A semicolon-separated list of filename extensions of header files (the filename
extensions should not include "." prefix). Default is ";h;hh;hpp;hxx".
For extension-less header files, using an empty string or leaving an
Expand Down
18 changes: 10 additions & 8 deletions clang-tools-extra/docs/clang-tidy/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ An overview of all the command-line options:
When the value is empty, clang-tidy will
attempt to find a file named .clang-tidy for
each source file in its parent directories.
--config-file=<string> -
--config-file=<string> -
Specify the path of .clang-tidy or custom config file:
e.g. --config-file=/some/path/myTidyConfigFile
This option internally works exactly the same way as
Expand Down Expand Up @@ -237,7 +237,7 @@ An overview of all the command-line options:
format to stderr. When this option is passed,
these per-TU profiles are instead stored as JSON.
--system-headers - Display the errors from system headers.
--use-color -
--use-color -
Use colors in diagnostics. If not set, colors
will be used if the terminal connected to
standard output supports colors.
Expand Down Expand Up @@ -287,12 +287,14 @@ An overview of all the command-line options:
$ clang-tidy -dump-config
---
Checks: '-*,some-check'
WarningsAsErrors: ''
HeaderFilterRegex: ''
FormatStyle: none
InheritParentConfig: true
User: user
Checks: '-*,some-check'
WarningsAsErrors: ''
HeaderFileExtensions: ['', 'h','hh','hpp','hxx']
ImplementationFileExtensions: ['c','cc','cpp','cxx']
HeaderFilterRegex: ''
FormatStyle: none
InheritParentConfig: true
User: user
CheckOptions:
some-check.SomeOption: 'some value'
...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

// RUN: not clang-tidy -verify-config \
// RUN: --checks='-*,bad*glob,llvm*,llvm-includeorder,my-made-up-check' --config='{Checks: "readability-else-after-ret", \
// RUN: HeaderFileExtensions: ["h", "hh", "hpp"], \
// RUN: ImplementationFileExtensions: ["c", "cc", "hpp"], \
// RUN: CheckOptions: [{key: "IgnoreMacros", value: "true"}, \
// RUN: {key: "StriceMode", value: "true"}, \
// RUN: {key: modernize-lop-convert.UseCxx20ReverseRanges, value: true} \
Expand All @@ -12,6 +14,7 @@
// CHECK-VERIFY-DAG: command-line option '-config': warning: unknown check 'readability-else-after-ret'; did you mean 'readability-else-after-return' [-verify-config]
// CHECK-VERIFY-DAG: command-line option '-config': warning: unknown check option 'modernize-lop-convert.UseCxx20ReverseRanges'; did you mean 'modernize-loop-convert.UseCxx20ReverseRanges' [-verify-config]
// CHECK-VERIFY-DAG: command-line option '-config': warning: unknown check option 'StriceMode'; did you mean 'StrictMode' [-verify-config]
// CHECK-VERIFY-DAG: command-line option '-config': warning: HeaderFileExtension 'hpp' is the same as ImplementationFileExtension 'hpp' [-verify-config]
// CHECK-VERIFY: command-line option '-checks': warning: check glob 'bad*glob' doesn't match any known check [-verify-config]
// CHECK-VERIFY: command-line option '-checks': warning: unknown check 'llvm-includeorder'; did you mean 'llvm-include-order' [-verify-config]
// CHECK-VERIFY: command-line option '-checks': warning: unknown check 'my-made-up-check' [-verify-config]
Loading

0 comments on commit 4240c91

Please sign in to comment.