Skip to content

[include-cleaner] rename enabled flags to disable-* #132991

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.

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
8 changes: 8 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@ Improvements to clang-doc
Improvements to clang-query
---------------------------

Improvements to include-cleaner
-------------------------------
- Deprecated the ``-insert`` and ``-remove`` command line options, and added
the ``-disable-remove`` and ``-disable-insert`` command line options as
replacements. The previous command line options were confusing because they
did not imply the default state of the option (which is inserts and removes
being enabled). The new options are easier to understand the semantics of.

Improvements to clang-tidy
--------------------------

Expand Down
17 changes: 15 additions & 2 deletions clang-tools-extra/include-cleaner/test/tool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ int x = foo();
// CHANGE: - "foobar.h"
// CHANGE-NEXT: + "foo.h"

// RUN: clang-include-cleaner -remove=0 -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=INSERT %s
// RUN: clang-include-cleaner -disable-remove -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=INSERT %s
// INSERT-NOT: - "foobar.h"
// INSERT: + "foo.h"

// RUN: clang-include-cleaner -insert=0 -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=REMOVE %s
// RUN: clang-include-cleaner -disable-insert -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=REMOVE %s
// REMOVE: - "foobar.h"
// REMOVE-NOT: + "foo.h"

Expand Down Expand Up @@ -58,3 +58,16 @@ int x = foo();
// RUN: FileCheck --match-full-lines --check-prefix=EDIT3 %s < %t.cpp
// EDIT3: #include "foo.h"
// EDIT3-NOT: {{^}}#include "foobar.h"{{$}}

// RUN: clang-include-cleaner -insert=false -print=changes %s -- -I%S/Inputs/ 2>&1 | \
// RUN: FileCheck --check-prefix=DEPRECATED-INSERT %s
// DEPRECATED-INSERT: warning: '-insert=0' is deprecated in favor of '-disable-insert'. The old flag was confusing since it suggested that inserts were disabled by default, when they were actually enabled.

// RUN: clang-include-cleaner -remove=false -print=changes %s -- -I%S/Inputs/ 2>&1 | \
// RUN: FileCheck --check-prefix=DEPRECATED-REMOVE %s
// DEPRECATED-REMOVE: warning: '-remove=0' is deprecated in favor of '-disable-remove'. The old flag was confusing since it suggested that removes were disabled by default, when they were actually enabled.

// RUN: clang-include-cleaner -insert=false -remove=false -print=changes %s -- -I%S/Inputs/ 2>&1 | \
// RUN: FileCheck --check-prefix=DEPRECATED-BOTH %s
// DEPRECATED-BOTH: warning: '-insert=0' is deprecated in favor of '-disable-insert'. The old flag was confusing since it suggested that inserts were disabled by default, when they were actually enabled.
// DEPRECATED-BOTH: warning: '-remove=0' is deprecated in favor of '-disable-remove'. The old flag was confusing since it suggested that removes were disabled by default, when they were actually enabled.
39 changes: 34 additions & 5 deletions clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,31 @@ cl::opt<bool> Edit{
cl::desc("Apply edits to analyzed source files"),
cl::cat(IncludeCleaner),
};

cl::opt<bool> Insert{
"insert",
cl::desc("Allow header insertions"),
cl::desc(
"Allow header insertions (deprecated. Use -disable-insert instead)"),
cl::init(true),
cl::cat(IncludeCleaner),
};
cl::opt<bool> Remove{
"remove",
cl::desc("Allow header removals"),
cl::desc("Allow header removals (deprecated. Use -disable-remove instead)"),
cl::init(true),
cl::cat(IncludeCleaner),
};
cl::opt<bool> DisableInsert{
"disable-insert",
cl::desc("Disable header insertions"),
cl::init(false),
cl::cat(IncludeCleaner),
};
cl::opt<bool> DisableRemove{
"disable-remove",
cl::desc("Disable header removals"),
cl::init(false),
cl::cat(IncludeCleaner),
};

std::atomic<unsigned> Errors = ATOMIC_VAR_INIT(0);

Expand Down Expand Up @@ -183,9 +195,26 @@ class Action : public clang::ASTFrontendAction {
auto Results =
analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI,
getCompilerInstance().getPreprocessor(), HeaderFilter);
if (!Insert)

if (!Insert) {
llvm::errs()
<< "warning: '-insert=0' is deprecated in favor of "
"'-disable-insert'. "
"The old flag was confusing since it suggested that inserts "
"were disabled by default, when they were actually enabled.\n";
}

if (!Remove) {
llvm::errs()
<< "warning: '-remove=0' is deprecated in favor of "
"'-disable-remove'. "
"The old flag was confusing since it suggested that removes "
"were disabled by default, when they were actually enabled.\n";
}

if (!Insert || DisableInsert)
Results.Missing.clear();
if (!Remove)
if (!Remove || DisableRemove)
Results.Unused.clear();
std::string Final = fixIncludes(Results, AbsPath, Code, getStyle(AbsPath));

Expand Down
Loading