Notify users for invalid client settings (#16361)

## Summary

As mentioned in
https://github.com/astral-sh/ruff/pull/16296#discussion_r1967047387

This PR updates the client settings resolver to notify the user if there
are any errors in the config using a very basic approach. In addition,
each error related to specific settings are logged.

This isn't the best approach because it can log the same message
multiple times when both workspace and global settings are provided and
they both are the same. This is the case for a single workspace VS Code
instance.

I do have some ideas on how to improve this and will explore them during
my free time (low priority):
* Avoid resolving the global settings multiple times as they're static
* Include the source of the setting (workspace or global?)
* Maybe use a struct (`ResolvedClientSettings` +
`Vec<ClientSettingsResolverError>`) instead to make unit testing easier

## Test Plan

Using:
```jsonc
{
  "ruff.logLevel": "debug",
	
  // Invalid settings
  "ruff.configuration": "$RANDOM",
  "ruff.lint.select": ["RUF000", "I001"],
  "ruff.lint.extendSelect": ["B001", "B002"],
  "ruff.lint.ignore": ["I999", "F401"]
}
```

The error logs:
```
2025-02-27 12:30:04.318736000 ERROR Failed to load settings from `configuration`: error looking key 'RANDOM' up: environment variable not found
2025-02-27 12:30:04.319196000 ERROR Failed to load settings from `configuration`: error looking key 'RANDOM' up: environment variable not found
2025-02-27 12:30:04.320549000 ERROR Unknown rule selectors found in `lint.select`: ["RUF000"]
2025-02-27 12:30:04.320669000 ERROR Unknown rule selectors found in `lint.extendSelect`: ["B001"]
2025-02-27 12:30:04.320764000 ERROR Unknown rule selectors found in `lint.ignore`: ["I999"]
```

Notification preview:

<img width="470" alt="Screenshot 2025-02-27 at 12 29 06 PM"
src="https://github.com/user-attachments/assets/61f41d5c-2558-46b3-a1ed-82114fd8ec22"
/>
This commit is contained in:
Dhruv Manilawala 2025-02-27 13:58:29 +05:30 committed by GitHub
parent 7dad0c471d
commit d56d241317
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -6,7 +6,9 @@ use serde::Deserialize;
use serde_json::{Map, Value};
use thiserror::Error;
use ruff_linter::{line_width::LineLength, RuleSelector};
use ruff_linter::line_width::LineLength;
use ruff_linter::rule_selector::ParseError;
use ruff_linter::RuleSelector;
use ruff_workspace::options::Options;
/// Maps a workspace URI to its associated client settings. Used during server initialization.
@ -319,7 +321,9 @@ impl ResolvedClientSettings {
}
fn new_impl(all_settings: &[&ClientSettings]) -> Self {
Self {
let mut contains_invalid_settings = false;
let settings = Self {
fix_all: Self::resolve_or(all_settings, |settings| settings.fix_all, true),
organize_imports: Self::resolve_or(
all_settings,
@ -369,6 +373,7 @@ impl ResolvedClientSettings {
tracing::error!(
"Failed to load settings from `configuration`: {err}"
);
contains_invalid_settings = true;
None
}
}
@ -381,34 +386,25 @@ impl ResolvedClientSettings {
settings.format.as_ref()?.preview
}),
select: Self::resolve_optional(all_settings, |settings| {
settings
.lint
.as_ref()?
.select
.as_ref()?
.iter()
.map(|rule| RuleSelector::from_str(rule).ok())
.collect()
Self::resolve_rules(
settings.lint.as_ref()?.select.as_ref()?,
RuleSelectorKey::Select,
&mut contains_invalid_settings,
)
}),
extend_select: Self::resolve_optional(all_settings, |settings| {
settings
.lint
.as_ref()?
.extend_select
.as_ref()?
.iter()
.map(|rule| RuleSelector::from_str(rule).ok())
.collect()
Self::resolve_rules(
settings.lint.as_ref()?.extend_select.as_ref()?,
RuleSelectorKey::ExtendSelect,
&mut contains_invalid_settings,
)
}),
ignore: Self::resolve_optional(all_settings, |settings| {
settings
.lint
.as_ref()?
.ignore
.as_ref()?
.iter()
.map(|rule| RuleSelector::from_str(rule).ok())
.collect()
Self::resolve_rules(
settings.lint.as_ref()?.ignore.as_ref()?,
RuleSelectorKey::Ignore,
&mut contains_invalid_settings,
)
}),
exclude: Self::resolve_optional(all_settings, |settings| settings.exclude.clone()),
line_length: Self::resolve_optional(all_settings, |settings| settings.line_length),
@ -418,6 +414,37 @@ impl ResolvedClientSettings {
ConfigurationPreference::EditorFirst,
),
},
};
if contains_invalid_settings {
show_err_msg!(
"Ruff received invalid settings from the editor. Refer to the logs for more information."
);
}
settings
}
fn resolve_rules(
rules: &[String],
key: RuleSelectorKey,
contains_invalid_settings: &mut bool,
) -> Option<Vec<RuleSelector>> {
let (mut known, mut unknown) = (vec![], vec![]);
for rule in rules {
match RuleSelector::from_str(rule) {
Ok(selector) => known.push(selector),
Err(ParseError::Unknown(_)) => unknown.push(rule),
}
}
if !unknown.is_empty() {
*contains_invalid_settings = true;
tracing::error!("Unknown rule selectors found in `{key}`: {unknown:?}");
}
if known.is_empty() {
None
} else {
Some(known)
}
}
@ -427,7 +454,7 @@ impl ResolvedClientSettings {
/// Use [`ResolvedClientSettings::resolve_or`] for settings that should have default values.
fn resolve_optional<T>(
all_settings: &[&ClientSettings],
get: impl Fn(&ClientSettings) -> Option<T>,
get: impl FnMut(&ClientSettings) -> Option<T>,
) -> Option<T> {
all_settings.iter().map(Deref::deref).find_map(get)
}
@ -446,6 +473,23 @@ impl ResolvedClientSettings {
}
}
#[derive(Copy, Clone)]
enum RuleSelectorKey {
Select,
ExtendSelect,
Ignore,
}
impl std::fmt::Display for RuleSelectorKey {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
RuleSelectorKey::Select => f.write_str("lint.select"),
RuleSelectorKey::ExtendSelect => f.write_str("lint.extendSelect"),
RuleSelectorKey::Ignore => f.write_str("lint.ignore"),
}
}
}
impl ResolvedClientSettings {
pub(crate) fn fix_all(&self) -> bool {
self.fix_all