Skip to content

Commit

Permalink
Names coming from query::FindTargets(): assemble directly.
Browse files Browse the repository at this point in the history
Instead of using ParseFrom(), always assemble names coming from
name = ... values directly with current package. This makes parsing
target that have slashes in it (foo:bar/baz) robust.
  • Loading branch information
hzeller committed Oct 2, 2024
1 parent fd8b496 commit 1f3f163
Show file tree
Hide file tree
Showing 11 changed files with 29 additions and 22 deletions.
2 changes: 1 addition & 1 deletion bant/explore/aliased-by.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ OneToN<BazelTarget, BazelTarget> ExtractAliasedBy(const ParsedProject &p) {
for (const auto &[_, build_file] : p.ParsedFiles()) {
query::FindTargets(
build_file->ast, {"alias"}, [&](const query::Result &details) {
auto alias = BazelTarget::ParseFrom(details.name, build_file->package);
auto alias = build_file->package.QualifiedTarget(details.name);
auto actual =
BazelTarget::ParseFrom(details.actual, build_file->package);
if (!alias.has_value() || !actual.has_value()) return;
Expand Down
4 changes: 2 additions & 2 deletions bant/explore/dependency-graph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ DependencyGraph BuildDependencyGraph(Session &session,
query::FindTargets(parsed->ast, kRulesOfInterest, //
[&](const query::Result &result) {
auto target_or =
BazelTarget::ParseFrom(result.name, current_package);
current_package.QualifiedTarget(result.name);
if (!target_or || !pattern.Match(*target_or)) return;
deps_to_resolve_todo.insert(*target_or);
});
Expand Down Expand Up @@ -154,7 +154,7 @@ DependencyGraph BuildDependencyGraph(Session &session,
if (!parsed) continue;
query::FindTargets(
parsed->ast, kRulesOfInterest, [&](const query::Result &result) {
auto target_or = BazelTarget::ParseFrom(result.name, current_package);
auto target_or = current_package.QualifiedTarget(result.name);
if (!target_or.has_value()) return;
const bool interested = (deps_to_resolve_todo.erase(*target_or) == 1);
if (!interested) return;
Expand Down
9 changes: 4 additions & 5 deletions bant/explore/header-providers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ static void IterateCCLibraryHeaders(const ParsedBuildFile &build_file,

query::FindTargets(
build_file.ast, kInterestingLibRules, [&](const query::Result &cc_lib) {
auto cc_library = BazelTarget::ParseFrom(cc_lib.name, build_file.package);
auto cc_library = build_file.package.QualifiedTarget(cc_lib.name);
if (!cc_library.has_value()) return;

auto hdrs = query::ExtractStringList(cc_lib.hdrs_list);
Expand Down Expand Up @@ -227,7 +227,7 @@ static void AppendProtoLibraryHeaders(const ParsedBuildFile &build_file,
OneToOne<BazelTarget, BazelTarget> proto_lib2cc_proto_lib[2];
query::FindTargets(
build_file.ast, kInterestingLibRules, [&](const query::Result &cc_plib) {
auto target = BazelTarget::ParseFrom(cc_plib.name, build_file.package);
auto target = build_file.package.QualifiedTarget(cc_plib.name);
if (!target.has_value()) return;

const bool is_grpc = (cc_plib.rule == "cc_grpc_library");
Expand All @@ -251,7 +251,7 @@ static void AppendProtoLibraryHeaders(const ParsedBuildFile &build_file,
// Putting it all together.
query::FindTargets(
build_file.ast, {"proto_library"}, [&](const query::Result &proto_lib) {
auto target = BazelTarget::ParseFrom(proto_lib.name, build_file.package);
auto target = build_file.package.QualifiedTarget(proto_lib.name);
if (!target.has_value()) return;

for (const bool is_grpc : {false, true}) {
Expand Down Expand Up @@ -324,8 +324,7 @@ ProvidedFromTarget ExtractGeneratedFromGenrule(const ParsedProject &project,
file_content->ast, {"genrule"}, [&](const query::Result &params) {
const auto genfiles = query::ExtractStringList(params.outs_list);

auto target =
BazelTarget::ParseFrom(params.name, file_content->package);
auto target = file_content->package.QualifiedTarget(params.name);
if (!target.has_value()) return;

for (const std::string_view generated : genfiles) {
Expand Down
2 changes: 1 addition & 1 deletion bant/frontend/parsed-project.cc
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ void PrintProject(Session &session, const BazelPattern &pattern,
file_content->ast, {}, [&](const query::Result &result) {
std::optional<BazelTarget> maybe_target;
if (!result.name.empty()) {
maybe_target = BazelTarget::ParseFrom(result.name, package);
maybe_target = package.QualifiedTarget(result.name);
}
if (!pattern.is_recursive()) {
// If pattern requires some match, need to check now.
Expand Down
1 change: 0 additions & 1 deletion bant/tool/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,5 @@ cc_library(
"//bant/frontend:parsed-project",
"//bant/util:file-utils",
"//bant/util:table-printer",
"@abseil-cpp//absl/strings",
],
)
2 changes: 1 addition & 1 deletion bant/tool/canon-targets.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ size_t CreateCanonicalizeEdits(Session &session, const ParsedProject &project,
const BazelPackage &current_package = parsed_package->package;
query::FindTargets(
parsed_package->ast, {}, [&](const query::Result &target) {
auto self = BazelTarget::ParseFrom(target.name, current_package);
auto self = current_package.QualifiedTarget(target.name);
if (!self.has_value()) {
return;
}
Expand Down
6 changes: 2 additions & 4 deletions bant/tool/compilation-db.cc
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,7 @@ static std::vector<std::string> CollectIncDirs(const ParsedProject &project) {
parsed_package->ast,
{"cc_library", "cc_binary", "cc_test", "proto_library"},
[&](const query::Result &details) {
auto target = BazelTarget::ParseFrom(absl::StrCat(":", details.name),
current_package);
auto target = current_package.QualifiedTarget(details.name);
// If we're one of those targets that come with the own -I prefix,
// add all these.
const auto inc_dirs = query::ExtractStringList(details.includes_list);
Expand Down Expand Up @@ -302,8 +301,7 @@ static void WriteCompilationDB(Session &session, const ParsedProject &project,
query::FindTargets(
parsed_package->ast, {"cc_library", "cc_binary", "cc_test"},
[&](const query::Result &details) {
auto target = BazelTarget::ParseFrom(absl::StrCat(":", details.name),
current_package);
auto target = current_package.QualifiedTarget(details.name);
if (!target.has_value() || !pattern.Match(*target)) {
return;
}
Expand Down
7 changes: 3 additions & 4 deletions bant/tool/dwyu.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ void DWYUGenerator::InitKnownLibraries() {
"cc_proto_library", "grpc_cc_library", // specialized
"cc_test"}, // also indexing test for testonly check.
[&](const query::Result &target) {
auto self = BazelTarget::ParseFrom(
absl::StrCat(":", target.name), current_package);
auto self =
current_package.QualifiedTarget(target.name);
if (!self.has_value()) {
return;
}
Expand Down Expand Up @@ -582,8 +582,7 @@ size_t DWYUGenerator::CreateEditsForPattern(const BazelPattern &pattern) {
query::FindTargets(
parsed_package->ast, {"cc_library", "cc_binary", "cc_test"},
[&](const query::Result &details) {
auto target = BazelTarget::ParseFrom(absl::StrCat(":", details.name),
current_package);
auto target = current_package.QualifiedTarget(details.name);
if (!target.has_value() || !pattern.Match(*target)) {
return;
}
Expand Down
4 changes: 1 addition & 3 deletions bant/tool/workspace.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include <string_view>
#include <vector>

#include "absl/strings/str_cat.h"
#include "bant/explore/query-utils.h"
#include "bant/frontend/parsed-project.h"
#include "bant/session.h"
Expand Down Expand Up @@ -70,8 +69,7 @@ BazelWorkspace CreateFilteredWorkspace(Session &session,
query::ExtractStringList(details.node->argument());
} else {
// Classical cc_library(), cc_binary() etc that has dependencies.
auto target = BazelTarget::ParseFrom(absl::StrCat(":", details.name),
current_package);
auto target = current_package.QualifiedTarget(details.name);
if (!target.has_value() || !pattern.Match(*target)) {
return;
}
Expand Down
8 changes: 8 additions & 0 deletions bant/types-bazel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ std::string BazelPackage::ToString() const {
return absl::StrCat(project, "//", path);
}

std::optional<BazelTarget> BazelPackage::QualifiedTarget(
std::string_view name) const {
if (name.empty()) return std::nullopt;
if (name[0] == ':') name.remove_prefix(1);
if (name.empty()) return std::nullopt;
return BazelTarget(*this, name);
}

std::string BazelPackage::QualifiedFile(std::string_view relative_file) const {
if (relative_file.starts_with(':')) {
relative_file.remove_prefix(1);
Expand Down
6 changes: 6 additions & 0 deletions bant/types-bazel.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
#include "re2/re2.h"

namespace bant {
class BazelTarget;

// Something like //foo/bar or @baz//foo/bar
struct BazelPackage {
BazelPackage() = default;
Expand All @@ -38,6 +40,9 @@ struct BazelPackage {
// Parse and create package if possible.
static std::optional<BazelPackage> ParseFrom(std::string_view str);

// Given a name of a target without package, reutrn a fully qualified target.
std::optional<BazelTarget> QualifiedTarget(std::string_view name) const;

std::string project; // either empty, or something like @foo_bar_baz
std::string path; // path relative to project w/o leading/trailing '/'

Expand Down Expand Up @@ -84,6 +89,7 @@ class BazelTarget {
bool operator!=(const BazelTarget &) const = default;

private:
friend BazelPackage;
// Make sure we only use the ParseFrom(). Not always needed, but this way
// it is harder to accidentally have broken targets.
BazelTarget(BazelPackage package, std::string_view target)
Expand Down

0 comments on commit 1f3f163

Please sign in to comment.