Support conditional sources for gn check.
Suppose you have foo_linux.cc including bar.h, and:
```
source_set("foo") {
sources = [ "foo.cc" ]
if (is_linux) {
sources += [ "foo_linux.cc" ]
}
}
```
Previously, we would suggest `gn edit "add deps //:bar"` //:foo`.
Now, we no longer output the gn edit suggestion, and instead suggest
doing so inside an is_linux conditional.
Change-Id: Iac7fb49959fb6437b5ffd2595614bf526a6a6964
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/26680
Reviewed-by: Takuto Ikuta <tikuta@google.com>
Commit-Queue: Matt Stark <msta@google.com>
diff --git a/src/gn/build_file_editor.cc b/src/gn/build_file_editor.cc
index 3e3ec0e..a471447 100644
--- a/src/gn/build_file_editor.cc
+++ b/src/gn/build_file_editor.cc
@@ -8,6 +8,7 @@
#include "base/files/file_enumerator.h"
#include "base/files/file_util.h"
+#include "base/strings/string_util.h"
#include "gn/build_settings.h"
#include "gn/command_format.h"
#include "gn/edit_subcommands.h"
@@ -373,6 +374,60 @@
return false;
}
+std::optional<StringAtom> TreeNode::GetConditionString() const {
+ if (!is_conditional()) {
+ return std::nullopt;
+ }
+
+ std::vector<std::string> clauses;
+ std::vector<const ConditionNode*> chain;
+
+ auto get_condition_text = [](const ConditionNode* node) {
+ return base::CollapseWhitespaceASCII(
+ std::string(node->condition()->GetRange().GetText()), false);
+ };
+
+ for (const ParseNode* node : stack_) {
+ if (node->AsFunctionCall()) {
+ // Remove the conditionals outside the target definition.
+ clauses.clear();
+ chain.clear();
+ continue;
+ }
+
+ const auto* condition = node->AsCondition();
+ if (!chain.empty() &&
+ (condition == nullptr || chain.back()->if_false() != condition)) {
+ std::string head = get_condition_text(chain.front());
+ if (node == chain.back()->if_false()) {
+ if (chain.size() == 1) {
+ clauses.push_back("if (" + head + ")'s else");
+ } else if (chain.size() == 2) {
+ clauses.push_back("if (" + head + ")'s else (after else if)");
+ } else {
+ clauses.push_back("if (" + head + ")'s else (after else ifs)");
+ }
+ } else if (chain.size() == 1) {
+ clauses.push_back("if (" + head + ")");
+ } else {
+ clauses.push_back("if (" + head + ")'s else if (" +
+ get_condition_text(chain.back()) + ")");
+ }
+ chain.clear();
+ }
+
+ if (condition) {
+ chain.push_back(condition);
+ }
+ }
+
+ if (clauses.empty()) {
+ return std::nullopt;
+ }
+
+ return StringAtom(base::JoinString(clauses, " -> "));
+}
+
bool TreeNode::is_modification() const {
if (const auto* op = node()->AsBinaryOp()) {
return op->op().type() == Token::PLUS_EQUALS ||
@@ -774,3 +829,29 @@
}
return result;
}
+
+TargetSourcesMap GetSourcesForTargets(BuildFile& build_file) {
+ TargetSourcesMap result;
+ for (const auto& target :
+ build_file.targets([](EditTarget&) { return true; })) {
+ SourceConditionsMap source_conditions;
+ for (const auto& assignment : target.assignments({"sources", "public"})) {
+ auto condition = assignment.GetConditionString();
+ for (const auto& element : FindAllListElements(assignment)) {
+ auto str = AsStringLiteral(element.node());
+ if (!str)
+ continue;
+ Err err;
+ SourceFile source_file =
+ build_file.source_file().GetDir().ResolveRelativeFile(
+ Value(nullptr, *str), &err);
+ if (err.has_error() || source_file.is_null())
+ continue;
+
+ source_conditions.try_emplace(source_file, condition);
+ }
+ }
+ result.emplace(target.label.name_atom(), std::move(source_conditions));
+ }
+ return result;
+}
diff --git a/src/gn/build_file_editor.h b/src/gn/build_file_editor.h
index 8872f29..b62174d 100644
--- a/src/gn/build_file_editor.h
+++ b/src/gn/build_file_editor.h
@@ -19,6 +19,7 @@
#include "gn/label_pattern.h"
#include "gn/parse_tree.h"
#include "gn/source_file.h"
+#include "gn/string_atom.h"
class BuildSettings;
class Loader;
@@ -26,6 +27,10 @@
struct EditState;
struct EditTarget;
+using SourceConditionsMap =
+ std::unordered_map<SourceFile, std::optional<StringAtom>>;
+using TargetSourcesMap = std::unordered_map<StringAtom, SourceConditionsMap>;
+
// A TreeNode represents a node in a tree.
// It fundamentally represents a ParseNode, but differs from one as it
// understands where it exists in the tree.
@@ -47,6 +52,10 @@
// Note that if the target itself is conditional, this will return false.
bool is_conditional() const;
+ // Returns the human-readable condition string if this node is inside a
+ // conditional statement within a target (e.g. "if (foo)'s else -> if (bar)").
+ std::optional<StringAtom> GetConditionString() const;
+
// Returns whether the node is a "+=" or "-=" operation.
bool is_modification() const;
@@ -301,4 +310,8 @@
const Loader* loader,
const std::vector<LabelPattern>& patterns);
+// Extracts all targets in the build file along with their sources and any
+// enclosing conditions.
+TargetSourcesMap GetSourcesForTargets(BuildFile& build_file);
+
#endif // TOOLS_GN_BUILD_FILE_EDITOR_H_
diff --git a/src/gn/command_suggest.cc b/src/gn/command_suggest.cc
index fab7418..28b21f6 100644
--- a/src/gn/command_suggest.cc
+++ b/src/gn/command_suggest.cc
@@ -16,6 +16,7 @@
#include "base/command_line.h"
#include "base/files/file_util.h"
#include "base/strings/string_split.h"
+#include "gn/build_file_editor.h"
#include "gn/commands.h"
#include "gn/config_values_extractors.h"
#include "gn/edit_command.h"
@@ -104,17 +105,40 @@
// Finds all targets that use a file as a source from a specific toolchain and
// adds them to results. Checks every toolchain if current_toolchain is null.
-bool AddToolchainSources(
- const std::vector<const Target*>& all_targets,
- const Label* current_toolchain,
- const SourceFile& file,
- TargetResolutionCache& cache,
- std::vector<std::pair<const Target*, commands::ApiScope>>& results) {
+bool AddToolchainSources(const std::vector<const Target*>& all_targets,
+ const Label* current_toolchain,
+ const SourceFile& file,
+ const BuildSettings* build_settings,
+ TargetResolutionCache& cache,
+ std::vector<ResolvedTarget>& results) {
for (const auto& [target, scope] :
cache.GetTargetsForFile(file, all_targets)) {
if (!current_toolchain ||
target->label().GetToolchainLabel() == *current_toolchain) {
- results.emplace_back(target, scope);
+ // Gracefully fall back to assuming it's unconditional if the build file
+ // is badly formatted or the source was added via a .gni file, for
+ // example.
+ std::optional<StringAtom> condition;
+ Err err;
+ SourceFile build_file = target->label().dir().ResolveRelativeFile(
+ Value(nullptr, "BUILD.gn"), &err);
+ if (!err.has_error() && !build_file.is_null()) {
+ if (const auto* sources_map =
+ cache.GetSourcesForBuildFile(build_file, build_settings)) {
+ auto target_it = sources_map->find(target->label().name_atom());
+ if (target_it != sources_map->end()) {
+ auto source_it = target_it->second.find(file);
+ if (source_it != target_it->second.end()) {
+ condition = source_it->second;
+ }
+ }
+ }
+ }
+ results.push_back(ResolvedTarget{
+ .target = target,
+ .scope = scope,
+ .conditional = std::move(condition),
+ });
}
}
return !results.empty();
@@ -303,6 +327,23 @@
return *it->second;
}
+const TargetSourcesMap* TargetResolutionCache::GetSourcesForBuildFile(
+ const SourceFile& build_file,
+ const BuildSettings* build_settings) {
+ std::lock_guard<std::mutex> lock(sources_cache_lock_);
+ auto it = build_file_sources_cache_.find(build_file);
+ if (it != build_file_sources_cache_.end()) {
+ return &it->second;
+ }
+ auto parsed = BuildFile::Create(build_settings, build_file, {});
+ if (parsed.has_error()) {
+ return nullptr;
+ }
+ auto [inserted, _] = build_file_sources_cache_.emplace(
+ build_file, GetSourcesForTargets(*parsed));
+ return &inserted->second;
+}
+
// Resolves an input to a list of targets, and whether each are private.
// The input can be:
// * A module name for a target
@@ -311,20 +352,20 @@
// * Targets defined in the current toolchain that contain the file
// * Targets defined in the default toolchain that contain the file
// * Targets defined in any toolchain that contain the file
-std::pair<std::vector<std::pair<const Target*, commands::ApiScope>>, bool>
-ResolveSuggestionToTarget(const BuildSettings* build_settings,
- const std::vector<const Target*>& all_targets,
- const Label& current_toolchain,
- std::string_view input,
- bool must_be_file,
- TargetResolutionCache& cache,
- const Target* includer) {
+std::pair<std::vector<ResolvedTarget>, bool> ResolveSuggestionToTarget(
+ const BuildSettings* build_settings,
+ const std::vector<const Target*>& all_targets,
+ const Label& current_toolchain,
+ std::string_view input,
+ bool must_be_file,
+ TargetResolutionCache& cache,
+ const Target* includer) {
auto sort_results = [](auto& vec) {
std::sort(vec.begin(), vec.end(), [](const auto& lhs, const auto& rhs) {
- return lhs.first->label() < rhs.first->label();
+ return lhs.target->label() < rhs.target->label();
});
};
- std::vector<std::pair<const Target*, commands::ApiScope>> results;
+ std::vector<ResolvedTarget> results;
if (!must_be_file) {
std::string_view module_name = input;
commands::ApiScope is_private = commands::ApiScope::kPublic;
@@ -336,7 +377,7 @@
// Try to resolve as a module name.
for (const Target* target : all_targets) {
if (target->module_name() == module_name) {
- results.emplace_back(target, is_private);
+ results.push_back(ResolvedTarget{target, is_private, std::nullopt});
}
}
if (!results.empty()) {
@@ -354,7 +395,7 @@
if (!err.has_error()) {
for (const Target* target : all_targets) {
if (target->label() == want) {
- results.emplace_back(target, is_private);
+ results.push_back(ResolvedTarget{target, is_private, std::nullopt});
// We know each label corresponds to exactly one target, so we don't
// need to keep going.
return {results, true};
@@ -373,21 +414,22 @@
// If we see //foo(:toolchain) request bar.h, prefer //:bar(:toolchain)
// over other toolchains.
- if (!AddToolchainSources(all_targets, ¤t_toolchain, file, cache,
- results)) {
- AddToolchainSources(all_targets, nullptr, file, cache, results);
+ if (!AddToolchainSources(all_targets, ¤t_toolchain, file,
+ build_settings, cache, results)) {
+ AddToolchainSources(all_targets, nullptr, file, build_settings, cache,
+ results);
}
// If we have an action that generates "gen/foo.h", we should prefer
// depending on the source set that declares it as a header.
if (std::ranges::all_of(results, [](const auto& result) {
- return result.second == commands::ApiScope::kOutput;
+ return result.scope == commands::ApiScope::kOutput;
})) {
for (auto& result : results) {
- result.second = commands::ApiScope::kPublic;
+ result.scope = commands::ApiScope::kPublic;
}
} else {
std::erase_if(results, [](const auto& result) {
- return result.second == commands::ApiScope::kOutput;
+ return result.scope == commands::ApiScope::kOutput;
});
}
sort_results(results);
@@ -575,8 +617,8 @@
return SuggestResult::kFailure;
if (includer_target) {
- std::erase_if(includer_targets, [&](const auto& pair) {
- return pair.first != includer_target;
+ std::erase_if(includer_targets, [&](const auto& match) {
+ return match.target != includer_target;
});
}
@@ -589,14 +631,16 @@
StartError();
OutputQuoted(includer_name);
OutputString(" resolved to multiple targets\n");
- for (const auto& [target, is_private] : includer_targets) {
+ for (const auto& match : includer_targets) {
OutputString("* ");
- OutputTarget(target);
+ OutputTarget(match.target);
OutputString("\n");
}
return SuggestResult::kFailure;
}
- const auto& [includer, dep_kind] = includer_targets.front();
+ const auto& includer_match = includer_targets.front();
+ const Target* includer = includer_match.target;
+ commands::ApiScope dep_kind = includer_match.scope;
current_toolchain = includer->label().GetToolchainLabel();
std::string_view dep_field =
@@ -622,11 +666,12 @@
}
std::set<Label> labels_without_toolchain;
- for (const auto& [target, _] : targets) {
- labels_without_toolchain.insert(target->label().GetWithNoToolchain());
+ for (const auto& match : targets) {
+ labels_without_toolchain.insert(match.target->label().GetWithNoToolchain());
}
if (labels_without_toolchain.size() == 1 &&
- targets.front().first->label().GetToolchainLabel() != current_toolchain) {
+ targets.front().target->label().GetToolchainLabel() !=
+ current_toolchain) {
// The resolution requires that if //:bar(:toolchain1) contained bar.h, we
// would have returned no targets from any other toolchain. Thus, we now
// have:
@@ -641,7 +686,7 @@
OutputString("\n");
SourceFile file = ResolveFilePath(build_settings, all_targets,
included_name, cache, includer);
- const Target* target = targets.front().first;
+ const Target* target = targets.front().target;
std::string path = file.is_null()
? std::string(included_name)
: RebasePath(file.value(), target->label().dir(),
@@ -658,9 +703,9 @@
StartWarning();
OutputQuoted(included_name);
OutputString(" is ambiguous because it belongs to multiple targets:\n");
- for (const auto& [target, _] : targets) {
+ for (const auto& match : targets) {
OutputString("* ");
- OutputTarget(target);
+ OutputTarget(match.target);
OutputString("\n");
}
StartSuggestion();
@@ -675,7 +720,8 @@
return result;
}
- const auto& [included, included_dep_kind] = targets.front();
+ const Target* included = targets.front().target;
+ commands::ApiScope included_dep_kind = targets.front().scope;
if (included_dep_kind == commands::ApiScope::kPrivate) {
StartWarning();
OutputQuoted(included_name);
@@ -873,6 +919,37 @@
return std::tie(lhs_abs, lhs.label) <
std::tie(rhs_abs, rhs.label);
});
+ if (includer_match.conditional) {
+ if (apply) {
+ result = SuggestResult::kUnapplied;
+ }
+ StartSuggestion();
+ if (candidate_deps.size() == 1) {
+ OutputString("Under ");
+ OutputString(includer_match.conditional->str());
+ OutputString(", add ");
+ OutputString(dep_field);
+ OutputString(" = [ ");
+ OutputQuoted(candidate_deps.front().label);
+ OutputString(" ] to ");
+ OutputDefinition(includer);
+ OutputString("\n");
+ } else {
+ SetAmbiguous();
+ OutputString("Under ");
+ OutputString(includer_match.conditional->str());
+ OutputString(", add one of the following to ");
+ OutputString(dep_field);
+ OutputString(" in ");
+ OutputDefinition(includer);
+ OutputString(":\n");
+ for (const auto& c : candidate_deps) {
+ OutputString("* " + c.label + "\n");
+ }
+ }
+ return;
+ }
+
if (candidate_deps.size() == 1) {
OutputEditCommand(candidate_deps.front().edit, includer);
return;
diff --git a/src/gn/command_suggest_unittest.cc b/src/gn/command_suggest_unittest.cc
index eda3a32..4ddc70c 100644
--- a/src/gn/command_suggest_unittest.cc
+++ b/src/gn/command_suggest_unittest.cc
@@ -111,7 +111,7 @@
auto [results, ok] = commands::ResolveSuggestionToTarget(
setup_scope.build_settings(), all_targets, default_toolchain,
"my_module", /*must_be_file=*/false, cache);
- std::vector<std::pair<const Target*, commands::ApiScope>> expected = {
+ std::vector<commands::ResolvedTarget> expected = {
{&target, commands::ApiScope::kPublic}};
EXPECT_EQ(expected, results);
EXPECT_TRUE(ok);
@@ -122,7 +122,7 @@
auto [results, ok] = commands::ResolveSuggestionToTarget(
setup_scope.build_settings(), all_targets, default_toolchain,
"my_module_Private", /*must_be_file=*/false, cache);
- std::vector<std::pair<const Target*, commands::ApiScope>> expected = {
+ std::vector<commands::ResolvedTarget> expected = {
{&target, commands::ApiScope::kPrivate}};
EXPECT_EQ(expected, results);
EXPECT_TRUE(ok);
@@ -151,7 +151,7 @@
setup_scope.toolchain()->label(), "//:hello", /*must_be_file=*/false,
cache);
- std::vector<std::pair<const Target*, commands::ApiScope>> expected_label = {
+ std::vector<commands::ResolvedTarget> expected_label = {
{&target, commands::ApiScope::kPublic}};
EXPECT_EQ(expected_label, results_label);
EXPECT_TRUE(ok_label);
@@ -161,8 +161,8 @@
setup_scope.build_settings(), all_targets, default_toolchain,
"//:hello(//build/toolchain:gcc)", /*must_be_file=*/false, cache);
- std::vector<std::pair<const Target*, commands::ApiScope>> expected_toolchain =
- {{&target_gcc, commands::ApiScope::kPublic}};
+ std::vector<commands::ResolvedTarget> expected_toolchain = {
+ {&target_gcc, commands::ApiScope::kPublic}};
EXPECT_EQ(expected_toolchain, results_toolchain);
EXPECT_TRUE(ok_toolchain);
}
@@ -269,7 +269,7 @@
auto [results, ok] = commands::ResolveSuggestionToTarget(
setup_scope.build_settings(), all_targets, current_toolchain,
"//public.h", /*must_be_file=*/true, cache);
- std::vector<std::pair<const Target*, commands::ApiScope>> expected = {
+ std::vector<commands::ResolvedTarget> expected = {
{&explicit_target, commands::ApiScope::kPublic}};
EXPECT_TRUE(ok);
EXPECT_EQ(expected, results);
@@ -279,7 +279,7 @@
auto [results, ok] = commands::ResolveSuggestionToTarget(
setup_scope.build_settings(), all_targets, current_toolchain,
"../../private.h", /*must_be_file=*/true, cache);
- std::vector<std::pair<const Target*, commands::ApiScope>> expected = {
+ std::vector<commands::ResolvedTarget> expected = {
{&explicit_target, commands::ApiScope::kPrivate}};
EXPECT_TRUE(ok);
EXPECT_EQ(expected, results);
@@ -289,7 +289,7 @@
auto [results, ok] = commands::ResolveSuggestionToTarget(
setup_scope.build_settings(), all_targets, current_toolchain,
"//implicit_public.h", /*must_be_file=*/true, cache);
- std::vector<std::pair<const Target*, commands::ApiScope>> expected = {
+ std::vector<commands::ResolvedTarget> expected = {
{&implicit_target, commands::ApiScope::kPublic}};
EXPECT_TRUE(ok);
EXPECT_EQ(expected, results);
@@ -306,7 +306,7 @@
auto [results, ok] = commands::ResolveSuggestionToTarget(
setup_scope.build_settings(), all_targets, current_toolchain,
"//out/Debug/generated_file.h", /*must_be_file=*/true, cache);
- std::vector<std::pair<const Target*, commands::ApiScope>> expected = {
+ std::vector<commands::ResolvedTarget> expected = {
{&generated, commands::ApiScope::kPublic}};
EXPECT_TRUE(ok);
EXPECT_EQ(expected, results);
@@ -318,7 +318,7 @@
auto [results, ok] = commands::ResolveSuggestionToTarget(
setup_scope.build_settings(), all_targets, current_toolchain,
"//out/Debug/generated_file.h", /*must_be_file=*/true, consumer_cache);
- std::vector<std::pair<const Target*, commands::ApiScope>> expected = {
+ std::vector<commands::ResolvedTarget> expected = {
{&consumer, commands::ApiScope::kPublic}};
EXPECT_TRUE(ok);
EXPECT_EQ(expected, results);
@@ -328,7 +328,7 @@
auto [results, ok] = commands::ResolveSuggestionToTarget(
setup_scope.build_settings(), all_targets, current_toolchain,
"//no_target.h", /*must_be_file=*/true, consumer_cache);
- std::vector<std::pair<const Target*, commands::ApiScope>> expected_targets;
+ std::vector<commands::ResolvedTarget> expected_targets;
EXPECT_TRUE(ok);
EXPECT_EQ(expected_targets, results);
}
@@ -337,10 +337,9 @@
auto [results, ok] = commands::ResolveSuggestionToTarget(
setup_scope.build_settings(), all_targets, current_toolchain,
"//default_toolchain.h", /*must_be_file=*/true, consumer_cache);
- std::vector<std::pair<const Target*, commands::ApiScope>> expected_targets =
- {
- {&simple_secondary, commands::ApiScope::kPublic},
- {&simple_default, commands::ApiScope::kPublic},
+ std::vector<commands::ResolvedTarget> expected_targets = {
+ {&simple_secondary, commands::ApiScope::kPublic},
+ {&simple_default, commands::ApiScope::kPublic},
};
EXPECT_TRUE(ok);
EXPECT_EQ(expected_targets, results);
@@ -350,8 +349,8 @@
auto [results, ok] = commands::ResolveSuggestionToTarget(
setup_scope.build_settings(), all_targets, current_toolchain,
"//secondary_toolchain.h", /*must_be_file=*/true, consumer_cache);
- std::vector<std::pair<const Target*, commands::ApiScope>> expected_targets =
- {{{&simple_secondary, commands::ApiScope::kPublic}}};
+ std::vector<commands::ResolvedTarget> expected_targets = {
+ {&simple_secondary, commands::ApiScope::kPublic}};
EXPECT_TRUE(ok);
EXPECT_EQ(expected_targets, results);
}
@@ -361,8 +360,8 @@
setup_scope.build_settings(), all_targets, current_toolchain,
"my_header.h", /*must_be_file=*/true, consumer_cache, &consumer);
EXPECT_TRUE(ok);
- std::vector<std::pair<const Target*, commands::ApiScope>> expected_targets =
- {{{&included_target, commands::ApiScope::kPublic}}};
+ std::vector<commands::ResolvedTarget> expected_targets = {
+ {&included_target, commands::ApiScope::kPublic}};
EXPECT_EQ(expected_targets, results);
}
}
@@ -613,8 +612,13 @@
TEST_F(SuggestTest, ApplyValidSuggestion) {
TestProject project({
- {SourceFile("//BUILD.gn"), R"(executable("includer") {
+ {SourceFile("//BUILD.gn"), R"(is_win = true
+
+executable("includer") {
sources = [ "includer.cc" ]
+ if (is_win) {
+ sources += [ "win.cc" ]
+ }
}
source_set("included") {
@@ -623,6 +627,7 @@
}
)"},
{SourceFile("//includer.cc"), ""},
+ {SourceFile("//win.cc"), ""},
{SourceFile("//included.h"), ""},
{SourceFile("//private.h"), ""},
});
@@ -642,11 +647,16 @@
EXPECT_EQ(commands::SuggestResult::kSuccess, result);
EXPECT_EQ(
"[APPLIED] Suggestion: Add deps = [ \":included\" ] to :includer "
- "(defined at //BUILD.gn:1)\n",
+ "(defined at //BUILD.gn:3)\n",
output);
- std::string expected_build_gn = R"(executable("includer") {
+ std::string expected_build_gn = R"(is_win = true
+
+executable("includer") {
sources = [ "includer.cc" ]
+ if (is_win) {
+ sources += [ "win.cc" ]
+ }
deps = [ ":included" ]
}
@@ -658,11 +668,10 @@
EXPECT_EQ(expected_build_gn, project.Read(SourceFile("//BUILD.gn")));
output.clear();
- commands::TargetResolutionCache same_target_cache;
commands::SuggestResult same_target_result = commands::OutputSuggestions(
project.targets(), &project.setup.build_settings(),
project.default_toolchain(), "//included.h",
- /*includer_target=*/nullptr, "//private.h", collect, same_target_cache,
+ /*includer_target=*/nullptr, "//private.h", collect, cache,
/*must_be_file=*/false, /*apply=*/true, &project.setup);
EXPECT_EQ(commands::SuggestResult::kUnapplied, same_target_result);
@@ -670,13 +679,28 @@
R"(Warning: "//private.h" is in the private API of //:included
[AMBIGUOUS] Suggestion: Choose one of the following to resolve the intra-target include:
* Create a new source_set for "private.h" and add it to public_deps in //:included (preferred)
-* Move "private.h" from `sources` to `public` in :included (defined at //BUILD.gn:5)
+* Move "private.h" from `sources` to `public` in :included (defined at //BUILD.gn:10)
(`gn edit "move sources public private.h" //:included`)
-* Move "included.h" from `public` to `sources` in :included (defined at //BUILD.gn:5)
+* Move "included.h" from `public` to `sources` in :included (defined at //BUILD.gn:10)
(`gn edit "move public sources included.h" //:included`)
)",
output);
EXPECT_EQ(expected_build_gn, project.Read(SourceFile("//BUILD.gn")));
+
+ output.clear();
+
+ commands::SuggestResult conditional_result = commands::OutputSuggestions(
+ project.targets(), &project.setup.build_settings(),
+ project.default_toolchain(), "//win.cc",
+ /*includer_target=*/nullptr, "//included.h", collect, cache,
+ /*must_be_file=*/false, /*apply=*/true, &project.setup);
+
+ EXPECT_EQ(commands::SuggestResult::kUnapplied, conditional_result);
+ EXPECT_EQ(
+ "Suggestion: Under if (is_win), add deps = [ \":included\" ] to "
+ ":includer (defined at //BUILD.gn:3)\n",
+ output);
+ EXPECT_EQ(expected_build_gn, project.Read(SourceFile("//BUILD.gn")));
}
TEST_F(SuggestTest, CheckAppliesSuggestions) {
diff --git a/src/gn/commands.h b/src/gn/commands.h
index 168cb26..08917b9 100644
--- a/src/gn/commands.h
+++ b/src/gn/commands.h
@@ -17,6 +17,7 @@
#include <vector>
#include "base/values.h"
+#include "gn/build_file_editor.h"
#include "gn/header_checker.h"
#include "gn/standard_out.h"
#include "gn/target.h"
@@ -130,6 +131,26 @@
kOutput,
};
+// Represents something that has resolved to a target.
+// May be a source file, a module name, or a target label.
+struct ResolvedTarget {
+ // Can never be null. Unfortunately can't use a reference because it's stored
+ // in a vector.
+ const Target* target;
+ ApiScope scope;
+ // If the input was a conditional source, this is a human-readable
+ // representation of the condition. eg. When looking up foo.cc in a target
+ // with: if (foo) {
+ // if (bar) {
+ // sources += [ "foo.cc" ]
+ // }
+ // }
+ // Conditional could be "if (foo) and if (bar)"
+ std::optional<StringAtom> conditional = std::nullopt;
+
+ bool operator==(const ResolvedTarget&) const = default;
+};
+
// Caches the mapping of SourceFile -> target(s) to accelerate suggestion
// resolution.
// May potentially be used in the future to cache other properties of the build
@@ -154,6 +175,12 @@
// Returns reference to ReachabilityCache for the given target.
HeaderChecker::ReachabilityCache& GetReachabilityCache(const Target* target);
+ // Returns cached source-to-condition map for all targets in the given build
+ // file.
+ const TargetSourcesMap* GetSourcesForBuildFile(
+ const SourceFile& build_file,
+ const BuildSettings* build_settings);
+
private:
std::once_flag file_to_target_initialized_;
std::unordered_map<SourceFile,
@@ -172,6 +199,11 @@
std::unordered_map<const Target*,
std::unique_ptr<HeaderChecker::ReachabilityCache>>
reachability_cache_;
+
+ // Maps a BUILD.gn file to the sources conditions map for targets in that
+ // file.
+ std::mutex sources_cache_lock_;
+ std::unordered_map<SourceFile, TargetSourcesMap> build_file_sources_cache_;
};
SuggestResult OutputSuggestions(const std::vector<const Target*>& all_targets,
@@ -355,15 +387,16 @@
// Resolves an input to a list of targets for suggestion.
// Specifically also decides whether it resolves to the public or private API
-// of the target.
-std::pair<std::vector<std::pair<const Target*, ApiScope>>, bool>
-ResolveSuggestionToTarget(const BuildSettings* build_settings,
- const std::vector<const Target*>& all_targets,
- const Label& current_toolchain,
- std::string_view input,
- bool must_be_file,
- TargetResolutionCache& cache,
- const Target* includer = nullptr);
+// of the target, and any enclosing conditions if it resolves to a conditional
+// source.
+std::pair<std::vector<ResolvedTarget>, bool> ResolveSuggestionToTarget(
+ const BuildSettings* build_settings,
+ const std::vector<const Target*>& all_targets,
+ const Label& current_toolchain,
+ std::string_view input,
+ bool must_be_file,
+ TargetResolutionCache& cache,
+ const Target* includer = nullptr);
// Resolves a vector of command line inputs and figures out the full set of
// things they resolve to.
diff --git a/src/gn/edit_command_unittest.cc b/src/gn/edit_command_unittest.cc
index 0f2f4ea..d61120d 100644
--- a/src/gn/edit_command_unittest.cc
+++ b/src/gn/edit_command_unittest.cc
@@ -10,9 +10,11 @@
#include "base/files/file_util.h"
#include "base/files/scoped_temp_dir.h"
+#include "gn/build_file_editor.h"
#include "gn/err.h"
#include "gn/filesystem_utils.h"
#include "gn/setup.h"
+#include "gn/string_atom.h"
#include "gn/test_with_scheduler.h"
#include "util/test/test.h"
@@ -965,4 +967,56 @@
)"));
}
+TEST_F(EditCommandTest, ExtractTargetSourcesConditions) {
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ base::FilePath root_path = base::MakeAbsoluteFilePath(temp_dir.GetPath());
+
+ base::FilePath build_gn_path = root_path.AppendASCII("BUILD.gn");
+ std::string content = R"(
+source_set("my_target") {
+ sources = [ "uncond.cc" ]
+
+ if (is_win) {
+ sources += [ "win1.cc", "win2.cc" ]
+ if (nested) {
+ sources += [ "nested.cc" ]
+ }
+ } else if (is_linux) {
+ sources += [ "linux.cc" ]
+ } else {
+ sources += [ "generic.cc" ]
+ public = [ "generic.h" ]
+ }
+}
+)";
+ ASSERT_TRUE(WriteFile(build_gn_path, content, nullptr));
+
+ Setup setup;
+ setup.build_settings().SetRootPath(root_path);
+
+ auto build_file =
+ BuildFile::Create(&setup.build_settings(), SourceFile("//BUILD.gn"), {});
+ ASSERT_TRUE(build_file.has_value());
+
+ TargetSourcesMap target_sources = GetSourcesForTargets(*build_file);
+ ASSERT_TRUE(target_sources.contains(StringAtom("my_target")));
+
+ const auto& sources = target_sources[StringAtom("my_target")];
+
+ SourceConditionsMap expected = {
+ {SourceFile("//uncond.cc"), std::nullopt},
+ {SourceFile("//win1.cc"), StringAtom("if (is_win)")},
+ {SourceFile("//win2.cc"), StringAtom("if (is_win)")},
+ {SourceFile("//nested.cc"), StringAtom("if (is_win) -> if (nested)")},
+ {SourceFile("//linux.cc"),
+ StringAtom("if (is_win)'s else if (is_linux)")},
+ {SourceFile("//generic.cc"),
+ StringAtom("if (is_win)'s else (after else if)")},
+ {SourceFile("//generic.h"),
+ StringAtom("if (is_win)'s else (after else if)")},
+ };
+ EXPECT_EQ(expected, sources);
+}
+
} // namespace commands
diff --git a/src/gn/location.cc b/src/gn/location.cc
index cba44c8..1cfe4b7 100644
--- a/src/gn/location.cc
+++ b/src/gn/location.cc
@@ -9,6 +9,7 @@
#include "base/logging.h"
#include "base/strings/string_number_conversions.h"
#include "gn/input_file.h"
+#include "gn/tokenizer.h"
Location::Location() = default;
@@ -67,3 +68,29 @@
return LocationRange(begin_ < other.begin_ ? begin_ : other.begin_,
end_ < other.end_ ? other.end_ : end_);
}
+
+std::string_view LocationRange::GetText() const {
+ if (is_null() || !begin_.file()) {
+ return {};
+ }
+ const std::string& contents = begin_.file()->contents();
+ size_t begin_line_off =
+ Tokenizer::ByteOffsetOfNthLine(contents, begin_.line_number());
+ if (begin_line_off == static_cast<size_t>(-1)) {
+ return {};
+ }
+ size_t begin_off = begin_line_off + (begin_.column_number() - 1);
+
+ size_t end_line_off =
+ Tokenizer::ByteOffsetOfNthLine(contents, end_.line_number());
+ if (end_line_off == static_cast<size_t>(-1)) {
+ return {};
+ }
+ size_t end_off = end_line_off + (end_.column_number() - 1);
+
+ if (begin_off > end_off || end_off > contents.size()) {
+ return {};
+ }
+
+ return std::string_view(contents).substr(begin_off, end_off - begin_off);
+}
diff --git a/src/gn/location.h b/src/gn/location.h
index 79af5dd..b6186a7 100644
--- a/src/gn/location.h
+++ b/src/gn/location.h
@@ -6,6 +6,7 @@
#define TOOLS_GN_LOCATION_H_
#include <string>
+#include <string_view>
class InputFile;
@@ -53,6 +54,10 @@
LocationRange Union(const LocationRange& other) const;
+ // Returns the text covered by this range in the source file, or an empty
+ // string_view if the range is null or invalid.
+ std::string_view GetText() const;
+
private:
Location begin_;
Location end_;
diff --git a/src/gn/tokenizer_unittest.cc b/src/gn/tokenizer_unittest.cc
index 6f6f0e0..e7e9c4a 100644
--- a/src/gn/tokenizer_unittest.cc
+++ b/src/gn/tokenizer_unittest.cc
@@ -162,6 +162,27 @@
EXPECT_EQ(2u, Tokenizer::ByteOffsetOfNthLine(input2, 2));
}
+TEST(Tokenizer, LocationRangeGetText) {
+ InputFile file(SourceFile("//test"));
+ file.SetContents("first line\nsecond line\nthird line");
+
+ // Entire first line
+ LocationRange first_line(Location(&file, 1, 1), Location(&file, 1, 11));
+ EXPECT_EQ(first_line.GetText(), "first line");
+
+ // Part of second line
+ LocationRange second_part(Location(&file, 2, 8), Location(&file, 2, 12));
+ EXPECT_EQ(second_part.GetText(), "line");
+
+ // Multi-line range
+ LocationRange multi_line(Location(&file, 1, 7), Location(&file, 2, 7));
+ EXPECT_EQ(multi_line.GetText(), "line\nsecond");
+
+ // Null range
+ LocationRange null_range;
+ EXPECT_TRUE(null_range.GetText().empty());
+}
+
TEST(Tokenizer, Comments) {
TokenExpectation fn[] = {
{Token::LINE_COMMENT, "# Stuff"},
