Improve how Zeekygen generated record/enum redefinition docs

It now provides a summary of the new fields/enums added by any given
redefinition along with associated commentary.

src/Type.cc

@@ -1271,7 +1271,7 @@ EnumType::~EnumType() = default;
 // location in the error message, rather than the one where the type was
 // originally defined.
 void EnumType::AddName(const string& module_name, const char* name, bool is_export,
-                       detail::Expr* deprecation)
+                       detail::Expr* deprecation, bool from_redef)
 	{
 	/* implicit, auto-increment */
 	if ( counter < 0)
@@ -1280,12 +1280,12 @@ void EnumType::AddName(const string& module_name, const char* name, bool is_expo
 		SetError();
 		return;
 		}
-	CheckAndAddName(module_name, name, counter, is_export, deprecation);
+	CheckAndAddName(module_name, name, counter, is_export, deprecation, from_redef);
 	counter++;
 	}
 
 void EnumType::AddName(const string& module_name, const char* name, bro_int_t val,
-                       bool is_export, detail::Expr* deprecation)
+                       bool is_export, detail::Expr* deprecation, bool from_redef)
 	{
 	/* explicit value specified */
 	if ( counter > 0 )
@@ -1295,11 +1295,12 @@ void EnumType::AddName(const string& module_name, const char* name, bro_int_t va
 		return;
 		}
 	counter = -1;
-	CheckAndAddName(module_name, name, val, is_export, deprecation);
+	CheckAndAddName(module_name, name, val, is_export, deprecation, from_redef);
 	}
 
 void EnumType::CheckAndAddName(const string& module_name, const char* name,
-                               bro_int_t val, bool is_export, detail::Expr* deprecation)
+                               bro_int_t val, bool is_export, detail::Expr* deprecation,
+                               bool from_redef)
 	{
 	if ( Lookup(val) )
 		{
@@ -1320,7 +1321,7 @@ void EnumType::CheckAndAddName(const string& module_name, const char* name,
 		if ( deprecation )
 			id->MakeDeprecated({NewRef{}, deprecation});
 
-		detail::zeekygen_mgr->Identifier(std::move(id));
+		detail::zeekygen_mgr->Identifier(std::move(id), from_redef);
 		}
 	else
 		{

src/Type.h

@@ -693,12 +693,12 @@ public:
 
 	// The value of this name is next internal counter value, starting
 	// with zero. The internal counter is incremented.
-	void AddName(const std::string& module_name, const char* name, bool is_export, detail::Expr* deprecation = nullptr);
+	void AddName(const std::string& module_name, const char* name, bool is_export, detail::Expr* deprecation = nullptr, bool from_redef = false);
 
 	// The value of this name is set to val. Once a value has been
 	// explicitly assigned using this method, no further names can be
 	// added that aren't likewise explicitly initalized.
-	void AddName(const std::string& module_name, const char* name, bro_int_t val, bool is_export, detail::Expr* deprecation = nullptr);
+	void AddName(const std::string& module_name, const char* name, bro_int_t val, bool is_export, detail::Expr* deprecation = nullptr, bool from_redef = false);
 
 	// -1 indicates not found.
 	bro_int_t Lookup(const std::string& module_name, const char* name) const;
@@ -721,7 +721,8 @@ protected:
 
 	void CheckAndAddName(const std::string& module_name,
 	                     const char* name, bro_int_t val, bool is_export,
-	                     detail::Expr* deprecation = nullptr);
+	                     detail::Expr* deprecation = nullptr,
+	                     bool from_redef = false);
 
 	typedef std::map<std::string, bro_int_t> NameMap;
 	NameMap names;

src/parse.y

@@ -130,6 +130,8 @@ static int in_when_cond = 0;
 static int in_hook = 0;
 int in_init = 0;
 int in_record = 0;
+static int in_record_redef = 0;
+static int in_enum_redef = 0;
 bool resolving_global_ID = false;
 bool defining_global_ID = false;
 std::vector<int> saved_in_init;
@@ -842,7 +844,8 @@ enum_body_elem:
 				zeek::reporter->Error("enumerator is not a count constant");
 			else
 				cur_enum_type->AddName(zeek::detail::current_module, $1,
-				                       $3->InternalUnsigned(), is_export, $4);
+				                       $3->InternalUnsigned(), is_export, $4,
+				                       in_enum_redef != 0);
 			}
 
 	|	TOK_ID '=' '-' TOK_CONSTANT
@@ -858,7 +861,8 @@ enum_body_elem:
 			{
 			zeek::detail::set_location(@1);
 			assert(cur_enum_type);
-			cur_enum_type->AddName(zeek::detail::current_module, $1, is_export, $2);
+			cur_enum_type->AddName(zeek::detail::current_module, $1, is_export, $2,
+			                       in_enum_redef != 0);
 			}
 	;
 
@@ -1070,7 +1074,8 @@ type_decl:
 			$$ = new zeek::TypeDecl($1, {zeek::AdoptRef{}, $3}, std::move(attrs));
 
 			if ( in_record > 0 && cur_decl_type_id )
-				zeek::detail::zeekygen_mgr->RecordField(cur_decl_type_id, $$, ::filename);
+				zeek::detail::zeekygen_mgr->RecordField(cur_decl_type_id, $$, ::filename,
+				                                        in_record_redef != 0);
 			}
 	;
 
@@ -1149,18 +1154,19 @@ decl:
 			}
 
 	|	TOK_REDEF TOK_ENUM global_id TOK_ADD_TO '{'
-			{ parser_redef_enum($3); zeek::detail::zeekygen_mgr->Redef($3, ::filename); }
+			{ ++in_enum_redef; parser_redef_enum($3); zeek::detail::zeekygen_mgr->Redef($3, ::filename); }
 		enum_body '}' ';'
 			{
+			--in_enum_redef;
 			// Zeekygen already grabbed new enum IDs as the type created them.
 			}
 
 	|	TOK_REDEF TOK_RECORD global_id
 			{ cur_decl_type_id = $3; zeek::detail::zeekygen_mgr->Redef($3, ::filename); }
 		TOK_ADD_TO '{'
-			{ ++in_record; }
+			{ ++in_record; ++in_record_redef; }
 		type_decl_list
-			{ --in_record; }
+			{ --in_record; --in_record_redef; }
 		'}' opt_attr ';'
 			{
 			cur_decl_type_id = 0;

src/zeekygen/IdentifierInfo.cc

@@ -12,10 +12,11 @@ using namespace std;
 
 namespace zeek::zeekygen::detail {
 
-IdentifierInfo::IdentifierInfo(zeek::detail::IDPtr arg_id, ScriptInfo* script)
+IdentifierInfo::IdentifierInfo(zeek::detail::IDPtr arg_id, ScriptInfo* script,
+                               bool redef)
 	: Info(),
 	  comments(), id(std::move(arg_id)), initial_val(), redefs(), fields(),
-	  last_field_seen(), declaring_script(script)
+	  last_field_seen(), declaring_script(script), from_redef(redef)
 	{
 	if ( id->GetVal() && (id->IsOption() || id->IsRedefinable()) )
 		initial_val = id->GetVal()->Clone();
@@ -41,12 +42,14 @@ void IdentifierInfo::AddRedef(const string& script, zeek::detail::InitClass ic,
 
 void IdentifierInfo::AddRecordField(const TypeDecl* field,
                                     const string& script,
-                                    vector<string>& comments)
+                                    vector<string>& comments,
+                                    bool from_redef)
 	{
 	RecordField* rf = new RecordField();
 	rf->field = new TypeDecl(*field);
 	rf->from_script = script;
 	rf->comments = comments;
+	rf->from_redef = from_redef;
 
 	auto [it, inserted] = fields.emplace(rf->field->id, rf);
 
@@ -99,6 +102,16 @@ string IdentifierInfo::GetDeclaringScriptForField(const string& field) const
 	return it->second->from_script;
 	}
 
+bool IdentifierInfo::FieldIsFromRedef(const std::string& field) const
+	{
+	auto it = fields.find(field);
+
+	if ( it == fields.end() )
+		return false;
+
+	return it->second->from_redef;
+	}
+
 string IdentifierInfo::DoReStructuredText(bool roles_only) const
 	{
 	ODesc d;

src/zeekygen/IdentifierInfo.h

@@ -31,8 +31,11 @@ public:
 	 * @param id The script-level identifier.
 	 * @param script The info object associated with the script in which \a id
 	 * is declared.
+	 * @param from_redef  Whether the identifier was create as part of a
+	 * redefinition (e.g. an enum).
 	 */
-	IdentifierInfo(zeek::detail::IDPtr id, ScriptInfo* script);
+	IdentifierInfo(zeek::detail::IDPtr id, ScriptInfo* script,
+	               bool from_redef = false);
 
 	/**
 	 * Dtor.  Releases any references to script-level objects.
@@ -81,9 +84,10 @@ public:
 	 * @param script The script in which the field was declared.  This may
 	 * differ from the script in which a record type is declared due to redefs.
 	 * @param comments Comments associated with the record field.
+	 * @param from_redef  The field is from a record redefinition.
 	 */
 	void AddRecordField(const TypeDecl* field, const std::string& script,
-	                    std::vector<std::string>& comments);
+	                    std::vector<std::string>& comments, bool from_redef);
 
 	/**
 	 * Signals that a record type has been completely parsed.  This resets
@@ -111,6 +115,19 @@ public:
 	 */
 	std::string GetDeclaringScriptForField(const std::string& field) const;
 
+	/**
+	 * @return True if the identifier was created as part of a redefinition
+	 * (e.g. an enum).
+	 */
+	bool IsFromRedef() const
+		{ return from_redef; }
+
+	/**
+	 * @param field A record field name.
+	 * @return True if the field name was added to a record as part of a redef.
+	 */
+	bool FieldIsFromRedef(const std::string& field) const;
+
 	/**
 	 * @return All Zeekygen comments associated with the identifier.
 	 */
@@ -168,6 +185,7 @@ private:
 		TypeDecl* field;
 		std::string from_script;
 		std::vector<std::string> comments;
+		bool from_redef;
 	};
 
 	typedef std::list<Redefinition*> redef_list;
@@ -180,6 +198,7 @@ private:
 	record_field_map fields;
 	RecordField* last_field_seen;
 	ScriptInfo* declaring_script;
+	bool from_redef = false;
 };
 
 } // namespace zeek::zeekygen::detail

src/zeekygen/Manager.cc

@@ -219,11 +219,12 @@ void Manager::ModuleUsage(const string& path, const string& module)
 	        module.c_str(), name.c_str());
 	}
 
-IdentifierInfo* Manager::CreateIdentifierInfo(zeek::detail::IDPtr id, ScriptInfo* script)
+IdentifierInfo* Manager::CreateIdentifierInfo(zeek::detail::IDPtr id, ScriptInfo* script, bool from_redef)
 	{
 	const auto& id_name = id->Name();
 	auto prev = identifiers.GetInfo(id_name);
-	IdentifierInfo* rval = prev ? prev : new IdentifierInfo(std::move(id), script);
+	IdentifierInfo* rval = prev ? prev : new IdentifierInfo(std::move(id), script,
+	                                                        from_redef);
 
 	rval->AddComments(comment_buffer);
 	comment_buffer.clear();
@@ -281,7 +282,7 @@ static bool IsEnumType(zeek::detail::ID* id)
 	return id->IsType() ? id->GetType()->Tag() == TYPE_ENUM : false;
 	}
 
-void Manager::Identifier(zeek::detail::IDPtr id)
+void Manager::Identifier(zeek::detail::IDPtr id, bool from_redef)
 	{
 	if ( disabled )
 		return;
@@ -322,7 +323,7 @@ void Manager::Identifier(zeek::detail::IDPtr id)
 		// Handled specially since they don't have a script location.
 		DBG_LOG(DBG_ZEEKYGEN, "Made internal IdentifierInfo %s",
 		        id->Name());
-		CreateIdentifierInfo(id, nullptr);
+		CreateIdentifierInfo(id, nullptr, from_redef);
 		return;
 		}
 
@@ -337,11 +338,11 @@ void Manager::Identifier(zeek::detail::IDPtr id)
 
 	DBG_LOG(DBG_ZEEKYGEN, "Making IdentifierInfo %s, in script %s",
 	        id->Name(), script.c_str());
-	CreateIdentifierInfo(std::move(id), script_info);
+	CreateIdentifierInfo(std::move(id), script_info, from_redef);
 	}
 
 void Manager::RecordField(const zeek::detail::ID* id, const TypeDecl* field,
-                          const string& path)
+                          const string& path, bool from_redef)
 	{
 	if ( disabled )
 		return;
@@ -357,7 +358,7 @@ void Manager::RecordField(const zeek::detail::ID* id, const TypeDecl* field,
 		}
 
 	string script = NormalizeScriptPath(path);
-	idd->AddRecordField(field, script, comment_buffer);
+	idd->AddRecordField(field, script, comment_buffer, from_redef);
 	comment_buffer.clear();
 	DBG_LOG(DBG_ZEEKYGEN, "Document record field %s, identifier %s, script %s",
 	        field->id, id->Name(), script.c_str());

src/zeekygen/Manager.h

@@ -115,8 +115,9 @@ public:
 	 * Register a script-level identifier for which information/documentation
 	 * will be gathered.
 	 * @param id The script-level identifier.
+	 * @param from_redef  The identifier was created from a redef (e.g. an enum).
 	 */
-	void Identifier(zeek::detail::IDPtr id);
+	void Identifier(zeek::detail::IDPtr id, bool from_redef = false);
 
 	/**
 	 * Register a record-field for which information/documentation will be
@@ -126,9 +127,10 @@ public:
 	 * @param path Absolute path to a Bro script in which this field is
 	 * declared.  This can be different from the place where the record type
 	 * is declared due to redefs.
+	 * @param from_redef  The field is from a record redefinition.
 	 */
 	void RecordField(const zeek::detail::ID* id, const TypeDecl* field,
-	                 const std::string& path);
+	                 const std::string& path, bool from_redef);
 
 	/**
 	 * Register a redefinition of a particular identifier.
@@ -216,7 +218,8 @@ private:
 	typedef std::vector<std::string> comment_buffer_t;
 	typedef std::map<std::string, comment_buffer_t> comment_buffer_map_t;
 
-	IdentifierInfo* CreateIdentifierInfo(zeek::detail::IDPtr id, ScriptInfo* script);
+	IdentifierInfo* CreateIdentifierInfo(zeek::detail::IDPtr id, ScriptInfo* script,
+	                                     bool from_redef = false);
 
 	bool disabled;
 	comment_buffer_t comment_buffer; // For whatever next identifier comes in.

src/zeekygen/ScriptInfo.cc

@@ -116,7 +116,106 @@ static string make_redef_summary(const string& heading, char underline,
 
 		for ( redef_list::const_iterator iit = redefs.begin();
 		      iit != redefs.end(); ++iit )
+			{
 			add_summary_rows(d, summary_comment(iit->comments), &table);
+
+			if ( ! id->IsType() )
+				continue;
+
+			if ( id->GetType()->Tag() == TYPE_ENUM )
+				{
+				for ( const auto& [enum_name, v] : id->GetType()->AsEnumType()->Names() )
+					{
+					auto info = zeek::detail::zeekygen_mgr->GetIdentifierInfo(enum_name);
+
+					if ( ! info )
+						continue;
+
+					if ( ! info->IsFromRedef() )
+						continue;
+
+					if ( ! info->GetDeclaringScript() )
+						continue;
+
+					if ( info->GetDeclaringScript()->Name() != from_script )
+						continue;
+
+					vector<string> row;
+					row.emplace_back("");
+					row.emplace_back("");
+					table.AddRow(row);
+
+					auto comments = info->GetComments();
+					auto summary_comments = summary_comment(comments);
+					auto enum_id = info->GetID();
+
+					auto colon = summary_comments.empty() ? "" : ":";
+					row[1] = util::fmt("* :zeek:enum:`%s`%s", enum_id->Name(), colon);
+					table.AddRow(row);
+
+					for ( auto& sc : summary_comments )
+						{
+						row[1] = util::fmt("  %s", sc.data());
+						table.AddRow(row);
+						}
+					}
+				}
+				else if ( id->GetType()->Tag() == TYPE_RECORD )
+					{
+					auto info = zeek::detail::zeekygen_mgr->GetIdentifierInfo(id->Name());
+
+					if ( ! info || ! info->GetDeclaringScript() )
+						continue;
+
+					auto rt = id->GetType()->AsRecordType();
+					bool added_new_field_docs = false;
+
+					for ( auto i = 0; i < rt->NumFields(); ++i )
+						{
+						auto field_name = rt->FieldName(i);
+
+						if ( ! info->FieldIsFromRedef(field_name) )
+							continue;
+
+						auto declaring_script = info->GetDeclaringScriptForField(field_name);
+
+						if ( declaring_script != from_script )
+							continue;
+
+						vector<string> row;
+						row.emplace_back("");
+						row.emplace_back("");
+						table.AddRow(row);
+
+						if ( ! added_new_field_docs )
+							{
+							added_new_field_docs = true;
+							row[1] = util::fmt(":New Fields: :zeek:type:`%s`", id->Name());
+							table.AddRow(row);
+							row[1] = "";
+							table.AddRow(row);
+							}
+
+						auto td = rt->FieldDecl(i);
+
+						ODesc fd;
+						fd.SetQuotes(true);
+						td->DescribeReST(&fd, true);
+
+						row[1] = util::fmt("  %s", fd.Description());
+						table.AddRow(row);
+
+						auto comments = info->GetFieldComments(field_name);
+						auto summary_comments = summary_comment(comments);
+
+						for ( auto& sc : summary_comments )
+							{
+							row[1] = util::fmt("    %s", sc.data());
+							table.AddRow(row);
+							}
+						}
+					}
+			}
 		}
 
 	return make_heading(heading, underline) + table.AsString(border)

testing/btest/Baseline/doc.zeekygen.example/example.rst

@@ -57,13 +57,40 @@ Types
 
 Redefinitions
 #############
-=============================================================== ====================================================================
+=============================================================== =====================================================================
 :zeek:type:`Log::ID`: :zeek:type:`enum`                         
+                                                                
+                                                                * :zeek:enum:`ZeekygenExample::LOG`
 :zeek:type:`Notice::Type`: :zeek:type:`enum`                    
+                                                                
+                                                                * :zeek:enum:`ZeekygenExample::Zeekygen_Four`:
+                                                                  Omitting comments is fine, and so is mixing ``##`` and ``##<``, but
+                                                                  it's probably best to use only one style consistently.
+                                                                
+                                                                * :zeek:enum:`ZeekygenExample::Zeekygen_One`:
+                                                                  Any number of this type of comment
+                                                                  will document "Zeekygen_One".
+                                                                
+                                                                * :zeek:enum:`ZeekygenExample::Zeekygen_Three`
+                                                                
+                                                                * :zeek:enum:`ZeekygenExample::Zeekygen_Two`:
+                                                                  Any number of this type of comment
+                                                                  will document "ZEEKYGEN_TWO".
 :zeek:type:`ZeekygenExample::SimpleEnum`: :zeek:type:`enum`     Document the "SimpleEnum" redef here with any special info regarding
                                                                 the *redef* itself.
+                                                                
+                                                                * :zeek:enum:`ZeekygenExample::FIVE`:
+                                                                  Also "FIVE".
+                                                                
+                                                                * :zeek:enum:`ZeekygenExample::FOUR`:
+                                                                  And some documentation for "FOUR".
 :zeek:type:`ZeekygenExample::SimpleRecord`: :zeek:type:`record` Document the record extension *redef* itself here.
-=============================================================== ====================================================================
+                                                                
+                                                                :New Fields: :zeek:type:`ZeekygenExample::SimpleRecord`
+                                                                
+                                                                  field_ext: :zeek:type:`string` :zeek:attr:`&optional`
+                                                                    Document the extending field like this.
+=============================================================== =====================================================================
 
 Events
 ######