No longer automatically add files to public. It is more preferred to split into a source set if you have public depending on a private header. Conveniently, this also means that `gn check --fix` will never succeed and then break if you run it immediately after (which it would previously do because the header that was made public may require something to be moved from deps to public_deps) Change-Id: I762a529c279232c6d7edb94bf99eecbc6a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/26400 Commit-Queue: Matt Stark <msta@google.com> Reviewed-by: Takuto Ikuta <tikuta@google.com>
diff --git a/src/gn/command_suggest.cc b/src/gn/command_suggest.cc index 613e79b..6a13507 100644 --- a/src/gn/command_suggest.cc +++ b/src/gn/command_suggest.cc
@@ -692,6 +692,51 @@ ? "$HEADER" : RebasePath(file.value(), included->label().dir(), build_settings->root_path_utf8()); + + // A public header attempted to include a private header. + if (includer == included) { + SourceFile includer_file = ResolveFilePath( + build_settings, all_targets, includer_name, cache, includer); + std::string includer_path = + includer_file.is_null() + ? "$HEADER" + : RebasePath(includer_file.value(), included->label().dir(), + build_settings->root_path_utf8()); + + SetAmbiguous(); + StartSuggestion(); + OutputString( + "Choose one of the following to resolve the intra-target include:\n"); + OutputString("* Create a new source_set for "); + OutputQuoted(path); + OutputString(" and add it to public_deps in "); + OutputTarget(included); + OutputString(" (preferred)\n"); + + EditCommand move_to_public{ + .command = {"move", "sources", "public", path}, + .target = included->label().GetUserVisibleName(current_toolchain), + }; + OutputString("* Move "); + OutputQuoted(path); + OutputString(" from `sources` to `public` in "); + OutputDefinition(included); + OutputString("\n"); + OutputString(" (`" + move_to_public.ToString() + "`)\n"); + + OutputString("* Move "); + OutputQuoted(includer_path); + OutputString(" from `public` to `sources` in "); + OutputDefinition(included); + OutputString("\n"); + EditCommand move_to_sources{ + .command = {"move", "public", "sources", std::move(includer_path)}, + .target = included->label().GetUserVisibleName(current_toolchain), + }; + OutputString(" (`" + move_to_sources.ToString() + "`)\n"); + return result; + } + EditCommand edit{ .command = {"move", "sources", "public", std::move(path)}, .target = included->label().GetUserVisibleName(current_toolchain),
diff --git a/src/gn/command_suggest_unittest.cc b/src/gn/command_suggest_unittest.cc index 68fce78..3f2833a 100644 --- a/src/gn/command_suggest_unittest.cc +++ b/src/gn/command_suggest_unittest.cc
@@ -587,11 +587,13 @@ } source_set("included") { - sources = [ "included.h" ] + public = [ "included.h" ] + sources = [ "private.h" ] } )"}, {SourceFile("//includer.cc"), ""}, {SourceFile("//included.h"), ""}, + {SourceFile("//private.h"), ""}, }); std::string output; @@ -617,10 +619,32 @@ } source_set("included") { - sources = [ "included.h" ] + public = [ "included.h" ] + sources = [ "private.h" ] } )"; 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", "//private.h", collect, + same_target_cache, /*must_be_file=*/false, /*apply=*/true, + &project.setup); + + EXPECT_EQ(commands::SuggestResult::kUnapplied, same_target_result); + EXPECT_EQ( + 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) + (`gn edit "move sources public private.h" //:included`) +* Move "included.h" from `public` to `sources` in :included (defined at //BUILD.gn:5) + (`gn edit "move public sources included.h" //:included`) +)", + output); + EXPECT_EQ(expected_build_gn, project.Read(SourceFile("//BUILD.gn"))); } TEST_F(SuggestTest, CheckAppliesSuggestions) {