warn-if-undocumented option

#feat

fix #758

Author: alandefreitas

docs/mrdocs.schema.json

@@ -93,7 +93,7 @@
     },
     "extract-all": {
       "default": true,
-      "description": "When set to `true`, MrDocs extracts all symbols from the source code, even if no documentation is provided. When set to `false`, it's still recommendable to provide file and symbol filters so that only the desired symbols are traversed by MrDocs.",
+      "description": "When set to `true`, MrDocs extracts all symbols from the source code, even if no documentation is provided. MrDocs can only identify whether a symbol is ultimated documented after extracting information from all translation units. For this reason, when this option is set to `false`, it's still recommendable to provide file and symbol filters so that only the desired symbols are traversed and stored by MrDocs.",
       "enum": [
         true,
         false
@@ -450,6 +450,16 @@
       "title": "Verbose output",
       "type": "boolean"
     },
+    "warn-if-undocumented": {
+      "default": true,
+      "description": "When set to `true`, MrDocs outputs a warning message if a symbol that passes all filters is not documented. This option is only effective when `extract-all` is set to `false`.",
+      "enum": [
+        true,
+        false
+      ],
+      "title": "Warn if symbols are not documented",
+      "type": "boolean"
+    },
     "warnings": {
       "default": true,
       "description": "When set to `true`, MrDocs outputs warning messages during the generation of the documentation. It is usually recommended to enable warnings while writing the documentation.",

src/lib/AST/ASTVisitor.cpp

@@ -73,7 +73,7 @@ void
 ASTVisitor::
 build()
 {
-    // traverse the translation unit, only extracting
+    // Traverse the translation unit, only extracting
     // declarations which satisfy all filter conditions.
     // dependencies will be tracked, but not extracted
     TranslationUnitDecl const* TU = context_.getTranslationUnitDecl();
@@ -3291,16 +3291,10 @@ upsert(DeclType const* D)
             return Unexpected(Error("Explicit declaration in dependency mode"));
         }
     }
-    else if (
-        !InfoParent<R> &&
-        mode_ == Regular &&
-        !config_->extractAll &&
-        !D->getASTContext().getRawCommentForDeclNoCache(D))
-    {
-        return Unexpected(formatError("{}: Symbol is undocumented", extractName(D)));
-    }
 
     SymbolID const id = generateID(D);
+    MRDOCS_TRY(checkUndocumented<R>(id, D));
+
     MRDOCS_CHECK_MSG(id, "Failed to extract symbol ID");
     auto [I, isNew] = upsert<R>(id);
 
@@ -3314,4 +3308,52 @@ upsert(DeclType const* D)
     return upsertResult<R>{std::ref(I), isNew};
 }
 
+template <
+    std::derived_from<Info> InfoTy,
+    std::derived_from<Decl> DeclTy>
+Expected<void>
+ASTVisitor::
+checkUndocumented(
+    SymbolID const& id,
+    DeclTy const* D)
+{
+    // If `extract-all` is enabled, we don't need to
+    // check for undocumented symbols
+    MRDOCS_CHECK_OR(!config_->extractAll, {});
+    // If the symbol is a namespace, the `extract-all`
+    // doesn't apply to it
+    MRDOCS_CHECK_OR((!std::same_as<InfoTy,NamespaceInfo>), {});
+    // If the symbol is not being extracted as a Regular
+    // symbol, we don't need to check for undocumented symbols
+    // These are expected to be potentially undocumented
+    MRDOCS_CHECK_OR(mode_ == Regular, {});
+    // Check if the symbol is documented, ensure this symbol is not in the set
+    // of undocumented symbols in this translation unit and return
+    // without an error if it is
+    if (D->getASTContext().getRawCommentForDeclNoCache(D))
+    {
+        if (config_->warnIfUndocumented)
+        {
+            undocumented_.erase({id, extractName(D)});
+        }
+        return {};
+    }
+    // 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)
+    {
+        return {};
+    }
+    // If the symbol is undocumented, and we haven't seen a documented
+    // version before, store this symbol in the set of undocumented
+    // symbols we've seen so far in this translation unit.
+    if (config_->warnIfUndocumented)
+    {
+        undocumented_.insert({id, extractName(D)});
+    }
+    return Unexpected(Error("Undocumented"));
+}
+
 } // clang::mrdocs

src/lib/AST/ASTVisitor.hpp

@@ -78,6 +78,29 @@ class ASTVisitor
     // An unordered set of all extracted Info declarations
     InfoSet info_;
 
+    /*  The symbols we would extract if they were documented
+
+        When `extract-all` is false, we only extract symbols
+        that are documented. If a symbol reappears in the
+        translation unit, we only extract the declaration
+        that's documented.
+
+        When `extract-all` is false and `warn-if-undocumented`
+        is true, we also warn if a symbol is not documented.
+        However, because a symbol can appear multiple times
+        in multiple translation units, we cannot be sure
+        a symbol is undocumented until we have processed
+        all translation units.
+
+        For this reason, this set stores the symbols that
+        are not documented but would otherwise have been
+        extracted as regular symbols in the current
+        translation unit. After symbols from all translation
+        units are merged, we will iterate these symbols
+        and warn if they are not documented.
+     */
+    UndocumentedInfoSet undocumented_;
+
     /* Struct to hold pre-processed file information.
 
         This struct stores information about a file, including its full path,
@@ -288,6 +311,19 @@ class ASTVisitor
         return info_;
     }
 
+    /** Get the set of extracted Info declarations.
+
+        This function returns a reference to the set of Info
+        declarations that have been extracted by the ASTVisitor.
+
+        @return A reference to the InfoSet containing the extracted Info declarations.
+     */
+    UndocumentedInfoSet&
+    undocumented()
+    {
+        return undocumented_;
+    }
+
 private:
     // =================================================
     // AST Traversal
@@ -1156,6 +1192,14 @@ class ASTVisitor
                 InfoTypeFor_t<DeclType>,
                 InfoTy>>>
     upsert(DeclType const* D);
+
+    template <
+        std::derived_from<Info> InfoTy,
+        std::derived_from<Decl> DeclTy>
+    Expected<void>
+    checkUndocumented(
+        SymbolID const& id,
+        DeclTy const* D);
 };
 
 } // clang::mrdocs

src/lib/AST/ASTVisitorConsumer.cpp

@@ -15,8 +15,7 @@
 #include "lib/AST/ASTVisitor.hpp"
 #include "lib/Support/Path.hpp"
 
-namespace clang {
-namespace mrdocs {
+namespace clang::mrdocs {
 
 void
 ASTVisitorConsumer::
@@ -31,8 +30,7 @@ HandleTranslationUnit(ASTContext& Context)
         Context,
         *sema_);
     visitor.build();
-    ex_.report(std::move(visitor.results()), std::move(diags));
+    ex_.report(std::move(visitor.results()), std::move(diags), std::move(visitor.undocumented()));
 }
 
-} // mrdocs
-} // clang
+} // clang::mrdocs

src/lib/Lib/ConfigOptions.json

@@ -181,7 +181,7 @@
       {
         "name": "extract-all",
         "brief": "Extract all symbols",
-        "details": "When set to `true`, MrDocs extracts all symbols from the source code, even if no documentation is provided. When set to `false`, it's still recommendable to provide file and symbol filters so that only the desired symbols are traversed by MrDocs.",
+        "details": "When set to `true`, MrDocs extracts all symbols from the source code, even if no documentation is provided. MrDocs can only identify whether a symbol is ultimated documented after extracting information from all translation units. For this reason, when this option is set to `false`, it's still recommendable to provide file and symbol filters so that only the desired symbols are traversed and stored by MrDocs.",
         "type": "bool",
         "default": true
       },
@@ -478,6 +478,13 @@
         "details": "When set to `true`, MrDocs outputs warning messages during the generation of the documentation. It is usually recommended to enable warnings while writing the documentation.",
         "type": "bool",
         "default": true
+      },
+      {
+        "name": "warn-if-undocumented",
+        "brief": "Warn if symbols are not documented",
+        "details": "When set to `true`, MrDocs outputs a warning message if a symbol that passes all filters is not documented. This option is only effective when `extract-all` is set to `false`.",
+        "type": "bool",
+        "default": true
       }
     ]
   },

src/lib/Lib/CorpusImpl.cpp

@@ -524,7 +524,9 @@ build(
     }
 
     MRDOCS_TRY(auto results, context.results());
+    auto undocumented = context.undocumented();
     corpus->info_ = std::move(results);
+    corpus->undocumented_ = std::move(undocumented);
 
     report::info(
         "Extracted {} declarations in {}",
@@ -598,6 +600,12 @@ CorpusImpl::finalize()
     report::debug("Finalizing javadoc");
     JavadocFinalizer finalizer(*this);
     finalizer.build();
+
+    if (!config->extractAll && config->warnIfUndocumented)
+    {
+        finalizer.warnUndocumented();
+    }
+    undocumented_.clear();
 }
 
 

src/lib/Lib/CorpusImpl.hpp

@@ -16,15 +16,16 @@
 #include "lib/Lib/ConfigImpl.hpp"
 #include "lib/Lib/Info.hpp"
 #include "lib/Support/Debug.hpp"
-#include <map>
-#include <mutex>
-#include <string>
 #include <clang/Tooling/CompilationDatabase.h>
 #include <mrdocs/ADT/UnorderedStringMap.hpp>
 #include <mrdocs/Corpus.hpp>
 #include <mrdocs/Metadata.hpp>
 #include <mrdocs/Platform.hpp>
 #include <mrdocs/Support/Error.hpp>
+#include <map>
+#include <mutex>
+#include <string>
+#include <set>
 
 namespace clang::mrdocs {
 
@@ -45,6 +46,9 @@ class CorpusImpl final : public Corpus
     // Info keyed on Symbol ID.
     InfoSet info_;
 
+    // Undocumented symbols
+    UndocumentedInfoSet undocumented_;
+
     // Lookup cache
     // The key represents the context symbol ID.
     // The value is another map from the name to the Info.

src/lib/Lib/ExecutionContext.cpp

@@ -55,7 +55,8 @@ void
 InfoExecutionContext::
 report(
     InfoSet&& results,
-    Diagnostics&& diags)
+    Diagnostics&& diags,
+    UndocumentedInfoSet&& undocumented)
 {
     InfoSet info = std::move(results);
     // KRYSTIAN TODO: read stage will be required to
@@ -67,6 +68,7 @@ report(
     #endif
 
     std::unique_lock<std::shared_mutex> write_lock(mutex_);
+
     // Add all new Info to the existing set.
     info_.merge(info);
 
@@ -80,6 +82,26 @@ 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() &&
+            infoIt->get()->javadoc)
+        {
+            it = undocumented_.erase(it);
+        }
+        else
+        {
+            ++it;
+        }
+    }
 }
 
 void
@@ -89,12 +111,19 @@ reportEnd(report::Level level)
     diags_.reportTotals(level);
 }
 
-mrdocs::Expected<InfoSet>
+Expected<InfoSet>
 InfoExecutionContext::
 results()
 {
     return std::move(info_);
 }
 
+UndocumentedInfoSet
+InfoExecutionContext::
+undocumented()
+{
+    return std::move(undocumented_);
+}
+
 } // mrdocs
 } // clang