Implement check_includes_strict parameter This parameter allows target-level strict include checking. When enabled on a target: 1. Public headers of this target cannot include its private deps. 2. Dependents of this target cannot transitively include its public_deps (they must depend on them directly). 3. Public headers of this target cannot include private headers. Bug: 500845363 Change-Id: I1000f3abcd0853c83c2a1232efa97f206a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/24240 Commit-Queue: Matt Stark <msta@google.com> Reviewed-by: Takuto Ikuta <tikuta@google.com>
diff --git a/docs/reference.md b/docs/reference.md index 75d3b42..e4c9013 100644 --- a/docs/reference.md +++ b/docs/reference.md
@@ -115,6 +115,7 @@ * [cflags_objc: [string list] Flags passed to the Objective C compiler.](#var_cflags_objc) * [cflags_objcc: [string list] Flags passed to the Objective C++ compiler.](#var_cflags_objcc) * [check_includes: [boolean] Controls whether a target's files are checked.](#var_check_includes) + * [check_includes_strict: [boolean] Controls whether strict include checking is enforced.](#var_check_includes_strict) * [complete_static_lib: [boolean] Links all deps into a static library.](#var_complete_static_lib) * [configs: [label list] Configs applying to this target or config.](#var_configs) * [contents: Contents to write to file.](#var_contents) @@ -5580,6 +5581,40 @@ ... } ``` +### <a name="var_check_includes_strict"></a>**check_includes_strict**: [boolean] Enforce strict include checking. [Back to Top](#gn-reference) + +``` + When true, the "gn check" command (as well as "gn gen" with the --check flag) + will enforce strict header checking rules on this target: + + 1. Public headers of this target cannot include headers from its private + "deps". + 2. Public headers of this target cannot include its own private headers + (headers in "sources" when "public" is explicitly specified). + 3. Dependents of this target cannot transitively include headers from this + target's "public_deps" (they must depend on those targets directly). + + Note that targets listed in "allow_circular_includes_from" act as an escape + hatch and bypass these strict checks. + + This enforces strict API boundary separation. Under this strict model: + * "public_deps" act as interface dependencies (dependencies required to + compile this target's public headers), but they are not transitively + forwarded to dependents. + * "deps" act as private implementation dependencies (dependencies required to + compile this target's sources). + + When false (the default), the default loose include checking rules apply. +``` + +#### **Example** + +``` + source_set("strict_target") { + check_includes_strict = true + ... + } +``` ### <a name="var_complete_static_lib"></a>**complete_static_lib**: [boolean] Links all deps into a static library. [Back to Top](#gn-reference) ```
diff --git a/src/gn/binary_target_generator_unittest.cc b/src/gn/binary_target_generator_unittest.cc index 66add7a..75660eb 100644 --- a/src/gn/binary_target_generator_unittest.cc +++ b/src/gn/binary_target_generator_unittest.cc
@@ -88,3 +88,49 @@ EXPECT_TRUE(target->module_type().test(Target::MODULEMAP_IS_GENERATED)); EXPECT_TRUE(target->module_type().test(Target::MODULEMAP_IS_TEXTUAL)); } + +TEST_F(BinaryTargetGeneratorTest, CheckIncludesStrict) { + TestWithScope setup; + Scope::ItemVector items_; + setup.scope()->set_item_collector(&items_); + setup.scope()->set_source_dir(SourceDir("//test/")); + + // Check default value is false. + { + TestParseInput input( + R"(static_library("foo") { + sources = [ "//foo.cc" ] + })"); + ASSERT_SUCCESS(input); + + Err err; + input.parsed()->Execute(setup.scope(), &err); + ASSERT_SUCCESS(err); + + ASSERT_EQ(1u, items_.size()); + Target* target = items_[0]->AsTarget(); + ASSERT_TRUE(target); + EXPECT_FALSE(target->check_includes_strict()); + items_.clear(); + } + + // Check overriding to true. + { + TestParseInput input( + R"(static_library("bar") { + sources = [ "//bar.cc" ] + check_includes_strict = true + })"); + ASSERT_SUCCESS(input); + + Err err; + input.parsed()->Execute(setup.scope(), &err); + ASSERT_SUCCESS(err); + + ASSERT_EQ(1u, items_.size()); + Target* target = items_[0]->AsTarget(); + ASSERT_TRUE(target); + EXPECT_TRUE(target->check_includes_strict()); + items_.clear(); + } +}
diff --git a/src/gn/command_desc.cc b/src/gn/command_desc.cc index 2e8f18a..e7b0c99 100644 --- a/src/gn/command_desc.cc +++ b/src/gn/command_desc.cc
@@ -271,6 +271,7 @@ {variables::kMetadata, MetadataHandler}, {variables::kTestonly, DefaultHandler}, {variables::kCheckIncludes, DefaultHandler}, + {variables::kCheckIncludesStrict, DefaultHandler}, {variables::kAllowCircularIncludesFrom, DefaultHandler}, {variables::kSources, DefaultHandler}, {variables::kPublic, PublicHandler}, @@ -369,6 +370,7 @@ HandleProperty(variables::kMetadata, handler_map, v, dict); HandleProperty(variables::kTestonly, handler_map, v, dict); HandleProperty(variables::kCheckIncludes, handler_map, v, dict); + HandleProperty(variables::kCheckIncludesStrict, handler_map, v, dict); HandleProperty(variables::kAllowCircularIncludesFrom, handler_map, v, dict); HandleProperty(variables::kSources, handler_map, v, dict); HandleProperty(variables::kSwiftBridgeHeader, handler_map, v, dict);
diff --git a/src/gn/desc_builder.cc b/src/gn/desc_builder.cc index 8dce294..67d28af 100644 --- a/src/gn/desc_builder.cc +++ b/src/gn/desc_builder.cc
@@ -379,6 +379,10 @@ if (what(variables::kTestonly)) res->SetKey(variables::kTestonly, base::Value(target_->testonly())); + if (what(variables::kCheckIncludesStrict)) + res->SetKey(variables::kCheckIncludesStrict, + base::Value(target_->check_includes_strict())); + if (is_binary_output) { if (what(variables::kCheckIncludes)) res->SetKey(variables::kCheckIncludes,
diff --git a/src/gn/header_checker.cc b/src/gn/header_checker.cc index 8044896..3e3a9f1 100644 --- a/src/gn/header_checker.cc +++ b/src/gn/header_checker.cc
@@ -94,14 +94,31 @@ ret += "There is no dependency chain between these targets."; } else { // Indirect dependency chain, print the chain. - ret += - "\nIt's usually best to depend directly on the destination target.\n" - "In some cases, the destination target is considered a subcomponent\n" - "of an intermediate target. In this case, the intermediate target\n" - "should depend publicly on the destination to forward the ability\n" - "to include headers.\n" - "\n" - "Dependency chain (there may also be others):\n"; + const Target* strict_target = nullptr; + for (size_t i = 1; i < chain.size() - 1; i++) { + if (chain[i].target->check_includes_strict()) { + strict_target = chain[i].target; + break; + } + } + + if (strict_target) { + ret += + "\nPlease depend directly on the destination target.\n" + "Including headers from the public_deps of " + + strict_target->label().GetUserVisibleName(false) + + " is blocked because check_includes_strict = true is enabled on it.\n" + "check_includes_strict = true is highly recommended for all new code " + "to ensure a correct build graph.\n"; + } else { + ret += + "\nIt's usually best to depend directly on the destination target.\n" + "In some cases, the destination target is considered a subcomponent\n" + "of an intermediate target. In this case, the intermediate target\n" + "should depend publicly on the destination to forward the ability\n" + "to include headers.\n"; + } + ret += "\nDependency chain (there may also be others):\n"; for (int i = static_cast<int>(chain.size()) - 1; i >= 0; i--) { ret.append(" " + chain[i].target->label().GetUserVisibleName(false)); @@ -110,10 +127,16 @@ // dependency chain things went bad. Don't list this for the first link // in the chain since direct dependencies are OK, and listing that as // "private" may make people feel like they need to fix it. - if (i == static_cast<int>(chain.size()) - 1 || chain[i - 1].is_public) - ret.append(" -->"); - else + if (i == static_cast<int>(chain.size()) - 1 || chain[i - 1].is_public) { + if (i != static_cast<int>(chain.size()) - 1 && + chain[i].target->check_includes_strict()) { + ret.append(" --[check_includes_strict = true]-->"); + } else { + ret.append(" -->"); + } + } else { ret.append(" --[private]-->"); + } } ret.append("\n"); } @@ -234,10 +257,10 @@ continue; } - std::vector<const Target*> targets_to_check; + TargetVector targets_to_check; for (const auto& vect_i : file.second) { if (vect_i.target->check_includes()) { - targets_to_check.push_back(vect_i.target); + targets_to_check.push_back(vect_i); } } if (targets_to_check.empty()) @@ -259,7 +282,7 @@ task_count_cv_.wait(auto_lock); } -void HeaderChecker::DoWork(const std::vector<const Target*>& targets, +void HeaderChecker::DoWork(const TargetVector& targets, const SourceFile& file) { std::vector<Err> errors; if (!CheckFile(targets, file, &errors)) { @@ -392,6 +415,11 @@ const Target* target = work_queue.front(); work_queue.pop(); + if (permitted && target != source_target_ && + target->check_includes_strict()) { + continue; + } + for (const auto& dep : target->public_deps()) { if (breadcrumbs.Insert(dep.ptr, target, true)) work_queue.push(dep.ptr); @@ -458,7 +486,7 @@ return true; } -bool HeaderChecker::CheckFile(const std::vector<const Target*>& targets, +bool HeaderChecker::CheckFile(const TargetVector& targets, const SourceFile& file, std::vector<Err>* errors) const { ScopedTrace trace(TraceItem::TRACE_CHECK_HEADER, file.value()); @@ -478,7 +506,8 @@ if (IsFileInOuputDir(file)) return true; - for (const Target* from_target : targets) { + for (const TargetInfo& from_target_info : targets) { + const Target* from_target = from_target_info.target; errors->emplace_back( from_target->defined_from(), "Source file not found.", "The target:\n " + from_target->label().GetUserVisibleName(false) + @@ -505,7 +534,8 @@ size_t error_count_before = errors->size(); - for (const Target* from_target : targets) { + for (const TargetInfo& from_target_info : targets) { + const Target* from_target = from_target_info.target; std::vector<SourceDir> include_dirs; for (ConfigValuesIterator target_iter(from_target); !target_iter.done(); target_iter.Next()) { @@ -523,8 +553,10 @@ SourceFile included_file = SourceFileForInclude(inc, include_dirs, input_file, &err); if (!included_file.is_null()) { - CheckInclude(from_target_cache, input_file, included_file, inc.location, - errors); + CheckInclude(from_target_cache, + from_target_info.is_public && + file.GetType() == SourceFile::SOURCE_H, + input_file, included_file, inc.location, errors); } } } @@ -539,6 +571,7 @@ // - If there are multiple targets with the header in it, only one need be // valid for the check to pass. void HeaderChecker::CheckInclude(ReachabilityCache& from_target_cache, + bool is_public_header, const InputFile& source_file, const SourceFile& include_file, const LocationRange& range, @@ -597,10 +630,26 @@ bool found_dependency = false; for (const auto& target : targets) { // We always allow source files in a target to include headers also in that - // target. + // target, unless strict checking is enabled and a public header includes + // a private header. const Target* to_target = target.target; - if (to_target == from_target) + if (to_target == from_target) { + if (from_target->check_includes_strict() && is_public_header && + !target.is_public) { + last_error = Err( + CreatePersistentRange(source_file, range), + "Public headers cannot include private headers of the same target.", + "The public header:\n " + source_file.name().value() + + "\nis including a private header of the same target:\n " + + include_file.value() + + "\nEither make the included header public, make the includer " + "private,\n" + "or make a source_set containing public = [private_headers] " + "and add it to public_deps."); + errors->push_back(std::move(last_error)); + } return; + } bool is_permitted_chain = false; if (IsDependencyOf(to_target, from_target_cache, &chain, @@ -614,6 +663,18 @@ target.is_public || FriendMatches(to_target, from_target); if (effectively_public && is_permitted_chain) { + if (from_target->check_includes_strict() && is_public_header && + !chain[chain.size() - 2].is_public) { + last_error = Err( + CreatePersistentRange(source_file, range), + "Public headers cannot include private dependencies.", + "The public header:\n " + source_file.name().value() + + "\nis including a header from private dependency:\n " + + to_target->label().GetUserVisibleName(false) + + "\nEither move the dependency to public_deps, or make this " + "header private."); + continue; + } // This one is OK, we're done. last_error = Err(); break;
diff --git a/src/gn/header_checker.h b/src/gn/header_checker.h index c3c688b..60f485d 100644 --- a/src/gn/header_checker.h +++ b/src/gn/header_checker.h
@@ -85,6 +85,11 @@ FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest, SourceFileForInclude_FileNotFound); FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest, Friend); + FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest, CheckIncludesStrictTransitive); + FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest, + CheckIncludesStrictPrivateInPublicHeader); + FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest, + CheckIncludesStrictSameTargetPrivateHeader); ~HeaderChecker(); @@ -247,8 +252,7 @@ bool force_check, WorkerPool* pool); - void DoWork(const std::vector<const Target*>& targets, - const SourceFile& file); + void DoWork(const TargetVector& targets, const SourceFile& file); // Adds the sources and public files from the given target to the given map. static void AddTargetToFileMap(const Target* target, FileMap* dest); @@ -264,7 +268,7 @@ // targets is a list of targets using the source file. They will be used in // error messages. - bool CheckFile(const std::vector<const Target*>& targets, + bool CheckFile(const TargetVector& targets, const SourceFile& file, std::vector<Err>* errors) const; @@ -273,6 +277,7 @@ // the errors array. The range indicates the location of the // include in the file for error reporting. void CheckInclude(ReachabilityCache& from_target_cache, + bool is_public_header, const InputFile& source_file, const SourceFile& include_file, const LocationRange& range,
diff --git a/src/gn/header_checker_unittest.cc b/src/gn/header_checker_unittest.cc index 99a3606..173a0f7 100644 --- a/src/gn/header_checker_unittest.cc +++ b/src/gn/header_checker_unittest.cc
@@ -5,7 +5,6 @@ #include <ostream> #include <vector> -#include "gn/config.h" #include "gn/header_checker.h" #include "gn/scheduler.h" #include "gn/target.h" @@ -206,32 +205,32 @@ // dependency on D. std::vector<Err> errors; auto& a_cache = checker->GetReachabilityCacheForTarget(&a_); - checker->CheckInclude(a_cache, input_file, d_header, range, &errors); + checker->CheckInclude(a_cache, false, input_file, d_header, range, &errors); EXPECT_GT(errors.size(), 0); // A can include the public header in B. errors.clear(); - checker->CheckInclude(a_cache, input_file, b_public, range, &errors); + checker->CheckInclude(a_cache, false, input_file, b_public, range, &errors); EXPECT_EQ(errors.size(), 0); // Check A depending on the public and private headers in C. errors.clear(); - checker->CheckInclude(a_cache, input_file, c_public, range, &errors); + checker->CheckInclude(a_cache, false, input_file, c_public, range, &errors); EXPECT_EQ(errors.size(), 0); errors.clear(); - checker->CheckInclude(a_cache, input_file, c_private, range, &errors); + checker->CheckInclude(a_cache, false, input_file, c_private, range, &errors); EXPECT_GT(errors.size(), 0); // A can depend on a random file unknown to the build. errors.clear(); - checker->CheckInclude(a_cache, input_file, SourceFile("//random.h"), range, - &errors); + checker->CheckInclude(a_cache, false, input_file, SourceFile("//random.h"), + range, &errors); EXPECT_EQ(errors.size(), 0); // A can depend on a file present only in another toolchain even with no // dependency path. errors.clear(); - checker->CheckInclude(a_cache, input_file, otc_header, range, &errors); + checker->CheckInclude(a_cache, false, input_file, otc_header, range, &errors); EXPECT_EQ(errors.size(), 0); } @@ -298,7 +297,7 @@ // A depends on B. So B normally can't include headers from A. std::vector<Err> errors; - checker->CheckInclude(b_cache, input_file, a_public, range, &errors); + checker->CheckInclude(b_cache, false, input_file, a_public, range, &errors); EXPECT_GT(errors.size(), 0); // Add an allow_circular_includes_from on A that lists B. @@ -306,7 +305,7 @@ // Now the include from B to A should be allowed. errors.clear(); - checker->CheckInclude(b_cache, input_file, a_public, range, &errors); + checker->CheckInclude(b_cache, false, input_file, a_public, range, &errors); EXPECT_EQ(errors.size(), 0); } @@ -345,12 +344,14 @@ // Check that unrelated target D cannot include header generated by S. errors.clear(); - checker->CheckInclude(d_cache, input_file, generated_header, range, &errors); + checker->CheckInclude(d_cache, false, input_file, generated_header, range, + &errors); EXPECT_GT(errors.size(), 0); // Check that unrelated target D cannot include S's bridge header. errors.clear(); - checker->CheckInclude(d_cache, input_file, bridge_header, range, &errors); + checker->CheckInclude(d_cache, false, input_file, bridge_header, range, + &errors); EXPECT_GT(errors.size(), 0); } @@ -455,11 +456,137 @@ // B should not be allowed to include C's private header. std::vector<Err> errors; - checker->CheckInclude(b_cache, input_file, c_private, range, &errors); + checker->CheckInclude(b_cache, false, input_file, c_private, range, &errors); EXPECT_GT(errors.size(), 0); // A should be able to because of the friend declaration. errors.clear(); - checker->CheckInclude(a_cache, input_file, c_private, range, &errors); + checker->CheckInclude(a_cache, false, input_file, c_private, range, &errors); EXPECT_EQ(errors.size(), 0); } + +TEST_F(HeaderCheckerTest, CheckIncludesStrictTransitive) { + InputFile input_file(SourceFile("//some_file.cc")); + input_file.SetContents(std::string()); + LocationRange range; // Dummy value. + + SourceFile c_public("//c_public.h"); + c_.sources().push_back(c_public); + + // Enable strict check on B. + // This means A (which depends on B) can no longer transitively include C's + // headers. + b_.set_check_includes_strict(true); + + auto checker = CreateChecker(); + auto& a_cache = checker->GetReachabilityCacheForTarget(&a_); + + // A cannot include C's header because B blocks transitively forwarding it. + std::vector<Err> errors; + checker->CheckInclude(a_cache, false, input_file, c_public, range, &errors); + ASSERT_EQ(errors.size(), 1u); + EXPECT_TRUE( + errors[0].message().contains("Can't include this header from here.")); + EXPECT_TRUE(errors[0].help_text().contains("check_includes_strict = true")) + << "Actual help text:\n" + << errors[0].help_text(); + EXPECT_TRUE(errors[0].help_text().contains( + "Including headers from the public_deps of //b:b " + "is blocked because check_includes_strict = true is enabled " + "on it.")) + << "Actual help text:\n" + << errors[0].help_text(); + EXPECT_TRUE(errors[0].help_text().contains( + "check_includes_strict = true is highly recommended for all " + "new code to ensure a correct build graph.")) + << "Actual help text:\n" + << errors[0].help_text(); +} + +TEST_F(HeaderCheckerTest, CheckIncludesStrictPrivateInPublicHeader) { + InputFile input_file(SourceFile("//a_public.h")); + input_file.SetContents(std::string()); + LocationRange range; // Dummy value. + + // B is a private dependency of A. + a_.public_deps().clear(); + a_.private_deps().push_back(LabelTargetPair(&b_)); + Err err; + a_.OnResolved(&err); + ASSERT_SUCCESS(err); + + SourceFile b_public("//b_public.h"); + b_.sources().push_back(b_public); + + // Set strict headers on A. + a_.set_check_includes_strict(true); + + // We are checking a public header file of A. + // Since A depends on B privately, this include should be blocked under strict + // check. + auto checker = CreateChecker(); + auto& a_cache = checker->GetReachabilityCacheForTarget(&a_); + + std::vector<Err> errors; + // pass is_public_header = true + checker->CheckInclude(a_cache, true, input_file, b_public, range, &errors); + ASSERT_EQ(errors.size(), 1u); + EXPECT_TRUE(errors[0].message().contains( + "Public headers cannot include private dependencies.")) + << "Actual message:\n" + << errors[0].message(); + + // If we check a private file (is_public_header = false), it should pass. + errors.clear(); + checker->CheckInclude(a_cache, false, input_file, b_public, range, &errors); + EXPECT_TRUE(errors.empty()); +} + +TEST_F(HeaderCheckerTest, CheckIncludesStrictSameTargetPrivateHeader) { + InputFile input_file(SourceFile("//a_public.h")); + input_file.SetContents(std::string()); + LocationRange range; // Dummy value. + + // a_private.h is a private header of A. + SourceFile a_private("//a_private.h"); + a_.sources().push_back(a_private); + + // Set strict headers on A. + a_.set_check_includes_strict(true); + + // Set public list on A explicitly so that a_private.h becomes private. + SourceFile a_public("//a_public.h"); + a_.sources().push_back(a_public); + a_.public_headers().push_back(a_public); + a_.set_all_headers_public(false); + + // Re-resolve target. + Err err; + a_.OnResolved(&err); + ASSERT_SUCCESS(err); + + // Re-create checker because we updated the target definitions. + auto checker = CreateChecker(); + auto& a_cache = checker->GetReachabilityCacheForTarget(&a_); + + std::vector<Err> errors; + // A's public header includes A's private header. + // Pass is_public_header = true, it should fail. + checker->CheckInclude(a_cache, true, input_file, a_private, range, &errors); + ASSERT_EQ(errors.size(), 1u); + EXPECT_TRUE( + errors[0].message().contains("Public headers cannot include private " + "headers of the same target.")) + << "Actual message:\n" + << errors[0].message(); + EXPECT_TRUE(errors[0].help_text().contains( + "make a source_set containing public = [private_headers] and " + "add it to public_deps")) + << "Actual help text:\n" + << errors[0].help_text(); + + // If we check a private file (is_public_header = false), it should pass. + errors.clear(); + checker->CheckInclude(a_cache, false, input_file, a_private, range, &errors); + EXPECT_TRUE(errors.empty()); +}
diff --git a/src/gn/json_project_writer_unittest.cc b/src/gn/json_project_writer_unittest.cc index 88b2842..b56c6f5 100644 --- a/src/gn/json_project_writer_unittest.cc +++ b/src/gn/json_project_writer_unittest.cc
@@ -73,6 +73,7 @@ "targets": { "//foo:bar()": { "args": [ "{{response_file_name}}" ], + "check_includes_strict": false, "deps": [ ], "inputs": [ "//foo/input1.txt" ], "metadata": { @@ -311,6 +312,7 @@ "//foo:bar()": { "allow_circular_includes_from": [ ], "check_includes": true, + "check_includes_strict": false, "crate_name": "foo", "crate_root": "//foo/lib.rs", "deps": [ ], @@ -574,6 +576,7 @@ "targets": { "//foo:bar()": { "args": [ "{{source}}", "{{source_file_part}}", "{{response_file_name}}" ], + "check_includes_strict": false, "deps": [ ], "metadata": { @@ -863,6 +866,7 @@ }, "targets": { "//foo:docs()": { + "check_includes_strict": false, "data": [ "README.md", "docs/help.txt" ], "deps": [ ], "metadata": {
diff --git a/src/gn/target.h b/src/gn/target.h index 12cbef8..19972b3 100644 --- a/src/gn/target.h +++ b/src/gn/target.h
@@ -188,6 +188,10 @@ bool check_includes() const { return check_includes_; } void set_check_includes(bool ci) { check_includes_ = ci; } + // Whether this target enforces strict include checking. + bool check_includes_strict() const { return check_includes_strict_; } + void set_check_includes_strict(bool value) { check_includes_strict_ = value; } + // Whether this static_library target should have code linked in. bool complete_static_lib() const { return complete_static_lib_; } void set_complete_static_lib(bool complete) { @@ -565,6 +569,7 @@ bool all_headers_public_ = true; FileList public_headers_; bool check_includes_ = true; + bool check_includes_strict_ = false; bool complete_static_lib_ = false; std::vector<std::string> data_; std::unique_ptr<BundleData> bundle_data_;
diff --git a/src/gn/target_generator.cc b/src/gn/target_generator.cc index bde52e4..69cbaea 100644 --- a/src/gn/target_generator.cc +++ b/src/gn/target_generator.cc
@@ -69,6 +69,9 @@ if (!FillAssertNoDeps()) return; + if (!FillCheckIncludesStrict()) + return; + if (!Visibility::FillItemVisibility(target_, scope_, err_)) return; @@ -380,6 +383,16 @@ return true; } +bool TargetGenerator::FillCheckIncludesStrict() { + const Value* value = scope_->GetValue(variables::kCheckIncludesStrict, true); + if (!value) + return true; + if (!value->VerifyTypeIs(Value::BOOLEAN, err_)) + return false; + target_->set_check_includes_strict(value->boolean_value()); + return true; +} + bool TargetGenerator::FillOutputExtension() { const Value* value = scope_->GetValue(variables::kOutputExtension, true); if (!value)
diff --git a/src/gn/target_generator.h b/src/gn/target_generator.h index b34e43a..442e9a8 100644 --- a/src/gn/target_generator.h +++ b/src/gn/target_generator.h
@@ -71,6 +71,7 @@ bool FillMetadata(); bool FillTestonly(); bool FillAssertNoDeps(); + bool FillCheckIncludesStrict(); bool FillWriteRuntimeDeps(); // Reads configs/deps from the given var name, and uses the given setting on
diff --git a/src/gn/variables.cc b/src/gn/variables.cc index 9e99985..a1ec0ba 100644 --- a/src/gn/variables.cc +++ b/src/gn/variables.cc
@@ -905,6 +905,43 @@ } )"; +const char kCheckIncludesStrict[] = "check_includes_strict"; +const char kCheckIncludesStrict_HelpShort[] = + "check_includes_strict: [boolean] Controls whether strict include checking " + "is enforced."; +const char kCheckIncludesStrict_Help[] = + R"(check_includes_strict: [boolean] Enforce strict include checking. + + When true, the "gn check" command (as well as "gn gen" with the --check flag) + will enforce strict header checking rules on this target: + + 1. Public headers of this target cannot include headers from its private + "deps". + 2. Public headers of this target cannot include its own private headers + (headers in "sources" when "public" is explicitly specified). + 3. Dependents of this target cannot transitively include headers from this + target's "public_deps" (they must depend on those targets directly). + + Note that targets listed in "allow_circular_includes_from" act as an escape + hatch and bypass these strict checks. + + This enforces strict API boundary separation. Under this strict model: + * "public_deps" act as interface dependencies (dependencies required to + compile this target's public headers), but they are not transitively + forwarded to dependents. + * "deps" act as private implementation dependencies (dependencies required to + compile this target's sources). + + When false (the default), the default loose include checking rules apply. + +Example + + source_set("strict_target") { + check_includes_strict = true + ... + } +)"; + const char kCompleteStaticLib[] = "complete_static_lib"; const char kCompleteStaticLib_HelpShort[] = "complete_static_lib: [boolean] Links all deps into a static library."; @@ -2553,6 +2590,7 @@ INSERT_VARIABLE(CflagsObjC) INSERT_VARIABLE(CflagsObjCC) INSERT_VARIABLE(CheckIncludes) + INSERT_VARIABLE(CheckIncludesStrict) INSERT_VARIABLE(CompleteStaticLib) INSERT_VARIABLE(Configs) INSERT_VARIABLE(Data)
diff --git a/src/gn/variables.h b/src/gn/variables.h index 3ecb62a..7e39f47 100644 --- a/src/gn/variables.h +++ b/src/gn/variables.h
@@ -166,6 +166,10 @@ extern const char kCheckIncludes_HelpShort[]; extern const char kCheckIncludes_Help[]; +extern const char kCheckIncludesStrict[]; +extern const char kCheckIncludesStrict_HelpShort[]; +extern const char kCheckIncludesStrict_Help[]; + extern const char kCompleteStaticLib[]; extern const char kCompleteStaticLib_HelpShort[]; extern const char kCompleteStaticLib_Help[];