UsageAnalyzer: Collect identifiers found in attributes as seeds

This marks every identifier used within an attribute as seeds. The scenario
this avoids is functions referenced through attributes on unused tables or
record types (&default, &expire_func, ...) being dinged as unused as
that's rather confusing.

Also adds test for the above and a light smoke test into language/ as it
doesn't appear we had coverage here.

Closes #3122

src/script_opt/UsageAnalyzer.cc

@@ -98,8 +98,60 @@ UsageAnalyzer::UsageAnalyzer(std::vector<FuncInfo>& funcs)
 		}
 	}
 
+// Find all identifiers in attributes for seeding.
+//
+// General motivation: Dinging functions referenced from &default or &expire_func
+// on tables or &default on record types is confusing when really the containing
+// table/type is unused. Any IDs references from attributes are therefore
+// implicitly used as seeds.
+class AttrExprIdsCollector : public TraversalCallback
+	{
+public:
+	TraversalCode PreAttr(const Attr* attr) override
+		{
+		++attr_depth;
+		return TC_CONTINUE;
+		}
+
+	TraversalCode PostAttr(const Attr* attr) override
+		{
+		--attr_depth;
+		return TC_CONTINUE;
+		}
+
+	TraversalCode PreType(const Type* t) override
+		{
+		if ( analyzed_types.count(t) > 0 )
+			return TC_ABORTSTMT;
+
+		analyzed_types.insert(t);
+		return TC_CONTINUE;
+		}
+
+	TraversalCode PreID(const ID* id) override
+		{
+		if ( ids.count(id) > 0 )
+			return TC_ABORTSTMT;
+
+		if ( attr_depth > 0 )
+			ids.insert(id);
+
+		id->GetType()->Traverse(this);
+
+		if ( auto& attrs = id->GetAttrs() )
+			attrs->Traverse(this);
+
+		return TC_CONTINUE;
+		}
+
+	int attr_depth = 0; // Are we in an attribute?
+	std::set<const detail::ID*> ids; // List of IDs found in attributes.
+	std::set<const Type*> analyzed_types; // Endless recursion avoidance.
+	};
+
 void UsageAnalyzer::FindSeeds(IDSet& seeds) const
 	{
+	AttrExprIdsCollector attr_ids_collector;
 	for ( auto& gpair : global_scope()->Vars() )
 		{
 		auto& id = gpair.second;
@@ -125,7 +177,13 @@ void UsageAnalyzer::FindSeeds(IDSet& seeds) const
 		// use it.
 		if ( id->IsExport() || id->ModuleName() == "GLOBAL" )
 			seeds.insert(id.get());
+		else
+			// ...otherwise, find all IDs referenced from attribute expressions
+			// found through this identifier.
+			id->Traverse(&attr_ids_collector);
 		}
+
+	seeds.insert(attr_ids_collector.ids.begin(), attr_ids_collector.ids.end());
 	}
 
 const Func* UsageAnalyzer::GetFuncIfAny(const ID* id) const

testing/btest/Baseline/language.usage-analyzer-record-attributes/.stderr

@@ -0,0 +1 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.

testing/btest/Baseline/language.usage-analyzer-table-attributes/.stderr

@@ -0,0 +1 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.

testing/btest/Baseline/language.usage-analyzer/.stderr

@@ -0,0 +1,5 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+warning in <...>/usage-analyzer.zeek, line 11: handler for non-existing event cannot be invoked (MyModule::unused)
+warning in <...>/usage-analyzer.zeek, line 7: non-exported function does not have any callers (MyModule::gen_id)
+warning in <...>/usage-analyzer.zeek, line 8: non-exported function does not have any callers (MyModule::gen_id2)
+warning in <...>/usage-analyzer.zeek, line 10: non-exported function does not have any callers (MyModule::helper)

testing/btest/language/usage-analyzer-record-attributes.zeek

@@ -0,0 +1,19 @@
+# @TEST-DOC: Usage analyzer marked lambdas and functions in attribute expressions of unused tables or record types as unused. That is a bit confusing. Regression test for #3122.
+# @TEST-EXEC: zeek -b %INPUT
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr
+
+module MyModule;
+
+function gen_id(): string {
+	return cat(rand(10000));
+}
+
+type R1: record {
+	xxx_id: string &default=gen_id();
+};
+
+# Seems we can't actually put functions on &default on records, so the
+# following uses a directly invoked lambda instead.
+type R2: record {
+	xxx_id: string &default=(function(): string { return cat(rand(10000)); })();
+};

testing/btest/language/usage-analyzer-table-attributes.zeek

@@ -0,0 +1,44 @@
+# @TEST-DOC: Usage analyzer marked lambdas and functions in attribute expressions of unused tables or record types as unused. That is a bit confusing. Regression test for #3122.
+# @TEST-EXEC: zeek -b %INPUT
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr
+
+module MyModule;
+
+## Lambda on table.
+const ids1: table[count] of string = {
+        [1] = "One",
+} &default=function(c: count): string {
+	return fmt("unknown-%d", c);
+};
+
+
+## External default function
+function default_id(c: count): string {
+	return fmt("unknown-%d", c);
+}
+
+const ids2: table[count] of string = {
+        [1] = "One",
+} &default=default_id;
+
+
+## &default expression using function
+function default_id2(): string {
+	return "";
+}
+
+const ids3: table[count] of string = {
+        [1] = "One",
+} &default=default_id2() + "";
+
+
+## &expire_func lambda using another function
+function expire_f(t: table[count] of string, c: count): interval {
+	return 0.0sec;
+}
+
+const ids4: table[count] of string = {
+        [1] = "One",
+} &expire_func=function(t: table[count] of string, c: count): interval {
+	return expire_f(t, c);
+};

testing/btest/language/usage-analyzer.zeek

@@ -0,0 +1,11 @@
+# @TEST-DOC: Simple testing for unused function/event detection to ensure nothing breaks when modifying it.
+# @TEST-EXEC: zeek -b %INPUT
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr
+
+module MyModule;
+
+function gen_id(): string { return cat(rand(10000)); }
+function gen_id2(): string { return gen_id2(); }
+
+function helper() { }
+event MyModule::unused(c: count) { helper(); }