Fix lint-collection-docs --plugin-docs subcommand (#47)

* Fix collection copier.

* Prevent errors to be dumped twice.

* Also use --plugin-docs in tests.

* Improve file name guessing.

* Add changelog fragment.

Author: felixfontein

.github/workflows/antsibull-docs.yml

@@ -80,7 +80,7 @@ jobs:
 
       - name: Lint collection docs
         run: |
-          poetry run coverage run -p --source antsibull_docs --source sphinx_antsibull_ext -m antsibull_docs.cli.antsibull_docs lint-collection-docs ~/.ansible/collections/ansible_collections/community/docker
+          poetry run coverage run -p --source antsibull_docs --source sphinx_antsibull_ext -m antsibull_docs.cli.antsibull_docs lint-collection-docs ~/.ansible/collections/ansible_collections/community/docker --plugin-docs
           poetry run coverage run -p --source antsibull_docs --source sphinx_antsibull_ext -m antsibull_docs.cli.antsibull_docs lint-collection-docs ~/.ansible/collections/ansible_collections/community/crypto
           poetry run coverage run -p --source antsibull_docs --source sphinx_antsibull_ext -m antsibull_docs.cli.antsibull_docs lint-collection-docs ~/.ansible/collections/ansible_collections/sensu/sensu_go
         working-directory: antsibull-docs

changelogs/fragments/47-fix-lint-plugin-docs.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - "Make ``lint-collection-docs --plugin-docs`` subcommand actually work (https://github.com/ansible-community/antsibull-docs/pull/47)."

src/antsibull_docs/lint_plugin_docs.py

@@ -37,7 +37,11 @@
 )
 from .jinja2.environment import doc_environment
 from .utils.collection_name_transformer import CollectionNameTransformer
-from .write_docs import create_plugin_rst
+from .write_docs import (
+    create_plugin_rst,
+    guess_relative_filename,
+    has_broken_docs,
+)
 from .rstcheck import check_rst_content
 
 
@@ -48,7 +52,7 @@ def __init__(self):
         self.dir = None
 
     def __enter__(self):
-        if self.dir is None:
+        if self.dir is not None:
             raise AssertionError('Collection copier already initialized')
         self.dir = os.path.realpath(tempfile.mkdtemp(prefix='antsibull-docs-'))
         return self
@@ -107,17 +111,6 @@ def _lint_collection_plugin_docs(collections_dir: str, collection_name: str,
     collection_to_plugin_info = get_collection_contents(plugin_contents)
     for collection in collection_metadata:
         collection_to_plugin_info[collection]  # pylint:disable=pointless-statement
-    # Collect non-fatal errors
-    result = []
-    for plugin_type, plugins in sorted(nonfatal_errors.items()):
-        for plugin_name, errors in sorted(plugins.items()):
-            for error in errors:
-                result.append((
-                    os.path.join(original_path_to_collection, 'plugins', plugin_type, plugin_name),
-                    0,
-                    0,
-                    error,
-                ))
     # Compose RST files and check for errors
     # Setup the jinja environment
     env = doc_environment(
@@ -129,21 +122,38 @@ def _lint_collection_plugin_docs(collections_dir: str, collection_name: str,
     role_tmpl = env.get_template('role.rst.j2')
     error_tmpl = env.get_template('plugin-error.rst.j2')
 
+    result = []
     for collection_name_, plugins_by_type in collection_to_plugin_info.items():
         for plugin_type, plugins_dict in plugins_by_type.items():
             plugin_type_tmpl = plugin_tmpl
             if plugin_type == 'role':
                 plugin_type_tmpl = role_tmpl
             for plugin_short_name, dummy_ in plugins_dict.items():
                 plugin_name = '.'.join((collection_name_, plugin_short_name))
+                plugin_record = new_plugin_info[plugin_type].get(plugin_name) or {}
+                filename = os.path.join(
+                    original_path_to_collection,
+                    guess_relative_filename(
+                        plugin_record,
+                        plugin_short_name,
+                        plugin_type,
+                        collection_name_,
+                        collection_metadata[collection_name_]))
+                if has_broken_docs(plugin_record, plugin_type):
+                    result.append((filename, 0, 0, 'Did not return correct DOCUMENTATION'))
+                for error in nonfatal_errors[plugin_type][plugin_name]:
+                    result.append((filename, 0, 0, error))
                 rst_content = create_plugin_rst(
-                    collection_name_, collection_metadata[collection_name_],
+                    collection_name_,
+                    collection_metadata[collection_name_],
                     link_data[collection_name_],
-                    plugin_short_name, plugin_type,
-                    new_plugin_info[plugin_type].get(plugin_name) or {},
+                    plugin_short_name,
+                    plugin_type,
+                    plugin_record,
                     nonfatal_errors[plugin_type][plugin_name],
                     plugin_type_tmpl, error_tmpl,
                     use_html_blobs=False,
+                    log_errors=False,
                 )
                 path = os.path.join(
                     original_path_to_collection, 'plugins', plugin_type,

src/antsibull_docs/write_docs.py

@@ -92,14 +92,50 @@ def follow_relative_links(path: str) -> str:
         path = os.path.join(os.path.dirname(path), link)
 
 
+def has_broken_docs(plugin_record: t.Mapping[str, t.Any], plugin_type: str) -> bool:
+    """
+    Determine whether the plugin record is completely broken or not.
+    """
+    expected_fields = ('entry_points',) if plugin_type == 'role' else ('doc', 'examples', 'return')
+    return not plugin_record or not all(field in plugin_record for field in expected_fields)
+
+
+def guess_relative_filename(plugin_record: t.Mapping[str, t.Any],
+                            plugin_short_name: str,
+                            plugin_type: str,
+                            collection_name: str,
+                            collection_meta: AnsibleCollectionMetadata) -> str:
+    """
+    Make an educated guess on the documentation source file.
+    """
+    if plugin_record and plugin_record.get('doc') and plugin_record['doc'].get('filename'):
+        filename = follow_relative_links(plugin_record['doc']['filename'])
+        return os.path.relpath(filename, collection_meta.path)
+    if plugin_type == 'role':
+        return f"roles/{plugin_short_name}/meta/argument_specs.yml"
+    plugin_dir = (
+        # Modules in ansible-core:
+        'modules' if plugin_type == 'module' and collection_name == 'ansible.builtin' else
+        # Modules in collections:
+        'plugins/modules' if plugin_type == 'module' else
+        # Plugins in ansible-core or collections:
+        'plugins/' + plugin_type
+    )
+    # Guess path inside collection tree
+    return f"{plugin_dir}/{plugin_short_name}.py"
+
+
 def create_plugin_rst(collection_name: str,
                       collection_meta: AnsibleCollectionMetadata,
                       collection_links: CollectionLinks,
-                      plugin_short_name: str, plugin_type: str,
-                      plugin_record: t.Dict[str, t.Any], nonfatal_errors: t.Sequence[str],
+                      plugin_short_name: str,
+                      plugin_type: str,
+                      plugin_record: t.Dict[str, t.Any],
+                      nonfatal_errors: t.Sequence[str],
                       plugin_tmpl: Template, error_tmpl: Template,
                       use_html_blobs: bool = False,
-                      for_official_docsite: bool = False) -> str:
+                      for_official_docsite: bool = False,
+                      log_errors: bool = True) -> str:
     """
     Create the rst page for one plugin.
 
@@ -118,6 +154,7 @@ def create_plugin_rst(collection_name: str,
                          tables instead of using RST tables.
     :kwarg for_official_docsite: Default False.  Set to True to use wording specific for the
         official docsite on docs.ansible.com.
+    :kwarg log_errors: Default True.  Set to False to avoid errors to be logged.
     """
     flog = mlog.fields(func='create_plugin_rst')
     flog.debug('Enter')
@@ -126,38 +163,21 @@ def create_plugin_rst(collection_name: str,
 
     edit_on_github_url = None
     eog = collection_links.edit_on_github
-    if eog and plugin_type != 'role':
-        gh_plugin_dir = (
-            # Modules in ansible-core:
-            'modules' if plugin_type == 'module' and collection_name == 'ansible.builtin' else
-            # Modules in collections:
-            'plugins/modules' if plugin_type == 'module' else
-            # Plugins in ansible-core or collections:
-            'plugins/' + plugin_type
-        )
-        # Guess path inside collection tree
-        gh_path = f"{gh_plugin_dir}/{plugin_short_name}.py"
-        # If we have more precise information, use that!
-        if plugin_record and plugin_record.get('doc') and plugin_record['doc'].get('filename'):
-            filename = follow_relative_links(plugin_record['doc']['filename'])
-            gh_path = os.path.relpath(filename, collection_meta.path)
-        # Compose path
+    if eog:
+        # Compose Edit on GitHub URL
+        gh_path = guess_relative_filename(
+            plugin_record, plugin_short_name, plugin_type, collection_name, collection_meta)
         edit_on_github_url = (
             f"https://github.com/{eog.repository}/edit/{eog.branch}/{eog.path_prefix}{gh_path}"
         )
-    if eog and plugin_type == 'role':
-        edit_on_github_url = (
-            f"https://github.com/{eog.repository}/edit/{eog.branch}/{eog.path_prefix}"
-            f"roles/{plugin_short_name}/meta/argument_specs.yml"
-        )
 
-    expected_fields = ('entry_points',) if plugin_type == 'role' else ('doc', 'examples', 'return')
-    if not plugin_record or not all(field in plugin_record for field in expected_fields):
-        flog.fields(plugin_type=plugin_type,
-                    plugin_name=plugin_name,
-                    nonfatal_errors=nonfatal_errors
-                    ).error('{plugin_name} did not return correct DOCUMENTATION.  An error page'
-                            ' will be generated.', plugin_name=plugin_name)
+    if has_broken_docs(plugin_record, plugin_type):
+        if log_errors:
+            flog.fields(plugin_type=plugin_type,
+                        plugin_name=plugin_name,
+                        nonfatal_errors=nonfatal_errors
+                        ).error('{plugin_name} did not return correct DOCUMENTATION.  An error'
+                                ' page will be generated.', plugin_name=plugin_name)
         plugin_contents = _render_template(
             error_tmpl,
             plugin_name + '_' + plugin_type,
@@ -172,7 +192,7 @@ def create_plugin_rst(collection_name: str,
             for_official_docsite=for_official_docsite,
         )
     else:
-        if nonfatal_errors:
+        if log_errors and nonfatal_errors:
             flog.fields(plugin_type=plugin_type,
                         plugin_name=plugin_name,
                         nonfatal_errors=nonfatal_errors