Skip to content

Commit

Permalink
Provide a way to tell bant to keep a dependency.
Browse files Browse the repository at this point in the history
Adding a comment `# keep` in the line of a dependency that otherwise
would be removed will tell bant to _not_ emit a remove.
  • Loading branch information
hzeller committed Jun 4, 2024
1 parent 5eff334 commit c11a053
Show file tree
Hide file tree
Showing 12 changed files with 143 additions and 14 deletions.
29 changes: 22 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,24 @@ The following auto-fixes all dependencies of the project:
. <(bant dwyu ...) # source the buildozer calls in shell
```

Features

* If there is a dependency that `bant` would like to remove, but it is needed
for some reason that can't be derived from the include files it provides
(and you can't correctly mark it as `alwayslink` because it is coming
from some external project for instance), you can add a comment that
contains the word 'keep' in the line in question.
```
cc_library(
deps = [
":foo" # build_cleaner keep
]
)
```
Note, a well-defined project should not need this. This features provides
an escape hatch if needed. With `-k`, `bant` will be
strict and ignore `# keep` comments and emit the removal edit anyway.

Caveats

* Does not understand package groups in visibility yet; these will be
Expand All @@ -110,10 +128,6 @@ Caveats
do _not_ provide any headerss are considered alwayslink implicitly).
(this is not really a caveat, it just emphasizes that it is important to
properly declare the intent in BUILD files).
* Any `#keep` or similar pragmas on dependencies are ignored. A well-defined
project should not need them anyway (e.g. if a dependency is suggested to
removed, but you want to keep it - maybe it was supposed to be marked
alwayslink?).

The `dwyu` command is essentially a [`build_cleaner`][build_cleaner] for
C++ targets.
Expand Down Expand Up @@ -179,7 +193,7 @@ need for it).
$ bazel-bin/bant/bant -h
bant v0.1.4 <http://bant.build/>
Copyright (c) 2024 Henner Zeller. This program is free software; license GPL 2.0.
Usage: bant [options] <command> [bazel-target-pattern]
Usage: bazel-bin/bant/bant [options] <command> [bazel-target-pattern]
Options
-C <directory> : Change to this project directory first (default = '.')
-q : Quiet: don't print info messages to stderr.
Expand All @@ -192,13 +206,13 @@ Options
An optional parameter allows to limit the nesting depth,
e.g. -r2 just follows two levels after the toplevel
pattern. -r0 is equivalent to not providing -r.
-v : Verbose; print some stats.
-v : Verbose; print some stats. Multiple times: more verbose.
-h : This help.
Commands (unique prefix sufficient):
== Parsing ==
print : Print AST matching pattern. -e : only files w/ parse errors
: -b : elaBorate
-b : elaBorate
parse : Parse all BUILD files from pattern. Follow deps with -r
Emit parse errors. Silent otherwise: No news are good news.
Expand All @@ -224,6 +238,7 @@ Commands (unique prefix sufficient):
== Tools ==
dwyu : DWYU: Depend on What You Use (emit buildozer edit script)
-k strict: emit remove even if # keep comment in line.
canonicalize : Emit rename edits to canonicalize targets.
```

Expand Down
11 changes: 8 additions & 3 deletions bant/bant.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,13 @@ static int usage(const char *prog, const char *message, int exit_code) {
An optional parameter allows to limit the nesting depth,
e.g. -r2 just follows two levels after the toplevel
pattern. -r0 is equivalent to not providing -r.
-v : Verbose; print some stats.
-v : Verbose; print some stats. Multiple times: more verbose.
-h : This help.
Commands (unique prefix sufficient):
%s== Parsing ==%s
print : Print AST matching pattern. -e : only files w/ parse errors
: -b : elaBorate
-b : elaBorate
parse : Parse all BUILD files from pattern. Follow deps with -r
Emit parse errors. Silent otherwise: No news are good news.
Expand All @@ -107,6 +107,7 @@ Commands (unique prefix sufficient):
%s== Tools ==%s
dwyu : DWYU: Depend on What You Use (emit buildozer edit script)
-k strict: emit remove even if # keep comment in line.
canonicalize : Emit rename edits to canonicalize targets.
)",
BOLD, RESET, BOLD, RESET, BOLD, RESET);
Expand Down Expand Up @@ -135,7 +136,7 @@ int main(int argc, char *argv[]) {
{"json", OutputFormat::kJSON}, {"graphviz", OutputFormat::kGraphviz},
};
int opt;
while ((opt = getopt(argc, argv, "C:qo:vhpecbf:r::V")) != -1) {
while ((opt = getopt(argc, argv, "C:qo:vhpecbf:r::Vk")) != -1) {
switch (opt) {
case 'C': {
std::error_code err;
Expand Down Expand Up @@ -172,6 +173,10 @@ int main(int argc, char *argv[]) {
: std::numeric_limits<int>::max();
break;

case 'k':
flags.ignore_keep_comment = true;
break;

// "print" options
case 'p': flags.print_ast = true; break;
case 'e': flags.print_only_errors = true; break;
Expand Down
10 changes: 10 additions & 0 deletions bant/frontend/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ cc_library(
],
)

cc_test(
name = "named-content_test",
srcs = ["named-content_test.cc"],
deps = [
":named-content",
"@googletest//:gtest",
"@googletest//:gtest_main",
],
)

cc_library(
name = "parser",
srcs = [
Expand Down
19 changes: 19 additions & 0 deletions bant/frontend/named-content.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include "bant/frontend/named-content.h"

#include <cstdlib>
#include <string_view>

#include "absl/log/check.h"
Expand All @@ -30,4 +31,22 @@ FileLocation NamedLineIndexedContent::GetLocation(std::string_view text) const {
return {name_, line_index_.GetRange(text)};
}

std::string_view NamedLineIndexedContent::GetSurroundingLine(
std::string_view text) const {
CHECK(text.begin() >= content().begin() && text.end() <= content().end())
<< "Attempt to pass '" << text << "' which is not within " << name_;

const char *start = text.data();
while (start > content_.data() && *(start - 1) != '\n') {
--start;
}

const char *end = text.data() + text.size();
const char *const end_of_content = content_.data() + content_.size();
while (end < end_of_content && *end != '\n') {
++end;
}

return {start, static_cast<size_t>(end - start)};
}
} // namespace bant
1 change: 1 addition & 0 deletions bant/frontend/named-content.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class NamedLineIndexedContent : public SourceLocator {

// Given "text", that must be a substring of content(), return location.
FileLocation GetLocation(std::string_view text) const final;
std::string_view GetSurroundingLine(std::string_view text) const final;

private:
const std::string name_;
Expand Down
45 changes: 45 additions & 0 deletions bant/frontend/named-content_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// bant - Bazel Navigation Tool
// Copyright (C) 2024 Henner Zeller <[email protected]>
//
// This program is free software; you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation; either version 2 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License along
// with this program; if not, write to the Free Software Foundation, Inc.,
// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA

#include "bant/frontend/named-content.h"

#include <string_view>

#include "gtest/gtest.h"

namespace bant {
TEST(NamedContent, Surrounding) {
{
constexpr std::string_view content = "foo";
const NamedLineIndexedContent nc("file.txt", content);
const auto full_line = nc.GetSurroundingLine(content.substr(1, 1));
EXPECT_EQ(full_line, content);
}
{
constexpr std::string_view content = "\nfoo\n";
const NamedLineIndexedContent nc("file.txt", content);
const auto full_line = nc.GetSurroundingLine(content.substr(1, 1));
EXPECT_EQ(full_line, content.substr(1, 3));
}
{
constexpr std::string_view content = "foo\nbar\nbaz";
const NamedLineIndexedContent nc("file.txt", content);
const auto full_line = nc.GetSurroundingLine(content.substr(5, 1));
EXPECT_EQ(full_line, content.substr(4, 3));
}
}
} // namespace bant
8 changes: 8 additions & 0 deletions bant/frontend/parsed-project.cc
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,14 @@ FileLocation ParsedProject::GetLocation(std::string_view text) const {
return found.value()->GetLocation(text);
}

std::string_view ParsedProject::GetSurroundingLine(
std::string_view text) const {
auto found = location_maps_.FindBySubrange(text);
CHECK(found.has_value())
<< "Not in any of the files managed by ParsedProject '" << text << "'";
return found.value()->GetSurroundingLine(text);
}

const ParsedBuildFile *ParsedProject::FindParsedOrNull(
const BazelPackage &package) const {
auto found = package_to_parsed_.find(package);
Expand Down
1 change: 1 addition & 0 deletions bant/frontend/parsed-project.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class ParsedProject : public SourceLocator {

// -- SourceLocator implementation
FileLocation GetLocation(std::string_view text) const final;
std::string_view GetSurroundingLine(std::string_view text) const final;

private:
friend class ParsedProjectTestUtil;
Expand Down
9 changes: 9 additions & 0 deletions bant/frontend/source-locator.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ class SourceLocator {
// return file and location .
virtual FileLocation GetLocation(std::string_view text) const = 0;

// Return an expansion of the string view representing the full line the
// text snippets resides in. Line will be without newline separators.
// If there is no context known beyond the given text, just returns text.
virtual std::string_view GetSurroundingLine(std::string_view text) const = 0;

// Convenience functions.

// Given the string_view, that must be a substring handled by this locator,
Expand All @@ -80,6 +85,10 @@ class FixedSourceLocator : public SourceLocator {
return location_; // Unconditional fixed location.
}

std::string_view GetSurroundingLine(std::string_view text) const final {
return text;
}

private:
const FileLocation location_;
};
Expand Down
1 change: 1 addition & 0 deletions bant/session.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ struct CommandlineFlags {
bool print_ast = false;
bool print_only_errors = false;
bool elaborate = false;
bool ignore_keep_comment = false;
int recurse_dependency_depth = 0;
OutputFormat output_format = OutputFormat::kNative;
};
Expand Down
7 changes: 6 additions & 1 deletion bant/tool/dwyu.cc
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,12 @@ void DWYUGenerator::CreateEditsForTarget(const BazelTarget &target,

// Emit the edits.
if (potential_remove_suggestion_safe) {
emit_deps_edit_(EditRequest::kRemove, target, dependency_target, "");
static const LazyRE2 kExcludeVetoUserCommentRe{"#.*keep"};
const auto line = project_.GetSurroundingLine(dependency_target);
if (session_.flags().ignore_keep_comment ||
!RE2::PartialMatch(line, *kExcludeVetoUserCommentRe)) {
emit_deps_edit_(EditRequest::kRemove, target, dependency_target, "");
}
} else if (!all_header_deps_known && session_.flags().verbose > 1) {
project_.Loc(session_.info(), dependency_target)
<< ": Unsure what " << requested_target->ToString()
Expand Down
16 changes: 13 additions & 3 deletions bant/tool/dwyu_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -497,14 +497,24 @@ cc_library(
cc_library(
name = "bar",
srcs = ["bar.cc"],
deps = [":foo"],
hdrs = ["bar.h"],
)
cc_library(
name = "baz",
srcs = ["baz.cc"],
deps = [
":foo"
":bar" # random text dwyu that needs to contain: keep
],
)
)");

DWYUTestFixture tester(pp.project());
tester.ExpectRemove(":foo");
tester.AddSource("some/path/bar.cc", "/* no include */");
tester.RunForTarget("//some/path:bar");
// :bar should be removed, but is kept due to comment
tester.AddSource("some/path/baz.cc", "/* no include */");
tester.RunForTarget("//some/path:baz");
}

TEST(DWYUTest, DoNotRemove_IfThereIsAHeaderThatIsUnaccounted) {
Expand Down

0 comments on commit c11a053

Please sign in to comment.