Skip to content

Commit

Permalink
Consider --preview flag for server subcommand (#12208)
Browse files Browse the repository at this point in the history
## Summary

This PR removes the requirement of `--preview` flag to run the `ruff
server` and instead considers it to be an indicator to turn on preview
mode for the linter and the formatter.

resolves: #12161 

## Test Plan

Add test cases to assert the `preview` value is updated accordingly.

In an editor context, I used the local `ruff` executable in Neovim with
the `--preview` flag and verified that the preview-only violations are
being highlighted.

Running with:
```lua
require('lspconfig').ruff.setup({
  cmd = {
    '/Users/dhruv/work/astral/ruff/target/debug/ruff',
    'server',
    '--preview',
  },
})
```
The screenshot shows that `E502` is highlighted with the below config in
`pyproject.toml`:

<img width="877" alt="Screenshot 2024-07-17 at 16 43 09"
src="https://github.com/user-attachments/assets/c7016ef3-55b1-4a14-bbd3-a07b1bcdd323">
  • Loading branch information
dhruvmanila authored Jul 18, 2024
1 parent ebe5b06 commit 2e77b77
Show file tree
Hide file tree
Showing 7 changed files with 145 additions and 37 deletions.
17 changes: 14 additions & 3 deletions crates/ruff/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,9 +494,20 @@ pub struct FormatCommand {

#[derive(Copy, Clone, Debug, clap::Parser)]
pub struct ServerCommand {
/// Enable preview mode; required for regular operation
#[arg(long)]
pub(crate) preview: bool,
/// Enable preview mode. Use `--no-preview` to disable.
///
/// This enables unstable server features and turns on the preview mode for the linter
/// and the formatter.
#[arg(long, overrides_with("no_preview"))]
preview: bool,
#[clap(long, overrides_with("preview"), hide = true)]
no_preview: bool,
}

impl ServerCommand {
pub(crate) fn resolve_preview(self) -> Option<bool> {
resolve_bool_arg(self.preview, self.no_preview)
}
}

#[derive(Debug, Clone, Copy, clap::ValueEnum)]
Expand Down
12 changes: 5 additions & 7 deletions crates/ruff/src/commands/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@ use crate::ExitStatus;
use anyhow::Result;
use ruff_server::Server;

pub(crate) fn run_server(preview: bool, worker_threads: NonZeroUsize) -> Result<ExitStatus> {
if !preview {
tracing::error!("--preview needs to be provided as a command line argument while the server is still unstable.\nFor example: `ruff server --preview`");
return Ok(ExitStatus::Error);
}

let server = Server::new(worker_threads)?;
pub(crate) fn run_server(
worker_threads: NonZeroUsize,
preview: Option<bool>,
) -> Result<ExitStatus> {
let server = Server::new(worker_threads, preview)?;

server.run().map(|()| ExitStatus::Success)
}
4 changes: 1 addition & 3 deletions crates/ruff/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,15 +200,13 @@ fn format(args: FormatCommand, global_options: GlobalConfigArgs) -> Result<ExitS
}

fn server(args: ServerCommand) -> Result<ExitStatus> {
let ServerCommand { preview } = args;

let four = NonZeroUsize::new(4).unwrap();

// by default, we set the number of worker threads to `num_cpus`, with a maximum of 4.
let worker_threads = std::thread::available_parallelism()
.unwrap_or(four)
.max(four);
commands::server::run_server(preview, worker_threads)
commands::server::run_server(worker_threads, args.resolve_preview())
}

pub fn check(args: CheckCommand, global_options: GlobalConfigArgs) -> Result<ExitStatus> {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"settings": [
{
"cwd": "/Users/test/projects/first",
"workspace": "file:///Users/test/projects/first"
},
{
"cwd": "/Users/test/projects/second",
"workspace": "file:///Users/test/projects/second"
}
],
"globalSettings": {
"cwd": "/",
"workspace": "/"
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"settings": [
{
"experimentalServer": true,
"nativeServer": "on",
"cwd": "/Users/test/projects/pandas",
"workspace": "file:///Users/test/projects/pandas",
"path": [],
Expand All @@ -21,20 +21,19 @@
"lint": {
"enable": true,
"run": "onType",
"args": [
"--preview"
]
"args": []
},
"format": {
"args": []
},
"enable": true,
"organizeImports": true,
"fixAll": true,
"showNotifications": "off"
"showNotifications": "off",
"showSyntaxErrors": true
},
{
"experimentalServer": true,
"nativeServer": "on",
"cwd": "/Users/test/projects/scipy",
"workspace": "file:///Users/test/projects/scipy",
"path": [],
Expand All @@ -55,21 +54,20 @@
"enable": true,
"preview": false,
"run": "onType",
"args": [
"--preview"
]
"args": []
},
"format": {
"args": []
},
"enable": true,
"organizeImports": true,
"fixAll": true,
"showNotifications": "off"
"showNotifications": "off",
"showSyntaxErrors": true
}
],
"globalSettings": {
"experimentalServer": true,
"nativeServer": "on",
"cwd": "/",
"workspace": "/",
"path": [],
Expand All @@ -89,16 +87,15 @@
"preview": true,
"select": ["F", "I"],
"run": "onType",
"args": [
"--preview"
]
"args": []
},
"format": {
"args": []
},
"enable": true,
"organizeImports": true,
"fixAll": false,
"showNotifications": "off"
"showNotifications": "off",
"showSyntaxErrors": true
}
}
14 changes: 9 additions & 5 deletions crates/ruff_server/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub struct Server {
}

impl Server {
pub fn new(worker_threads: NonZeroUsize) -> crate::Result<Self> {
pub fn new(worker_threads: NonZeroUsize, preview: Option<bool>) -> crate::Result<Self> {
let connection = ConnectionInitializer::stdio();

let (id, init_params) = connection.initialize_start()?;
Expand All @@ -70,14 +70,18 @@ impl Server {

crate::message::init_messenger(connection.make_sender());

let AllSettings {
global_settings,
mut workspace_settings,
} = AllSettings::from_value(
let mut all_settings = AllSettings::from_value(
init_params
.initialization_options
.unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::default())),
);
if let Some(preview) = preview {
all_settings.set_preview(preview);
}
let AllSettings {
global_settings,
mut workspace_settings,
} = all_settings;

crate::trace::init_tracing(
connection.make_sender(),
Expand Down
92 changes: 88 additions & 4 deletions crates/ruff_server/src/session/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,20 @@ pub struct ClientSettings {
pub(crate) tracing: TracingSettings,
}

impl ClientSettings {
/// Update the preview flag for the linter and the formatter with the given value.
pub(crate) fn set_preview(&mut self, preview: bool) {
match self.lint.as_mut() {
None => self.lint = Some(LintOptions::default().with_preview(preview)),
Some(lint) => lint.set_preview(preview),
}
match self.format.as_mut() {
None => self.format = Some(FormatOptions::default().with_preview(preview)),
Some(format) => format.set_preview(preview),
}
}
}

/// Settings needed to initialize tracing. These will only be
/// read from the global configuration.
#[derive(Debug, Deserialize, Default)]
Expand All @@ -107,7 +121,7 @@ struct WorkspaceSettings {
workspace: Url,
}

#[derive(Debug, Deserialize)]
#[derive(Debug, Default, Deserialize)]
#[cfg_attr(test, derive(PartialEq, Eq))]
#[serde(rename_all = "camelCase")]
struct LintOptions {
Expand All @@ -118,13 +132,35 @@ struct LintOptions {
ignore: Option<Vec<String>>,
}

impl LintOptions {
fn with_preview(mut self, preview: bool) -> LintOptions {
self.preview = Some(preview);
self
}

fn set_preview(&mut self, preview: bool) {
self.preview = Some(preview);
}
}

#[derive(Debug, Default, Deserialize)]
#[cfg_attr(test, derive(PartialEq, Eq))]
#[serde(rename_all = "camelCase")]
struct FormatOptions {
preview: Option<bool>,
}

impl FormatOptions {
fn with_preview(mut self, preview: bool) -> FormatOptions {
self.preview = Some(preview);
self
}

fn set_preview(&mut self, preview: bool) {
self.preview = Some(preview);
}
}

#[derive(Debug, Default, Deserialize)]
#[cfg_attr(test, derive(PartialEq, Eq))]
#[serde(rename_all = "camelCase")]
Expand Down Expand Up @@ -159,6 +195,7 @@ enum InitializationOptions {
}

/// Built from the initialization options provided by the client.
#[derive(Debug)]
pub(crate) struct AllSettings {
pub(crate) global_settings: ClientSettings,
/// If this is `None`, the client only passed in global settings.
Expand All @@ -179,6 +216,16 @@ impl AllSettings {
)
}

/// Update the preview flag for both the global and all workspace settings.
pub(crate) fn set_preview(&mut self, preview: bool) {
self.global_settings.set_preview(preview);
if let Some(workspace_settings) = self.workspace_settings.as_mut() {
for settings in workspace_settings.values_mut() {
settings.set_preview(preview);
}
}
}

fn from_init_options(options: InitializationOptions) -> Self {
let (global_settings, workspace_settings) = match options {
InitializationOptions::GlobalOnly { settings } => (settings, None),
Expand Down Expand Up @@ -393,6 +440,11 @@ mod tests {
const EMPTY_INIT_OPTIONS_FIXTURE: &str =
include_str!("../../resources/test/fixtures/settings/empty.json");

// This fixture contains multiple workspaces with empty initialization options. It only sets
// the `cwd` and the `workspace` value.
const EMPTY_MULTIPLE_WORKSPACE_INIT_OPTIONS_FIXTURE: &str =
include_str!("../../resources/test/fixtures/settings/empty_multiple_workspace.json");

fn deserialize_fixture<T: DeserializeOwned>(content: &str) -> T {
serde_json::from_str(content).expect("test fixture JSON should deserialize")
}
Expand Down Expand Up @@ -456,7 +508,9 @@ mod tests {
exclude: None,
line_length: None,
configuration_preference: None,
show_syntax_errors: None,
show_syntax_errors: Some(
true,
),
tracing: TracingSettings {
log_level: None,
log_file: None,
Expand Down Expand Up @@ -509,7 +563,9 @@ mod tests {
exclude: None,
line_length: None,
configuration_preference: None,
show_syntax_errors: None,
show_syntax_errors: Some(
true,
),
tracing: TracingSettings {
log_level: None,
log_file: None,
Expand Down Expand Up @@ -575,7 +631,9 @@ mod tests {
exclude: None,
line_length: None,
configuration_preference: None,
show_syntax_errors: None,
show_syntax_errors: Some(
true,
),
tracing: TracingSettings {
log_level: None,
log_file: None,
Expand Down Expand Up @@ -771,4 +829,30 @@ mod tests {

assert_eq!(options, InitializationOptions::default());
}

fn assert_preview_client_settings(settings: &ClientSettings, preview: bool) {
assert_eq!(settings.lint.as_ref().unwrap().preview.unwrap(), preview);
assert_eq!(settings.format.as_ref().unwrap().preview.unwrap(), preview);
}

fn assert_preview_all_settings(all_settings: &AllSettings, preview: bool) {
assert_preview_client_settings(&all_settings.global_settings, preview);
if let Some(workspace_settings) = all_settings.workspace_settings.as_ref() {
for settings in workspace_settings.values() {
assert_preview_client_settings(settings, preview);
}
}
}

#[test]
fn test_preview_flag() {
let options = deserialize_fixture(EMPTY_MULTIPLE_WORKSPACE_INIT_OPTIONS_FIXTURE);
let mut all_settings = AllSettings::from_init_options(options);

all_settings.set_preview(false);
assert_preview_all_settings(&all_settings, false);

all_settings.set_preview(true);
assert_preview_all_settings(&all_settings, true);
}
}

0 comments on commit 2e77b77

Please sign in to comment.