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.

Sign up for GitHub

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

Jump to bottom

ruff server: Fix multiple issues with Neovim and Helix #11497

Merged
merged 2 commits into from
May 22, 2024
Merged

ruff server: Fix multiple issues with Neovim and Helix #11497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc8d79f
Fix miscellaneous issues with configuration and diagnostic publishing
snowsignal May 22, 2024
66f3f59
Filter workspace folders
snowsignal May 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 13 additions & 15 deletions crates/ruff_server/resources/test/fixtures/settings/global_only.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
{
"settings": {
"codeAction": {
"disableRuleComment": {
"enable": false
}
},
"lint": {
"ignore": ["RUF001"],
"run": "onSave"
},
"fixAll": false,
"logLevel": "warn",
"lineLength": 80,
"exclude": ["third_party"]
}
"codeAction": {
"disableRuleComment": {
"enable": false
}
},
"lint": {
"ignore": ["RUF001"],
"run": "onSave"
},
"fixAll": false,
"logLevel": "warn",
"lineLength": 80,
"exclude": ["third_party"]
}
6 changes: 4 additions & 2 deletions crates/ruff_server/src/lint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,14 @@ pub(crate) fn check(

let mut diagnostics = Diagnostics::default();

// Populate all cell URLs with an empty diagnostic list.
// This ensures that cells without diagnostics still get updated.
// Populates all relevant URLs with an empty diagnostic list.
// This ensures that documents without diagnostics still get updated.
if let Some(notebook) = query.as_notebook() {
for url in notebook.urls() {
diagnostics.entry(url.clone()).or_default();
}
} else {
diagnostics.entry(query.make_key().into_url()).or_default();
}

let lsp_diagnostics = data
Expand Down
9 changes: 7 additions & 2 deletions crates/ruff_server/src/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,11 @@ impl Server {
let AllSettings {
global_settings,
mut workspace_settings,
} = AllSettings::from_value(init_params.initialization_options.unwrap_or_default());
} = AllSettings::from_value(
init_params
.initialization_options
.unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::default())),
snowsignal marked this conversation as resolved.
Show resolved Hide resolved
);

let mut workspace_for_path = |path: PathBuf| {
let Some(workspace_settings) = workspace_settings.as_mut() else {
Expand All @@ -84,11 +88,12 @@ impl Server {

let workspaces = init_params
.workspace_folders
.filter(|folders| !folders.is_empty())
.map(|folders| folders.into_iter().map(|folder| {
workspace_for_path(folder.uri.to_file_path().unwrap())
}).collect())
.or_else(|| {
tracing::debug!("No workspace(s) were provided during initialization. Using the current working directory as a default workspace...");
tracing::warn!("No workspace(s) were provided during initialization. Using the current working directory as a default workspace...");
let uri = types::Url::from_file_path(std::env::current_dir().ok()?).ok()?;
Some(vec![workspace_for_path(uri.to_file_path().unwrap())])
})
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_server/src/session.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl Session {
Some(DocumentSnapshot {
resolved_client_capabilities: self.resolved_client_capabilities.clone(),
client_settings: self.index.client_settings(&key, &self.global_settings),
document_ref: self.index.make_document_ref(key)?,
document_ref: self.index.make_document_ref(key, &self.global_settings)?,
position_encoding: self.position_encoding,
})
}
Expand Down
23 changes: 19 additions & 4 deletions crates/ruff_server/src/session/index.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,27 @@ impl Index {
Ok(())
}

pub(super) fn make_document_ref(&self, key: DocumentKey) -> Option<DocumentQuery> {
pub(super) fn make_document_ref(
&self,
key: DocumentKey,
global_settings: &ClientSettings,
) -> Option<DocumentQuery> {
let path = self.path_for_key(&key)?.clone();
let document_settings = self
.settings_for_path(&path)?
.workspace_settings_index
.get(&path);
.settings_for_path(&path)
.map(|settings| settings.workspace_settings_index.get(&path))
.unwrap_or_else(|| {
tracing::warn!(
"No settings available for {} - falling back to default settings",
path.display()
);
let resolved_global = ResolvedClientSettings::global(global_settings);
let root = path.parent().unwrap_or(&path);
Arc::new(RuffSettings::fallback(
resolved_global.editor_settings(),
root,
))
});

let controller = self.documents.get(&path)?;
let cell_uri = match key {
Expand Down
58 changes: 28 additions & 30 deletions crates/ruff_server/src/session/index/ruff_settings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,32 @@ impl std::fmt::Display for RuffSettings {
}

impl RuffSettings {
pub(crate) fn fallback(editor_settings: &ResolvedEditorSettings, root: &Path) -> RuffSettings {
let fallback = find_user_settings_toml()
.and_then(|user_settings| {
ruff_workspace::resolver::resolve_root_settings(
&user_settings,
Relativity::Cwd,
&EditorConfigurationTransformer(editor_settings, root),
)
.ok()
})
.unwrap_or_else(|| {
let default_configuration = ruff_workspace::configuration::Configuration::default();
EditorConfigurationTransformer(editor_settings, root)
.transform(default_configuration)
.into_settings(root)
.expect(
"editor configuration should merge successfully with default configuration",
)
});

RuffSettings {
formatter: fallback.formatter,
linter: fallback.linter,
}
}

pub(crate) fn linter(&self) -> &ruff_linter::settings::LinterSettings {
&self.linter
}
Expand Down Expand Up @@ -80,32 +106,9 @@ impl RuffSettingsIndex {
}
}

let fallback = find_user_settings_toml()
.and_then(|user_settings| {
ruff_workspace::resolver::resolve_root_settings(
&user_settings,
Relativity::Cwd,
&EditorConfigurationTransformer(editor_settings, root),
)
.ok()
})
.unwrap_or_else(|| {
let default_configuration = ruff_workspace::configuration::Configuration::default();
EditorConfigurationTransformer(editor_settings, root)
.transform(default_configuration)
.into_settings(root)
.expect(
"editor configuration should merge successfully with default configuration",
)
});
let fallback = Arc::new(RuffSettings::fallback(editor_settings, root));

Self {
index,
fallback: Arc::new(RuffSettings {
formatter: fallback.formatter,
linter: fallback.linter,
}),
}
Self { index, fallback }
}

pub(super) fn get(&self, document_path: &Path) -> Arc<RuffSettings> {
Expand All @@ -118,11 +121,6 @@ impl RuffSettingsIndex {
return settings.clone();
}

tracing::info!(
"No Ruff settings file found for {}; falling back to default configuration",
document_path.display()
);

self.fallback.clone()
}
}
Expand Down
95 changes: 48 additions & 47 deletions crates/ruff_server/src/session/settings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ enum InitializationOptions {
workspace_settings: Vec<WorkspaceSettings>,
},
GlobalOnly {
settings: Option<ClientSettings>,
#[serde(flatten)]
settings: ClientSettings,
Comment on lines -133 to +134
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@snowsignal when you're back, can you explain why is this being flattened? This is causing #11507 because now it's not under settings but rather in the global table.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I understand why it was flattened - to account for the empty initialization options. I think we need to re-think about how to handle these cases.

},
}

Expand All @@ -157,7 +158,7 @@ impl AllSettings {

fn from_init_options(options: InitializationOptions) -> Self {
let (global_settings, workspace_settings) = match options {
InitializationOptions::GlobalOnly { settings } => (settings.unwrap_or_default(), None),
InitializationOptions::GlobalOnly { settings } => (settings, None),
InitializationOptions::HasWorkspaces {
global_settings,
workspace_settings,
Expand Down Expand Up @@ -341,7 +342,9 @@ impl ResolvedClientSettings {

impl Default for InitializationOptions {
fn default() -> Self {
Self::GlobalOnly { settings: None }
Self::GlobalOnly {
settings: ClientSettings::default(),
}
}
}

Expand Down Expand Up @@ -626,52 +629,50 @@ mod tests {

assert_debug_snapshot!(options, @r###"
GlobalOnly {
settings: Some(
ClientSettings {
configuration: None,
fix_all: Some(
false,
),
organize_imports: None,
lint: Some(
LintOptions {
enable: None,
preview: None,
select: None,
extend_select: None,
ignore: Some(
[
"RUF001",
],
),
},
),
format: None,
code_action: Some(
CodeActionOptions {
disable_rule_comment: Some(
CodeActionParameters {
enable: Some(
false,
),
},
),
fix_violation: None,
},
),
exclude: Some(
[
"third_party",
],
),
line_length: Some(
LineLength(
80,
settings: ClientSettings {
configuration: None,
fix_all: Some(
false,
),
organize_imports: None,
lint: Some(
LintOptions {
enable: None,
preview: None,
select: None,
extend_select: None,
ignore: Some(
[
"RUF001",
],
),
},
),
format: None,
code_action: Some(
CodeActionOptions {
disable_rule_comment: Some(
CodeActionParameters {
enable: Some(
false,
),
},
),
fix_violation: None,
},
),
exclude: Some(
[
"third_party",
],
),
line_length: Some(
LineLength(
80,
),
configuration_preference: None,
},
),
),
configuration_preference: None,
},
}
"###);
}
Expand Down
Loading