diff --git a/CHANGES b/CHANGES index f9b4f0fe54..65d0e96364 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,10 @@ +3.3.0-dev.488 | 2020-10-26 11:03:51 -0700 + + * 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. (Jon Siwek, Corelight) + 3.3.0-dev.486 | 2020-10-26 10:41:48 -0700 * GH-1245: require TLD of hostname literals to start with a letter (Jon Siwek, Corelight) diff --git a/VERSION b/VERSION index d882a2ad36..1c295388c6 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.3.0-dev.486 +3.3.0-dev.488 diff --git a/src/Type.cc b/src/Type.cc index 35b3cfdc94..30e05805e3 100644 --- a/src/Type.cc +++ b/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 { diff --git a/src/Type.h b/src/Type.h index 545fb54b3f..52dc2264a5 100644 --- a/src/Type.h +++ b/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 NameMap; NameMap names; diff --git a/src/parse.y b/src/parse.y index 56c63e69d3..83e57e8aa3 100644 --- a/src/parse.y +++ b/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 saved_in_init; @@ -838,7 +840,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 @@ -854,7 +857,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); } ; @@ -1066,7 +1070,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); } ; @@ -1145,18 +1150,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; diff --git a/src/zeekygen/IdentifierInfo.cc b/src/zeekygen/IdentifierInfo.cc index 5946432778..df088c2bc6 100644 --- a/src/zeekygen/IdentifierInfo.cc +++ b/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& comments) + vector& 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; diff --git a/src/zeekygen/IdentifierInfo.h b/src/zeekygen/IdentifierInfo.h index c04aa9f206..739fbc31f2 100644 --- a/src/zeekygen/IdentifierInfo.h +++ b/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& comments); + std::vector& 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 comments; + bool from_redef; }; typedef std::list 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 diff --git a/src/zeekygen/Manager.cc b/src/zeekygen/Manager.cc index 285edc8fb7..0a85f8cb3f 100644 --- a/src/zeekygen/Manager.cc +++ b/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()); diff --git a/src/zeekygen/Manager.h b/src/zeekygen/Manager.h index 7bdd0205d1..c0dbdd4e18 100644 --- a/src/zeekygen/Manager.h +++ b/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 comment_buffer_t; typedef std::map 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. diff --git a/src/zeekygen/ScriptInfo.cc b/src/zeekygen/ScriptInfo.cc index 06221734db..2e45df3abd 100644 --- a/src/zeekygen/ScriptInfo.cc +++ b/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 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 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) diff --git a/testing/btest/Baseline/doc.zeekygen.example/example.rst b/testing/btest/Baseline/doc.zeekygen.example/example.rst index 141a06cc2a..ca00c8e4a2 100644 --- a/testing/btest/Baseline/doc.zeekygen.example/example.rst +++ b/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 ######