Repository: ansible/ansible
Pull request: More information on doc parsing errors
  fixes #77675

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Bugfix Pull Request

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below -->
ansible-doc

Problem statement:
ansible-test sanity - Include which documentation block is being parsed when raising "Unknown DOCUMENTATION error"
### Summary

It would be helpful if the generic `Unknown DOCUMENTATION error` indicated which of the documentation blocks was being parsed the the time the exception was hit.

### Issue Type

Feature Idea

### Component Name

ansible-test

### Additional Information

Locally I run `ansible-test sanity --docker default -vv` using the devel branch to try and keep ahead of new sanity tests (I know this is expected to break from time to time, and I'm good with that).

I hit the following error:
```
ERROR: plugins/modules/ec2_snapshot_info.py:0:0: documentation-error: Unknown DOCUMENTATION error, see TRACE: while scanning a simple key
  in "<unicode string>", line 72, column 13
could not find expected ':'
  in "<unicode string>", line 73, column 13
```

Because I hadn't changed this module it took me some time to figure out where the failure was occurring, being able to narrow down which of the documentation blocks had some bad YAML would have saved me time.  This is especially the case for these documentation tests because the line numbers aren't easy to trace.

### Code of Conduct

- [X] I agree to follow the Ansible Code of Conduct

Hints from issue discussion:
Files identified in the description:
* [`bin/ansible-test`](https://github.com/ansible/ansible/blob/devel/bin/ansible-test)

If these files are incorrect, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/devel/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
This has been caused by a change from b439e41a915ccec0ccbabecc966919ea406db74e

The change is arguably more correct, but as a result of using `tokenize` instead of `ast`, the `\` are not stripped from the data.  This is why the `yamllint` sanity test passes, why older versions of ansible-core passed with `validate-modules`, but why devel is now failing partially in `validate-modules`

The complicated part here is that we have no way of knowing what section caused the issue from within `validate-modules`.  All of the methods in ansible-core to retrieve the documentation currently fetch all documentation blocks, which means that a call to `get_docstring` pulls them all out and parses them all.

I suppose we should update `read_docstring_from_python_module` (and related) to also include the section in the name in the error, which might be a good idea, since `ansible-doc` also just fails with the same generic error:

```
$ ansible-doc --playbook-dir $PWD amazon.aws.ec2_snapshot_info
ERROR! module amazon.aws.ec2_snapshot_info missing documentation (or could not parse documentation): while scanning a simple key
  in "<unicode string>", line 72, column 13
could not find expected ':'
  in "<unicode string>", line 73, column 13
```

The improved error could look something like:

```
ERROR! module amazon.aws.ec2_snapshot_info missing documentation (or could not parse documentation): RETURN section: while scanning a simple key
  in "<unicode string>", line 72, column 13
could not find expected ':'
  in "<unicode string>", line 73, column 13
```

```diff
diff --git a/lib/ansible/parsing/plugin_docs.py b/lib/ansible/parsing/plugin_docs.py
index 31d083e5da..a50995f5ca 100644
--- a/lib/ansible/parsing/plugin_docs.py
+++ b/lib/ansible/parsing/plugin_docs.py
@@ -72,11 +72,13 @@ def read_docstring_from_python_module(filename, verbose=True, ignore_errors=True
     data = _init_doc_dict()
 
     next_string = None
+    var_name = None
     with tokenize.open(filename) as f:
         tokens = tokenize.generate_tokens(f.readline)
         for token in tokens:
             if token.type == tokenize.NAME:
                 if token.string in string_to_vars:
+                    var_name = token.string
                     next_string = string_to_vars[token.string]
 
             if next_string is not None and token.type == tokenize.STRING:
@@ -101,7 +103,7 @@ def read_docstring_from_python_module(filename, verbose=True, ignore_errors=True
                             if verbose:
                                 display.error("unable to parse %s" % filename)
                         else:
-                            raise
+                            raise AnsibleError(f"{var_name} section: {e}") from e
 
                 next_string = None
 
```

Commit message: reverse the lookup

Patch to review:
diff --git a/lib/ansible/parsing/plugin_docs.py b/lib/ansible/parsing/plugin_docs.py
index 31d083e5dabd34..780c505805291a 100644
--- a/lib/ansible/parsing/plugin_docs.py
+++ b/lib/ansible/parsing/plugin_docs.py
@@ -5,12 +5,10 @@
 __metaclass__ = type
 
 import ast
-import os
-import pyclbr
 import tokenize
 
 from ansible import constants as C
-from ansible.errors import AnsibleError
+from ansible.errors import AnsibleError, AnsibleParserError
 from ansible.module_utils._text import to_text, to_native
 from ansible.parsing.yaml.loader import AnsibleLoader
 from ansible.utils.display import Display
@@ -26,6 +24,17 @@
 }
 
 
+def _var2string(value):
+    ''' reverse lookup of the dict above '''
+    global string_to_vars
+    string = None
+    for k, v in string_to_vars.items():
+        if v == value:
+            string = k
+            break
+    return string
+
+
 def _init_doc_dict():
     ''' initialize a return dict for docs with the expected structure '''
     return {k: None for k in string_to_vars.values()}
@@ -41,12 +50,12 @@ def read_docstring_from_yaml_file(filename, verbose=True, ignore_errors=True):
     try:
         with open(filename, 'rb') as yamlfile:
             file_data = AnsibleLoader(yamlfile.read(), file_name=filename).get_single_data()
-
-    except Exception:
-        if verbose:
-            display.error("unable to parse %s" % filename)
+    except Exception as e:
+        msg = "Unable to parse yaml file '%s': %s" % (filename, to_native(e))
         if not ignore_errors:
-            raise
+            raise AnsibleParserError(msg, orig_exc=e)
+        elif verbose:
+            display.error(msg)
 
     for key in string_to_vars:
         data[string_to_vars[key]] = file_data.get(key, None)
@@ -54,14 +63,6 @@ def read_docstring_from_yaml_file(filename, verbose=True, ignore_errors=True):
     return data
 
 
-def _find_jinja_function(filename, verbose=True, ignore_errors=True):
-
-    # for finding filters/tests
-    module_name = os.path.splitext(os.path.basename(filename))[0]
-    paths = [os.path.dirname(filename)]
-    mdata = pyclbr.readmodule_ex(module_name, paths)
-
-
 def read_docstring_from_python_module(filename, verbose=True, ignore_errors=True):
     """
     Use tokenization to search for assignment of the documentation variables in the given file.
@@ -97,11 +98,11 @@ def read_docstring_from_python_module(filename, verbose=True, ignore_errors=True
                     try:
                         data[next_string] = AnsibleLoader(value, file_name=filename).get_single_data()
                     except Exception as e:
-                        if ignore_errors:
-                            if verbose:
-                                display.error("unable to parse %s" % filename)
-                        else:
-                            raise
+                        msg = "Unable to parse docs '%s' in python file '%s': %s" % (_var2string(next_string), filename, to_native(e))
+                        if not ignore_errors:
+                            raise AnsibleParserError(msg, orig_exc=e)
+                        elif verbose:
+                            display.error(msg)
 
                 next_string = None
 
@@ -148,13 +149,14 @@ def read_docstring_from_python_file(filename, verbose=True, ignore_errors=True):
                                 # string should be yaml if already not a dict
                                 data[varkey] = AnsibleLoader(child.value.s, file_name=filename).get_single_data()
 
-                        display.debug('assigned: %s' % varkey)
+                        display.debug('doc assigned: %s' % varkey)
 
-    except Exception:
-        if verbose:
-            display.error("unable to parse %s" % filename)
+    except Exception as e:
+        msg = "Unable to parse docs in python file '%s': %s" % (filename, to_native(e))
         if not ignore_errors:
-            raise
+            raise AnsibleParserError(msg, orig_exc=e)
+        elif verbose:
+            display.error(msg)
 
     return data
 
@@ -162,7 +164,7 @@ def read_docstring_from_python_file(filename, verbose=True, ignore_errors=True):
 def read_docstring(filename, verbose=True, ignore_errors=True):
     ''' returns a documentation dictionary from Ansible plugin docstrings '''
 
-    # TODO: ensure adjacency to code (including ps1 for py files)
+    # NOTE: adjacency of doc file to code file is responsibility of caller
     if filename.endswith(C.YAML_DOC_EXTENSIONS):
         docstring = read_docstring_from_yaml_file(filename, verbose=verbose, ignore_errors=ignore_errors)
     elif filename.endswith(C.PYTHON_DOC_EXTENSIONS):


Code review comments:
diff_hunk: @@ -26,6 +24,17 @@
 }
 
 
+def _var2string(value):
+    ''' reverse lookup of the dict above '''
+    global string_to_vars
+    string = None
+    for k, v in string_to_vars.items():
+        if v == value:
+            string = k
path: lib/ansible/parsing/plugin_docs.py
text: @user1:
I don't really love this function, but since it's an "end of the road" call, I suppose it doesn't matter much.  Seems like you could just `return` here instead of setting a var and doing a `break`.  And potentially have the function raise an exception at the end otherwise.

@author:
initial versino had `else: raise keyerror`  but i really didn't see this useful/possible since at this point