Skip to content

Commit 39e0463

Browse files
SysixSysix
committed
refactor(language_server): move nested_configs to ServerLinter (#10775)
Normally we would just restart the `ServerLinter.isolated_linter` or the `Worker.nested_configs`. I do not see the benefits to store the `nested_configs` separately and changing them + the `isolated_linter` when needed. And I do not want to change `isolated_linter` from `Arc` to `RwLock` / `Mutex`. When a `.oxlintrc.json` file changes, we just set up the linter and searching for all nested configs again.
1 parent 0f7e755 commit 39e0463

File tree

2 files changed

+72
-136
lines changed

2 files changed

+72
-136
lines changed

‎crates/oxc_language_server/src/linter/server_linter.rs‎

Lines changed: 65 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,73 @@ pub struct ServerLinter {
2424
}
2525

2626
impl ServerLinter {
27+
pub fn new(root_uri: &Uri, options: &Options) -> Self {
28+
let nested_configs = Self::create_nested_configs(root_uri, options);
29+
let root_path = root_uri.to_file_path().unwrap();
30+
let relative_config_path = options.config_path.clone();
31+
let oxlintrc = if relative_config_path.is_some() {
32+
let config = root_path.join(relative_config_path.unwrap());
33+
if config.try_exists().is_ok_and(|exists| exists) {
34+
if let Ok(oxlintrc) = Oxlintrc::from_file(&config) {
35+
oxlintrc
36+
} else {
37+
warn!("Failed to initialize oxlintrc config: {}", config.to_string_lossy());
38+
Oxlintrc::default()
39+
}
40+
} else {
41+
warn!(
42+
"Config file not found: {}, fallback to default config",
43+
config.to_string_lossy()
44+
);
45+
Oxlintrc::default()
46+
}
47+
} else {
48+
Oxlintrc::default()
49+
};
50+
51+
// clone because we are returning it for ignore builder
52+
let config_builder =
53+
ConfigStoreBuilder::from_oxlintrc(false, oxlintrc.clone()).unwrap_or_default();
54+
55+
// TODO(refactor): pull this into a shared function, because in oxlint we have the same functionality.
56+
let use_nested_config = options.use_nested_configs();
57+
58+
let use_cross_module = if use_nested_config {
59+
nested_configs.pin().values().any(|config| config.plugins().has_import())
60+
} else {
61+
config_builder.plugins().has_import()
62+
};
63+
64+
let config_store = config_builder.build().expect("Failed to build config store");
65+
66+
let lint_options = LintOptions { fix: options.fix_kind(), ..Default::default() };
67+
68+
let linter = if use_nested_config {
69+
let nested_configs = nested_configs.pin();
70+
let nested_configs_copy: FxHashMap<PathBuf, ConfigStore> = nested_configs
71+
.iter()
72+
.map(|(key, value)| (key.clone(), value.clone()))
73+
.collect::<FxHashMap<_, _>>();
74+
75+
Linter::new_with_nested_configs(lint_options, config_store, nested_configs_copy)
76+
} else {
77+
Linter::new(lint_options, config_store)
78+
};
79+
80+
let isolated_linter = IsolatedLintHandler::new(
81+
linter,
82+
IsolatedLintHandlerOptions { use_cross_module, root_path: root_path.to_path_buf() },
83+
);
84+
85+
Self {
86+
isolated_linter: Arc::new(isolated_linter),
87+
gitignore_glob: Self::create_ignore_glob(root_uri, &oxlintrc),
88+
}
89+
}
90+
2791
/// Searches inside root_uri recursively for the default oxlint config files
2892
/// and insert them inside the nested configuration
29-
pub fn create_nested_configs(
93+
fn create_nested_configs(
3094
root_uri: &Uri,
3195
options: &Options,
3296
) -> ConcurrentHashMap<PathBuf, ConfigStore> {
@@ -115,73 +179,6 @@ impl ServerLinter {
115179
gitignore_globs
116180
}
117181

118-
pub fn create_server_linter(
119-
root_uri: &Uri,
120-
options: &Options,
121-
nested_configs: &ConcurrentHashMap<PathBuf, ConfigStore>,
122-
) -> Self {
123-
let root_path = root_uri.to_file_path().unwrap();
124-
let relative_config_path = options.config_path.clone();
125-
let oxlintrc = if relative_config_path.is_some() {
126-
let config = root_path.join(relative_config_path.unwrap());
127-
if config.try_exists().expect("Could not get fs metadata for config") {
128-
if let Ok(oxlintrc) = Oxlintrc::from_file(&config) {
129-
oxlintrc
130-
} else {
131-
warn!("Failed to initialize oxlintrc config: {}", config.to_string_lossy());
132-
Oxlintrc::default()
133-
}
134-
} else {
135-
warn!(
136-
"Config file not found: {}, fallback to default config",
137-
config.to_string_lossy()
138-
);
139-
Oxlintrc::default()
140-
}
141-
} else {
142-
Oxlintrc::default()
143-
};
144-
145-
// clone because we are returning it for ignore builder
146-
let config_builder =
147-
ConfigStoreBuilder::from_oxlintrc(false, oxlintrc.clone()).unwrap_or_default();
148-
149-
// TODO(refactor): pull this into a shared function, because in oxlint we have the same functionality.
150-
let use_nested_config = options.use_nested_configs();
151-
152-
let use_cross_module = if use_nested_config {
153-
nested_configs.pin().values().any(|config| config.plugins().has_import())
154-
} else {
155-
config_builder.plugins().has_import()
156-
};
157-
158-
let config_store = config_builder.build().expect("Failed to build config store");
159-
160-
let lint_options = LintOptions { fix: options.fix_kind(), ..Default::default() };
161-
162-
let linter = if use_nested_config {
163-
let nested_configs = nested_configs.pin();
164-
let nested_configs_copy: FxHashMap<PathBuf, ConfigStore> = nested_configs
165-
.iter()
166-
.map(|(key, value)| (key.clone(), value.clone()))
167-
.collect::<FxHashMap<_, _>>();
168-
169-
Linter::new_with_nested_configs(lint_options, config_store, nested_configs_copy)
170-
} else {
171-
Linter::new(lint_options, config_store)
172-
};
173-
174-
let isolated_linter = IsolatedLintHandler::new(
175-
linter,
176-
IsolatedLintHandlerOptions { use_cross_module, root_path: root_path.to_path_buf() },
177-
);
178-
179-
Self {
180-
isolated_linter: Arc::new(isolated_linter),
181-
gitignore_glob: Self::create_ignore_glob(root_uri, &oxlintrc),
182-
}
183-
}
184-
185182
fn is_ignored(&self, uri: &Uri) -> bool {
186183
for gitignore in &self.gitignore_glob {
187184
if let Some(uri_path) = uri.to_file_path() {

‎crates/oxc_language_server/src/worker.rs‎

Lines changed: 7 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
1-
use std::{path::PathBuf, str::FromStr, vec};
1+
use std::{str::FromStr, vec};
22

3-
use log::{debug, info};
4-
use oxc_linter::{ConfigStore, ConfigStoreBuilder, Oxlintrc};
3+
use log::debug;
54
use rustc_hash::FxBuildHasher;
65
use tokio::sync::{Mutex, OnceCell, RwLock};
76
use tower_lsp_server::{
87
UriExt,
9-
lsp_types::{CodeActionOrCommand, Diagnostic, FileChangeType, FileEvent, Range, TextEdit, Uri},
8+
lsp_types::{CodeActionOrCommand, Diagnostic, FileEvent, Range, TextEdit, Uri},
109
};
1110

1211
use crate::{
13-
ConcurrentHashMap, OXC_CONFIG_FILE, Options, Run,
12+
ConcurrentHashMap, Options, Run,
1413
code_actions::{
1514
apply_all_fix_code_action, apply_fix_code_action, ignore_this_line_code_action,
1615
ignore_this_rule_code_action,
@@ -23,22 +22,19 @@ pub struct WorkspaceWorker {
2322
server_linter: RwLock<ServerLinter>,
2423
diagnostics_report_map: RwLock<ConcurrentHashMap<String, Vec<DiagnosticReport>>>,
2524
options: Mutex<Options>,
26-
nested_configs: RwLock<ConcurrentHashMap<PathBuf, ConfigStore>>,
2725
}
2826

2927
impl WorkspaceWorker {
3028
pub fn new(root_uri: &Uri, options: Options) -> Self {
3129
let root_uri_cell = OnceCell::new();
3230
root_uri_cell.set(root_uri.clone()).unwrap();
3331

34-
let nested_configs = ServerLinter::create_nested_configs(root_uri, &options);
35-
let server_linter = ServerLinter::create_server_linter(root_uri, &options, &nested_configs);
32+
let server_linter = ServerLinter::new(root_uri, &options);
3633
Self {
3734
root_uri: root_uri_cell,
3835
server_linter: RwLock::new(server_linter),
3936
diagnostics_report_map: RwLock::new(ConcurrentHashMap::default()),
4037
options: Mutex::new(options),
41-
nested_configs: RwLock::const_new(nested_configs),
4238
}
4339
}
4440

@@ -59,22 +55,9 @@ impl WorkspaceWorker {
5955
self.diagnostics_report_map.read().await.pin().remove(&uri.to_string());
6056
}
6157

62-
async fn refresh_nested_configs(&self) {
63-
let options = self.options.lock().await;
64-
let nested_configs =
65-
ServerLinter::create_nested_configs(self.root_uri.get().unwrap(), &options);
66-
67-
*self.nested_configs.write().await = nested_configs;
68-
}
69-
7058
async fn refresh_server_linter(&self) {
7159
let options = self.options.lock().await;
72-
let nested_configs = self.nested_configs.read().await;
73-
let server_linter = ServerLinter::create_server_linter(
74-
self.root_uri.get().unwrap(),
75-
&options,
76-
&nested_configs,
77-
);
60+
let server_linter = ServerLinter::new(self.root_uri.get().unwrap(), &options);
7861

7962
*self.server_linter.write().await = server_linter;
8063
}
@@ -224,48 +207,8 @@ impl WorkspaceWorker {
224207

225208
pub async fn did_change_watched_files(
226209
&self,
227-
file_event: &FileEvent,
210+
_file_event: &FileEvent,
228211
) -> Option<ConcurrentHashMap<String, Vec<DiagnosticReport>>> {
229-
if self.options.lock().await.use_nested_configs() {
230-
let nested_configs = self.nested_configs.read().await;
231-
let nested_configs = nested_configs.pin();
232-
let Some(file_path) = file_event.uri.to_file_path() else {
233-
info!("Unable to convert {:?} to a file path", file_event.uri);
234-
return None;
235-
};
236-
let Some(file_name) = file_path.file_name() else {
237-
info!("Unable to retrieve file name from {file_path:?}");
238-
return None;
239-
};
240-
241-
if file_name != OXC_CONFIG_FILE {
242-
return None;
243-
}
244-
245-
let Some(dir_path) = file_path.parent() else {
246-
info!("Unable to retrieve parent from {file_path:?}");
247-
return None;
248-
};
249-
250-
// spellchecker:off -- "typ" is accurate
251-
if file_event.typ == FileChangeType::CREATED
252-
|| file_event.typ == FileChangeType::CHANGED
253-
{
254-
// spellchecker:on
255-
let oxlintrc =
256-
Oxlintrc::from_file(&file_path).expect("Failed to parse config file");
257-
let config_store_builder = ConfigStoreBuilder::from_oxlintrc(false, oxlintrc)
258-
.expect("Failed to create config store builder");
259-
let config_store =
260-
config_store_builder.build().expect("Failed to build config store");
261-
nested_configs.insert(dir_path.to_path_buf(), config_store);
262-
// spellchecker:off -- "typ" is accurate
263-
} else if file_event.typ == FileChangeType::DELETED {
264-
// spellchecker:on
265-
nested_configs.remove(&dir_path.to_path_buf());
266-
}
267-
}
268-
269212
self.refresh_server_linter().await;
270213
Some(self.revalidate_diagnostics().await)
271214
}
@@ -288,10 +231,6 @@ impl WorkspaceWorker {
288231

289232
*self.options.lock().await = changed_options.clone();
290233

291-
if changed_options.use_nested_configs() != current_option.use_nested_configs() {
292-
self.refresh_nested_configs().await;
293-
}
294-
295234
if Self::needs_linter_restart(current_option, &changed_options) {
296235
self.refresh_server_linter().await;
297236
return Some(self.revalidate_diagnostics().await);

0 commit comments

Comments
 (0)