Checker python-ruff fails for Python buffers not visiting a file.

[joostkremers]

Bug description

Support for the Python checker ruff was added in #1974. Unfortunately, the way the checker is defined it cannot handle Python buffers that are not visiting a file. When I edit a Python source block in an Org file, I consistently get the following error:

Suspicious state from syntax checker python-ruff: Flycheck checker python-ruff returned 2, but its output
contained no errors: error: a value is required for '--stdin-filename <STDIN_FILENAME>' but none was supplied

This appears to be due to the definition of the checker python-flycheck, which has the following:

:command ("ruff"
            "check"
            (config-file "--config" flycheck-python-ruff-config)
            "--output-format=text"
            "--stdin-filename" source-original
            "-")

Specifically, the option ---stdin-filename to ruff requires a filename, but source-original is replaced with an empty string if the source buffer is not visiting a file, which is the case when editing an Org source block (i.e., with C-c ').

Steps to reproduce

Steps to reproduce the behavior:

1. Set up Emacs to use ruff in Python source buffers (I use eglot with python-lsp-server and python-lsp-ruff).
2. Create an Org file with a Python source block.
3. Edit the block with C-c '.
4. See error.

It is possible to pass code to ruff via stdin that's not associated with any file. Ruff will not apply any fixes, but it will report errors, so it should be usable in Org source buffers. I just don't know the best way to define the checker so that this works and other use cases aren't affected.

System configuration

Syntax checkers for buffer *Org Src titanic.org[ jupyter-python ]* in python-ts-mode:

First checker to run:

  python-ruff
    - may enable:         yes
    - executable:         Found at /usr/bin/ruff
    - configuration file: Not found
    - next checkers:      python-mypy

Checkers that could run if selected:

  python-pycompile  select
    - may enable:    yes
    - executable:    Found at /home/joost/.cache/pypoetry/virtualenvs/kaggle-MP1dYBDK-py3.11/bin/python3
    - next checkers: python-mypy

Checkers that are compatible with this mode, but will not run until properly configured:

  eglot-check
    - may enable: yes
    - may run:    nil

  python-flake8 (automatically disabled) reset
    - may enable:         no
    - executable:         Found at /home/joost/.cache/pypoetry/virtualenvs/kaggle-MP1dYBDK-py3.11/bin/python3
    - configuration file: Not found
    - `flake8' module:    Missing; sys.path is ['/home/joost/.pyenv/versions/3.11.7/lib/python311.zip', '/home/joost/.pyenv/versions/3.11.7/lib/python3.11', '/home/joost/.pyenv/versions/3.11.7/lib/python3.11/lib-dynload', '/home/joost/.cache/pypoetry/virtualenvs/kaggle-MP1dYBDK-py3.11/lib/python3.11/site-packages']
    - next checkers:      python-pylint, python-mypy

  python-pylint (automatically disabled) reset
    - may enable:         no
    - executable:         Found at /home/joost/.cache/pypoetry/virtualenvs/kaggle-MP1dYBDK-py3.11/bin/python3
    - configuration file: Not found
    - `pylint' module:    Missing; sys.path is ['/home/joost/.pyenv/versions/3.11.7/lib/python311.zip', '/home/joost/.pyenv/versions/3.11.7/lib/python3.11', '/home/joost/.pyenv/versions/3.11.7/lib/python3.11/lib-dynload', '/home/joost/.cache/pypoetry/virtualenvs/kaggle-MP1dYBDK-py3.11/lib/python3.11/site-packages']
    - next checkers:      python-mypy

  python-pyright (automatically disabled) reset
    - may enable: no
    - executable: Not found

  python-mypy (automatically disabled) reset
    - may enable:         no
    - may run:            nil
    - executable:         Not found
    - configuration file: Not found

Flycheck Mode is enabled. Use C-u C-c ! x to enable disabled checkers.

--------------------

Flycheck version: 20240229.628
Emacs version:    29.2
System:           x86_64-pc-linux-gnu
Window system:    pgtk

Emacs configuration:

• Plain Emacs / Custom configuration
• Spacemacs
• Doom Emacs
• Other shared configuration

Emacs 29.1, flycheck installed from Melpa (20240229.628).

[bbatsov]

It is possible to pass code to ruff via stdin that's not associated with any file. Ruff will not apply any fixes, but it will report errors, so it should be usable in Org source buffers. I just don't know the best way to define the checker so that this works and other use cases aren't affected.

Perhaps we can make this some configuration option or something contingent on the presence of an actual python source buffer?

[joostkremers]

As a test, I replaced "--stdin-filename" source-original with "--stdin-filename" source, since the doc string of flycheck-substitute-argument says that source "create[s] a temporary file and returns its path", which seems exactly what's needed. It also says that source is "the preferred way to pass the input file to a syntax checker", which to me sounds like it is in fact not wrong to use it for checking buffers that visit a file.

I've made the change locally and so far everything seems to work. Of course, I don't know what the original motivation was for using source-original, so it's possible I'm missing something. I will certainly report back if I run into any issues.

[cpitclaudel]

flycheck-substitute-argument says that source "create[s] a temporary file and returns its path", which seems exactly what's needed.

I don't think so, see below

I've made the change locally and so far everything seems to work.

It will (or at least should?) break imports. The --stdin-filename flag is intended to indicate what file (on disk) the file contents sent on the stdin come from, to help resolve imports. Changing it to source will create a temporary copy of the file and pass its name to the checker, which won't be very useful.

(source is very useful for checkers that don't support stdin)

[joostkremers]

It will (or at least should?) break imports.

I haven't seen any issues yet. Which is no guarantee that there won't be any, of course.

Looking at the doc string of flycheck-substitute-argument a bit more, though, the symbol to use in that case would seem to be source-inplace, not source-original:

     ‘source’ is the preferred way to pass the input file to a
     syntax checker.  ‘source-inplace’ should only be used if the
     syntax checker needs other files from the source directory,
     such as include files in C.

Regarding source-original, the doc string says:

      [...] Do not use this
      as primary input to a checker, unless absolutely necessary.

and also:

     When using this symbol as primary input to the syntax
     checker, add ‘flycheck-buffer-saved-p’ to the ‘:predicate’.

which seems to suggest that the best way would be to define two ruff checkers, one for buffers backed by a file and one for buffers not backed by a file, and to use :predicate to make sure only one is actually used.

Anyway, I'm no expert by any means on flycheck or on ruff, so I'm not the person to say which solution should be chosen.

[cpitclaudel]

Do not use this as primary input to a checker
When using this symbol as primary input to the syntax

This is the important part: the symbol isn't being used as the primary input here. (the primary input is stdin)

[cpitclaudel]

Can you try replacing "--stdin-filename" source-original with (option "--stdin-filename" buffer-file-name)?

[joostkremers]

Can you try replacing "--stdin-filename" source-original with (option "--stdin-filename" buffer-file-name)?

Sure. Just tried it, but I don't see any difference.

To be clear, I've now tried four different ways to define the checker:

1. "--stdin-filename" source-original
2. "--stdin-filename" source
3. "--stdin-filename" source-inplace
4. (option "--stdin-filename" buffer-file-name)

I've tried all four by opening a Python source file and by opening an Org file containing Python source blocks and editing a source block with C-c '.

Option 1, which is the original definition, gives the error above in Org source blocks. The other three options all appear to behave the same: I experience no issues in a Python source buffer (imports, even local ones, are recognised correctly), and in Org source blocks the checker works as well.

The only issue in Org source blocks is that identifiers that are defined in an earlier source block are not recognised in later source blocks, even though all source blocks are evaluated in the same session. But that's to be expected, I assume. (Ruff doesn't have access to the Python session in which the source blocks are evaluated, of course. It can only look at the source block itself.)

[cpitclaudel]

Perfect, if number 4 works then that's what we should use. Thanks a lot for testing.

(The differences with the others are subtle: they depend on exactly what ruff does with stdin-filename, but broadly speaking they will give it incorrect information (either a temp file in the wrong directory, or a temp file in the same directory but with the wrong name)

[cpitclaudel]

Sorry, didn't mean to close

[cpitclaudel]

Perhaps we should do the same thing for other checkers that depend on an stdin filename. I suspect that the same issue would pop up in most of them.

[joostkremers]

Actually, I just realised I do see a difference between options 2 & 3 on the one hand and option 4 (your suggestion) on the other: with option 4, ruff doesn't seem to provide line/column positions for the errors it finds: if an error is found, Emacs just underlines the first line in the buffer, not the character(s) where the error actually occurs.

With options 2 & 3 OTOH the errors are indicated where they occur.

Edit: I'm talking about Org source buffers here, in case that wasn't clear...

[cpitclaudel]

Right, you'd need to add

  :error-filter
  (lambda (errors)
    (flycheck-sanitize-errors
     (flycheck-remove-error-file-names "-" errors)))

Maybe that too should be the default when using :standard-input t

[cpitclaudel]

This works for me:

(flycheck-define-checker python-ruff
  "A Python syntax and style checker using the ruff.
To override the path to the ruff executable, set
`flycheck-python-ruff-executable'.

See URL `https://beta.ruff.rs/docs/'."
  :command ("ruff"
            "check"
            (config-file "--config" flycheck-python-ruff-config)
            "--output-format=text"
            (option "--stdin-filename" buffer-file-name)
            "-")
  :standard-input t
  :error-filter (lambda (errors)
                  (seq-map #'flycheck-flake8-fix-error-level
                           (flycheck-sanitize-errors
                            (flycheck-remove-error-file-names "-" errors))))
  :error-patterns
  ((warning line-start
            (file-name) ":" line ":" (optional column ":") " "
            (id (one-or-more (any alpha)) (one-or-more digit)) " "
            (message (one-or-more not-newline))
            line-end))
  :modes (python-mode python-ts-mode)
  :next-checkers ((warning . python-mypy)))

I tested using a buffer without a file name in which I explicitly enabled python-mode. Can you confirm on your side?

[joostkremers]

Can you confirm on your side?

Yup. I don't know how you did it, but it works. 😉 Thanks!

[cpitclaudel]

Yup.

Thanks a lot!

I don't know how you did it, but it works.

I ran ruff on the command line. If you don't pass it a filename, it prints -, which Flycheck doesn't treat specially.

We should patch this, sorry for the trouble you ran into. We should also review the other checkers that use stdin; my hunch is that almost all of them will have this exact same issue.

[joostkremers]

BTW, while I'm here, allow me to nitpick a bit. The doc string is this:

  "A Python syntax and style checker using the ruff.
To override the path to the ruff executable, set
`flycheck-python-ruff-executable'.

See URL `https://beta.ruff.rs/docs/'."

Ruff's documentation capitalises the name and doesn't use the definite article, so it should probably read:

  "A Python syntax and style checker using Ruff.
To override the path to the Ruff executable, set
`flycheck-python-ruff-executable'.

See URL `https://beta.ruff.rs/docs/'."

[joostkremers]

[...] sorry for the trouble you ran into.

Not at all! That's how the process works. 🙂 Thanks for all the work you put into flycheck!

[cpitclaudel]

#2065