Add hy.inspect module, extending inspect.

[atisharma]

Introduces hy.inspect to support Hy-specific introspection, extending
python's inspect module.

It does this by shadowing the functions getfile, findsource,
getsourcelines, getcomments, getsource for Hy constructs. It adds
isExpression and isLazy, and hy_getblock (the equivalent to getblock).
Works for macros via get-macro.

Has test coverage equivalent to python's inspect for the new features.
Tests include xfails where Hy's class docstring inheritance differs from
python's Tests are skipped where class source is unavailable (pre-Python
3.13).

Fixes #1696.

[Kodiologist]

As I understand, you're providing some functions that can be used in place of inspect, but the linked issue is asking to make inspect functions work for Hy. Then, other tools that use inspect, like debuggers, will (in theory) work with Hy. So I think you'll need to monkey-patch inspect.

getsource on hy __code__ object is not implemented

Isn't that the main point of the linked issue, though? Without this, are we really addressing the issue?

If I understand your xfail marks correctly, you believe that methods should actually inherit the docstrings of superclass methods. This doesn't happen in Python, so it would be weird for it to happen in Hy. Rather, as I understand it, inspect.getdoc does the work of climbing up the inheritance hierarchy to get a superclass's docstring when the subclass's method doesn't have one.

Using Python's test is probably a bad idea since "The test package is meant for internal use by Python only.… Any use of this package outside of Python’s standard library is discouraged as code mentioned here can change or be removed without notice between releases of Python."

[atisharma]

As I understand, you're providing some functions that can be used in place of inspect, but the linked issue is asking to make inspect functions work for Hy. Then, other tools that use inspect, like debuggers, will (in theory) work with Hy. So I think you'll need to monkey-patch inspect.

Yes, I suppose, but as per a previous discussion I had understood that you were against monkey-patching inspect. It would only be a few lines to do so. Even without monkey-patching I still think it's worth having hy.inspect available. If it's preferred to monkey-patch inspect, I can do that.

getsource on hy __code__ object is not implemented

The issue shows (inspect.findsource func2) and (inspect.getsource func2) failing in cPython. Those are implemented.

The comment you mention refers to the missing test

def test_getsource_on_code_object(self):
    self.assertSourceEqual(mod.eggs.__code__, 12, 18)

which would test for (inspect.findsource func2.__code__), for instance. I've pushed a fix.

If I understand your xfail marks correctly, you believe that methods should actually inherit the docstrings of superclass methods. This doesn't happen in Python, so it would be weird for it to happen in Hy. Rather, as I understand it, inspect.getdoc does the work of climbing up the inheritance hierarchy to get a superclass's docstring when the subclass's method doesn't have one.

I see. I haven't reimplemented inspect.getdoc or inspect._finddoc, so they aren't testing hy.inspect, but rather python's inspect. Perhaps they should instead be Hy issues, but I thought it made sense to have those tests here.

Using Python's test is probably a bad idea [...]

I've rewritten the two tests that used the test.support.os_helper.temp_cwd to use pytest's equivalent.

[Kodiologist]

as per a previous discussion I had understood that you were against monkey-patching inspect.

You mean the discussion in #2578? I was complaining about undecidable syntax and surprise code execution, not monkey-patching.

If it's preferred to monkey-patch inspect, I can do that.

The goal was to support "code that implicitly uses inspect (including other standard library modules)". If you can do that without monkey-patching, more power to you. For example: does ll work now?

[atisharma]

Curiously, it seems you can.

For example, the file test_pdb.hy:

;; test pdb and hy.inspect

(defn foo []
  (setv x 1)
  (for [y (range 2)]
    (print y))
  (breakpoint)
  (print x))

(defclass A []

  (setv x 0)

  (defn [property] y [] self.x)

  (defn incr [self]
    (setv self.x (+ 1 self.x))
    (breakpoint)))
 

(setv a (A))

(foo)

(a.incr)
(breakpoint)

when run as hy test_pdb.hy gives:

0
1
> /src/hy/test_pdb.hy(7)foo()
-> (breakpoint)
(Pdb) ll
  3  	(defn foo []
  4  	  (setv x 1)
  5  	  (for [y (range 2)]
  6  	    (print y))
  7  ->	  (breakpoint)
  8  	  (print x))
(Pdb) c
1
> /src/hy/test_pdb.hy(18)incr()
-> (breakpoint)))
(Pdb) ll
 16  	  (defn incr [self]
 17  	    (setv self.x (+ 1 self.x))
 18  ->	    (breakpoint)))
(Pdb) p self.x
1
(Pdb) c
> /src/hy/test_pdb.hy(26)<module>()
-> (breakpoint)
(Pdb) ll
  1  	;; test pdb and hy.inspect
  2  	
  3  	(defn foo []
  4  	  (setv x 1)
  5  	  (for [y (range 2)]
  6  	    (print y))
  7  	  (breakpoint)
  8  	  (print x))
  9  	
 10  	(defclass A []
 11  	
 12  	  (setv x 0)
 13  	
 14  	  (defn [property] y [] self.x)
 15  	
 16  	  (defn incr [self]
 17  	    (setv self.x (+ 1 self.x))
 18  	    (breakpoint)))
 19  	
 20  	
 21  	(setv a (A))
 22  	
 23  	(foo)
 24  	
 25  	(a.incr)
 26  ->	(breakpoint)
(Pdb) 
Quitting pdb will kill the process. Quit anyway? [y/n] y

I'm not familiar enough with Hy's import priority to know why it prioritises hy.inspect, but it seems to, and this avoids monkey-patching.

[Kodiologist]

Hmm, I get a different result:

$ hy test_pdb.hy        
0
1
> /tmp/test_pdb.hy(7)foo()
-> (breakpoint)
(Pdb) ll
  3     (defn foo []
(Pdb)

Is it possible that your environment was a bit screwy when you ran that? In Python in general, a danger of giving a module the same name as a standard module, like inspect, is that it can be imported in place of the standard module when you don't expect it. I think that monkey-patching a standard module should have a more reliable effect than trying to shadow the module name.

[atisharma]

It was a clean venv, so that's odd. I'll go the monkey-patch route then.

[Kodiologist]

To be clear, you should rename the module to something else unless you're confident you can keep it from shadowing the standard inspect. Situations where what module you get for a certain name depends on the order of imports are ugly.

[atisharma]

Quite so. Thanks for your help with this.

I have taken the following approach, based on your feedback:

• renamed hy.inspect to hy.hy_inspect (and some rearrangement therein, docstring change etc)
• added a function hy.importer._patch_inspect akin to hy.importer._inject_builtins
• called the patch in hy.__init__.
• flag inspect.__is_patched__.

This 'works for me'.

[atisharma]

Just to add - I don't know if __init__ and hy.importer are the correct places to do this, being unfamiliar with the Hy internals.

[Kodiologist]

Thanks, ll works for me now.

I did a deeper dive into this and accumulated a long list of nitpicks, but one last big-picture issue we should resolve before worrying about that stuff is the use of read. @scauligi Here in hy_inspect.py line 168 there is read(''.join(lines), reader=HyReader(use_current_readers=False)) (which I believe is equivalent to just read(''.join(lines))) called by inspect.getblock. Can this execute arbitrary code, or am I right in thinking that we're using only the standard reader-macro table, which doesn't have a reader macro that would let you do this? This is one advantage of not having a reader-macro equivalent of hy.M.

[atisharma]

If it helps,

=> (import hy.reader.hy_reader [HyReader])
=> (import hy [read])
=> (defreader m 'x)
=>
=> (read "#_(ignored) (symbol #[[string]] #m)" :reader (HyReader :use-current-readers False))
Traceback (most recent call last):
  File "stdin-646bae656e43034675a545ddad37cffa4d69bd84", line 1, in <module>
    (read "#_(ignored) (symbol #[[string]] #m)" :reader (HyReader :use-current-readers False))
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<string>", line 1
    #_(ignored) (symbol #[[string]] #m)
                                     ^^
hy.reader.exceptions.LexException: reader macro '#m' is not defined
=>
=> (read "#_(ignored) (symbol #[[string]] #m)" :reader (HyReader :use-current-readers True))
'(symbol #[[string]] x)

This does raise the question of whether hy.hy_inspect.hy_getblock ought to choke on reader macros, but if they are not to be expanded, I think it has to.

[scauligi]

@Kodiologist correct, nothing in the standard reader table allows for arbitrary code execution

@atisharma I think we might need to use something different than the standard HyReader to parse "raw" hy forms, since ll now blows up in the presence of any user-defined reader macros:

(defreader m
  '(print "hi"))

(defn foo []
  #m
  (breakpoint))
(foo)

^{the half-highlight of breakpoint is pretty amusing}

% hy test_pdb.hy
hi
> test_pdb.hy(8)foo()
-> (breakpoint))
(Pdb) ll
Traceback (most recent call last):
  File "<frozen runpy>", line 287, in run_path
  File "<frozen runpy>", line 98, in _run_module_code
  File "<frozen runpy>", line 88, in _run_code
  File "test_pdb.hy", line 9, in <module>
    (foo)
     ^^^^
  [...]
  File "<string>", line 2
    #m
     ^
hy.reader.exceptions.LexException: reader macro '#m' is not defined

Potentially it might be easier to make a separate reader class? Since we don't actually need most of the machinery of HyReader generating hy models, we just need text extraction.

[Kodiologist]

ll now blows up in the presence of any user-defined reader macros

Isn't that kinda what it should do? We don't want to execute the macro, but we also can't read without executing the macro. Refusing to proceed seems like the right choice.

[scauligi]

Well, blowing up on reader macros now kind of makes this even worse than before, since instead of at least printing a single line with ll you instead get a huge exception dump.

I'd argue we shouldn't be using HyReader in the first place, and we should be using something different/simpler to extract raw text segments. Either that or have a like "raw mode" option in HyReader to skim over reader macros instead of raising a LexException.

[Kodiologist]

I see your point. Is that something we could do separately? This is already a big PR and I'm asking a lot of the author.

[Kodiologist]

I feel like trying to skip reader-macro calls could be a losing game, because after the macro name, there is no guarantee that what follows is anything resembling legal ordinary Hy syntax. All kinds of parse errors could result. But the proof is in the pudding.

[scauligi]

there is no guarantee that what follows is anything resembling legal ordinary Hy syntax.

I think it's fine for these inspect functions to assume "loosely well-formed" Hy; if someone is using custom readers that would mess with their own editor's syntax highlighting, then I think it's reasonable to assume they would understand why the debugger isn't displaying their function properly.

[Kodiologist]

Okay. So we could add something like an option to HyReader, or a subclass of it, that ignores calls to reader macros other than the standard ones, as if they were comments or whitespace.

[atisharma]

I agree that this approach is reasonable because it mirrors of python's inspect. Although, I do feel like we need to be explicit about what types of reader macro can be expected to work and which should not, and document the choice.

I have tried subclassing HyReader and overriding the tag_dispatch method. It is a bit unobvious to do because of how reader_for is implemented and you have to jump through some hoops to get the reader macro table to appear in the subclassed reader. I think it still needs some tidying. Feel free to have at it.

I also would like your views on how to handle the error when you hit a reader macro that doesn't work when you skip over the reader macro identifier. At the moment hy_getblock just kicks the LexError upstairs. Python's getblock tries to handle a couple of typical problems using heuristics, otherwise also kicks the problem upstairs. So my guess is that this is the best approach.

[Kodiologist]

Here's a long list of (mostly) small comments. I've made you do a lot for this PR already, so let me know if any of this is too troublesome and I'll take care of it myself.

1. Add a test for ll.

2. In NEWS, we want to describe changes to Hy in terms of what they mean for users, rather than how Hy changed internally. So, say which inspect functions now work with Hy (and didn't already work in Hy 1.1.0).

3. Code that's copied, translated, or otherwise derived from CPython (or anywhere else, for that matter) should be noted in LICENSE. See the current bottom of the file for an example.

4. Use __init__.py instead of __init__.hy for a blank file.

5. Don't forget to remove all trailing whitespace.

6. "I don't know if __init__ and hy.importer are the correct places to do this, being unfamiliar with the Hy internals." — To me, it seems overcomplicated. You can assign to inspect.getfile etc. directly at the top of level of hy_inspect.py; the assignments to _inspect_getfile etc. are no-ops; and checking that the assignments only happen once is unnecessary because they're idempotent.

7. "Hy class source code requires cPython 3.13" — Are you sure you don't mean "Python" instead of "CPython"? No PyPy exists for 3.13 yet, but I don't see why it wouldn't work, and you're only checking PY3_13, not PYPY. Also, shouldn't the error message start with "Finding"?

8. If you find isExpression and isLazy useful in hy_inspect.py itself, that's fine, but I don't think they should be public: in user code, isinstance(x, hy.models.Expression) seems more straightforward than hy.inspect.isExpression(x). And if they're not public, they don't need tests. In any case, don't assign them into inspect.

9. Just say inspect.getfile in place of from inspect import getfile as py_getfile etc.

10. not hasattr(object, 'start_line') or object.start_line is None can be written getattr(object, 'start_line', None) is None.

11. Try not to shadow built-in Python names (like file) with local variables. Keeping the exact parameter names of the functions you're replacing is probably more important (in case they're called with keyword arguments) but e.g. file isn't a parameter.

12. The line-number counts in fodder_2.hy are noisy, and could create diff noise in the future. Let the text editors take care of line numbering.

13. The files tests/native_tests/inspect.hy and tests/resources/inspect also create modules named inspect (the second one is a directory containing __init__.hy) and hence should probably be named hy_inspect for the same reason as in the original case.

14. The docstring for tests/native_tests/inspect.hy is mostly uninformative. You can just remove it. The bit about translation belongs in the commit message, not the source.

15. The features tested by the xfail tests may never be implemented, so let's not commit them to Hy.

16. Avoid using let where setv will do, and avoid assigning a variable that's used only once. So e.g. (let [path (.joinpath tmp_path "empty_file.hy")] (with [_ (open path "w")])) can be just (with [(open (.joinpath tmp_path "empty_file.hy") "w")]), or better yet, (.write-text (/ tmp_path "empty_file.hy")).

17. In (when PY3_13 ; can't import before 3.12, the comment and the check don't quite match.

18. Combine the two versions of test_getsource_empty_file, putting the PY3_13 checks inside instead.

19. Are test-getfunctions, test_getmodule, or test_getmodule_file_not_found testing any of your code? I thought inspect.getmembers and inspect.getmodule already worked fine with Hy.

20. Similarly, I'm not sure test_getfile or test_getfile_class_without_module are helpful because they don't test Lazy or Expression models, which are the only things hy_inspect.getfile was written to handle specially, right?

21. Expressions like (is None FORM) are better written (is FORM None).

22. Expressions like (. mod.FesteringGob abuse) are better written mod.FestingGob.abuse.

23. Expressions like (with [_ (pytest.raises Error)] …) are better written (with [(pytest.raises Error)] …).

[scauligi]

@atisharma good catch on the inheritance bug! Turns out it's an easy fix; if you patch the following:

--- a/hy/reader/reader.py
+++ b/hy/reader/reader.py
@@ -36,6 +36,9 @@ class ReaderMeta(type):
     def __new__(cls, name, bases, namespace):
         del namespace["reader_for"]
         default_table = {}
+        for base in bases:
+            if hasattr(base, "DEFAULT_TABLE"):
+                default_table |= base.DEFAULT_TABLE
         for method in namespace.values():
             if callable(method) and hasattr(method, "_readers"):
                 default_table.update(method._readers)

then you can vastly simplify HySafeReader:

class HySafeReader(HyReader):
    @reader_for("#")
    def _safe_tag_dispatch(self, key):
        with suppress(LexException):
            return super().tag_dispatch(key)

---

Adding on a few more comments (which I'm also happy to jump in and do):
1. in _patch_inspect, you're checking for __is_patched__ on the builtins module but setting it on the inspect module
2. I'd rename __is_patched__ to something like __hy_patched__ just to err on the side of caution with name clashes
3. also in _patch_inspect, saving the original inspect functions to local variables is unnecessary
(superseded by comment 6 above)
4. I would suggest running black and isort (and also usort or something similar) on hy_inspect.py, to clean up the formatting and unnecessary/redundant imports
5. use_current_readers is already default False, so no need to set it explicitly when instantiating HySafeReaders
6. # `form.source` also carries the source, but as one line. not exactly; it's assigned directly from the input to read so it's exactly ''.join(lines) (I think just remove this comment)
7. you should just rename the test resource files to mod1.hy etc instead of renaming them on import
8. (also just for my own sake please organize the test module imports to be in numerical order)

[atisharma]

Great. I can do most of that over the next day or two. There are a few items I'll leave for you both.

[atisharma]

@Kodiologist

I've had a go at most of these. I have left a few that are subjective, or quicker or easier for you to do it, or where I'm not quite sure what you mean.

1. left
2. left
3. left. Note that the inspect module tests (in this case, only the tests are derivative) is marked public domain. I think a translation of public domain work does not have a licence to append. Though of course attribution is polite. You may want to check.
4. done
5. done
6. Partly done. I don't think the injection is idempotent. If called twice from hy_inspect, or the module is reloaded, you can end up with a situation where hy_inspect calls its own injected functions recursively instead of inspect's native ones. I'll let you iterate further to your taste.
7. done
8. done
9. See 6.
10. done
11. done (just for file, I couldn't see any other instances)
12. left; the line numbers are in python inspect's equivalent test fodder, and though it looks stupid it turned out to be convenient to compare against the editor's line numbers as a guard against accidental line removal.
13. done
14. done
15. removed
16. done
17. done
18. done
19. removed
20. left
21. done
22. left - I don't think it's quite the same thing in this case
23. done

@scauligi

The reader after the fix is indeed much cleaner.

1. oops.
2. done
3. __
4. done
5. done
6. done
7. done
8. done

[Kodiologist]

Turns out that the only meanginful test against fodder_2 is redundant with a test against fodder_1, so I can remove fodder_2 entirely, shaving a good 400 lines from this PR.

[Kodiologist]

Note that the inspect module tests (in this case, only the tests are derivative) is marked public domain. I think a translation of public domain work does not have a licence to append.

I agree with the second sentence, but I can't find the indication of public domain. In https://www.python.org/ftp/python/3.14.2/Python-3.14.2.tar.xz , looking at Lib/test/test_inspect/inspect_fodder.py and Lib/test/test_inspect/inspect_fodder2.py, I don't see anything about licensing; I see only the overall LICENSE, which doesn't indicate that anything is public domain. Am I missing something?

[atisharma]

My mistake; the comment about public domain is the actual inspect module:
https://github.com/python/cpython/blob/84b7e6970f4252ec6b82f50997e880662ec0a146/Lib/inspect.py#L29
So you are probably correct about the tests being PSF.

[Kodiologist]

Okay, I'll take care of it.

[Kodiologist]

I've finished the aforementioned tasks. There's a wrinkle, which is that better testing seems to have revealed some bugs. @atisharma, could you take a look at these test failures? test_pdb_ll is fine on Pythons ≥ 3.13 but appers to fail on ≤ 3.12.

I'll clean up the commit structure later.

[atisharma]

For the test that looks here in fodder-1,

 55 ; Test that getsource works on a line that includes
 56 ; a closing parenthesis with the opening paren being in another line
 57 #(
 58 ) (setv after_closing (fn [] 1))

it's tripping up here because it starts reading at line 58 column 0, which causes a LexError because of the unpaired closing paren. This is quite a good test; I'd normally not encounter it because parinfer would move the closing paren up, but it is of course valid hy.

For this particular case (a code object) we could use PEP 626's co_lines method to get the bytecode offset for the start. But I'm not sure that would help in a similar situation with a class definition, for example, where there is no code object. The alternative is to further abuse the reader to just carry on instead of raising LexError, by overwriting the reader's INVALID method. Which is less pretty but probably robust.

Unless you prefer otherwise, I'll abuse the reader to do this.

[atisharma]

For the ll test, as you point out, it seems like for 3.9, it fails to print the source.

For 3.10, 3.11, 3.12, ll actually works, but pdb thinks it's on l4, so needs a special case:

AAA
> /tmp/ex.hy(4)f()
-> (print (.upper "bbb")))
(Pdb) ll
  1  	(defn f []
  2  	  (print (.upper "aaa"))
  3  	  (breakpoint)
  4  ->	  (print (.upper "bbb")))
(Pdb) quit

For 3.13, 3.14, works as-is.

I'm not sure whether it's worth pursuing 3.9, since it was EOL earlier this year.

[atisharma]

Also I notice hy.compat does some monkey patching of getdoc, which falls back to inspect.getcomments. I don't know whether that logic ought to change.

[Kodiologist]

The alternative is to further abuse the reader to just carry on instead of raising LexError

I suspect that's the way to do it, because there are probably other ways to provoke an error. For example, you could use a reader macro, which, by design, our new safe reader won't parse correctly.

For 3.10, 3.11, 3.12, ll actually works, but pdb thinks it's on l4, so needs a special case

Is the misplaced arrow not a bug?

I'm not sure whether it's worth pursuing 3.9, since it was EOL earlier this year.

Yeah, you can just add a test skip for Python 3.9 if you want. We're dropping support for that as soon as it becomes more than slightly inconvenient.

Also I notice hy.compat does some monkey patching of getdoc

It might no longer be necessary.

[atisharma]

Cool. I pushed the reader change.

Probably the l4 arrow is a bug, but I don't think that it's in hy_inspect. I think the line number is generated from the bytecode object. The implementation was overhauled in 3.13.

[Kodiologist]

Did you commit the wrong thing? All I see is a whitespace change.

[atisharma]

I did, sorry. Parinfer automatically messes with the whitespace.

[Kodiologist]

Thanks.

Probably the l4 arrow is a bug, but I don't think that it's in hy_inspect. I think the line number is generated from the bytecode object. The implementation was overhauled in 3.13.

You're probably right. I'll have to conditionally xfail the test for now.

[Kodiologist]

@scauligi I'd like to clean up the commits and then merge this. Is that okay by you?

[atisharma]

Ah, while you were doing that I captured the pre-3.13 output and rewrote the test to handle that case, so it passes. Do you want me to push it?

[Kodiologist]

No, that output appears to be wrong; I think it would be counterproductive to test for it.

[scauligi]

@scauligi I'd like to clean up the commits and then merge this. Is that okay by you?

lemme give it one more quick once-over today, but otherwise I'm ideologically unopposed 👍

[scauligi]

Lazy or Expression object

doesn't seem to be true; passing in the output of hy.read or hy.read-all throws AttributeError: 'NoneType' object has no attribute 'endswith' (from inspect.getsourcefile below).

---

in any case, is there a reason we need this to work on models? I'd expect inspect functions to only be called on (compiled) objects (@atisharma ?)

[atisharma]

I pushed a fix. It's maybe useful when you want to read a file and you've set the filename.

[atisharma]

I slept on this and realised you're right - there's not a great need to support Lazy or Expression. Removing that path doesn't break any tests. Still, it's there and is not much code, and can always be removed if it causes problems later.

[scauligi]

aight that's everything I had, @Kodiologist I assume you're handling the fixup/squash and rebase?

thanks for the contribution @atisharma !

[Kodiologist]

I will, yes.

[atisharma]

That you both very much for your help. I am pleased to be able to give something back. It should help some with the tooling situation.

[Kodiologist]

All done, and it only took 3 PhDs to do it. Thanks, guys.