public:
HeaderCheckerTest()
: a_(setup_.settings(), Label(SourceDir("//a/"), "a")),
- b_(setup_.settings(), Label(SourceDir("//b/"), "a")),
+ b_(setup_.settings(), Label(SourceDir("//b/"), "b")),
c_(setup_.settings(), Label(SourceDir("//c/"), "c")),
d_(setup_.settings(), Label(SourceDir("//d/"), "d")) {
- a_.deps().push_back(LabelTargetPair(&b_));
- b_.deps().push_back(LabelTargetPair(&c_));
+ a_.set_output_type(Target::SOURCE_SET);
+ b_.set_output_type(Target::SOURCE_SET);
+ c_.set_output_type(Target::SOURCE_SET);
+ d_.set_output_type(Target::SOURCE_SET);
+
+ Err err;
+ a_.SetToolchain(setup_.toolchain(), &err);
+ b_.SetToolchain(setup_.toolchain(), &err);
+ c_.SetToolchain(setup_.toolchain(), &err);
+ d_.SetToolchain(setup_.toolchain(), &err);
+
+ a_.public_deps().push_back(LabelTargetPair(&b_));
+ b_.public_deps().push_back(LabelTargetPair(&c_));
// Start with all public visibility.
a_.visibility().SetPublic();
c_.visibility().SetPublic();
d_.visibility().SetPublic();
+ d_.OnResolved(&err);
+ c_.OnResolved(&err);
+ b_.OnResolved(&err);
+ a_.OnResolved(&err);
+
targets_.push_back(&a_);
targets_.push_back(&b_);
targets_.push_back(&c_);
TestWithScope setup_;
- // Some headers that are automatically set up with a dependency chain.
- // a -> b -> c
+ // Some headers that are automatically set up with a public dependency chain.
+ // a -> b -> c. D is unconnected.
Target a_;
Target b_;
Target c_;
scoped_refptr<HeaderChecker> checker(
new HeaderChecker(setup_.build_settings(), targets_));
- std::vector<const Target*> chain;
- EXPECT_FALSE(checker->IsDependencyOf(&a_, &a_, false, &chain, NULL));
+ // Add a target P ("private") that privately depends on C, and hook up the
+ // chain so that A -> P -> C. A will depend on C via two different paths.
+ Err err;
+ Target p(setup_.settings(), Label(SourceDir("//p/"), "p"));
+ p.set_output_type(Target::SOURCE_SET);
+ p.SetToolchain(setup_.toolchain(), &err);
+ EXPECT_FALSE(err.has_error());
+ p.private_deps().push_back(LabelTargetPair(&c_));
+ p.visibility().SetPublic();
+ p.OnResolved(&err);
+
+ a_.public_deps().push_back(LabelTargetPair(&p));
+ // A does not depend on itself.
+ bool is_permitted = false;
+ HeaderChecker::Chain chain;
+ EXPECT_FALSE(checker->IsDependencyOf(&a_, &a_, &chain, &is_permitted));
+
+ // A depends publicly on B.
chain.clear();
- EXPECT_TRUE(checker->IsDependencyOf(&b_, &a_, false, &chain, NULL));
+ is_permitted = false;
+ EXPECT_TRUE(checker->IsDependencyOf(&b_, &a_, &chain, &is_permitted));
ASSERT_EQ(2u, chain.size());
- EXPECT_EQ(&b_, chain[0]);
- EXPECT_EQ(&a_, chain[1]);
+ EXPECT_EQ(HeaderChecker::ChainLink(&b_, true), chain[0]);
+ EXPECT_EQ(HeaderChecker::ChainLink(&a_, true), chain[1]);
+ EXPECT_TRUE(is_permitted);
+ // A indirectly depends on C. The "public" dependency path through B should
+ // be identified.
chain.clear();
- EXPECT_TRUE(checker->IsDependencyOf(&c_, &a_, false, &chain, NULL));
+ is_permitted = false;
+ EXPECT_TRUE(checker->IsDependencyOf(&c_, &a_, &chain, &is_permitted));
ASSERT_EQ(3u, chain.size());
- EXPECT_EQ(&c_, chain[0]);
- EXPECT_EQ(&b_, chain[1]);
- EXPECT_EQ(&a_, chain[2]);
+ EXPECT_EQ(HeaderChecker::ChainLink(&c_, true), chain[0]);
+ EXPECT_EQ(HeaderChecker::ChainLink(&b_, true), chain[1]);
+ EXPECT_EQ(HeaderChecker::ChainLink(&a_, true), chain[2]);
+ EXPECT_TRUE(is_permitted);
+ // C does not depend on A.
chain.clear();
- EXPECT_FALSE(checker->IsDependencyOf(&a_, &c_, false, &chain, NULL));
+ is_permitted = false;
+ EXPECT_FALSE(checker->IsDependencyOf(&a_, &c_, &chain, &is_permitted));
EXPECT_TRUE(chain.empty());
+ EXPECT_FALSE(is_permitted);
- // If an a -> c dependency exists, this should be chosen for the chain.
+ // Remove the B -> C public dependency, leaving P's private dep on C the only
+ // path from A to C. This should now be found.
chain.clear();
- a_.deps().push_back(LabelTargetPair(&c_));
- EXPECT_TRUE(checker->IsDependencyOf(&c_, &a_, false, &chain, NULL));
- EXPECT_EQ(&c_, chain[0]);
- EXPECT_EQ(&a_, chain[1]);
-}
-
-TEST_F(HeaderCheckerTest, IsDependencyOf_ForwardsDirectDependentConfigs) {
- scoped_refptr<HeaderChecker> checker(
- new HeaderChecker(setup_.build_settings(), targets_));
-
- // The a -> b -> c chain is found, since no chains that forward direct-
- // dependent configs exist.
- std::vector<const Target*> chain;
- bool direct_dependent_configs_apply = false;
- EXPECT_TRUE(checker->IsDependencyOf(
- &c_, &a_, true, &chain, &direct_dependent_configs_apply));
- EXPECT_FALSE(direct_dependent_configs_apply);
+ EXPECT_EQ(&c_, b_.public_deps()[0].ptr); // Validate it's the right one.
+ b_.public_deps().erase(b_.public_deps().begin());
+ EXPECT_TRUE(checker->IsDependencyOf(&c_, &a_, &chain, &is_permitted));
EXPECT_EQ(3u, chain.size());
- EXPECT_EQ(&c_, chain[0]);
- EXPECT_EQ(&b_, chain[1]);
- EXPECT_EQ(&a_, chain[2]);
+ EXPECT_EQ(HeaderChecker::ChainLink(&c_, false), chain[0]);
+ EXPECT_EQ(HeaderChecker::ChainLink(&p, true), chain[1]);
+ EXPECT_EQ(HeaderChecker::ChainLink(&a_, true), chain[2]);
+ EXPECT_FALSE(is_permitted);
- // Create a chain a -> d -> c where d forwards direct-dependent configs.
- // This path should be preferred when dependency chains which forward
- // direct-dependent configs are preferred.
+ // P privately depends on C. That dependency should be OK since it's only
+ // one hop.
chain.clear();
- direct_dependent_configs_apply = false;
- d_.deps().push_back(LabelTargetPair(&c_));
- d_.forward_dependent_configs().push_back(LabelTargetPair(&c_));
- a_.deps().push_back(LabelTargetPair(&d_));
- EXPECT_TRUE(checker->IsDependencyOf(
- &c_, &a_, true, &chain, &direct_dependent_configs_apply));
- EXPECT_TRUE(direct_dependent_configs_apply);
- EXPECT_EQ(3u, chain.size());
- EXPECT_EQ(&c_, chain[0]);
- EXPECT_EQ(&d_, chain[1]);
- EXPECT_EQ(&a_, chain[2]);
-
- // d also forwards direct-dependent configs if it is a group.
- chain.clear();
- direct_dependent_configs_apply = false;
- d_.set_output_type(Target::GROUP);
- d_.forward_dependent_configs().clear();
- EXPECT_TRUE(checker->IsDependencyOf(
- &c_, &a_, true, &chain, &direct_dependent_configs_apply));
- EXPECT_TRUE(direct_dependent_configs_apply);
- EXPECT_EQ(3u, chain.size());
- EXPECT_EQ(&c_, chain[0]);
- EXPECT_EQ(&d_, chain[1]);
- EXPECT_EQ(&a_, chain[2]);
-
- // A direct dependency a -> c carries direct-dependent configs.
- chain.clear();
- direct_dependent_configs_apply = false;
- a_.deps().push_back(LabelTargetPair(&c_));
- EXPECT_TRUE(checker->IsDependencyOf(
- &c_, &a_, true, &chain, &direct_dependent_configs_apply));
- EXPECT_TRUE(direct_dependent_configs_apply);
- EXPECT_EQ(2u, chain.size());
- EXPECT_EQ(&c_, chain[0]);
- EXPECT_EQ(&a_, chain[1]);
+ is_permitted = false;
+ EXPECT_TRUE(checker->IsDependencyOf(&c_, &p, &chain, &is_permitted));
+ ASSERT_EQ(2u, chain.size());
+ EXPECT_EQ(HeaderChecker::ChainLink(&c_, false), chain[0]);
+ EXPECT_EQ(HeaderChecker::ChainLink(&p, true), chain[1]);
+ EXPECT_TRUE(is_permitted);
}
TEST_F(HeaderCheckerTest, CheckInclude) {
c_.public_headers().push_back(c_public);
c_.set_all_headers_public(false);
- targets_.push_back(&d_);
scoped_refptr<HeaderChecker> checker(
new HeaderChecker(setup_.build_settings(), targets_));
EXPECT_TRUE(checker->CheckInclude(&a_, input_file, SourceFile("//random.h"),
range, &err));
EXPECT_FALSE(err.has_error());
+}
- // If C is not visible from A, A can't include public headers even if there
- // is a dependency path.
- c_.visibility().SetPrivate(c_.label().dir());
- err = Err();
- EXPECT_FALSE(checker->CheckInclude(&a_, input_file, c_public, range, &err));
- EXPECT_TRUE(err.has_error());
- c_.visibility().SetPublic();
-
- // If C has direct-dependent configs, then B must forward them to A.
- // If B is a group, that suffices to forward direct-dependent configs.
- {
- Config direct(setup_.settings(), Label(SourceDir("//c/"), "config"));
- direct.config_values().cflags().push_back("-DSOME_DEFINE");
-
- c_.direct_dependent_configs().push_back(LabelConfigPair(&direct));
- err = Err();
- EXPECT_FALSE(checker->CheckInclude(&a_, input_file, c_public, range, &err));
- EXPECT_TRUE(err.has_error());
-
- b_.forward_dependent_configs().push_back(LabelTargetPair(&c_));
- err = Err();
- EXPECT_TRUE(checker->CheckInclude(&a_, input_file, c_public, range, &err));
- EXPECT_FALSE(err.has_error());
-
- b_.forward_dependent_configs().clear();
- b_.set_output_type(Target::GROUP);
- err = Err();
- EXPECT_TRUE(checker->CheckInclude(&a_, input_file, c_public, range, &err));
- EXPECT_FALSE(err.has_error());
-
- b_.set_output_type(Target::UNKNOWN);
- c_.direct_dependent_configs().clear();
- }
+// A public chain of dependencies should always be identified first, even if
+// it is longer than a private one.
+TEST_F(HeaderCheckerTest, PublicFirst) {
+ // Now make a A -> Z -> D private dependency chain (one shorter than the
+ // public one to get to D).
+ Target z(setup_.settings(), Label(SourceDir("//a/"), "a"));
+ z.set_output_type(Target::SOURCE_SET);
+ Err err;
+ EXPECT_TRUE(z.SetToolchain(setup_.toolchain(), &err));
+ z.private_deps().push_back(LabelTargetPair(&d_));
+ EXPECT_TRUE(z.OnResolved(&err));
+ targets_.push_back(&z);
+
+ a_.private_deps().push_back(LabelTargetPair(&z));
+
+ // Check that D can be found from A, but since it's private, it will be
+ // marked as not permitted.
+ bool is_permitted = false;
+ HeaderChecker::Chain chain;
+ scoped_refptr<HeaderChecker> checker(
+ new HeaderChecker(setup_.build_settings(), targets_));
+ EXPECT_TRUE(checker->IsDependencyOf(&d_, &a_, &chain, &is_permitted));
+
+ EXPECT_FALSE(is_permitted);
+ ASSERT_EQ(3u, chain.size());
+ EXPECT_EQ(HeaderChecker::ChainLink(&d_, false), chain[0]);
+ EXPECT_EQ(HeaderChecker::ChainLink(&z, false), chain[1]);
+ EXPECT_EQ(HeaderChecker::ChainLink(&a_, true), chain[2]);
+
+ // Hook up D to the existing public A -> B -> C chain to make a long one, and
+ // search for D again.
+ c_.public_deps().push_back(LabelTargetPair(&d_));
+ checker = new HeaderChecker(setup_.build_settings(), targets_);
+ chain.clear();
+ EXPECT_TRUE(checker->IsDependencyOf(&d_, &a_, &chain, &is_permitted));
+
+ // This should have found the long public one.
+ EXPECT_TRUE(is_permitted);
+ ASSERT_EQ(4u, chain.size());
+ EXPECT_EQ(HeaderChecker::ChainLink(&d_, true), chain[0]);
+ EXPECT_EQ(HeaderChecker::ChainLink(&c_, true), chain[1]);
+ EXPECT_EQ(HeaderChecker::ChainLink(&b_, true), chain[2]);
+ EXPECT_EQ(HeaderChecker::ChainLink(&a_, true), chain[3]);
}
-TEST_F(HeaderCheckerTest, GetDependentConfigChainProblemIndex) {
- // Assume we have a chain A -> B -> C -> D.
- Target target_a(setup_.settings(), Label(SourceDir("//a/"), "a"));
- Target target_b(setup_.settings(), Label(SourceDir("//b/"), "b"));
- Target target_c(setup_.settings(), Label(SourceDir("//c/"), "c"));
- Target target_d(setup_.settings(), Label(SourceDir("//d/"), "d"));
-
- // C is a group, and B forwards deps from C, so A should get configs from D.
- target_a.set_output_type(Target::SOURCE_SET);
- target_b.set_output_type(Target::SOURCE_SET);
- target_c.set_output_type(Target::GROUP);
- target_d.set_output_type(Target::SOURCE_SET);
- target_b.forward_dependent_configs().push_back(
- LabelTargetPair(&target_c));
-
- // Dependency chain goes from bottom to top.
- std::vector<const Target*> chain;
- chain.push_back(&target_d);
- chain.push_back(&target_c);
- chain.push_back(&target_b);
- chain.push_back(&target_a);
-
- // If C is not a group, it shouldn't work anymore.
- target_c.set_output_type(Target::SOURCE_SET);
- EXPECT_EQ(1u, HeaderChecker::GetDependentConfigChainProblemIndex(chain));
-
- // Or if B stops forwarding from C, it shouldn't work anymore.
- target_c.set_output_type(Target::GROUP);
- target_b.forward_dependent_configs().clear();
- EXPECT_EQ(2u, HeaderChecker::GetDependentConfigChainProblemIndex(chain));
+// Checks that the allow_circular_includes_from list works.
+TEST_F(HeaderCheckerTest, CheckIncludeAllowCircular) {
+ InputFile input_file(SourceFile("//some_file.cc"));
+ input_file.SetContents(std::string());
+ LocationRange range; // Dummy value.
+
+ // Add an include file to A.
+ SourceFile a_public("//a_public.h");
+ a_.sources().push_back(a_public);
+
+ scoped_refptr<HeaderChecker> checker(
+ new HeaderChecker(setup_.build_settings(), targets_));
+
+ // A depends on B. So B normally can't include headers from A.
+ Err err;
+ EXPECT_FALSE(checker->CheckInclude(&b_, input_file, a_public, range, &err));
+ EXPECT_TRUE(err.has_error());
+
+ // Add an allow_circular_includes_from on A that lists B.
+ a_.allow_circular_includes_from().insert(b_.label());
+
+ // Now the include from B to A should be allowed.
+ err = Err();
+ EXPECT_TRUE(checker->CheckInclude(&b_, input_file, a_public, range, &err));
+ EXPECT_FALSE(err.has_error());
}