diff --git a/include/mrdocs/Corpus.hpp b/include/mrdocs/Corpus.hpp index 39ba2bbb43..6dc05db34a 100644 --- a/include/mrdocs/Corpus.hpp +++ b/include/mrdocs/Corpus.hpp @@ -90,12 +90,12 @@ class MRDOCS_VISIBLE /** Return the Info for the matching string in a given context. */ virtual - Info const* + Expected lookup(SymbolID const& context, std::string_view name) const = 0; /** Return the Info for the matching string in the global context. */ - Info const* + Expected lookup(std::string_view name) const { return lookup(SymbolID::global, name); diff --git a/include/mrdocs/Metadata/Info.hpp b/include/mrdocs/Metadata/Info.hpp index 85dc1a172a..5964d997c2 100644 --- a/include/mrdocs/Metadata/Info.hpp +++ b/include/mrdocs/Metadata/Info.hpp @@ -355,6 +355,15 @@ tag_invoke( }); } +inline +OptionalLocation +getPrimaryLocation(Info const& I) +{ + return getPrimaryLocation( + dynamic_cast(I), + I.isRecord() || I.isEnum()); +} + } // clang::mrdocs #endif diff --git a/include/mrdocs/Metadata/Source.hpp b/include/mrdocs/Metadata/Source.hpp index d4d669b27c..8cfab3cb77 100644 --- a/include/mrdocs/Metadata/Source.hpp +++ b/include/mrdocs/Metadata/Source.hpp @@ -144,7 +144,7 @@ merge(SourceInfo& I, SourceInfo&& Other); MRDOCS_DECL OptionalLocation -getPrimaryLocation(SourceInfo const& I); +getPrimaryLocation(SourceInfo const& I, bool preferDefinition); void tag_invoke( diff --git a/src/lib/AST/ASTVisitor.cpp b/src/lib/AST/ASTVisitor.cpp index 26b39b0d6f..a5d81d236a 100644 --- a/src/lib/AST/ASTVisitor.cpp +++ b/src/lib/AST/ASTVisitor.cpp @@ -531,10 +531,24 @@ void ASTVisitor:: populate(Info& I, bool const isNew, DeclTy const* D) { - // Populate the documentation - bool const isDocumented = generateJavadoc(I.javadoc, D); + populate(I.javadoc, D); + populate(dynamic_cast(I), D); - // Populate the source info + // All other information is redundant if the symbol is not new + MRDOCS_CHECK_OR(isNew); + + // These should already have been populated by traverseImpl + MRDOCS_ASSERT(I.id); + MRDOCS_ASSERT(I.Kind != InfoKind::None); + + I.Name = extractName(D); +} + +template DeclTy> +void +ASTVisitor:: +populate(SourceInfo& I, DeclTy const* D) +{ clang::SourceLocation Loc = D->getBeginLoc(); if (Loc.isInvalid()) { @@ -544,17 +558,26 @@ populate(Info& I, bool const isNew, DeclTy const* D) { populate( dynamic_cast(I), - Loc, isDefinition(D), isDocumented); + Loc, + isDefinition(D), + D->getASTContext().getRawCommentForDeclNoCache(D)); } +} - // All other information is redundant if the symbol is not new - MRDOCS_CHECK_OR(isNew); - - // These should already have been populated by traverseImpl - MRDOCS_ASSERT(I.id); - MRDOCS_ASSERT(I.Kind != InfoKind::None); - - I.Name = extractName(D); +bool +ASTVisitor:: +populate( + std::optional& javadoc, + Decl const* D) +{ + RawComment const* RC = + D->getASTContext().getRawCommentForDeclNoCache(D); + MRDOCS_CHECK_OR(RC, false); + comments::FullComment* FC = + RC->parse(D->getASTContext(), &sema_.getPreprocessor(), D); + MRDOCS_CHECK_OR(FC, false); + parseJavadoc(javadoc, FC, D, config_, diags_); + return true; } void @@ -686,6 +709,13 @@ populate(RecordInfo& I, ClassTemplateSpecializationDecl const* D) populate(I, cast(D)); } +void +ASTVisitor:: +populate(RecordInfo& I, ClassTemplatePartialSpecializationDecl const* D) +{ + populate(I, dynamic_cast(D)); +} + void ASTVisitor:: populate( @@ -1023,6 +1053,13 @@ populate(VariableInfo& I, VarTemplateSpecializationDecl const* D) populate(I, cast(D)); } +void +ASTVisitor:: +populate(VariableInfo& I, VarTemplatePartialSpecializationDecl const* D) +{ + populate(I, dynamic_cast(D)); +} + void ASTVisitor:: populate( @@ -1883,22 +1920,6 @@ qualifiedName(NamedDecl const* ND) const return name; } -bool -ASTVisitor:: -generateJavadoc( - std::optional& javadoc, - Decl const* D) -{ - RawComment const* RC = - D->getASTContext().getRawCommentForDeclNoCache(D); - MRDOCS_CHECK_OR(RC, false); - comments::FullComment* FC = - RC->parse(D->getASTContext(), &sema_.getPreprocessor(), D); - MRDOCS_CHECK_OR(FC, false); - parseJavadoc(javadoc, FC, D, config_, diags_); - return true; -} - Polymorphic ASTVisitor:: toTypeInfo(QualType const qt, TraversalMode const mode) @@ -3341,9 +3362,9 @@ checkUndocumented( } // If the symbol is undocumented, check if we haven't seen a // documented version before. - auto const it = info_.find(id); - if (it != info_.end() && - it->get()->javadoc) + if (auto const infoIt = info_.find(id); + infoIt != info_.end() && + infoIt->get()->javadoc) { return {}; } @@ -3352,7 +3373,17 @@ checkUndocumented( // symbols we've seen so far in this translation unit. if (config_->warnIfUndocumented) { - undocumented_.insert({id, extractName(D)}); + auto const undocIt = undocumented_.find(id); + if (undocIt == undocumented_.end()) + { + InfoKind const kind = InfoTy::kind_id; + undocumented_.insert(UndocumentedInfo{id, extractName(D), kind}); + } + // Populate the location + auto handle = undocumented_.extract(undocIt); + UndocumentedInfo& UI = handle.value(); + populate(dynamic_cast(UI), D); + undocumented_.insert(std::move(handle)); } return Unexpected(Error("Undocumented")); } diff --git a/src/lib/AST/ASTVisitor.hpp b/src/lib/AST/ASTVisitor.hpp index 90f8aa65fd..ada6939e12 100644 --- a/src/lib/AST/ASTVisitor.hpp +++ b/src/lib/AST/ASTVisitor.hpp @@ -484,6 +484,24 @@ class ASTVisitor void populate(Info& I, bool isNew, DeclTy const* D); + template DeclTy> + void + populate(SourceInfo& I, DeclTy const* D); + + /* Parse the comments above a declaration as Javadoc + + This function will parse the comments above a declaration + as Javadoc, and store the results in the `javadoc` input + parameter. + + @return true if the comments were successfully parsed as + Javadoc, and false otherwise. + */ + bool + populate( + std::optional& javadoc, + Decl const* D); + void populate(SourceInfo& I, clang::SourceLocation loc, bool definition, bool documented); @@ -503,6 +521,9 @@ class ASTVisitor void populate(RecordInfo& I, ClassTemplateSpecializationDecl const* D); + void + populate(RecordInfo& I, ClassTemplatePartialSpecializationDecl const* D); + void populate(FunctionInfo& I, FunctionDecl const* D); @@ -548,6 +569,9 @@ class ASTVisitor void populate(VariableInfo& I, VarTemplateSpecializationDecl const* D); + void + populate(VariableInfo& I, VarTemplatePartialSpecializationDecl const* D); + void populate(FieldInfo& I, FieldDecl const* D); @@ -718,20 +742,6 @@ class ASTVisitor void addMember(std::vector& container, Info const& Member) const; - /* Parse the comments above a declaration as Javadoc - - This function will parse the comments above a declaration - as Javadoc, and store the results in the `javadoc` input - parameter. - - @return true if the comments were successfully parsed as - Javadoc, and false otherwise. - */ - bool - generateJavadoc( - std::optional& javadoc, - Decl const* D); - Polymorphic toTypeInfo(QualType qt, TraversalMode mode); diff --git a/src/lib/Gen/hbs/Builder.cpp b/src/lib/Gen/hbs/Builder.cpp index 5cf3352c57..7f041d7735 100644 --- a/src/lib/Gen/hbs/Builder.cpp +++ b/src/lib/Gen/hbs/Builder.cpp @@ -202,7 +202,7 @@ Builder( return nullptr; } dom::Value decls = sourceInfo.get("decl"); - if(dom::Value def = sourceInfo.get("def")) + if (dom::Value def = sourceInfo.get("def")) { // for classes/enums, prefer the definition if (dom::Value const kind = v.get("kind"); @@ -423,7 +423,7 @@ commonTemplatesDir() const std::string Builder:: -commonTemplatesDir(std::string_view subdir) const +commonTemplatesDir(std::string_view const subdir) const { Config const& config = domCorpus->config; return files::appendPath( diff --git a/src/lib/Lib/CorpusImpl.cpp b/src/lib/Lib/CorpusImpl.cpp index b9eda181fc..f4947366de 100644 --- a/src/lib/Lib/CorpusImpl.cpp +++ b/src/lib/Lib/CorpusImpl.cpp @@ -221,14 +221,14 @@ isTransparent(Info const& info) } } -Info const* +Expected CorpusImpl:: -lookup(SymbolID const& context, std::string_view name) const +lookup(SymbolID const& context, std::string_view const name) const { return lookupImpl(*this, context, name); } -Info const* +Expected CorpusImpl:: lookup(SymbolID const& context, std::string_view name) { @@ -236,7 +236,7 @@ lookup(SymbolID const& context, std::string_view name) } template -Info const* +Expected CorpusImpl:: lookupImpl(Self&& self, SymbolID const& context, std::string_view name) { @@ -246,16 +246,33 @@ lookupImpl(Self&& self, SymbolID const& context, std::string_view name) } if (auto [info, found] = self.lookupCacheGet(context, name); found) { + if (!info) + { + return Unexpected(formatError( + "Failed to find '{}' from context '{}'", + name, + self.Corpus::qualifiedName(*self.find(context)))); + } return info; } Expected const s = parseRef(name); if (!s) { - report::warn("Failed to parse '{}'\n {}", name, s.error().reason()); - self.lookupCacheSet(context, name, nullptr); - return nullptr; + return Unexpected(formatError("Failed to parse '{}'\n {}", name, s.error().reason())); } Info const* res = lookupImpl(self, context, *s, name, false); + if (!res) + { + auto const contextPtr = self.find(context); + if (!contextPtr) + { + return Unexpected(formatError("Failed to find '{}'", context)); + } + return Unexpected(formatError( + "Failed to find '{}' from context '{}'", + name, + self.Corpus::qualifiedName(*contextPtr))); + } return res; } @@ -600,7 +617,6 @@ CorpusImpl::finalize() report::debug("Finalizing javadoc"); JavadocFinalizer finalizer(*this); finalizer.build(); - finalizer.emitWarnings(); } diff --git a/src/lib/Lib/CorpusImpl.hpp b/src/lib/Lib/CorpusImpl.hpp index d72a7e6046..0502ad5053 100644 --- a/src/lib/Lib/CorpusImpl.hpp +++ b/src/lib/Lib/CorpusImpl.hpp @@ -91,10 +91,10 @@ class CorpusImpl final : public Corpus Info const* find(SymbolID const& id) const noexcept override; - Info const* + Expected lookup(SymbolID const& context, std::string_view name) const override; - Info const* + Expected lookup(SymbolID const& context, std::string_view name); /** Build metadata for a set of translation units. @@ -141,7 +141,7 @@ class CorpusImpl final : public Corpus template static - Info const* + Expected lookupImpl( Self&& self, SymbolID const& context, diff --git a/src/lib/Lib/ExecutionContext.cpp b/src/lib/Lib/ExecutionContext.cpp index 1bcb1612ce..dd8eaaac10 100644 --- a/src/lib/Lib/ExecutionContext.cpp +++ b/src/lib/Lib/ExecutionContext.cpp @@ -59,15 +59,7 @@ report( UndocumentedInfoSet&& undocumented) { InfoSet info = std::move(results); - // KRYSTIAN TODO: read stage will be required to - // update Info references once we switch to using Info* - #if 0 - { - std::shared_lock read_lock(mutex_); - } - #endif - - std::unique_lock write_lock(mutex_); + std::unique_lock write_lock(mutex_); // Add all new Info to the existing set. info_.merge(info); @@ -83,16 +75,14 @@ report( // Merge diagnostics and report any new messages. diags_.mergeAndReport(std::move(diags)); - - // Merge undocumented symbols and remove any symbols // from undocumented that we can find in info_ with // documentation from other translation units. undocumented_.merge(undocumented); for (auto it = undocumented_.begin(); it != undocumented_.end();) { - auto infoIt = info_.find(it->first); - if (infoIt != info_.end() && + if (auto infoIt = info_.find(it->id); + infoIt != info_.end() && infoIt->get()->javadoc) { it = undocumented_.erase(it); diff --git a/src/lib/Lib/Info.hpp b/src/lib/Lib/Info.hpp index a59ae83de9..d592aa58cd 100644 --- a/src/lib/Lib/Info.hpp +++ b/src/lib/Lib/Info.hpp @@ -116,7 +116,25 @@ struct InfoPtrEqual using InfoSet = std::unordered_set< std::unique_ptr, InfoPtrHasher, InfoPtrEqual>; -struct SymbolIDNameHasher { +struct UndocumentedInfo final : SourceInfo { + SymbolID id; + std::string name; + InfoKind kind; + + constexpr + UndocumentedInfo( + SymbolID id_, + std::string name_, + InfoKind kind_) noexcept + : SourceInfo() + , id(id_) + , name(std::move(name_)) + , kind(kind_) + { + } +}; + +struct UndocumentedInfoHasher { using is_transparent = void; std::size_t @@ -125,41 +143,41 @@ struct SymbolIDNameHasher { } std::size_t - operator()(std::pair const& I) const { - return std::hash()(I.first); + operator()(UndocumentedInfo const& I) const { + return std::hash()(I.id); } }; -struct SymbolIDNameEqual { +struct UndocumentedInfoEqual { using is_transparent = void; bool operator()( - std::pair const& a, - std::pair const& b) const + UndocumentedInfo const& a, + UndocumentedInfo const& b) const { - return a.first == b.first; + return a.id == b.id; } bool operator()( - std::pair const& a, + UndocumentedInfo const& a, SymbolID const& b) const { - return a.first == b; + return a.id == b; } bool operator()( SymbolID const& a, - std::pair const& b) const + UndocumentedInfo const& b) const { - return a == b.first; + return a == b.id; } }; using UndocumentedInfoSet = std::unordered_set< - std::pair, SymbolIDNameHasher, SymbolIDNameEqual>; + UndocumentedInfo, UndocumentedInfoHasher, UndocumentedInfoEqual>; } // clang::mrdocs diff --git a/src/lib/Metadata/Finalizers/JavadocFinalizer.cpp b/src/lib/Metadata/Finalizers/JavadocFinalizer.cpp index bf914dde36..c49314f639 100644 --- a/src/lib/Metadata/Finalizers/JavadocFinalizer.cpp +++ b/src/lib/Metadata/Finalizers/JavadocFinalizer.cpp @@ -110,34 +110,34 @@ operator()(InfoTy& I) void JavadocFinalizer:: -finalize(doc::Reference& ref) +finalize(doc::Reference& ref, bool const emitWarning) { - Info const* res = corpus_.lookup(current_context_->id, ref.string); - if (res) + if (Expected res + = corpus_.lookup(current_context_->id, ref.string)) { - ref.id = res->id; + Info const* resPtr = *res; + MRDOCS_ASSERT(resPtr); + ref.id = resPtr->id; } - if (res == nullptr && + else if ( + emitWarning && corpus_.config->warnings && corpus_.config->warnBrokenRef && // Only warn once per reference - !warned_.contains({ref.string, current_context_->Name}) && + !refWarned_.contains({ref.string, current_context_->Name}) && // Ignore std:: references !ref.string.starts_with("std::") && // Ignore other reference types that can't be replaced by `...` ref.Kind == doc::NodeKind::reference) { MRDOCS_ASSERT(current_context_); - if (auto primaryLoc = getPrimaryLocation(*current_context_)) - { - this->warn( - "{}:{}\n{}: Failed to resolve reference to '{}'", - primaryLoc->FullPath, - primaryLoc->LineNumber, - corpus_.Corpus::qualifiedName(*current_context_), - ref.string); - } - warned_.insert({ref.string, current_context_->Name}); + this->warn( + "{}: Failed to resolve reference to '{}'\n" + " {}", + corpus_.Corpus::qualifiedName(*current_context_), + ref.string, + res.error().reason()); + refWarned_.insert({ref.string, current_context_->Name}); } } @@ -154,11 +154,11 @@ finalize(doc::Node& node) if constexpr(std::same_as) { - finalize(dynamic_cast(N)); + finalize(dynamic_cast(N), true); } else if constexpr (std::same_as) { - finalize(dynamic_cast(N).exception); + finalize(dynamic_cast(N).exception, false); } }); } @@ -280,54 +280,47 @@ copyBriefAndDetails(Javadoc& javadoc) } // Find the node to copy from - Info const* res = corpus_.lookup(current_context_->id, copied->string); - if (!res) + Expected res = corpus_.lookup(current_context_->id, copied->string); + if (!res || !*res) { if (corpus_.config->warnings && corpus_.config->warnBrokenRef && - !warned_.contains({copied->string, current_context_->Name})) + !refWarned_.contains({copied->string, current_context_->Name})) { - MRDOCS_ASSERT(current_context_); - auto primaryLoc = getPrimaryLocation(*current_context_); this->warn( - "{}:{}\n" - "{}: Failed to copy documentation from '{}'\n" - " Note: Symbol '{}' not found.", - primaryLoc->FullPath, - primaryLoc->LineNumber, + "{}: Failed to copy documentation from '{}' (symbol not found)\n" + " {}", corpus_.Corpus::qualifiedName(*current_context_), copied->string, - copied->string); + res.error().reason()); } continue; } // Ensure the source node is finalized - if (!finalized_.contains(res)) + Info const* resPtr = *res; + MRDOCS_ASSERT(resPtr); + if (!finalized_.contains(resPtr)) { - operator()(const_cast(*res)); + operator()(const_cast(*resPtr)); } - if (!res->javadoc) + if (!resPtr->javadoc) { if (corpus_.config->warnings && corpus_.config->warnBrokenRef && - !warned_.contains({copied->string, current_context_->Name})) + !refWarned_.contains({copied->string, current_context_->Name})) { - auto ctxPrimaryLoc = getPrimaryLocation(*current_context_); - auto resPrimaryLoc = getPrimaryLocation(*res); + auto resPrimaryLoc = getPrimaryLocation(*resPtr); this->warn( - "{}:{}\n" - "{}: Failed to copy documentation from '{}'.\n" - "No documentation available.\n" - " {}:{}\n" - " Note: No documentation available for '{}'.", - ctxPrimaryLoc->FullPath, - ctxPrimaryLoc->LineNumber, + "{}: Failed to copy documentation from '{}' (no documentation available).\n" + " No documentation available.\n" + " {}:{}\n" + " Note: No documentation available for '{}'.", corpus_.Corpus::qualifiedName(*current_context_), copied->string, resPrimaryLoc->FullPath, resPrimaryLoc->LineNumber, - corpus_.Corpus::qualifiedName(*res)); + corpus_.Corpus::qualifiedName(*resPtr)); } continue; } @@ -335,15 +328,142 @@ copyBriefAndDetails(Javadoc& javadoc) // Copy brief and details bool const copyBrief = copied->parts == doc::Parts::all || copied->parts == doc::Parts::brief; bool const copyDetails = copied->parts == doc::Parts::all || copied->parts == doc::Parts::description; - Javadoc const& src = *res->javadoc; + Javadoc const& src = *resPtr->javadoc; if (copyBrief && !javadoc.brief) { javadoc.brief = src.brief; } - if (copyDetails && !src.blocks.empty()) + if (copyDetails) { - blockIt = javadoc.blocks.insert(blockIt, src.blocks.begin(), src.blocks.end()); - blockIt = std::next(blockIt, src.blocks.size()); + // Copy detail blocks + if (!src.blocks.empty()) + { + blockIt = javadoc.blocks.insert(blockIt, src.blocks.begin(), src.blocks.end()); + blockIt = std::next(blockIt, src.blocks.size()); + } + // Copy returns only if destination is empty + if (javadoc.returns.empty()) + { + javadoc.returns.insert( + javadoc.returns.end(), + src.returns.begin(), + src.returns.end()); + } + // Copy only params that don't exist at the destination + // documentation but that do exist in the destination + // function parameters declaration. + if (current_context_->isFunction()) + { + auto const& FI = dynamic_cast(*current_context_); + for (auto const& srcParam: src.params) + { + if (std::ranges::find_if(javadoc.params, + [&srcParam](doc::Param const& destParam) + { + return srcParam.name == destParam.name; + }) != javadoc.params.end()) + { + // param already exists at the destination, + // so the user attributed a new meaning to it + continue; + } + if (std::ranges::find_if(FI.Params, + [&srcParam](Param const& destParam) + { + return srcParam.name == destParam.Name; + }) == FI.Params.end()) + { + // param does not exist in the destination function + // so it would be an error there + continue; + } + // Push the new param + javadoc.params.push_back(srcParam); + } + } + // Copy only tparams that don't exist at the destination + // documentation but that do exist in the destination + // template parameters. + TemplateInfo const* destTemplate = visit( + *current_context_, + [](auto& I) -> TemplateInfo const* + { + if constexpr (requires { I.Template; }) + { + if (I.Template) + { + return &*I.Template; + } + } + return nullptr; + }); + if (destTemplate) + { + for (auto const& srcTParam: src.tparams) + { + if (std::ranges::find_if(javadoc.tparams, + [&srcTParam](doc::TParam const& destTParam) + { + return srcTParam.name == destTParam.name; + }) != javadoc.tparams.end()) + { + // tparam already exists at the destination, + // so the user attributed a new meaning to it + continue; + } + if (std::ranges::find_if(destTemplate->Params, + [&srcTParam](Polymorphic const& destTParam) + { + return srcTParam.name == destTParam->Name; + }) == destTemplate->Params.end()) + { + // TParam does not exist in the destination definition + // so it would be an error there + continue; + } + // Push the new param + javadoc.tparams.push_back(srcTParam); + } + } + // exceptions + if (javadoc.exceptions.empty()) + { + bool const isNoExcept = + current_context_->isFunction() ? + dynamic_cast(*current_context_).Noexcept.Kind == NoexceptKind::False : + false; + if (!isNoExcept) + { + javadoc.exceptions.insert( + javadoc.exceptions.end(), + src.exceptions.begin(), + src.exceptions.end()); + } + } + // sees + if (javadoc.sees.empty()) + { + javadoc.sees.insert( + javadoc.sees.end(), + src.sees.begin(), + src.sees.end()); + } + // preconditions + if (javadoc.preconditions.empty()) + { + javadoc.preconditions.insert( + javadoc.preconditions.end(), + src.preconditions.begin(), + src.preconditions.end()); + } + // postconditions + if (javadoc.postconditions.empty()) + { + javadoc.postconditions.insert( + javadoc.postconditions.end(), + src.postconditions.begin(), + src.postconditions.end()); + } continue; } // Erasing the paragraph could make the iterator == end() @@ -521,34 +641,54 @@ checkExists(SymbolID const& id) const void JavadocFinalizer:: -emitWarnings() const +emitWarnings() { MRDOCS_CHECK_OR(corpus_.config->warnings); warnUndocumented(); warnDocErrors(); warnNoParamDocs(); warnUndocEnumValues(); + + // Print to the console + auto const level = !corpus_.config->warnAsError ? report::Level::warn : report::Level::error; + for (auto const& [loc, msgs] : warnings_) + { + std::string locWarning = fmt::format( + "{}:{}\n", + loc.FullPath, + loc.LineNumber); + for (auto const& msg : msgs) + { + locWarning += fmt::format(" {}\n", msg); + } + report::log(level, locWarning); + } } void JavadocFinalizer:: -warnUndocumented() const +warnUndocumented() { MRDOCS_CHECK_OR(corpus_.config->warnIfUndocumented); - for (auto& [id, name] : corpus_.undocumented_) + for (auto& undocI : corpus_.undocumented_) { - if (Info const* I = corpus_.find(id)) + if (Info const* I = corpus_.find(undocI.id)) { - MRDOCS_CHECK_OR(!I->javadoc || I->Extraction == ExtractionMode::Regular); + MRDOCS_CHECK_OR( + !I->javadoc || I->Extraction == ExtractionMode::Regular); } - this->warn("{}: Symbol is undocumented", name); + bool const prefer_definition = undocI.kind == InfoKind::Record + || undocI.kind == InfoKind::Enum; + this->warn( + *getPrimaryLocation(undocI, prefer_definition), + "{}: Symbol is undocumented", undocI.name); } corpus_.undocumented_.clear(); } void JavadocFinalizer:: -warnDocErrors() const +warnDocErrors() { MRDOCS_CHECK_OR(corpus_.config->warnIfDocError); for (auto const& I : corpus_.info_) @@ -586,7 +726,7 @@ getJavadocParamNames(Javadoc const& javadoc) void JavadocFinalizer:: -warnParamErrors(FunctionInfo const& I) const +warnParamErrors(FunctionInfo const& I) { MRDOCS_CHECK_OR(I.javadoc); @@ -596,15 +736,12 @@ warnParamErrors(FunctionInfo const& I) const auto [firstDup, lastUnique] = std::ranges::unique(javadocParamNames); auto duplicateParamNames = std::ranges::subrange(firstDup, lastUnique); auto [firstDupDup, _] = std::ranges::unique(duplicateParamNames); - for (auto uniqueDuplicateParamNames = std::ranges::subrange(firstDup, firstDupDup); + for (auto const uniqueDuplicateParamNames = std::ranges::subrange(firstDup, firstDupDup); std::string_view duplicateParamName: uniqueDuplicateParamNames) { - auto primaryLoc = getPrimaryLocation(I); this->warn( - "{}:{}\n" + *getPrimaryLocation(I), "{}: Duplicate parameter documentation for '{}'", - primaryLoc->FullPath, - primaryLoc->LineNumber, corpus_.Corpus::qualifiedName(I), duplicateParamName); } @@ -618,12 +755,9 @@ warnParamErrors(FunctionInfo const& I) const { if (std::ranges::find(paramNames, javadocParamName) == paramNames.end()) { - auto primaryLoc = getPrimaryLocation(I); this->warn( - "{}:{}\n" + *getPrimaryLocation(I), "{}: Documented parameter '{}' does not exist", - primaryLoc->FullPath, - primaryLoc->LineNumber, corpus_.Corpus::qualifiedName(I), javadocParamName); } @@ -633,7 +767,7 @@ warnParamErrors(FunctionInfo const& I) const void JavadocFinalizer:: -warnNoParamDocs() const +warnNoParamDocs() { MRDOCS_CHECK_OR(corpus_.config->warnNoParamdoc); for (auto const& I : corpus_.info_) @@ -647,8 +781,9 @@ warnNoParamDocs() const void JavadocFinalizer:: -warnNoParamDocs(FunctionInfo const& I) const +warnNoParamDocs(FunctionInfo const& I) { + MRDOCS_CHECK_OR(!I.IsDeleted); // Check for function parameters that are not documented in javadoc auto javadocParamNames = getJavadocParamNames(*I.javadoc); auto paramNames = @@ -658,12 +793,9 @@ warnNoParamDocs(FunctionInfo const& I) const { if (std::ranges::find(javadocParamNames, paramName) == javadocParamNames.end()) { - auto primaryLoc = getPrimaryLocation(I); this->warn( - "{}:{}\n" + *getPrimaryLocation(I), "{}: Missing documentation for parameter '{}'", - primaryLoc->FullPath, - primaryLoc->LineNumber, corpus_.Corpus::qualifiedName(I), paramName); } @@ -683,12 +815,9 @@ warnNoParamDocs(FunctionInfo const& I) const }; if (!isVoid(*I.ReturnType)) { - auto primaryLoc = getPrimaryLocation(I); this->warn( - "{}:{}\n" + *getPrimaryLocation(I), "{}: Missing documentation for return type", - primaryLoc->FullPath, - primaryLoc->LineNumber, corpus_.Corpus::qualifiedName(I)); } } @@ -696,7 +825,7 @@ warnNoParamDocs(FunctionInfo const& I) const void JavadocFinalizer:: -warnUndocEnumValues() const +warnUndocEnumValues() { MRDOCS_CHECK_OR(corpus_.config->warnIfUndocEnumVal); for (auto const& I : corpus_.info_) @@ -704,12 +833,9 @@ warnUndocEnumValues() const MRDOCS_CHECK_OR_CONTINUE(I->isEnumConstant()); MRDOCS_CHECK_OR_CONTINUE(I->Extraction == ExtractionMode::Regular); MRDOCS_CHECK_OR_CONTINUE(!I->javadoc); - auto primaryLoc = getPrimaryLocation(*I); this->warn( - "{}:{}\n" + *getPrimaryLocation(*I), "{}: Missing documentation for enum value", - primaryLoc->FullPath, - primaryLoc->LineNumber, corpus_.Corpus::qualifiedName(*I)); } } diff --git a/src/lib/Metadata/Finalizers/JavadocFinalizer.hpp b/src/lib/Metadata/Finalizers/JavadocFinalizer.hpp index 2da3a604b7..ae720cda44 100644 --- a/src/lib/Metadata/Finalizers/JavadocFinalizer.hpp +++ b/src/lib/Metadata/Finalizers/JavadocFinalizer.hpp @@ -32,9 +32,50 @@ class JavadocFinalizer { CorpusImpl& corpus_; Info* current_context_ = nullptr; - std::set> warned_; + + /* Broken references for which we have already emitted + a warning. + */ + std::set> refWarned_; + + /* Info objects that have been finalized + + This is used to avoid allow recursion when + finalizing references. + */ std::set finalized_; + // A comparison function that sorts locations by: + // 1) ascending full path + // 2) descending line number + // This is the most convenient order to fix + // warnings in the source code because fixing a problem + // at a certain line will invalidate the line numbers + // of all subsequent warnings. + struct WarningLocationCompare + { + bool + operator()(Location const& lhs, Location const& rhs) const + { + if (lhs.FullPath != rhs.FullPath) + { + return lhs.FullPath < rhs.FullPath; + } + return lhs.LineNumber > rhs.LineNumber; + } + }; + + /* Warnings that should be emitted after finalization + + The warnings are initially stored in this container + where the messages are sorted by location. + + This makes it easier for the user to go through + the warnings in the order they appear in the source + code and fix them. + */ + std::map, WarningLocationCompare> warnings_; + public: JavadocFinalizer(CorpusImpl& corpus) : corpus_(corpus) @@ -50,11 +91,9 @@ class JavadocFinalizer MRDOCS_CHECK_OR_CONTINUE(I->Extraction != ExtractionMode::Dependency); visit(*I, *this); } + emitWarnings(); } - void - emitWarnings() const; - void operator()(Info& I) { @@ -69,7 +108,7 @@ class JavadocFinalizer private: // Look for symbol and set the id of a reference void - finalize(doc::Reference& ref); + finalize(doc::Reference& ref, bool emitWarning = true); // Recursively finalize references in a javadoc node void @@ -163,34 +202,50 @@ class JavadocFinalizer })); } + void + emitWarnings(); + + template + void + warn( + Location const& loc, + Located const format, + Args&&... args) + { + MRDOCS_CHECK_OR(corpus_.config->warnings); + std::string const str = fmt::vformat(format.value, fmt::make_format_args(args...)); + warnings_[loc].push_back(str); + } + template void warn( - Located format, - Args&&... args) const + Located const format, + Args&&... args) { MRDOCS_CHECK_OR(corpus_.config->warnings); - auto const level = !corpus_.config->warnAsError ? report::Level::warn : report::Level::error; - return log(level, format, std::forward(args)...); + MRDOCS_ASSERT(current_context_); + auto loc = getPrimaryLocation(*current_context_); + warn(*loc, format, std::forward(args)...); } void - warnUndocumented() const; + warnUndocumented(); void - warnDocErrors() const; + warnDocErrors(); void - warnParamErrors(FunctionInfo const& I) const; + warnParamErrors(FunctionInfo const& I); void - warnNoParamDocs() const; + warnNoParamDocs(); void - warnNoParamDocs(FunctionInfo const& I) const; + warnNoParamDocs(FunctionInfo const& I); void - warnUndocEnumValues() const; + warnUndocEnumValues(); }; } // clang::mrdocs diff --git a/src/lib/Metadata/Source.cpp b/src/lib/Metadata/Source.cpp index 81cbf615cf..04f19902fd 100644 --- a/src/lib/Metadata/Source.cpp +++ b/src/lib/Metadata/Source.cpp @@ -81,27 +81,21 @@ merge(SourceInfo& I, SourceInfo&& Other) } OptionalLocation -getPrimaryLocation(SourceInfo const& I) +getPrimaryLocation(SourceInfo const& I, bool const preferDefinition) { - OptionalLocation primaryLoc; - if (I.DefLoc) + if (I.Loc.empty() || + (preferDefinition && + I.DefLoc)) { - primaryLoc = *I.DefLoc; + return I.DefLoc; } - else if (!I.Loc.empty()) - { - auto const documentedIt = std::ranges::find_if( + auto const documentedIt = std::ranges::find_if( I.Loc, &Location::Documented); - if (documentedIt != I.Loc.end()) - { - primaryLoc = *documentedIt; - } - else - { - primaryLoc = I.Loc.front(); - } + if (documentedIt != I.Loc.end()) + { + return OptionalLocation(*documentedIt); } - return primaryLoc; + return OptionalLocation(I.Loc.front()); } template