Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lint.per-file-ignores option is ignored by native server in ruff.toml #14282

Closed
SXHRYU opened this issue Nov 11, 2024 · 12 comments · Fixed by #14352
Closed

lint.per-file-ignores option is ignored by native server in ruff.toml #14282

SXHRYU opened this issue Nov 11, 2024 · 12 comments · Fixed by #14352
Labels
bug Something isn't working server Related to the LSP server

Comments

@SXHRYU
Copy link

SXHRYU commented Nov 11, 2024

Basically it's the same issue as #11751 but the problem still occurs when using ruff.toml as config file.

jic: this happens in VS Code, when ruff checking via console everything's okay.

@AlexWaygood AlexWaygood added configuration Related to settings and configuration server Related to the LSP server labels Nov 11, 2024
@MichaReiser MichaReiser added needs-mre Needs more information for reproduction and removed configuration Related to settings and configuration labels Nov 12, 2024
@MichaReiser
Copy link
Member

Could you share a small reproduction? How does the directory layout look like? What files do you open in VS code.

I'm asking because I just tried what you described and VS Code correctly shows or doesn't show diagnostic based on the per-file-ignores in the ruff.toml

@amoralesc
Copy link

amoralesc commented Nov 14, 2024

@MichaReiser I'm experiencing the same problem. I might add that I'm using a custom location for my Ruff config file, setting it under .config/.ruff.toml. Also working on Dev Containers.

I want to ignore the pydocstyle linter on all files under src/migrations/versions. Here's my environment (simplified) and steps to reproduce:

Versions

  • ruff: 0.7.3
  • charliermarsh.ruff extension: latest
  • VSCode: 1.95.2

Project structure:

.
├─ .config
│  └─ .ruff.toml
├─ .venv
└─ src
   └─ migrations
      └─ versions
         └─ 1e8f6175aeef_a.py # <- file with errors

VSCode settings:

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

Ruff config file:

target-version = "py312"
line-length    = 80
indent-width   = 4

[lint]
select = [
  "D"
]

[lint.per-file-ignores]
"**/versions/*.py" = [
  "D",
]

[lint.pydocstyle]
convention = "numpy"

The problematic file in question:

"""Test

Revision ID: 1e8f6175aeef
Revises:
Create Date: 2024-11-14 02:42:01.465193

"""

from collections.abc import Sequence

# revision identifiers, used by Alembic.
revision: str = "1e8f6175aeef"
down_revision: str | None = None
branch_labels: str | Sequence[str] | None = None
depends_on: str | Sequence[str] | None = None


def upgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    pass
    # ### end Alembic commands ###


def downgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    pass
    # ### end Alembic commands ###

When the file is on focus:

image

Running ruff check from the project root:

ruff check --config .config/.ruff.toml .
All checks passed!

For good measure, commenting out the [lint.per-file-ignores] section and running ruff again returns:

src/migrations/versions/1e8f6175aeef_a.py:1:1: D400 First line should end with a period
  |
1 | / """Test
2 | | 
3 | | Revision ID: 1e8f6175aeef
4 | | Revises:
5 | | Create Date: 2024-11-14 02:42:01.465193
6 | | 
7 | | """
  | |___^ D400
8 |   
9 |   from collections.abc import Sequence
  |
  = help: Add period

src/migrations/versions/1e8f6175aeef_a.py:18:5: D103 Missing docstring in public function
   |
18 | def upgrade() -> None:
   |     ^^^^^^^ D103
19 |     # ### commands auto generated by Alembic - please adjust! ###
20 |     pass
   |

src/migrations/versions/1e8f6175aeef_a.py:24:5: D103 Missing docstring in public function
   |
24 | def downgrade() -> None:
   |     ^^^^^^^^^ D103
25 |     # ### commands auto generated by Alembic - please adjust! ###
26 |     pass
   |

Found 3 errors.
No fixes available (1 hidden fix can be enabled with the `--unsafe-fixes` option).

@amoralesc
Copy link

amoralesc commented Nov 14, 2024

Same behavior with exclude = [] on the Ruff config file.

Temporary workaround: use ruff.exclude in the VSCode settings.json. From the docs.

@dhruvmanila
Copy link
Member

@amoralesc Do you mind trying it outside the Dev Container? There have been a couple of issues when working inside a dev container and I want to find some time to look into those soon.

@amoralesc
Copy link

@dhruvmanila I can indeed reproduce this error outside of the dev container. Tested with extend-exclude and [lint.per-file-ignores].

My local environment is WSL. Worth noting that VSCode through WSL is still a remote server.

@SXHRYU Can you confirm how does your environment look like?

@dhruvmanila
Copy link
Member

Ok, thanks for confirming. Although I don't have a Windows machine I can try reproducing on a Mac.

@dhruvmanila
Copy link
Member

@amoralesc I'm looking at this now but just want to point out a couple of things:

  "ruff.interpreter": ["./.venv/bin/python"],

I don't think this will work (astral-sh/ruff-vscode#553), you'll need to update it to:

  "ruff.interpreter": ["${workspaceFolder}/.venv/bin/python"],

But, otherwise I can reproduce this on my end although I'm not sure why this is happening.

@amoralesc
Copy link

@dhruvmanila Yes, my bad there. I wanted to generalize the reproducible environment and totally forgot that dot access doesn't work in ruff.interpreter. I'm actually using ${containerWorkspaceFolder} there as well as in ruff.configuration. I'm gonna update the original snippet.

I may also add that the behavior is rather strange. Let's call the the problematic file file.py, deeply nested at /path/to/file.py. If I exclude the file like this:

# .config/.ruff.toml

# either like:
exclude = ["**/file.py"]
# or:
[lint.per-file-ignores]
"**/file.py" = ["D"]

That actually works correctly! This is not the ideal solution though, because I'd want to ignore everything under path/to. So when the pattern matching is changed to ./path/to/*.py, **/path/to/*.py or **/to/*.py, it fails.

@MichaReiser
Copy link
Member

MichaReiser commented Nov 14, 2024

@dhruvmanila and I narrowed the problem down to that patterns in .config/ruff.toml are resolved relative to the configuration directory, which I think is surprising. So changing per-file-ignores to ../**/file.py should work. We'll look into if it makes sense to change the behavior

@MichaReiser MichaReiser removed the needs-mre Needs more information for reproduction label Nov 14, 2024
@amoralesc
Copy link

I can confirm the fix on #14282 (comment) works on my environment.

I can't comment on this being the intended behavior, but it is still different from calling ruff from the project's root with:

ruff check --config .config/.ruff.toml

Thankfully, adding the .. does work on both calls (VSCode extension and ruff).

@dhruvmanila
Copy link
Member

@amoralesc I'd also recommend to use absolute path for ruff.configuration because this setting is applied for every workspaces that you open in VS Code. So, it could be that in a workspace there isn't a .config/ruff.toml file. Using absolute path is more useful in this context, we also support expanding tilde (~) here. I don't know your setup so I leave this up to you.

@amoralesc
Copy link

Since my environment is a Dev Container, I choose to use ${containerWorkspaceFolder}. I can confirm this resolves to an absolute path to the config file inside the container in the extension logs.

Thanks for the heads up though!

dhruvmanila added a commit that referenced this issue Nov 15, 2024
## Summary

This PR fixes a bug in the Ruff language server where the
editor-specified configuration was resolved relative to the
configuration directory and not the current working directory.

The existing behavior is confusing given that this config file is
specified by the user and is not _discovered_ by Ruff itself. The
behavior of resolving this configuration file should be similar to that
of the `--config` flag on the command-line which uses the current
working directory:
https://github.com/astral-sh/ruff/blob/3210f1a23bfe42c5d58c609b602861062eeed2ad/crates/ruff/src/resolve.rs#L34-L48

This creates problems where certain configuration options doesn't work
because the paths resolved in that case are relative to the
configuration directory and not the current working directory in which
the editor is expected to be in. For example, the
`lint.per-file-ignores` doesn't work as mentioned in the linked issue
along with `exclude`, `extend-exclude`, etc.

fixes: #14282 

## Test Plan

Using the following directory tree structure:
```
.
├── .config
│   └── ruff.toml
└── src
    └── migrations
        └── versions
            └── a.py
```

where, the `ruff.toml` is:
```toml
# 1. Comment this out to test `per-file-ignores`
extend-exclude = ["**/versions/*.py"]

[lint]
select = ["D"]

# 2. Comment this out to test `extend-exclude`
[lint.per-file-ignores]
"**/versions/*.py" = ["D"]

# 3. Comment both `per-file-ignores` and `extend-exclude` to test selection works
```

And, the content of `a.py`:
```py
"""Test"""
```

And, the VS Code settings:
```jsonc
{
  "ruff.nativeServer": "on",
  "ruff.path": ["/Users/dhruv/work/astral/ruff/target/debug/ruff"],
  // For single-file mode where current working directory is `/`
  // "ruff.configuration": "/tmp/ruff-repro/.config/ruff.toml",
  // When a workspace is opened containing this path
  "ruff.configuration": "./.config/ruff.toml",
  "ruff.trace.server": "messages",
  "ruff.logLevel": "trace"
}
```

I also tested out just opening the file in single-file mode where the
current working directory is `/` in VS Code. Here, the
`ruff.configuration` needs to be updated to use absolute path as shown
in the above VS Code settings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working server Related to the LSP server
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants