Checker `python-ruff` fails for Python buffers not visiting a file. about flycheck HOT 18 OPEN

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.

from flycheck.

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?

from flycheck.

#2065

from flycheck.

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?

from flycheck.

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.

from flycheck.

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)

from flycheck.

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.

from flycheck.

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)

from flycheck.

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

from flycheck.

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.)

from flycheck.

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)

from flycheck.

Sorry, didn't mean to close

from flycheck.

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...

from flycheck.

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

from flycheck.

Can you confirm on your side?

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

from flycheck.

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.

from flycheck.

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/'."

from flycheck.

[...] 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!

from flycheck.