Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
100 changes: 82 additions & 18 deletions crates/oxc_language_server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use futures::future::join_all;
use globset::Glob;
use ignore::gitignore::Gitignore;
use log::{debug, error, info};
use rustc_hash::FxBuildHasher;
use oxc_linter::{ConfigStore, ConfigStoreBuilder, FixKind, LintOptions, Linter, Oxlintrc};
use rustc_hash::{FxBuildHasher, FxHashMap};
use serde::{Deserialize, Serialize};
use tokio::sync::{Mutex, OnceCell, RwLock, SetError};
use tower_lsp::{
Expand All @@ -15,14 +16,12 @@ use tower_lsp::{
CodeAction, CodeActionKind, CodeActionOrCommand, CodeActionParams, CodeActionResponse,
ConfigurationItem, Diagnostic, DidChangeConfigurationParams, DidChangeTextDocumentParams,
DidChangeWatchedFilesParams, DidCloseTextDocumentParams, DidOpenTextDocumentParams,
DidSaveTextDocumentParams, ExecuteCommandParams, InitializeParams, InitializeResult,
InitializedParams, NumberOrString, Position, Range, ServerInfo, TextEdit, Url,
WorkspaceEdit,
DidSaveTextDocumentParams, ExecuteCommandParams, FileChangeType, InitializeParams,
InitializeResult, InitializedParams, NumberOrString, Position, Range, ServerInfo, TextEdit,
Url, WorkspaceEdit,
},
};

use oxc_linter::{ConfigStoreBuilder, FixKind, LintOptions, Linter, Oxlintrc};

use crate::capabilities::{CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC, Capabilities};
use crate::linter::error_with_position::DiagnosticReport;
use crate::linter::server_linter::ServerLinter;
Expand All @@ -33,13 +32,16 @@ mod linter;

type ConcurrentHashMap<K, V> = papaya::HashMap<K, V, FxBuildHasher>;

const OXC_CONFIG_FILE: &str = ".oxlintrc.json";

struct Backend {
client: Client,
root_uri: OnceCell<Option<Url>>,
server_linter: RwLock<ServerLinter>,
diagnostics_report_map: ConcurrentHashMap<String, Vec<DiagnosticReport>>,
options: Mutex<Options>,
gitignore_glob: Mutex<Vec<Gitignore>>,
nested_configs: RwLock<FxHashMap<PathBuf, ConfigStore>>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Store a map of directory to config for all nested configs in the workspace.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know if this is the correct type. RwLock<FxHashMap<PathBuf, ConfigStore>>. Maybe something else should be used, but this is the only one I could figure out how to make work.

Copy link
Member

Choose a reason for hiding this comment

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

Seems ok to me, but I'm not an expert in the async data structures area 😄 Might be worth looking at https://docs.rs/papaya/latest/papaya/, as I know we have been using that recently?

}
#[derive(Debug, Serialize, Deserialize, Default, PartialEq, PartialOrd, Clone, Copy)]
#[serde(rename_all = "camelCase")]
Expand All @@ -54,11 +56,17 @@ struct Options {
run: Run,
enable: bool,
config_path: String,
use_nested_configs: bool,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Allow opting into nested configs. If false, there shouldn't be any behavior change.

}

impl Default for Options {
fn default() -> Self {
Self { enable: true, run: Run::default(), config_path: ".oxlintrc.json".into() }
Self {
enable: true,
run: Run::default(),
config_path: OXC_CONFIG_FILE.into(),
use_nested_configs: false,
}
}
}

Expand Down Expand Up @@ -180,8 +188,38 @@ impl LanguageServer for Backend {
}
}

async fn did_change_watched_files(&self, _params: DidChangeWatchedFilesParams) {
async fn did_change_watched_files(&self, params: DidChangeWatchedFilesParams) {
debug!("watched file did change");
if self.options.lock().await.use_nested_configs {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If using nested configs, check the watched files to determine if they are .oxlintrc.json files (which are the only nested config files that are supported to my knowledge).

This just keeps the nested config map up to date with any workspace changes.

let mut config_files = FxHashMap::default();
let existing_nested_configs = (*self.nested_configs.read().await).clone();
for (key, value) in existing_nested_configs {
config_files.insert(key.clone(), value.clone());
Copy link
Member

Choose a reason for hiding this comment

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

instead of inserting one-by-one, maybe we can try extending the hash map all at once? this should be more efficient because it allocates the needed memory upfront: https://doc.rust-lang.org/nightly/std/collections/struct.HashMap.html#impl-Extend%3C(K,+V)%3E-for-HashMap%3CK,+V,+S%3E

}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am copying the map here, so I can mutate it later, before setting the value back into self.nested_configs. There might be a better way to do this that I don't know about.

Copy link
Member

Choose a reason for hiding this comment

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

Might be worth looking at https://doc.rust-lang.org/std/mem/fn.take.html. It still requires allocating an empty hash map, but that is much cheaper compared to cloning a full hash map. Then after you've taken the hash map, you have full ownership over it and can insert into as needed.


params.changes.iter().for_each(|x| {
if x.uri.to_file_path().unwrap().file_name().unwrap() != OXC_CONFIG_FILE {
return;
}
// spellchecker:off -- "typ" is accurate
if x.typ == FileChangeType::CREATED || x.typ == FileChangeType::CHANGED {
// spellchecker:on
let file_path = x.uri.to_file_path().unwrap();
Comment on lines +201 to +207
Copy link
Member

Choose a reason for hiding this comment

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

we could avoid a duplicate unwrap here and some work by saving to_file_path():

Suggested change
if x.uri.to_file_path().unwrap().file_name().unwrap() != OXC_CONFIG_FILE {
return;
}
// spellchecker:off -- "typ" is accurate
if x.typ == FileChangeType::CREATED || x.typ == FileChangeType::CHANGED {
// spellchecker:on
let file_path = x.uri.to_file_path().unwrap();
let Some(file_path) = x.uri.to_file_path() else {
return;
};
if file_path.file_name().unwrap() != OXC_CONFIG_FILE {
return;
}
// spellchecker:off -- "typ" is accurate
if x.typ == FileChangeType::CREATED || x.typ == FileChangeType::CHANGED {
// spellchecker:on

let dir_path = file_path.parent().unwrap();
let oxlintrc = Oxlintrc::from_file(&dir_path.join(OXC_CONFIG_FILE)).unwrap();
let config_store_builder =
ConfigStoreBuilder::from_oxlintrc(false, oxlintrc).unwrap();
let config_store = config_store_builder.build().unwrap();
config_files.insert(dir_path.to_path_buf(), config_store);
// spellchecker:off -- "typ" is accurate
} else if x.typ == FileChangeType::DELETED {
// spellchecker:on
config_files.remove(&x.uri.to_file_path().unwrap());
}
});
*self.nested_configs.write().await = config_files;
}

self.init_linter_config().await;
self.revalidate_open_files().await;
}
Expand Down Expand Up @@ -492,21 +530,46 @@ impl Backend {
if config.exists() {
config_path = Some(config);
}

let mut oxlintrc = None;
let mut config_store = None;
let mut linter = self.server_linter.write().await;
if let Some(config_path) = config_path {
let mut linter = self.server_linter.write().await;
let config = Oxlintrc::from_file(&config_path)
.expect("should have initialized linter with new options");
let config_store = ConfigStoreBuilder::from_oxlintrc(true, config.clone())
.expect("failed to build config")
.build()
.expect("failed to build config");
oxlintrc = Some(
Oxlintrc::from_file(&config_path)
.expect("should have initialized linter with new options"),
);
config_store = Some(
ConfigStoreBuilder::from_oxlintrc(true, oxlintrc.clone().unwrap())
.expect("failed to build config")
.build()
.expect("failed to build config"),
);
}

if self.options.lock().await.use_nested_configs {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If using nested configs, use Linter::new_with_nested_configs and pass in the nested config files. Otherwise, should be no behavior change. However, I did do some refactoring here to make this easier for me to read through.

let mut config_files: FxHashMap<PathBuf, ConfigStore> = FxHashMap::default();
let existing_nested_configs = (*self.nested_configs.read().await).clone();
for (key, value) in existing_nested_configs {
config_files.insert(key.clone(), value.clone());
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copying the map so that I can pass it into Linter::new_with_nested_configs. Might be a better way to do the copy that I'm not aware of.

Copy link
Member

Choose a reason for hiding this comment

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

maybe std::mem::take? https://doc.rust-lang.org/std/mem/fn.take.html but not sure if that works in async context like this


*linter = ServerLinter::new_with_linter(
Linter::new_with_nested_configs(
LintOptions::default(),
config_store.unwrap(),
config_files,
)
.with_fix(FixKind::SafeFix),
);
} else {
*linter = ServerLinter::new_with_linter(
Linter::new(LintOptions::default(), config_store).with_fix(FixKind::SafeFix),
Linter::new(LintOptions::default(), config_store.unwrap())
.with_fix(FixKind::SafeFix),
);
return Some(config);
}

None
Some(oxlintrc.clone().unwrap())
}

async fn handle_file_update(&self, uri: Url, content: Option<String>, version: Option<i32>) {
Expand Down Expand Up @@ -568,6 +631,7 @@ async fn main() {
diagnostics_report_map,
options: Mutex::new(Options::default()),
gitignore_glob: Mutex::new(vec![]),
nested_configs: RwLock::new(FxHashMap::default()),
})
.finish();

Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/config/config_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ impl Clone for ResolvedLinterState {
}
}

#[derive(Debug)]
#[derive(Debug, Clone)]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needed so that I can clone my map that contains this struct.

struct Config {
/// The basic linter state for this configuration.
base: ResolvedLinterState,
Expand All @@ -29,7 +29,7 @@ struct Config {
}

/// Resolves a lint configuration for a given file, by applying overrides based on the file's path.
#[derive(Debug)]
#[derive(Debug, Clone)]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needed so that I can clone my map that contains this struct.

pub struct ConfigStore {
base: Config,
}
Expand Down
File renamed without changes.
15 changes: 15 additions & 0 deletions editors/vscode/client/Config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export class Config implements ConfigInterface {
private _trace!: TraceLevel;
private _configPath!: string;
private _binPath: string | undefined;
private _useNestedConfigs!: boolean;

constructor() {
this.refresh();
Expand All @@ -21,6 +22,7 @@ export class Config implements ConfigInterface {
this._trace = conf.get<TraceLevel>('trace.server') || 'off';
this._configPath = conf.get<string>('configPath') || '.oxlintrc.json';
this._binPath = conf.get<string>('path.server');
this._useNestedConfigs = conf.get<boolean>('useNestedConfigs') ?? false;
}

get runTrigger(): Trigger {
Expand Down Expand Up @@ -78,11 +80,23 @@ export class Config implements ConfigInterface {
.update('path.server', value);
}

get useNestedConfigs(): boolean {
return this._useNestedConfigs;
}

updateUseNestedConfigs(value: boolean): PromiseLike<void> {
this._useNestedConfigs = value;
return workspace
.getConfiguration(Config._namespace)
.update('useNestedConfigs', value);
}

public toLanguageServerConfig(): LanguageServerConfig {
return {
run: this.runTrigger,
enable: this.enable,
configPath: this.configPath,
useNestedConfigs: this.useNestedConfigs,
};
}
}
Expand All @@ -91,6 +105,7 @@ interface LanguageServerConfig {
configPath: string;
enable: boolean;
run: Trigger;
useNestedConfigs: boolean;
}

export type Trigger = 'onSave' | 'onType';
Expand Down
5 changes: 4 additions & 1 deletion editors/vscode/client/config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Config } from './Config.js';
suite('Config', () => {
setup(async () => {
const wsConfig = workspace.getConfiguration('oxc');
const keys = ['lint.run', 'enable', 'trace.server', 'configPath', 'path.server'];
const keys = ['lint.run', 'enable', 'trace.server', 'configPath', 'path.server', 'useNestedConfigs'];

await Promise.all(keys.map(key => wsConfig.update(key, undefined)));
});
Expand All @@ -18,6 +18,7 @@ suite('Config', () => {
strictEqual(config.trace, 'off');
strictEqual(config.configPath, '.oxlintrc.json');
strictEqual(config.binPath, '');
strictEqual(config.useNestedConfigs, true);
});

test('updating values updates the workspace configuration', async () => {
Expand All @@ -29,6 +30,7 @@ suite('Config', () => {
config.updateTrace('messages'),
config.updateConfigPath('./somewhere'),
config.updateBinPath('./binary'),
config.updateUseNestedConfigs(false),
]);

const wsConfig = workspace.getConfiguration('oxc');
Expand All @@ -38,5 +40,6 @@ suite('Config', () => {
strictEqual(wsConfig.get('trace.server'), 'messages');
strictEqual(wsConfig.get('configPath'), './somewhere');
strictEqual(wsConfig.get('path.server'), './binary');
strictEqual(wsConfig.get('useNestedConfigs'), false);
});
});
42 changes: 37 additions & 5 deletions editors/vscode/client/extension.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,23 @@
import { promises as fsPromises } from 'node:fs';

import { commands, ExtensionContext, StatusBarAlignment, StatusBarItem, ThemeColor, window, workspace } from 'vscode';
import {
commands,
ExtensionContext,
RelativePattern,
StatusBarAlignment,
StatusBarItem,
ThemeColor,
window,
workspace,
} from 'vscode';

import { ExecuteCommandRequest, MessageType, ShowMessageNotification } from 'vscode-languageclient';
import {
DidChangeWatchedFilesNotification,
ExecuteCommandRequest,
FileChangeType,
MessageType,
ShowMessageNotification,
} from 'vscode-languageclient';

import { Executable, LanguageClient, LanguageClientOptions, ServerOptions } from 'vscode-languageclient/node';

Expand All @@ -12,6 +27,7 @@ import { ConfigService } from './ConfigService';
const languageClientName = 'oxc';
const outputChannelName = 'Oxc';
const commandPrefix = 'oxc';
const oxlintConfigFileGlob = '**/.oxlintrc.json';

const enum OxcCommands {
RestartServer = `${commandPrefix}.restartServer`,
Expand Down Expand Up @@ -98,6 +114,10 @@ export async function activate(context: ExtensionContext) {
configService,
);

const workspaceConfigPatterns = (workspace.workspaceFolders || []).map((workspaceFolder) =>
new RelativePattern(workspaceFolder, configService.config.configPath)
);

const outputChannel = window.createOutputChannel(outputChannelName, { log: true });

async function findBinary(): Promise<string> {
Expand Down Expand Up @@ -171,8 +191,10 @@ export async function activate(context: ExtensionContext) {
synchronize: {
// Notify the server about file config changes in the workspace
fileEvents: [
workspace.createFileSystemWatcher('**/.oxlint{.json,rc.json}'),
workspace.createFileSystemWatcher('**/oxlint{.json,rc.json}'),
workspace.createFileSystemWatcher(oxlintConfigFileGlob),
...workspaceConfigPatterns.map((workspaceConfigPattern) => {
return workspace.createFileSystemWatcher(workspaceConfigPattern);
}),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Watch any .oxlintrc.json file in the workspace.
Watch the file that the user has configured within their configuration.

This stops watching all variations of oxlint config files that were previously watched. Which, I think is safe.

],
},
initializationOptions: {
Expand Down Expand Up @@ -242,7 +264,17 @@ export async function activate(context: ExtensionContext) {
myStatusBarItem.backgroundColor = bgColor;
}
updateStatsBar(configService.config.enable);
client.start();
await client.start();

const initialConfigFiles =
(await Promise.all([oxlintConfigFileGlob, ...workspaceConfigPatterns].map(async globPattern => {
return workspace.findFiles(globPattern);
}))).flat();
await client.sendNotification(DidChangeWatchedFilesNotification.type, {
changes: initialConfigFiles.map(file => {
return { uri: file.toString(), type: FileChangeType.Created };
}),
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On startup, send all found config files to the language server as "created", so that they get added to the nested config map.

}

export function deactivate(): Thenable<void> | undefined {
Expand Down
8 changes: 7 additions & 1 deletion editors/vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@
"type": "string",
"scope": "window",
"description": "Path to Oxc language server binary."
},
"oxc.useNestedConfigs": {
"type": "boolean",
"scope": "window",
"default": false,
"description": "Whether to enable experimental nested config resolution for Oxc."
}
}
},
Expand Down Expand Up @@ -139,7 +145,7 @@
"install-extension": "code --install-extension oxc_language_server.vsix --force",
"server:build:debug": "cargo build -p oxc_language_server",
"server:build:release": "cross-env CARGO_TARGET_DIR=./target cargo build -p oxc_language_server --release",
"lint": "npx oxlint --config=oxlint.json --tsconfig=tsconfig.json",
"lint": "npx oxlint --config=.oxlintrc.json --tsconfig=tsconfig.json",
"test": "esbuild client/config.spec.ts --bundle --outfile=out/config.spec.js --external:vscode --format=cjs --platform=node --target=node16 --minify --sourcemap && vscode-test",
"type-check": "tsc --noEmit"
},
Expand Down
Loading