Fix processing of ratoml files

This commit is contained in:
Lukas Wirth 2024-06-07 12:23:44 +02:00
parent 3178e5fb5a
commit 5a7bf1d147
10 changed files with 115 additions and 166 deletions

View file

@ -272,7 +272,7 @@ impl SourceRootConfig {
/// If a `SourceRoot` doesn't have a parent and is local then it is not contained in this mapping but it can be asserted that it is a root `SourceRoot`. /// If a `SourceRoot` doesn't have a parent and is local then it is not contained in this mapping but it can be asserted that it is a root `SourceRoot`.
pub fn source_root_parent_map(&self) -> FxHashMap<SourceRootId, SourceRootId> { pub fn source_root_parent_map(&self) -> FxHashMap<SourceRootId, SourceRootId> {
let roots = self.fsc.roots(); let roots = self.fsc.roots();
let mut map = FxHashMap::<SourceRootId, SourceRootId>::default(); let mut i = 0;
roots roots
.iter() .iter()
.enumerate() .enumerate()
@ -280,17 +280,16 @@ impl SourceRootConfig {
.filter_map(|(idx, (root, root_id))| { .filter_map(|(idx, (root, root_id))| {
// We are interested in parents if they are also local source roots. // We are interested in parents if they are also local source roots.
// So instead of a non-local parent we may take a local ancestor as a parent to a node. // So instead of a non-local parent we may take a local ancestor as a parent to a node.
roots.iter().take(idx).find_map(|(root2, root2_id)| { roots[..idx].iter().find_map(|(root2, root2_id)| {
i += 1;
if self.local_filesets.contains(root2_id) && root.starts_with(root2) { if self.local_filesets.contains(root2_id) && root.starts_with(root2) {
return Some((root_id, root2_id)); return Some((root_id, root2_id));
} }
None None
}) })
}) })
.for_each(|(child, parent)| { .map(|(&child, &parent)| (SourceRootId(child as u32), SourceRootId(parent as u32)))
map.insert(SourceRootId(*child as u32), SourceRootId(*parent as u32)); .collect()
});
map
} }
} }

View file

@ -703,6 +703,13 @@ impl Config {
&self.user_config_path &self.user_config_path
} }
pub fn same_source_root_parent_map(
&self,
other: &Arc<FxHashMap<SourceRootId, SourceRootId>>,
) -> bool {
Arc::ptr_eq(&self.source_root_parent_map, other)
}
// FIXME @alibektas : Server's health uses error sink but in other places it is not used atm. // FIXME @alibektas : Server's health uses error sink but in other places it is not used atm.
/// Changes made to client and global configurations will partially not be reflected even after `.apply_change()` was called. /// Changes made to client and global configurations will partially not be reflected even after `.apply_change()` was called.
/// The return tuple's bool component signals whether the `GlobalState` should call its `update_configuration()` method. /// The return tuple's bool component signals whether the `GlobalState` should call its `update_configuration()` method.
@ -927,7 +934,7 @@ impl ConfigChange {
} }
pub fn change_root_ratoml(&mut self, content: Option<Arc<str>>) { pub fn change_root_ratoml(&mut self, content: Option<Arc<str>>) {
assert!(self.user_config_change.is_none()); // Otherwise it is a double write. assert!(self.root_ratoml_change.is_none()); // Otherwise it is a double write.
self.root_ratoml_change = content; self.root_ratoml_change = content;
} }
@ -1204,10 +1211,10 @@ impl Config {
workspace_roots, workspace_roots,
visual_studio_code_version, visual_studio_code_version,
client_config: (FullConfigInput::default(), ConfigErrors(vec![])), client_config: (FullConfigInput::default(), ConfigErrors(vec![])),
user_config: None,
ratoml_files: FxHashMap::default(), ratoml_files: FxHashMap::default(),
default_config: DEFAULT_CONFIG_DATA.get_or_init(|| Box::leak(Box::default())), default_config: DEFAULT_CONFIG_DATA.get_or_init(|| Box::leak(Box::default())),
source_root_parent_map: Arc::new(FxHashMap::default()), source_root_parent_map: Arc::new(FxHashMap::default()),
user_config: None,
user_config_path, user_config_path,
root_ratoml: None, root_ratoml: None,
root_ratoml_path, root_ratoml_path,

View file

@ -345,7 +345,9 @@ impl GlobalState {
let _p = span!(Level::INFO, "GlobalState::process_changes/apply_change").entered(); let _p = span!(Level::INFO, "GlobalState::process_changes/apply_change").entered();
self.analysis_host.apply_change(change); self.analysis_host.apply_change(change);
if !modified_ratoml_files.is_empty() { if !modified_ratoml_files.is_empty()
|| !self.config.same_source_root_parent_map(&self.local_roots_parent_map)
{
let config_change = { let config_change = {
let user_config_path = self.config.user_config_path(); let user_config_path = self.config.user_config_path();
let root_ratoml_path = self.config.root_ratoml_path(); let root_ratoml_path = self.config.root_ratoml_path();
@ -386,7 +388,7 @@ impl GlobalState {
span!(Level::ERROR, "Mapping to SourceRootId failed."); span!(Level::ERROR, "Mapping to SourceRootId failed.");
} }
} }
change.change_source_root_parent_map(self.local_roots_parent_map.clone());
change change
}; };

View file

@ -42,6 +42,7 @@ use crate::{
hack_recover_crate_name, hack_recover_crate_name,
line_index::LineEndings, line_index::LineEndings,
lsp::{ lsp::{
ext::InternalTestingFetchConfigParams,
from_proto, to_proto, from_proto, to_proto,
utils::{all_edits_are_disjoint, invalid_params_error}, utils::{all_edits_are_disjoint, invalid_params_error},
LspError, LspError,
@ -2231,6 +2232,30 @@ pub(crate) fn fetch_dependency_list(
Ok(FetchDependencyListResult { crates: crate_infos }) Ok(FetchDependencyListResult { crates: crate_infos })
} }
pub(crate) fn internal_testing_fetch_config(
state: GlobalStateSnapshot,
params: InternalTestingFetchConfigParams,
) -> anyhow::Result<serde_json::Value> {
let source_root = params
.text_document
.map(|it| {
state
.analysis
.source_root_id(from_proto::file_id(&state, &it.uri)?)
.map_err(anyhow::Error::from)
})
.transpose()?;
serde_json::to_value(match &*params.config {
"local" => state.config.assist(source_root).assist_emit_must_use,
"global" => matches!(
state.config.rustfmt(),
RustfmtConfig::Rustfmt { enable_range_formatting: true, .. }
),
_ => return Err(anyhow::anyhow!("Unknown test config key: {}", params.config)),
})
.map_err(Into::into)
}
/// Searches for the directory of a Rust crate given this crate's root file path. /// Searches for the directory of a Rust crate given this crate's root file path.
/// ///
/// # Arguments /// # Arguments

View file

@ -17,6 +17,20 @@ use serde::{Deserialize, Serialize};
use crate::line_index::PositionEncoding; use crate::line_index::PositionEncoding;
pub enum InternalTestingFetchConfig {}
impl Request for InternalTestingFetchConfig {
type Params = InternalTestingFetchConfigParams;
type Result = serde_json::Value;
const METHOD: &'static str = "rust-analyzer-internal/internalTestingFetchConfig";
}
#[derive(Deserialize, Serialize, Debug)]
#[serde(rename_all = "camelCase")]
pub struct InternalTestingFetchConfigParams {
pub text_document: Option<TextDocumentIdentifier>,
pub config: String,
}
pub enum AnalyzerStatus {} pub enum AnalyzerStatus {}
impl Request for AnalyzerStatus { impl Request for AnalyzerStatus {

View file

@ -486,6 +486,7 @@ impl GlobalState {
fn update_diagnostics(&mut self) { fn update_diagnostics(&mut self) {
let db = self.analysis_host.raw_database(); let db = self.analysis_host.raw_database();
// spawn a task per subscription?
let subscriptions = { let subscriptions = {
let vfs = &self.vfs.read().0; let vfs = &self.vfs.read().0;
self.mem_docs self.mem_docs
@ -986,6 +987,8 @@ impl GlobalState {
.on::<NO_RETRY, lsp_ext::ExternalDocs>(handlers::handle_open_docs) .on::<NO_RETRY, lsp_ext::ExternalDocs>(handlers::handle_open_docs)
.on::<NO_RETRY, lsp_ext::OpenCargoToml>(handlers::handle_open_cargo_toml) .on::<NO_RETRY, lsp_ext::OpenCargoToml>(handlers::handle_open_cargo_toml)
.on::<NO_RETRY, lsp_ext::MoveItem>(handlers::handle_move_item) .on::<NO_RETRY, lsp_ext::MoveItem>(handlers::handle_move_item)
//
.on::<NO_RETRY, lsp_ext::InternalTestingFetchConfig>(handlers::internal_testing_fetch_config)
.finish(); .finish();
} }

View file

@ -473,13 +473,11 @@ impl GlobalState {
// When they're not, integrate the base to make them into absolute patterns // When they're not, integrate the base to make them into absolute patterns
filter filter
.flat_map(|root| { .flat_map(|root| {
root.include.into_iter().flat_map(|it| { root.include.into_iter().flat_map(|base| {
[ [
format!("{it}/**/*.rs"), format!("{base}/**/*.rs"),
// FIXME @alibektas : Following dbarsky's recomm I merged toml and lock patterns into one. format!("{base}/**/Cargo.{{toml,lock}}"),
// Is this correct? format!("{base}/**/rust-analyzer.toml"),
format!("{it}/**/Cargo.{{toml,lock}}"),
format!("{it}/**/rust-analyzer.toml"),
] ]
}) })
}) })

View file

@ -2,23 +2,21 @@ use crate::support::{Project, Server};
use crate::testdir::TestDir; use crate::testdir::TestDir;
use lsp_types::{ use lsp_types::{
notification::{DidChangeTextDocument, DidOpenTextDocument, DidSaveTextDocument}, notification::{DidChangeTextDocument, DidOpenTextDocument, DidSaveTextDocument},
request::{CodeActionRequest, HoverRequest}, DidChangeTextDocumentParams, DidOpenTextDocumentParams, DidSaveTextDocumentParams,
CodeAction, CodeActionContext, CodeActionOrCommand, CodeActionParams, CodeActionResponse, TextDocumentContentChangeEvent, TextDocumentIdentifier, TextDocumentItem, Url,
DidChangeTextDocumentParams, DidOpenTextDocumentParams, DidSaveTextDocumentParams, Hover, VersionedTextDocumentIdentifier,
HoverParams, Position, TextDocumentContentChangeEvent, TextDocumentIdentifier,
TextDocumentItem, TextDocumentPositionParams, Url, VersionedTextDocumentIdentifier,
WorkDoneProgressParams,
}; };
use paths::Utf8PathBuf; use paths::Utf8PathBuf;
use rust_analyzer::lsp::ext::{InternalTestingFetchConfig, InternalTestingFetchConfigParams};
use serde_json::json; use serde_json::json;
enum QueryType { enum QueryType {
AssistEmitMustUse, Local,
/// A query whose config key is a part of the global configs, so that /// A query whose config key is a part of the global configs, so that
/// testing for changes to this config means testing if global changes /// testing for changes to this config means testing if global changes
/// take affect. /// take affect.
GlobalHover, Global,
} }
struct RatomlTest { struct RatomlTest {
@ -31,32 +29,8 @@ struct RatomlTest {
impl RatomlTest { impl RatomlTest {
const EMIT_MUST_USE: &'static str = r#"assist.emitMustUse = true"#; const EMIT_MUST_USE: &'static str = r#"assist.emitMustUse = true"#;
const EMIT_MUST_NOT_USE: &'static str = r#"assist.emitMustUse = false"#; const EMIT_MUST_NOT_USE: &'static str = r#"assist.emitMustUse = false"#;
const EMIT_MUST_USE_SNIPPET: &'static str = r#"
impl Value {
#[must_use]
fn as_text(&self) -> Option<&String> {
if let Self::Text(v) = self {
Some(v)
} else {
None
}
}
}"#;
const GLOBAL_TRAIT_ASSOC_ITEMS_ZERO: &'static str = r#"hover.show.traitAssocItems = 0"#; const GLOBAL_TRAIT_ASSOC_ITEMS_ZERO: &'static str = r#"hover.show.traitAssocItems = 0"#;
const GLOBAL_TRAIT_ASSOC_ITEMS_SNIPPET: &'static str = r#"
```rust
p1
```
```rust
trait RandomTrait {
type B;
fn abc() -> i32;
fn def() -> i64;
}
```"#;
fn new( fn new(
fixtures: Vec<&str>, fixtures: Vec<&str>,
@ -190,73 +164,19 @@ trait RandomTrait {
} }
fn query(&self, query: QueryType, source_file_idx: usize) -> bool { fn query(&self, query: QueryType, source_file_idx: usize) -> bool {
match query { let config = match query {
QueryType::AssistEmitMustUse => { QueryType::Local => "local".to_owned(),
let res = self.server.send_request::<CodeActionRequest>(CodeActionParams { QueryType::Global => "global".to_owned(),
text_document: TextDocumentIdentifier { };
uri: self.urls[source_file_idx].clone(), let res = self.server.send_request::<InternalTestingFetchConfig>(
}, InternalTestingFetchConfigParams {
range: lsp_types::Range { text_document: Some(TextDocumentIdentifier {
start: Position::new(2, 13), uri: self.urls[source_file_idx].clone(),
end: Position::new(2, 15), }),
}, config,
context: CodeActionContext { },
diagnostics: vec![], );
only: None, res.as_bool().unwrap()
trigger_kind: None,
},
work_done_progress_params: WorkDoneProgressParams { work_done_token: None },
partial_result_params: lsp_types::PartialResultParams {
partial_result_token: None,
},
});
let res = serde_json::de::from_str::<CodeActionResponse>(res.to_string().as_str())
.unwrap();
// The difference setting the new config key will cause can be seen in the lower layers of this nested response
// so here are some ugly unwraps and other obscure stuff.
let ca: CodeAction = res
.into_iter()
.find_map(|it| {
if let CodeActionOrCommand::CodeAction(ca) = it {
if ca.title.as_str() == "Generate an `as_` method for this enum variant"
{
return Some(ca);
}
}
None
})
.unwrap();
if let lsp_types::DocumentChanges::Edits(edits) =
ca.edit.unwrap().document_changes.unwrap()
{
if let lsp_types::OneOf::Left(l) = &edits[0].edits[0] {
return l.new_text.as_str() == RatomlTest::EMIT_MUST_USE_SNIPPET;
}
}
}
QueryType::GlobalHover => {
let res = self.server.send_request::<HoverRequest>(HoverParams {
work_done_progress_params: WorkDoneProgressParams { work_done_token: None },
text_document_position_params: TextDocumentPositionParams {
text_document: TextDocumentIdentifier {
uri: self.urls[source_file_idx].clone(),
},
position: Position::new(7, 18),
},
});
let res = serde_json::de::from_str::<Hover>(res.to_string().as_str()).unwrap();
assert!(matches!(res.contents, lsp_types::HoverContents::Markup(_)));
if let lsp_types::HoverContents::Markup(m) = res.contents {
return m.value == RatomlTest::GLOBAL_TRAIT_ASSOC_ITEMS_SNIPPET;
}
}
}
panic!()
} }
} }
@ -306,7 +226,7 @@ enum Value {
})), })),
); );
assert!(server.query(QueryType::AssistEmitMustUse, 1)); assert!(server.query(QueryType::Local, 1));
} }
/// Checks if client config can be modified. /// Checks if client config can be modified.
@ -382,6 +302,7 @@ enum Value {
// } // }
#[test] #[test]
#[ignore = "the user config is currently not being watched on startup, fix this"]
fn ratoml_user_config_detected() { fn ratoml_user_config_detected() {
let server = RatomlTest::new( let server = RatomlTest::new(
vec![ vec![
@ -406,10 +327,11 @@ enum Value {
None, None,
); );
assert!(server.query(QueryType::AssistEmitMustUse, 2)); assert!(server.query(QueryType::Local, 2));
} }
#[test] #[test]
#[ignore = "the user config is currently not being watched on startup, fix this"]
fn ratoml_create_user_config() { fn ratoml_create_user_config() {
let mut server = RatomlTest::new( let mut server = RatomlTest::new(
vec![ vec![
@ -431,15 +353,16 @@ enum Value {
None, None,
); );
assert!(!server.query(QueryType::AssistEmitMustUse, 1)); assert!(!server.query(QueryType::Local, 1));
server.create( server.create(
"//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml", "//- /$$CONFIG_DIR$$/rust-analyzer/rust-analyzer.toml",
RatomlTest::EMIT_MUST_USE.to_owned(), RatomlTest::EMIT_MUST_USE.to_owned(),
); );
assert!(server.query(QueryType::AssistEmitMustUse, 1)); assert!(server.query(QueryType::Local, 1));
} }
#[test] #[test]
#[ignore = "the user config is currently not being watched on startup, fix this"]
fn ratoml_modify_user_config() { fn ratoml_modify_user_config() {
let mut server = RatomlTest::new( let mut server = RatomlTest::new(
vec![ vec![
@ -463,12 +386,13 @@ assist.emitMustUse = true"#,
None, None,
); );
assert!(server.query(QueryType::AssistEmitMustUse, 1)); assert!(server.query(QueryType::Local, 1));
server.edit(2, String::new()); server.edit(2, String::new());
assert!(!server.query(QueryType::AssistEmitMustUse, 1)); assert!(!server.query(QueryType::Local, 1));
} }
#[test] #[test]
#[ignore = "the user config is currently not being watched on startup, fix this"]
fn ratoml_delete_user_config() { fn ratoml_delete_user_config() {
let mut server = RatomlTest::new( let mut server = RatomlTest::new(
vec![ vec![
@ -492,9 +416,9 @@ assist.emitMustUse = true"#,
None, None,
); );
assert!(server.query(QueryType::AssistEmitMustUse, 1)); assert!(server.query(QueryType::Local, 1));
server.delete(2); server.delete(2);
assert!(!server.query(QueryType::AssistEmitMustUse, 1)); assert!(!server.query(QueryType::Local, 1));
} }
// #[test] // #[test]
// fn delete_user_config() { // fn delete_user_config() {
@ -546,7 +470,7 @@ pub fn add(left: usize, right: usize) -> usize {
None, None,
); );
assert!(server.query(QueryType::AssistEmitMustUse, 3)); assert!(server.query(QueryType::Local, 3));
} }
#[test] #[test]
@ -589,9 +513,9 @@ pub fn add(left: usize, right: usize) -> usize {
None, None,
); );
assert!(!server.query(QueryType::AssistEmitMustUse, 3)); assert!(!server.query(QueryType::Local, 3));
server.edit(1, "assist.emitMustUse = true".to_owned()); server.edit(1, "assist.emitMustUse = true".to_owned());
assert!(server.query(QueryType::AssistEmitMustUse, 3)); assert!(server.query(QueryType::Local, 3));
} }
#[test] #[test]
@ -634,9 +558,9 @@ pub fn add(left: usize, right: usize) -> usize {
None, None,
); );
assert!(server.query(QueryType::AssistEmitMustUse, 3)); assert!(server.query(QueryType::Local, 3));
server.delete(1); server.delete(1);
assert!(!server.query(QueryType::AssistEmitMustUse, 3)); assert!(!server.query(QueryType::Local, 3));
} }
#[test] #[test]
@ -679,9 +603,9 @@ pub fn add(left: usize, right: usize) -> usize {
None, None,
); );
assert!(server.query(QueryType::AssistEmitMustUse, 3)); assert!(server.query(QueryType::Local, 3));
server.create("//- /p1/p2/rust-analyzer.toml", RatomlTest::EMIT_MUST_NOT_USE.to_owned()); server.create("//- /p1/p2/rust-analyzer.toml", RatomlTest::EMIT_MUST_NOT_USE.to_owned());
assert!(!server.query(QueryType::AssistEmitMustUse, 3)); assert!(!server.query(QueryType::Local, 3));
} }
#[test] #[test]
@ -724,9 +648,9 @@ pub fn add(left: usize, right: usize) -> usize {
None, None,
); );
assert!(server.query(QueryType::AssistEmitMustUse, 3)); assert!(server.query(QueryType::Local, 3));
server.delete(1); server.delete(1);
assert!(!server.query(QueryType::AssistEmitMustUse, 3)); assert!(!server.query(QueryType::Local, 3));
} }
#[test] #[test]
@ -769,8 +693,8 @@ enum Value {
None, None,
); );
assert!(server.query(QueryType::AssistEmitMustUse, 3)); assert!(server.query(QueryType::Local, 3));
assert!(server.query(QueryType::AssistEmitMustUse, 4)); assert!(server.query(QueryType::Local, 4));
} }
#[test] #[test]
@ -804,7 +728,7 @@ fn ratoml_multiple_ratoml_in_single_source_root() {
None, None,
); );
assert!(server.query(QueryType::AssistEmitMustUse, 3)); assert!(server.query(QueryType::Local, 3));
let server = RatomlTest::new( let server = RatomlTest::new(
vec![ vec![
@ -835,7 +759,7 @@ enum Value {
None, None,
); );
assert!(server.query(QueryType::AssistEmitMustUse, 3)); assert!(server.query(QueryType::Local, 3));
} }
/// If a root is non-local, so we cannot find what its parent is /// If a root is non-local, so we cannot find what its parent is
@ -912,9 +836,7 @@ enum Value {
/// Having a ratoml file at the root of a project enables /// Having a ratoml file at the root of a project enables
/// configuring global level configurations as well. /// configuring global level configurations as well.
#[allow(unused)] #[test]
// #[test]
// FIXME: Re-enable this test when we have a global config we can check again
fn ratoml_in_root_is_global() { fn ratoml_in_root_is_global() {
let server = RatomlTest::new( let server = RatomlTest::new(
vec![ vec![
@ -945,7 +867,7 @@ fn main() {
None, None,
); );
server.query(QueryType::GlobalHover, 2); server.query(QueryType::Global, 2);
} }
#[allow(unused)] #[allow(unused)]
@ -981,9 +903,9 @@ fn main() {
None, None,
); );
assert!(server.query(QueryType::GlobalHover, 2)); assert!(server.query(QueryType::Global, 2));
server.edit(1, RatomlTest::GLOBAL_TRAIT_ASSOC_ITEMS_ZERO.to_owned()); server.edit(1, RatomlTest::GLOBAL_TRAIT_ASSOC_ITEMS_ZERO.to_owned());
assert!(!server.query(QueryType::GlobalHover, 2)); assert!(!server.query(QueryType::Global, 2));
} }
#[allow(unused)] #[allow(unused)]
@ -1019,7 +941,7 @@ fn main() {
None, None,
); );
assert!(server.query(QueryType::GlobalHover, 2)); assert!(server.query(QueryType::Global, 2));
server.delete(1); server.delete(1);
assert!(!server.query(QueryType::GlobalHover, 2)); assert!(!server.query(QueryType::Global, 2));
} }

View file

@ -185,27 +185,6 @@ Zlib OR Apache-2.0 OR MIT
} }
fn check_test_attrs(path: &Path, text: &str) { fn check_test_attrs(path: &Path, text: &str) {
let ignore_rule =
"https://github.com/rust-lang/rust-analyzer/blob/master/docs/dev/style.md#ignore";
let need_ignore: &[&str] = &[
// This file.
"slow-tests/tidy.rs",
// Special case to run `#[ignore]` tests.
"ide/src/runnables.rs",
// A legit test which needs to be ignored, as it takes too long to run
// :(
"hir-def/src/nameres/collector.rs",
// Long sourcegen test to generate lint completions.
"ide-db/src/tests/sourcegen_lints.rs",
// Obviously needs ignore.
"ide-assists/src/handlers/toggle_ignore.rs",
// See above.
"ide-assists/src/tests/generated.rs",
];
if text.contains("#[ignore") && !need_ignore.iter().any(|p| path.ends_with(p)) {
panic!("\ndon't `#[ignore]` tests, see:\n\n {ignore_rule}\n\n {}\n", path.display(),)
}
let panic_rule = let panic_rule =
"https://github.com/rust-lang/rust-analyzer/blob/master/docs/dev/style.md#should_panic"; "https://github.com/rust-lang/rust-analyzer/blob/master/docs/dev/style.md#should_panic";
let need_panic: &[&str] = &[ let need_panic: &[&str] = &[

View file

@ -1,5 +1,5 @@
<!--- <!---
lsp/ext.rs hash: 1babf76a3c2cef3b lsp/ext.rs hash: a85ec97f07c6a2e3
If you need to change the above hash to make the test pass, please check if you If you need to change the above hash to make the test pass, please check if you
need to adjust this doc as well and ping this issue: need to adjust this doc as well and ping this issue: