Support for Python 3.12-style f-strings

[charliermarsh]

Great project!

I saw that you moved your Python parsing into Rust, and there's one limitation I want to bring to your attention. At a high level, the problem is that by using a third-party parser, you're not guaranteed to support parsing all valid Python programs for the current Python version (i.e., whatever version the user is running), since the parser may not support all syntax.

In this case, the acute limitation is that the RustPython parser doesn't support the revised f-string syntax from Python 3.12. So any projects that use Python 3.12-style f-strings won't be analyzable by Deptry right now.

To reproduce, create a file like foo/__init__.py, with f"abc{"def"}". Then run deptry foo:

❯ deptry foo
Scanning 1 file...
Traceback (most recent call last):
  File "/Users/crmarsh/.local/share/rtx/installs/python/3.12.1/bin/deptry", line 8, in <module>
    sys.exit(deptry())
             ^^^^^^^^
  File "/Users/crmarsh/.local/share/rtx/installs/python/3.12.1/lib/python3.12/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/crmarsh/.local/share/rtx/installs/python/3.12.1/lib/python3.12/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/Users/crmarsh/.local/share/rtx/installs/python/3.12.1/lib/python3.12/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/crmarsh/.local/share/rtx/installs/python/3.12.1/lib/python3.12/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/crmarsh/.local/share/rtx/installs/python/3.12.1/lib/python3.12/site-packages/deptry/cli.py", line 356, in deptry
    ).run()
      ^^^^^
  File "/Users/crmarsh/.local/share/rtx/installs/python/3.12.1/lib/python3.12/site-packages/deptry/core.py", line 83, in run
    for module, locations in get_imported_modules_from_list_of_files(all_python_files).items()
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/crmarsh/.local/share/rtx/installs/python/3.12.1/lib/python3.12/site-packages/deptry/imports/extract.py", line 28, in get_imported_modules_from_list_of_files
    rust_result = get_imports_from_py_files(py_files)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
SyntaxError: Error parsing file foo/__init__.py: SyntaxError: Error parsing file foo/__init__.py: invalid syntax. Got unexpected token 'def' at byte offset 7

We (Astral) did add support for type statements to the RustPython parser (RustPython/Parser#97), so it does support PEP 695, which was another big syntax change in Python 3.12. But we've since forked the parser. Our updates to Ruff's parser to support Python 3.12-style f-strings were never backported to RustPython, and our parser and AST have diverged a lot more since then.

(This genuinely isn't intended to be an advertisement for Ruff's parser. In fact, while you're obviously welcome to use it, we don't publish it to crates.io and we don't make any guarantees around its public API / stability right now, so having users that depend on it is actually not totally desirable for us. I just wanted you to be aware of some of the limitations that come with using a non-CPython parser.)

[mkniewallner]

Hi Charlie, nice to see you there 🙂

I must say that this is not something we were aware of, so thanks a lot for taking the time to write this heads up, this is indeed an important limitation to be aware of.

In fact, while you're obviously welcome to use it, we don't publish it to crates.io and we don't make any guarantees around its public API / stability right now, so having users that depend on it is actually not totally desirable for us. I just wanted you to be aware of some of the limitations that come with using a non-CPython parser.

The idea of supporting new Python 3.12 syntax (and future changes in newer versions) by switching to Ruff's parser instead of having syntax errors, even if this means having breaking changes to handle from time to time, looks like a good deal to me.

Given that we are mostly interested in imports and their locations when parsing the AST, the rate of breaking changes in the API might be something we could accommodate.

Do you know other projects that use Ruff's parser instead of RustPython one? And since you mention some reservation about having projects depend on Ruff's parser (I guess mostly because you don't want to commit on a public API, as this is intended to be use Ruff, which is perfectly understandable), how would you feel if deptry was to switch to Ruff's parser to support newer Python syntax?

[fpgmaas]

Thanks @charliermarsh for raising this issue and for your clear explanation.

I agree with @mkniewallner that this project could definitely benefit by switching to ruff's internal parser, with the obvious note that we would take responsibility for that and that, won't expect a stable API, and we would simply deal with any future breaking changes as they come. Give the limited scope of deptry I think that sounds doable.

I created a quick draft PR to see what the impact currently would be, and that is very limited.

Curious for your thoughts on this :)

[charliermarsh]

I think it's totally reasonable for you all to use Ruff's parser. It would obviously be a shame for us to invest all this effort in building a greater parser, only for other projects to have no way to benefit from it. I mostly just want to be clear in setting expectations around it, i.e.:

1. It'd need to be pulled in as a Git dependency, as in your PR, since we aren't planning to publish it to crates.io yet. (I assume we will publish it to crates.io some day when we feel we can adhere to SemVer.)
2. We just can't make any guarantees around the stability of the API. We generally develop those internal libraries with ourselves as the sole consumer in mind, and so you likely will experience breaking changes when you upgrade. Honestly, I wouldn't expect it to be that bad in practice, especially since you need such a narrow subset of the AST anyway, but I need to reserve the right for it to be fairly annoying, just in case... (Relatedly, we're in the process of rewriting the parser entirely. The API and AST will be similar, but the internals will be completely rewritten. So that may affect you in some way whenever you choose to bump your version.)

[charliermarsh]

In short, I would feel okay with it (and, in fact, glad that we could help out the project), just don't hate me when we ship breaking changes.

[mkniewallner]

Thank you for the details, we acknowledge that there could be breaking changes, and fully understand why, so we obviously won't hate you when that happens, especially since we will depend on the parser for free. Thanks for your hard work maintaining the parser 🙂