Skip to content

[clang-tidy] add depercation warning for non-whitelisted global options #121057

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 30, 2024
Merged

[clang-tidy] add depercation warning for non-whitelisted global options #121057

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
98d65a0
[clang-tidy] add depercation warning for non-whitelisted global options
HerrCai0907 Dec 24, 2024
179098a
update msg; only warn for global only options"
HerrCai0907 Dec 27, 2024
66004df
Update clang-tools-extra/test/clang-tidy/infrastructure/deprecation-g…
HerrCai0907 Dec 27, 2024
1289ede
add release note
HerrCai0907 Dec 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 21 additions & 11 deletions clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
//===----------------------------------------------------------------------===//

#include "ClangTidyCheck.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/Support/YAMLParser.h"
#include <optional>
#include <string>

namespace clang::tidy {

Expand Down Expand Up @@ -62,16 +62,29 @@ ClangTidyCheck::OptionsView::get(StringRef LocalName) const {
return std::nullopt;
}

static const llvm::StringSet<> DeprecatedGlobalOptions{
"StrictMode",
"IgnoreMacros",
};

static ClangTidyOptions::OptionMap::const_iterator
findPriorityOption(const ClangTidyOptions::OptionMap &Options,
StringRef NamePrefix, StringRef LocalName,
llvm::StringSet<> *Collector) {
ClangTidyContext *Context) {
llvm::StringSet<> *Collector = Context->getOptionsCollector();
if (Collector) {
Collector->insert((NamePrefix + LocalName).str());
Collector->insert(LocalName);
}
auto IterLocal = Options.find((NamePrefix + LocalName).str());
auto IterGlobal = Options.find(LocalName);
// FIXME: temporary solution for deprecation warnings, should be removed
Copy link
Contributor

@carlosgalvezp carlosgalvezp Dec 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this is merged, please create an issue on github to track this removal work for release 22.x

// after 22.x.
if (IterGlobal != Options.end() &&
DeprecatedGlobalOptions.contains(LocalName))
Context->configurationDiag(
"deprecation global option '%0', please use '%1%0'.")
Copy link
Contributor

@carlosgalvezp carlosgalvezp Dec 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this would be clearer:

"global option '%0' is deprecated, please use '%1%0' instead."

<< LocalName << NamePrefix;
if (IterLocal == Options.end())
return IterGlobal;
if (IterGlobal == Options.end())
Expand All @@ -83,8 +96,7 @@ findPriorityOption(const ClangTidyOptions::OptionMap &Options,

std::optional<StringRef>
ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName) const {
auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName,
Context->getOptionsCollector());
auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName, Context);
if (Iter != CheckOptions.end())
return StringRef(Iter->getValue().Value);
return std::nullopt;
Expand Down Expand Up @@ -117,8 +129,7 @@ ClangTidyCheck::OptionsView::get<bool>(StringRef LocalName) const {
template <>
std::optional<bool>
ClangTidyCheck::OptionsView::getLocalOrGlobal<bool>(StringRef LocalName) const {
auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName,
Context->getOptionsCollector());
auto Iter = findPriorityOption(CheckOptions, NamePrefix, LocalName, Context);
if (Iter != CheckOptions.end()) {
if (auto Result = getAsBool(Iter->getValue().Value, Iter->getKey()))
return Result;
Expand Down Expand Up @@ -157,10 +168,9 @@ std::optional<int64_t> ClangTidyCheck::OptionsView::getEnumInt(
bool IgnoreCase) const {
if (!CheckGlobal && Context->getOptionsCollector())
Context->getOptionsCollector()->insert((NamePrefix + LocalName).str());
auto Iter = CheckGlobal
? findPriorityOption(CheckOptions, NamePrefix, LocalName,
Context->getOptionsCollector())
: CheckOptions.find((NamePrefix + LocalName).str());
auto Iter = CheckGlobal ? findPriorityOption(CheckOptions, NamePrefix,
LocalName, Context)
: CheckOptions.find((NamePrefix + LocalName).str());
if (Iter == CheckOptions.end())
return std::nullopt;

Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-fmt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// RUN: %check_clang_tidy %s modernize-use-std-format %t -- \
// RUN: -config="{CheckOptions: { \
// RUN: StrictMode: true, \
// RUN: modernize-use-std-format.StrictMode: true, \
// RUN: modernize-use-std-format.StrFormatLikeFunctions: 'fmt::sprintf', \
// RUN: modernize-use-std-format.ReplacementFormatFunction: 'fmt::format', \
// RUN: modernize-use-std-format.FormatHeader: '<fmt/core.h>' \
Expand Down
3 changes: 3 additions & 0 deletions clang-tools-extra/test/clang-tidy/infrastructure/deprecation-global-option.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// RUN: clang-tidy %s --config="{CheckOptions:{StrictMode: true}}" -checks="-*,modernize-use-std-format" | FileCheck %s

// CHECK: warning: deprecation global option 'StrictMode', please use 'modernize-use-std-format.StrictMode'. [clang-tidy-config]
Loading