Repository: hylang/hy

Issue:
Unable to view some docstrings with => (help modulename)
I came across a strange error causing problems viewing the docstring of a module. Below is my setup and a minimum working example.

```bash
> python -V
Python 3.11.7

> hy -v
hy 0.28.0+82.g225017d

> uname -a
Linux jupiter 6.1.83_1 #1 SMP PREEMPT_DYNAMIC Thu Mar 28 23:12:50 UTC 2024 x86_64 GNU/Linux
> # also reproduced on a debian bookworm docker image with python 3.11.2 and hy 0.28.0 release

> cat ds.hy
"A test docstring."
(defclass ExampleError [Exception])

> HYSTARTUP="" PAGER=less hy
Hy 0.28.0+82.g225017d using CPython(main) 3.11.7 on Linux

=> (import ds)
=> (help ds)
```

results in the following error:

```
Traceback (most recent call last):
  File "stdin-851f42906b23b723c670e376d4ab82b6ce401c3d", line 1, in <module>
    (help ds)
     ^^^^^^^^
  File "/home/ati/.pyenv/versions/3.11.7/lib/python3.11/pydoc.py", line 2011, in __call__
    self.help(request)
  File "/home/ati/.pyenv/versions/3.11.7/lib/python3.11/pydoc.py", line 2072, in help
    else: doc(request, 'Help on %s:', output=self._output, is_cli=is_cli)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ati/.pyenv/versions/3.11.7/lib/python3.11/pydoc.py", line 1785, in doc
    pager(render_doc(thing, title, forceload))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ati/.pyenv/versions/3.11.7/lib/python3.11/pydoc.py", line 1778, in render_doc
    return title % desc + '\n\n' + renderer.document(object, name)
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ati/.pyenv/versions/3.11.7/lib/python3.11/pydoc.py", line 479, in document
    if inspect.ismodule(object): return self.docmodule(*args)
                                        ^^^^^^^^^^^^^^^^^^^^^
  File "/home/ati/.pyenv/versions/3.11.7/lib/python3.11/pydoc.py", line 1269, in docmodule
    contents.append(self.document(value, key, name))
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ati/.pyenv/versions/3.11.7/lib/python3.11/pydoc.py", line 480, in document
    if inspect.isclass(object): return self.docclass(*args)
                                       ^^^^^^^^^^^^^^^^^^^^
  File "/home/ati/.pyenv/versions/3.11.7/lib/python3.11/pydoc.py", line 1331, in docclass
    doc = getdoc(object)
          ^^^^^^^^^^^^^^
  File "/home/ati/.pyenv/versions/3.11.7/lib/python3.11/pydoc.py", line 188, in getdoc
    result = _getdoc(object) or inspect.getcomments(object)
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ati/.pyenv/versions/3.11.7/lib/python3.11/inspect.py", line 1129, in getcomments
    lines, lnum = findsource(object)
                  ^^^^^^^^^^^^^^^^^^
  File "/home/ati/.pyenv/versions/3.11.7/lib/python3.11/inspect.py", line 1089, in findsource
    tree = ast.parse(source)
           ^^^^^^^^^^^^^^^^^
  File "/home/ati/.pyenv/versions/3.11.7/lib/python3.11/ast.py", line 50, in parse
    return compile(source, filename, mode, flags,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<unknown>", line 2
    (defclass ExampleError [Exception])
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
SyntaxError: invalid syntax. Perhaps you forgot a comma?
=>
```

Commenting out the class definition removes the error. Removing the docstring preserves the error.

Discussion:
@Kodiologist: Perplexing. It looks like Python wants to try to reparse the source file (as Python) even though all the docstrings it wants should already exist at runtime. I'm not sure Hy can fix this, but I'll look into it.@Kodiologist: It seems to occur whenever a Hy source file contains a `defclass` with no docstring.@Kodiologist: There it is:

> If there is no docstring, pydoc [`pydoc.help` being called by `help`] tries to obtain a description from the block of comment lines just above the definition of the class, function or method in the source file, or at the top of the module (see `inspect.getcomments()`).

`inspect.getcomments` says "If the object's source code is unavailable, return `None`." So, short of monkey-patching `inspect.getcomments` to actually get Hy comments, which is a tall order, perhaps we can at least get it to return `None` when the source file is in Hy.@atisharma: Thank you for looking at that. Unfortunately, it breaks a [use case](https://github.com/atisharma/hyjinx/blob/a9397e74353a5405975742a650afd89db98c7df0/hyjinx/source.hy#L58) where I use `inspect.getsourcefile` for Hy objects. This allows me to print the source of a function or other object in the repl, which I find very helpful.

Since you are monkey-patching inspect anyway, could I suggest that monkey-patching `inspect.getcomments` might be the better solution? I think it would not be too difficult to write a regex to extract comments, and improving the functionality of the inspect module with Hy would have other benefits - e.g. #1696 #1680 #1397.

If you're agreeable to that approach, I'll work on a patch. I'm aware you would want a test.@Kodiologist: > I think it would not be too difficult to write a regex to extract comments

I'm not so sure. Hy's syntax is Turing-undecidable (because of reader macros); it has complex nested structures (such as bracket f-strings) that are difficult to match properly with regexes; and macros can produce code with bogus position information. But the proof is in the pudding.@atisharma: I think I have sorted this out by patching inspect. This would also solve #1696. See https://github.com/atisharma/hy/blob/inspect-compat/hy/_compat.py which fixes:

* inspect.findsource
* inspect.getcomments
* inspect.getsourcelines
* inspect.getsource
* (help class-with-no-docstring)

If this approach is acceptable then I'll finish writing tests and proceed. If you want changes (e.g. it's quite long - would you prefer hy._inspect.py?) or if I'm barking up the wrong tree, then of course let me know.@Kodiologist: I think calling `hy_compile` or `read_many` is too much, because it would mean that `help` can execute arbitrary code, which is a big change from the situation in plain Python.

I haven't tested your `hy_getcomments`, but it looks like it could be fooled with plain string literals, bracket strings, reader macros, etc. It may well still be better than nothing; the thing to do is probably just to ensure it will return the wrong answer or `None` instead of raising an error.@atisharma: > I think calling `hy_compile` or `read_many` is too much, because it would mean that `help` can execute arbitrary code, which is a big change from the situation in plain Python.

I see your point, but doesn't calling help on an object mean the object has already been through the reader once, since it's in the namespace? There is no way to locate a class without either following the syntax tree (which is what python's inspect._ClassFinder does) or guessing with regexes, which has its own problems, as you pointed out. I don't see any other way.

> I haven't tested your `hy_getcomments`, but it looks like it could be fooled with plain string literals, bracket strings, reader macros, etc. It may well still be better than nothing; the thing to do is probably just to ensure it will return the wrong answer or `None` instead of raising an error.

This is expected behaviour. Python's inspect.getcomment only shows comments with a hash `# like this` before the class or function definition, and does not show strings. I would expect `hy_getcomments` to also ignore string literals, bracket strings in the same way, and reader macros too, since they are not comments. And as you say, it's better than returning `None`.

Since all the functions rely on `findsource`; `getcomment` will not work without it.

I do appreciate that patching inspect is not to be taken lightly.@Kodiologist: > doesn't calling help on an object mean the object has already been through the reader once, since it's in the namespace?

Sure, but one of the things about arbitrary code is that running it can have a different effect the second time. Besides, there's no guarantee that the file hasn't changed, so you might execute different code, after all.

> I don't see any other way.

You could change the compiler to cache what you need when the code is originally compiled, perhaps. But simply not implementing this is probably reasonable.

> I would expect `hy_getcomments` to also ignore string literals, bracket strings in the same way, and reader macros too, since they are not comments.

When I say "fooled with", I mean that those things could be mistaken for comments by your text-processing code, not that the code ought to treat them as comments and fails to. I could construct an example if this isn't clear.@atisharma: Well, the user has already made the decision to trust the code by that point, but I can see why you're not keen.

I can implement caching the python ast for use in `findsource` but modifying the compiler seems rather brutal. I'll have a think about possible other approaches, maybe like a `HyReader` without reader macros.

In the meantime, your patch is lighter touch and fixes the `help` problem. I can get around my `getsource` problem just in that module.@Kodiologist: I also have another PR you might like better, which patches `pydoc.getdoc` rather than anything in `inspect`. To be uploaded shortly.

Solution patch:
diff --git a/NEWS.rst b/NEWS.rst
index 05731db4..45c6fd33 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -24,6 +24,7 @@ New Features
 Bug Fixes
 ------------------------------
 * Tracebacks now point to the correct code in more cases.
+* `help` should no longer crash when objects are missing docstrings.
 
 0.28.0 (released 2024-01-05)
 =============================
diff --git a/hy/cmdline.py b/hy/cmdline.py
index a7d74800..bdafee10 100644
--- a/hy/cmdline.py
+++ b/hy/cmdline.py
@@ -13,7 +13,7 @@ from contextlib import nullcontext
 from pathlib import Path
 
 import hy
-from hy._compat import PY3_9
+from hy.compat import PY3_9
 from hy.compiler import hy_compile, hy_eval
 from hy.errors import HyLanguageError, filtered_hy_exceptions, hy_exc_handler
 from hy.importer import runhy
diff --git a/hy/_compat.py b/hy/compat.py
similarity index 67%
rename from hy/_compat.py
rename to hy/compat.py
index 25e8872b..046033dc 100644
--- a/hy/_compat.py
+++ b/hy/compat.py
@@ -43,3 +43,21 @@ if "def" in ast.unparse(ast.parse("𝕕𝕖𝕗 = 1")):
         return true_unparse(ast_obj)
 
     ast.unparse = rewriting_unparse
+
+
+if True:
+    import pydoc, inspect, re
+    true_getdoc = pydoc.getdoc
+    if not PY3_9:
+        pydoc._getdoc = inspect.getdoc
+    def getdoc(object):
+        """A monkey-patched `pydoc.getdoc` that tries to avoid calling
+        `inspect.getcomments` for an object defined in Hy code, which would try
+        to parse the Hy as Python. The implementation is based on Python
+        3.12.3's `getdoc`."""
+        result = pydoc._getdoc(object) or (
+            None
+            if inspect.getfile(object).endswith('.hy')
+            else inspect.getcomments(object))
+        return result and re.sub('^ *\n', '', result.rstrip()) or ''
+    pydoc.getdoc = getdoc
diff --git a/hy/core/result_macros.py b/hy/core/result_macros.py
index c1638813..dc57a8f2 100644
--- a/hy/core/result_macros.py
+++ b/hy/core/result_macros.py
@@ -15,7 +15,7 @@ from itertools import dropwhile
 
 from funcparserlib.parser import finished, forward_decl, many, maybe, oneplus, some
 
-from hy._compat import PY3_11, PY3_12
+from hy.compat import PY3_11, PY3_12
 from hy.compiler import Result, asty, mkexpr
 from hy.errors import HyEvalError, HyInternalError, HyTypeError
 from hy.macros import pattern_macro, require, require_reader, local_macro_name
diff --git a/hy/errors.py b/hy/errors.py
index 3bba6f1f..d5d7ae10 100644
--- a/hy/errors.py
+++ b/hy/errors.py
@@ -6,7 +6,7 @@ import importlib.util
 from contextlib import contextmanager
 
 from hy import _initialize_env_var
-from hy._compat import PYPY
+from hy.compat import PYPY
 
 _hy_show_internal_errors = _initialize_env_var("HY_SHOW_INTERNAL_ERRORS", False)
 
@@ -221,7 +221,7 @@ _tb_hidden_modules = {
             "hy.repl",
             "hy.reader.parser",
             "hy.importer",
-            "hy._compat",
+            "hy.compat",
             "hy.macros",
             "hy.models",
             "hy.core.result_macros",
diff --git a/hy/macros.py b/hy/macros.py
index 0bbe87a5..ef591523 100644
--- a/hy/macros.py
+++ b/hy/macros.py
@@ -10,7 +10,7 @@ from ast import AST
 from funcparserlib.parser import NoParseError
 
 import hy.compiler
-from hy._compat import PY3_11
+from hy.compat import PY3_11
 from hy.errors import (
     HyLanguageError,
     HyMacroExpansionError,
diff --git a/hy/scoping.py b/hy/scoping.py
index fbfa4e6b..a2df341b 100644
--- a/hy/scoping.py
+++ b/hy/scoping.py
@@ -4,7 +4,7 @@ import ast
 import itertools
 from abc import ABC, abstractmethod
 
-import hy._compat
+import hy.compat
 from hy.models import Expression, List, Symbol, Tuple
 from hy.reader import mangle
 
@@ -85,7 +85,7 @@ class NodeRef:
         ast.Nonlocal: "names",
         OuterVar: "names",
     }
-    if hy._compat.PY3_10:
+    if hy.compat.PY3_10:
         ACCESSOR.update(
             {
                 ast.MatchAs: "name",
diff --git a/requirements-dev.txt b/requirements-dev.txt
index 1ef7278c..aa406d02 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -1,6 +1,6 @@
 pytest >= 7
 
 # documentation
-Pygments @ git+https://github.com/Kodiologist/pygments@hylex
+Pygments >= 2.18.0
 Sphinx == 5.0.2
 git+https://github.com/hylang/sphinxcontrib-hydomain.git


Test patch:
diff --git a/tests/compilers/test_ast.py b/tests/compilers/test_ast.py
index 4ca70a30..deea00ef 100644
--- a/tests/compilers/test_ast.py
+++ b/tests/compilers/test_ast.py
@@ -4,7 +4,7 @@ from textwrap import dedent
 import pytest
 
 import hy
-from hy._compat import PY3_11
+from hy.compat import PY3_11
 from hy.compiler import hy_compile
 from hy.errors import HyError, HyLanguageError
 from hy.reader import read_many
diff --git a/tests/resources/__init__.py b/tests/resources/__init__.py
index 458d85fd..4ecbd1d9 100644
--- a/tests/resources/__init__.py
+++ b/tests/resources/__init__.py
@@ -1,6 +1,6 @@
 import pytest
 
-from hy._compat import PYODIDE
+from hy.compat import PYODIDE
 
 in_init = "chippy"
 
diff --git a/tests/test_bin.py b/tests/test_bin.py
index 93926363..d9a97ea1 100644
--- a/tests/test_bin.py
+++ b/tests/test_bin.py
@@ -11,7 +11,7 @@ from pathlib import Path
 
 import pytest
 
-from hy._compat import PY3_9, PYODIDE
+from hy.compat import PY3_9, PYODIDE
 
 if PYODIDE:
     pytest.skip(
diff --git a/tests/test_hy2py.py b/tests/test_hy2py.py
index b88cd08a..ec18515e 100644
--- a/tests/test_hy2py.py
+++ b/tests/test_hy2py.py
@@ -6,7 +6,7 @@ import pytest
 
 import hy.importer
 from hy import mangle
-from hy._compat import PYODIDE
+from hy.compat import PYODIDE
 from tests.resources import can_test_async