[VS Code extension] Ruff doesn't fix I001 completely on save

Question

[VS Code extension] Ruff doesn't fix I001 completely on save

ReinforcedKnowledge opened this issue 3 months ago · 6 comments

ReinforcedKnowledge commented 3 months ago

Hi!

Issue

I have noticed that the Ruff VSCode extension doesn't fix imports I001 completely on save. It does fix it partially, fixes other issues , so I'm sure the extension is working properly, but doesn't fix I001 completely.

Context

I have tried a bunch of different settings with two different operating systems and I have observed the same issue. Otherwise, I tried to keep almost the same environment.

Operating systems

macOS Sequoia 15.0
Ubuntu LTS 24.04.1 LTS

Python versions

Python 3.12.6 and 3.12.3

Ruff versions

Ruff version 0.6.8

.vscode/settings.json

{
    "[python]": {
        "editor.formatOnSave": true,
        "editor.codeActionsOnSave": {
            "source.fixAll": "explicit",
            "source.organizeImports": "explicit"
        },
        "editor.defaultFormatter": "charliermarsh.ruff",
    },
    "python.defaultInterpreterPath": ".venv/bin/python",
    "ruff.enable": true,
    "ruff.configuration": "${workspaceFolder}/ruff.toml",
    "ruff.fixAll": true,
    "ruff.organizeImports": true,
    "python.analysis.autoImportCompletions": true,
}

I have also tried combining together all of the following options:

"ruff.nativeServer": "on",
"ruff.configuration": "${workspaceFolder}/ruff.toml",

with and without:

    "[python]": {
        "editor.formatOnSave": true,
        "editor.codeActionsOnSave": {
            "source.fixAll": "always",
            "source.organizeImports": "always"
        },
        "editor.defaultFormatter": "charliermarsh.ruff",
    },

with and without:

"ruff.organizeImports": true,
"ruff.fixAll": true,

I have done the same but with the Python language server instead of the native one, with and without the following settings:

    "ruff.lint.args": [
        "--config=${workspaceFolder}/ruff.toml", 
    ],

and:

    "ruff.format.args": [
        "--config=${workspaceFolder}/ruff.toml", 
    ],

I have tried all the options that make sense (not usign ruff.lint.args for example with the Rust based language server) in a combinatorial manner and still noticed the same issue.

ruff.toml

line-length  = 79
indent-width = 4
target-version = "py311"

exclude = [
    "*.venv/*",
    ".cache",
    ".mypy_cache",
    ".ruff_cache",
]


[lint]
    select = [
        "E",      # pycodestyle
        "F",      # pyflakes
        "I",      # isort
        "N",      # pep8-naming
        "T20",    # print
        "S",      # bandit
        "PL",     # pylint
        "D",      # pydocstyle
        "SIM",    # flake8-simplify
        "C90",    # mccabe (keep is simple)
        "ASYNC",  # flake8-async
        "ARG001", # unused-function-argument
        "ARG001", # unused-method-argument
        "ERA001", # commented-out-code
        "TID252", # absolute imports
    ]
    ignore = [
        "D100", # missing-docstring in module
        "D101", # missing-docstring in class
        "D104", # missing-docstring in public package
        "D106", # missing-docstring in nested class
        "D107", # missing-docstring in __init__,
        "D213", # multi-line-summary-first-line -> replaced by D212
        "D203", # 1 blank line required before class docstring
        "PLR0913"
    ]
    fixable = ["ALL"]
    unfixable = ["F401"] # disable: imported but unused

    [lint.isort]
        force-wrap-aliases   = true
        combine-as-imports   = true
        length-sort-straight = true
        lines-between-types  = 1

    [lint.flake8-tidy-imports]
    # Disallow all relative imports.
    ban-relative-imports = "all"

[format]
    quote-style                = "double"
    indent-style               = "space"
    skip-magic-trailing-comma  = false
    line-ending                = "auto"
    docstring-code-format      = true
    docstring-code-line-length = "dynamic"

[lint.per-file-ignores]
    "test_*" = [
        "S101", # allow assert
        "D102", # missing-docstring in class
        "D103", # missing-docstring in function
    ]

Example code:

import math
from datetime import date


def calculate_circle_area(radius):
    return math.pi * math.pow(radius, 2)


def get_today_date():
    today = date.today()
    return today.strftime("%B %d, %Y")

Reported problem

Import block is un-sorted or un-formatted Ruff(I001) [Ln 1, Col 1]

General information

I said it doesn't fix I001 completely because it still tries to apply some fix like with standard libraries. On save, it turns the following:

import math
from datetime import date
import os

Into:

import math
import os
from datetime import date

But not into:

import os
import math

from datetime import date

Running ruff check myfile.py --fix does fix the I001.

I have installed isort to try and check its behavior and running isort myfile.py doesn't fix the 001.

Answer 1 · 2024-10-04T13:24:40.000Z

Can you please help me understand the issue a bit? I'm confused.

General information

I said it doesn't fix I001 completely because it still tries to apply some fix like with standard libraries. On save, it turns the following:
import math
from datetime import date
import os
Into:
import math
import os
from datetime import date
But not into:
import os
import math

from datetime import date
Running ruff check myfile.py --fix does fix the I001.

Is the expectation that Ruff should organize the imports as per the final code block but is doing so as the one in the middle? I think what Ruff is doing is correct because all three imports are from standard library.

Answer 2 · 2024-10-04T13:52:37.000Z

Yes indeed, it should organize the imports as per the final block, though I'd prefer not to say correct since the topic relates more to a subjective experience.

In this block that I get by saving the file:

import math
import os
from datetime import date

We should have the os import first. Then it should put one line between the direct import and the from import.

This is further supported by the the fact that even after saving and having those imports "misorganized", Ruff still reports Import block is un-sorted or un-formatted Ruff(I001) [Ln 1, Col 1].

And doing ruff check myfile.py --fix sorts the imports "correctly" into:

import os
import math

from datetime import date

To be honest I couldn't care less about how the imports are organized but I wanted to report the difference in the sorting behavior between saving the files and running the command.

While it would be interesting to pinpoint the source of the problem just for the sake of knowing, it can be a bit frustrating in some cases.
For instance, if we assume that the way imports are organized with --fix is the correct one, it becomes annoying when Ruff automatically adjusts them on save in a different way that doesn’t pass a strict CI job checking for I001 in its linting rules. In such cases, it might be better to disable Ruff's auto-fixing of imports on save and instead run the command before pushing code.

Answer 3 · 2024-10-04T14:00:21.000Z

I want to add a quick follow-up comment (feel free to let me know if this isn't the right way to do it) to keep my initial response separate from this new observation.

The following settings.json work as intended.

{
    "[python]": {
        "editor.formatOnSave": true,
        "editor.codeActionsOnSave": {
            "source.fixAll": "explicit",
            "source.organizeImports": "never"
        },
        "editor.defaultFormatter": "charliermarsh.ruff",
    },
    "python.defaultInterpreterPath": ".venv/bin/python",
    "ruff.nativeServer": "on",
    "ruff.enable": true,
    "ruff.configuration": "${workspaceFolder}/ruff.toml",
    "python.analysis.autoImportCompletions": true,
}

I don't know why turning off the "source.organizeImports" at the editor level lets Ruff fix the imports as it'd have done when running the command but maybe it can lead you to something.

Let me know if you want to check the user settings as well, though they're overwritten by the workspace settings so I'm not sure that'll help.

Answer 4 · 2024-10-07T11:58:49.000Z

We should have the os import first. Then it should put one line between the direct import and the from import.

Do you have a Ruff config where you've updated the [tool.ruff.isort] section to organize it in the way that you've described? Because, by default, Ruff will organize it as per the second code block mentioned in my previous comment: https://play.ruff.rs/711e918f-aae6-4a62-82f7-39363473855c

And doing ruff check myfile.py --fix sorts the imports "correctly" into:
import os
import math

from datetime import date

So, there must be some configuration under [tool.ruff.isort] that's doing this and is not being picked up by the editor extension.

I don't know why turning off the "source.organizeImports" at the editor level lets Ruff fix the imports as it'd have done when running the command but maybe it can lead you to something.

Huh, I'm confused now. With source.organizeImports turned off, it must be some other extension that's organizing the imports.

A few suggestions to try:

Remove the ruff.configuration setting because it should be picked up automatically as it's in the current workspace.
Use source.organizeImports.ruff to explicitly use the imports organizer from Ruff
Do you have any other Python extension installed that might sort your imports like the isort extension?

Answer 5 · 2024-10-07T14:10:00.000Z

Hi!

Thank you for your response.

My bad I think my issue title is not totally clear as for the issue. I do indeed have a ruff.toml as provided in the first comment of the issue and the action on save is not respecting the isort rule though it's detected by ruff (thus the reported I001).

I think I figured out where the issue comes from.

Why the on-save sorting behaviour was different from running the command? Because in my user settings I had "ruff.organizeImports": false while I also had the isort extension and "source.organizeImports": "explicit". This made Ruff report the "I" error according to the linting rule I specified, but unable to solve it because I disabled its ability to organize imports, and isort would sort the imports in a way different from my ruff.toml.

Why setting "source.organizeImports": "never" seemed to provide the intended behaviour? Actually Ruff did fix the "I001" issue, due to it being in the linting rules and having "source.fixAll": "explicit" but since isort comes after the fix to organize the imports it seemed like nothing happened. But disabling the import sorting allowed for only the linting fixes to show hence why it appeared like Ruff sorted the imports though we disabled that option.

What I would recommend:

Check your user and global settings and if there is any keys that's not being overwritten in your workspace settings. Mine was ruff.organizeImports.
Check if you have rules that affect the behaviour described by that set of keys. Mine were a bunch of rules around how to sort imports.
Check if you have any extension that does the same thing but works when Ruff is disabled and that doesn't use your configuration files. Mine was isort.

My particular fix:

Removed the "ruff.organizeImports": false
Kept the workspace' settings.json as in the Ruff's extension documentation
Kept only the Ruff extension

Just in case anyone wondered was wondering what my ruff.tomlwas:

line-length  = 79
indent-width = 4
target-version = "py311"

exclude = [
    "*.venv/*",
    ".cache",
    ".mypy_cache",
    ".ruff_cache",
]


[lint]
    select = [
        "E",      # pycodestyle
        "F",      # pyflakes
        "I",      # isort
        "N",      # pep8-naming
        "T20",    # print
        "S",      # bandit
        "PL",     # pylint
        "D",      # pydocstyle
        "SIM",    # flake8-simplify
        "C90",    # mccabe (keep is simple)
        "ASYNC",  # flake8-async
        "ARG001", # unused-function-argument
        "ARG001", # unused-method-argument
        "ERA001", # commented-out-code
        "TID252", # absolute imports
    ]
    ignore = [
        "D100", # missing-docstring in module
        "D101", # missing-docstring in class
        "D104", # missing-docstring in public package
        "D106", # missing-docstring in nested class
        "D107", # missing-docstring in __init__,
        "D213", # multi-line-summary-first-line -> replaced by D212
        "D203", # 1 blank line required before class docstring
        "PLR0913"
    ]
    fixable = ["ALL"]
    unfixable = ["F401"] # disable: imported but unused

    [lint.isort]
        force-wrap-aliases   = true
        combine-as-imports   = true
        length-sort-straight = true
        lines-between-types  = 1

    [lint.flake8-tidy-imports]
    # Disallow all relative imports.
    ban-relative-imports = "all"

[format]
    quote-style                = "double"
    indent-style               = "space"
    skip-magic-trailing-comma  = false
    line-ending                = "auto"
    docstring-code-format      = true
    docstring-code-line-length = "dynamic"

[lint.per-file-ignores]
    "test_*" = [
        "S101", # allow assert
        "D102", # missing-docstring in class
        "D103", # missing-docstring in function
    ]

Answer 6 · 2024-10-08T04:30:15.000Z

Thanks for providing a thorough analysis on your end!