Skip to content

Comments

Allow arbitrary config keys in ruff.configuration#702

Merged
dhruvmanila merged 2 commits intomainfrom
dhruv/ruff-configuration
Feb 26, 2025
Merged

Allow arbitrary config keys in ruff.configuration#702
dhruvmanila merged 2 commits intomainfrom
dhruv/ruff-configuration

Conversation

@dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Feb 24, 2025

Summary

Closes: #6

This PR adds support for astral-sh/ruff#16296 for the VS Code extension by allowing any arbitrary object in ruff.configuration

Additionally, it will provide a warning message to the user if they're using inline configuration but the Ruff version does not support it.

It has one disadvantage that the user will see two popups where the error one is coming from the server because it cannot deserialize the options. We could not show the warning popup if this is too much. I'm worried that it might go unnoticed because the "Show Logs" in the below popup will open the server logs while the message would be in the client logs.

Screenshot 2025-02-25 at 1 24 15 PM

We'll still log it on the console (client log channel):

2025-02-25 13:24:10.067 [warning] Inline configuration support was added in Ruff 0.9.8 (current version is 0.9.7). Please update your Ruff version to use this feature.

I haven't provided more details in the warning message based on the assumption that if the user is using inline configuration then they're aware of it but it's just that the Ruff version is too old.

Test Plan

Refer to the test plan in astral-sh/ruff#16296

@dhruvmanila dhruvmanila marked this pull request as ready for review February 25, 2025 08:00
dhruvmanila added a commit to astral-sh/ruff that referenced this pull request Feb 26, 2025
## Summary

[Internal design
document](https://www.notion.so/astral-sh/In-editor-settings-19e48797e1ca807fa8c2c91b689d9070?pvs=4)

This PR expands `ruff.configuration` to allow inline configuration
directly in the editor. For example:

```json
{
	"ruff.configuration": {
		"line-length": 100,
		"lint": {
			"unfixable": ["F401"],
			"flake8-tidy-imports": {
				"banned-api": {
					"typing.TypedDict": {
						"msg": "Use `typing_extensions.TypedDict` instead"
					}
				}
			}
		},
		"format": {
			"quote-style": "single"
		}
	}
}
```

This means that now `ruff.configuration` accepts either a path to
configuration file or the raw config itself. It's _mostly_ similar to
`--config` with one difference that's highlighted in the following
section. So, it can be said that the format of `ruff.configuration` when
provided the config map is same as the one on the [playground] [^1].

## Limitations

<details><summary><b>Casing (<code>kebab-case</code> v/s/
<code>camelCase</code>)</b></summary>
<p>


The config keys needs to be in `kebab-case` instead of `camelCase` which
is being used for other settings in the editor.

This could be a bit confusing. For example, the `line-length` option can
be set directly via an editor setting or can be configured via
`ruff.configuration`:

```json
{
	"ruff.configuration": {
        "line-length": 100
    },
    "ruff.lineLength": 120
}
```

#### Possible solution

We could use feature flag with [conditional
compilation](https://doc.rust-lang.org/reference/conditional-compilation.html#the-cfg_attr-attribute)
to indicate that when used in `ruff_server`, we need the `Options`
fields to be renamed as `camelCase` while for other crates it needs to
be renamed as `kebab-case`. But, this might not work very easily because
it will require wrapping the `Options` struct and create two structs in
which we'll have to add `#[cfg_attr(...)]` because otherwise `serde`
will complain:

```
error: duplicate serde attribute `rename_all`
  --> crates/ruff_workspace/src/options.rs:43:38
   |
43 | #[cfg_attr(feature = "editor", serde(rename_all = "camelCase"))]
   |                                      ^^^^^^^^^^
```

</p>
</details> 

<details><summary><b>Nesting (flat v/s nested keys)</b></summary>
<p>

This is the major difference between `--config` flag on the command-line
v/s `ruff.configuration` and it makes it such that `ruff.configuration`
has same value format as [playground] [^1].

The config keys needs to be split up into keys which can result in
nested structure instead of flat structure:

So, the following **won't work**:

```json
{
	"ruff.configuration": {
		"format.quote-style": "single",
		"lint.flake8-tidy-imports.banned-api.\"typing.TypedDict\".msg": "Use `typing_extensions.TypedDict` instead"
	}
}
```

But, instead it would need to be split up like the following:
```json
{
	"ruff.configuration": {
		"format": {
			"quote-style": "single"
		},
		"lint": {
			"flake8-tidy-imports": {
				"banned-api": {
					"typing.TypedDict": {
						"msg": "Use `typing_extensions.TypedDict` instead"
					}
				}
			}
		}
	}
}
```

#### Possible solution (1)

The way we could solve this and make it same as `--config` would be to
add a manual logic of converting the JSON map into an equivalent TOML
string which would be then parsed into `Options`.

So, the following JSON map:
```json
{ "lint.flake8-tidy-imports": { "banned-api": {"\"typing.TypedDict\".msg": "Use typing_extensions.TypedDict instead"}}}
```

would need to be converted into the following TOML string:
```toml
lint.flake8-tidy-imports = { banned-api = { "typing.TypedDict".msg = "Use typing_extensions.TypedDict instead" } }
```

by recursively convering `"key": value` into `key = value` which is to
remove the quotes from key and replacing `:` with `=`.

#### Possible solution (2)

Another would be to just accept `Map<String, String>` strictly and
convert it into `key = value` and then parse it as a TOML string. This
would also match `--config` but quotes might become a nuisance because
JSON only allows double quotes and so it'll require escaping any inner
quotes or use single quotes.

</p>
</details> 

## Test Plan

### VS Code

**Requires astral-sh/ruff-vscode#702

**`settings.json`**:
```json
{
  "ruff.lint.extendSelect": ["TID"],
  "ruff.configuration": {
    "line-length": 50,
    "format": {
      "quote-style": "single"
    },
    "lint": {
      "unfixable": ["F401"],
      "flake8-tidy-imports": {
        "banned-api": {
          "typing.TypedDict": {
            "msg": "Use `typing_extensions.TypedDict` instead"
          }
        }
      }
    }
  }
}
```

Following video showcases me doing the following:
1. Check diagnostics that it includes `TID`
2. Run `Ruff: Fix all auto-fixable problems` to test `unfixable`
3. Run `Format: Document` to test `line-length` and `quote-style`


https://github.com/user-attachments/assets/0a38176f-3fb0-4960-a213-73b2ea5b1180

### Neovim

**`init.lua`**:
```lua
require('lspconfig').ruff.setup {
  init_options = {
    settings = {
      lint = {
        extendSelect = { 'TID' },
      },
      configuration = {
        ['line-length'] = 50,
        format = {
          ['quote-style'] = 'single',
        },
        lint = {
          unfixable = { 'F401' },
          ['flake8-tidy-imports'] = {
            ['banned-api'] = {
              ['typing.TypedDict'] = {
                msg = 'Use typing_extensions.TypedDict instead',
              },
            },
          },
        },
      },
    },
  },
}
```

Same steps as in the VS Code test:



https://github.com/user-attachments/assets/cfe49a9b-9a89-43d7-94f2-7f565d6e3c9d

## Documentation Preview



https://github.com/user-attachments/assets/e0062f58-6ec8-4e01-889d-fac76fd8b3c7



[playground]: https://play.ruff.rs

[^1]: This has one advantage that the value can be copy-pasted directly
into the playground
@dhruvmanila dhruvmanila merged commit 7c6b464 into main Feb 26, 2025
6 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/ruff-configuration branch February 26, 2025 04:47
dhruvmanila added a commit that referenced this pull request Feb 26, 2025
## Summary

fixup from #702 

## Test Plan

Verified that `null` (not setting `ruff.configuration`) doesn't give me
the warning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Ruff configuration settings as VS Code extension options

2 participants