diff --git a/Cargo.lock b/Cargo.lock index d95cdae0e..8618a4ed9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1160,12 +1160,6 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" -[[package]] -name = "foldhash" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77ce24cb58228fbb8aa041425bb1050850ac19177686ea6e0f41a70416f56fdb" - [[package]] name = "form_urlencoded" version = "1.2.2" @@ -1473,7 +1467,7 @@ checksum = "9229cfe53dfd69f0609a49f65461bd93001ea1ef889cd5529dd176593f5338a1" dependencies = [ "allocator-api2", "equivalent", - "foldhash 0.1.5", + "foldhash", ] [[package]] @@ -1481,11 +1475,6 @@ name = "hashbrown" version = "0.16.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" -dependencies = [ - "allocator-api2", - "equivalent", - "foldhash 0.2.0", -] [[package]] name = "hashlink" @@ -1989,15 +1978,6 @@ dependencies = [ "logos-codegen", ] -[[package]] -name = "lru" -version = "0.16.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f66e8d5d03f609abc3a39e6f08e4164ebf1447a732906d39eb9b99b7919ef39" -dependencies = [ - "hashbrown 0.16.1", -] - [[package]] name = "ls-types" version = "0.0.6" @@ -2991,7 +2971,6 @@ dependencies = [ "globset", "ignore", "json-strip-comments 3.1.0", - "lru", "pgls_analyse", "pgls_analyser", "pgls_catalog", diff --git a/crates/pgls_workspace/Cargo.toml b/crates/pgls_workspace/Cargo.toml index 280b3733c..bfdb1e9e5 100644 --- a/crates/pgls_workspace/Cargo.toml +++ b/crates/pgls_workspace/Cargo.toml @@ -13,7 +13,6 @@ version = "0.0.0" [dependencies] globset = "0.4.16" -lru = "0.16.3" ignore = { workspace = true } json-strip-comments = { workspace = true } diff --git a/crates/pgls_workspace/src/workspace/server.rs b/crates/pgls_workspace/src/workspace/server.rs index 14d207215..6bc5bae19 100644 --- a/crates/pgls_workspace/src/workspace/server.rs +++ b/crates/pgls_workspace/src/workspace/server.rs @@ -1,4 +1,5 @@ use std::{ + borrow::Borrow, collections::HashMap, fs, hash::{DefaultHasher, Hash, Hasher}, @@ -29,8 +30,8 @@ use pgls_analyser::{ Analyser, AnalyserConfig, AnalyserParams, AnalysisResult, Direction, FileKind, LinterDiagnostic, LinterOptions, MigrationSection, RuleFix, }; -use pgls_catalog::resolve::{Reference, ResolveParams, resolve}; -use pgls_catalog::{Catalog, CatalogBase, Session, TransactionMode}; +use pgls_catalog::resolve::{ColumnSource, Reference, ResolveParams, resolve}; +use pgls_catalog::{Catalog, CatalogBase, Origin, Session, TransactionMode}; use std::borrow::Cow; use pgls_catalog::Snapshot; @@ -109,13 +110,22 @@ pub struct WorkspaceServer { /// The indexed database snapshot the catalog starts from catalog_base: Mutex>>, - /// The last snapshot built for completions and hover, by the statements it includes - snapshot_before: Mutex)>>, + /// The last snapshot built for completions and hover + snapshot_before: Mutex>, #[cfg(feature = "db")] connection: ConnectionManager, } +/// A database snapshot with the changes of the statements before a statement. +struct SnapshotBefore { + /// The database snapshot it is built from. + base: Arc, + /// The text that the statement sees. + key: u64, + snapshot: Arc, +} + /// The `Workspace` object is long-lived, so we want it to be able to cross /// unwind boundaries. /// In return, we have to make sure operations on the workspace either do not @@ -431,6 +441,7 @@ impl WorkspaceServer { /// earlier in the file. Each migration section gets a session of its own, and the changes of /// a down section don't carry into the next section. `visit` gets every parsed statement with /// the catalog and the session as they are before it. `settings` are the ones of the file. + /// Returns the catalog after the walked statements. fn walk_statements( &self, settings: Option<&Settings>, @@ -438,7 +449,7 @@ impl WorkspaceServer { snapshot: &Arc, end: TextSize, mut visit: impl FnMut(&pgls_analyser::AnalysableStatement, &Catalog, &Session), - ) { + ) -> Catalog { let (search_path, sections) = settings .map(|settings| { ( @@ -507,35 +518,38 @@ impl WorkspaceServer { session.apply(&stmt.root); } } + catalog } - /// The references whose span contains `position`, in the innermost statement at `position` - /// (the body of a SQL function rather than its `CREATE FUNCTION`). - fn references_at( + /// For hover on the statement `id` at `range`: the references whose span contains + /// `position`, and the database snapshot as the statement sees it. + fn hover_references( &self, settings: Option<&Settings>, doc: &Document, - snapshot: &Arc, + id: &StatementId, + range: TextRange, position: TextSize, - ) -> Vec { - let mut innermost: Option<(TextRange, Vec)> = None; - self.walk_statements( + snapshot: Arc, + ) -> (Vec, Arc) { + let search_path = settings + .map(|settings| settings.typecheck.search_path.clone()) + .unwrap_or_default(); + let mut found = None; + let catalog = self.walk_statements( settings, doc, - snapshot, + &snapshot, position + TextSize::from(1), |stmt, catalog, session| { let Some(sql) = stmt.sql.as_deref() else { return; }; - if !stmt.range.contains(position) - || innermost - .as_ref() - .is_some_and(|(range, _)| !range.contains_range(stmt.range)) - { + if stmt.range != range { return; } - let references = resolve(ResolveParams { + let position = position - range.start(); + let (references, others): (Vec<_>, Vec<_>) = resolve(ResolveParams { stmt: &stmt.root, catalog, search_path: session.search_path(), @@ -543,22 +557,37 @@ impl WorkspaceServer { sql: Some(sql), collect_references: true, }) - .references; - innermost = Some((stmt.range, references)); + .references + .into_iter() + .partition(|reference| { + reference + .span() + .is_some_and(|span| span.contains_inclusive(position)) + }); + // Objects that the file didn't change are the same in the database snapshot. + let snapshot = if in_database(&references, &others) { + Arc::clone(&snapshot) + } else { + self.snapshot_before( + doc, + id, + range, + Arc::clone(&snapshot), + &search_path, + || catalog, + ) + }; + found = Some((references, snapshot)); }, ); - let Some((range, references)) = innermost else { - return Vec::new(); - }; - let position = position - range.start(); - references - .into_iter() - .filter(|reference| { - reference - .span() - .is_some_and(|span| span.contains_inclusive(position)) - }) - .collect() + // A statement that doesn't parse is not visited. It changes nothing, so it sees the + // catalog after the walk. + found.unwrap_or_else(|| { + ( + Vec::new(), + self.snapshot_before(doc, id, range, snapshot, &search_path, || catalog), + ) + }) } /// The indexed database snapshot, built once per snapshot. @@ -572,48 +601,43 @@ impl WorkspaceServer { base } - /// The database snapshot as the statement at `statement_start` sees it: with the objects - /// that the statements before it create, change, or drop. - fn snapshot_before( + /// The database snapshot as the statement `id` at `range` sees it: with the objects that the + /// statements before it create, change, or drop. `catalog` returns the catalog that the + /// statement sees, and is only called when the snapshot is not cached. + fn snapshot_before>( &self, doc: &Document, - statement_start: TextSize, + id: &StatementId, + range: TextRange, snapshot: Arc, search_path: &[String], + catalog: impl FnOnce() -> C, ) -> Arc { - let statements: Vec<_> = doc - .iter(ExecuteStatementMapper) - .take_while(|(_, range, _, _)| range.end() <= statement_start) - .filter(|(id, _, _, _)| !id.is_child()) - .filter_map(|(_, _, sql, ast)| Some((sql, ast?))) - .collect(); - if statements.is_empty() { - return snapshot; - } - + // The statement sees the text before it, and the body of a SQL function also sees its + // `CREATE FUNCTION`. Files with other settings can see another search path. let mut hasher = DefaultHasher::new(); - Arc::as_ptr(&snapshot).hash(&mut hasher); + doc.get_document_content()[..usize::from(range.start())].hash(&mut hasher); + id.parent_content().hash(&mut hasher); search_path.hash(&mut hasher); - for (sql, _) in &statements { - sql.hash(&mut hasher); - } let key = hasher.finish(); let mut cached = self.snapshot_before.lock().unwrap(); - if let Some((cached_key, cached_snapshot)) = cached.as_ref() - && *cached_key == key + if let Some(cached) = cached.as_ref() + && cached.key == key + && Arc::ptr_eq(&cached.base, &snapshot) { - return Arc::clone(cached_snapshot); + return Arc::clone(&cached.snapshot); } - let mut catalog = Catalog::new(Some(self.catalog_base(&snapshot))); - let mut session = Session::new(snapshot.expand_search_path(search_path)); - for (_, ast) in &statements { - catalog.apply(ast, session.search_path()); - session.apply(ast); - } - let result = catalog.snapshot().unwrap_or(snapshot); - *cached = Some((key, Arc::clone(&result))); + let result = catalog() + .borrow() + .snapshot() + .unwrap_or_else(|| Arc::clone(&snapshot)); + *cached = Some(SnapshotBefore { + base: snapshot, + key, + snapshot: Arc::clone(&result), + }); result } @@ -864,6 +888,8 @@ impl Workspace for WorkspaceServer { params.vcs_base_path, params.gitignore_matches.as_slice(), )?; + // The search path and the migration sections change what a statement sees. + *self.snapshot_before.lock().unwrap() = None; Ok(()) } @@ -932,6 +958,8 @@ impl Workspace for WorkspaceServer { self.workspaces_mut() .as_mut() .set_nested_settings(params.project_key, nested); + // The search path and the migration sections change what a statement sees. + *self.snapshot_before.lock().unwrap() = None; Ok(result) } @@ -1723,8 +1751,22 @@ impl Workspace for WorkspaceServer { } Some((id, range, cst)) => { let position = params.position - range.start(); - let snapshot = - self.snapshot_before(parsed_doc, range.start(), snapshot, &search_path); + let snapshot = self.snapshot_before( + parsed_doc, + &id, + range, + Arc::clone(&snapshot), + &search_path, + || { + self.walk_statements( + settings.settings_for(params.project_key, params.path.as_path()), + parsed_doc, + &snapshot, + range.start(), + |_, _, _| {}, + ) + }, + ); let items = pgls_completions::complete(pgls_completions::CompletionParams { position, @@ -1751,9 +1793,6 @@ impl Workspace for WorkspaceServer { let settings = self.workspaces(); let settings = settings.settings_for(params.project_key, params.path.as_path()); - let search_path = settings - .map(|settings| settings.typecheck.search_path.clone()) - .unwrap_or_default(); #[cfg(feature = "db")] let Some(snapshot) = settings @@ -1777,8 +1816,14 @@ impl Workspace for WorkspaceServer { match doc.iter_at(WithCSTandASTMapper, params.position).last() { Some((stmt_id, range, ts_tree, maybe_ast)) => { let position_in_stmt = params.position - range.start(); - let references = self.references_at(settings, doc, &snapshot, params.position); - let snapshot = self.snapshot_before(doc, range.start(), snapshot, &search_path); + let (references, snapshot) = self.hover_references( + settings, + doc, + &stmt_id, + range, + params.position, + snapshot, + ); let markdown_blocks = pgls_hover::on_hover(pgls_hover::OnHoverParams { ts_tree: &ts_tree, @@ -1867,6 +1912,34 @@ impl Workspace for WorkspaceServer { } } +/// Whether hover finds what `references` resolve to in the database snapshot: objects that the +/// file didn't change, and columns of CTEs and subqueries. `others` are the other references of +/// the statement. +fn in_database(references: &[Reference], others: &[Reference]) -> bool { + // The statement references each relation that it takes columns from. + let unchanged_relation = |schema: &str, name: &str| { + references.iter().chain(others).any(|reference| { + matches!( + reference, + Reference::Relation { relation, .. } + if relation.schema == schema + && relation.name == name + && relation.origin == Origin::Database + ) + }) + }; + !references.is_empty() + && references.iter().all(|reference| match reference { + Reference::Relation { relation, .. } => relation.origin == Origin::Database, + Reference::Function { function, .. } => function.origin == Origin::Database, + Reference::Column { + source: ColumnSource::Relation { schema, name }, + .. + } => unchanged_relation(schema, name), + Reference::Column { .. } => true, + }) +} + /// The migration sections of `doc`, like the up and down migrations of dbmate and goose. fn document_sections(settings: &Settings, doc: &Document) -> Vec { settings diff --git a/crates/pgls_workspace/src/workspace/server.tests.rs b/crates/pgls_workspace/src/workspace/server.tests.rs index 5761468e8..53673338e 100644 --- a/crates/pgls_workspace/src/workspace/server.tests.rs +++ b/crates/pgls_workspace/src/workspace/server.tests.rs @@ -2280,6 +2280,86 @@ async fn test_hover_shows_resolved_references(test_db: PgPool) { assert_eq!(blocks.len(), 2, "{blocks:?}"); } +/// Hover shows the objects as the statement sees them: created earlier in the file, changed by +/// the file, or unchanged in the database, also after the file changes. +#[sqlx::test(migrator = "pgls_test_utils::MIGRATIONS")] +async fn hover_follows_the_ddl_before_the_statement(test_db: PgPool) { + test_db + .execute( + "create table hover_db_items (id int); + create table hover_db_tags (id int);", + ) + .await + .expect("Failed to setup test database"); + + let mut conf = PartialConfiguration::init(); + conf.merge_with(PartialConfiguration { + db: Some(PartialDatabaseConfiguration { + database: Some( + test_db + .connect_options() + .get_database() + .unwrap() + .to_string(), + ), + ..Default::default() + }), + ..Default::default() + }); + let (workspace, project_key) = get_test_workspace(Some(conf)).expect("Unable to create test workspace"); + + let path = PgLSPath::new("test.sql"); + let statements = "alter table hover_db_items add column extra int;\n\ + select id from hover_file_items;\n\ + select extra from hover_db_items;\n\ + select id from hover_db_tags;\n"; + let hover = |content: &str, version: i32, needle: &str| { + if version == 1 { + workspace.open_file(OpenFileParams { + project_key, + path: path.clone(), + content: content.into(), + version, + }) + } else { + workspace.change_file(crate::workspace::ChangeFileParams { + project_key, + path: path.clone(), + content: content.into(), + version, + }) + } + .expect("Unable to update test file"); + let offset = content.find(needle).expect("needle in content") + 2; + let blocks = workspace + .on_hover(OnHoverParams { + project_key, + path: path.clone(), + position: u32::try_from(offset).unwrap().into(), + }) + .expect("Unable to hover") + .markdown_blocks; + assert_eq!(blocks.len(), 1, "{blocks:?}"); + blocks.into_iter().next().unwrap() + }; + + let content = format!("create table hover_file_items (id int);\n{statements}"); + let block = hover(&content, 1, "hover_file_items;"); + assert!(block.contains("- id: int4"), "{block}"); + assert!(!block.contains("- name"), "{block}"); + // A table of the database that the file changed. + let block = hover(&content, 1, "hover_db_items;"); + assert!(block.contains("- extra: int4"), "{block}"); + // A table of the database that the file didn't change. + let block = hover(&content, 1, "hover_db_tags;"); + assert!(block.contains("public.hover_db_tags"), "{block}"); + + // The statements before the hovered one changed. + let content = format!("create table hover_file_items (id int, name text);\n{statements}"); + let block = hover(&content, 2, "hover_file_items;"); + assert!(block.contains("- name: text"), "{block}"); +} + /// Hover and code actions on a file with thousands of statements, on a thread with the 2 MiB /// stack of the language server's tokio workers. They used to recurse once per statement before /// or after the cursor and overflow the stack, which aborts the server. diff --git a/crates/pgls_workspace/src/workspace/server/annotation.rs b/crates/pgls_workspace/src/workspace/server/annotation.rs index 3f2bb43ef..756deac2d 100644 --- a/crates/pgls_workspace/src/workspace/server/annotation.rs +++ b/crates/pgls_workspace/src/workspace/server/annotation.rs @@ -1,20 +1,17 @@ -use std::num::NonZeroUsize; use std::sync::{Arc, Mutex}; -use lru::LruCache; use pgls_lexer::SyntaxKind; +use rustc_hash::FxHashMap; use super::statement_identifier::StatementId; -const DEFAULT_CACHE_SIZE: usize = 1000; - #[derive(Debug, Clone, PartialEq, Eq)] pub struct StatementAnnotations { ends_with_semicolon: bool, } pub struct AnnotationStore { - db: Mutex>>, + db: Mutex>>, } const WHITESPACE_TOKENS: [SyntaxKind; 6] = [ @@ -29,12 +26,15 @@ const WHITESPACE_TOKENS: [SyntaxKind; 6] = [ impl AnnotationStore { pub fn new() -> AnnotationStore { AnnotationStore { - db: Mutex::new(LruCache::new( - NonZeroUsize::new(DEFAULT_CACHE_SIZE).unwrap(), - )), + db: Mutex::default(), } } + /// Drops the annotations of the statements for which `keep` returns `false`. + pub fn retain(&self, keep: impl Fn(&StatementId) -> bool) { + self.db.lock().unwrap().retain(|id, _| keep(id)); + } + #[allow(unused)] pub fn get_annotations( &self, @@ -58,7 +58,7 @@ impl AnnotationStore { ends_with_semicolon, }); - cache.put(statement_id.clone(), annotations.clone()); + cache.insert(statement_id.clone(), annotations.clone()); annotations } diff --git a/crates/pgls_workspace/src/workspace/server/document.rs b/crates/pgls_workspace/src/workspace/server/document.rs index 0d91cb3b2..6d3ffdcd0 100644 --- a/crates/pgls_workspace/src/workspace/server/document.rs +++ b/crates/pgls_workspace/src/workspace/server/document.rs @@ -5,6 +5,7 @@ use pgls_diagnostics::{Diagnostic, DiagnosticExt, serde::Diagnostic as SDiagnost use pgls_query_ext::{diagnostics::SyntaxDiagnostic, utils::sql_function_body}; use pgls_suppressions::Suppressions; use pgls_text_size::{TextRange, TextSize}; +use rustc_hash::FxHashSet; use super::{ annotation::AnnotationStore, @@ -55,6 +56,19 @@ impl Document { self.ranges = ranges; self.diagnostics = diagnostics; self.suppressions = Suppressions::from(self.content.as_str()); + + // Keep what is cached for the statements that are still in the document. The bodies + // of SQL functions are kept with their function. + let statements: FxHashSet<&str> = self + .ranges + .iter() + .map(|range| &self.content[*range]) + .collect(); + let is_present = + |id: &StatementId| statements.contains(id.parent_content().unwrap_or(id.content())); + self.ast_db.retain(is_present); + self.cst_db.retain(is_present); + self.annotation_db.retain(is_present); } pub fn suppressions(&self) -> &Suppressions { @@ -740,6 +754,28 @@ $$ LANGUAGE plpgsql;"; ); } + #[test] + fn keeps_the_parsed_statements_that_are_still_in_the_document() { + let mut d = Document::new("select 1;\nselect 2;".to_string(), 1); + let first = StatementId::new("select 1;"); + let second = StatementId::new("select 2;"); + let first_ast = d.ast_db.get_or_cache_ast(&first); + let second_ast = d.ast_db.get_or_cache_ast(&second); + let first_tree = d.cst_db.get_or_cache_tree(&first); + + d.update_content("select 1;\nselect 3;".to_string(), 2); + + assert!(Arc::ptr_eq(&first_ast, &d.ast_db.get_or_cache_ast(&first))); + assert!(Arc::ptr_eq( + &first_tree, + &d.cst_db.get_or_cache_tree(&first) + )); + assert!(!Arc::ptr_eq( + &second_ast, + &d.ast_db.get_or_cache_ast(&second) + )); + } + #[test] fn test_id_filter() { let input = "SELECT 1; SELECT 2;"; diff --git a/crates/pgls_workspace/src/workspace/server/pg_query.rs b/crates/pgls_workspace/src/workspace/server/pg_query.rs index a17f3eaba..80f8c64e2 100644 --- a/crates/pgls_workspace/src/workspace/server/pg_query.rs +++ b/crates/pgls_workspace/src/workspace/server/pg_query.rs @@ -1,33 +1,34 @@ -use std::num::NonZeroUsize; use std::sync::{Arc, LazyLock, Mutex}; -use lru::LruCache; pub use pgls_lexer::{convert_to_positional_params, convert_to_positional_params_with_metadata}; use pgls_query_ext::diagnostics::*; use pgls_text_size::TextRange; use regex::Regex; +use rustc_hash::FxHashMap; use super::statement_identifier::StatementId; -const DEFAULT_CACHE_SIZE: usize = 1000; - +/// The parse results of the statements of a document. They are kept until the statement is +/// no longer in the document, see [`PgQueryStore::retain`]. pub struct PgQueryStore { - ast_db: Mutex>>>, - plpgsql_db: Mutex>>, + ast_db: Mutex>>>, + plpgsql_db: Mutex>>, } impl PgQueryStore { pub fn new() -> PgQueryStore { PgQueryStore { - ast_db: Mutex::new(LruCache::new( - NonZeroUsize::new(DEFAULT_CACHE_SIZE).unwrap(), - )), - plpgsql_db: Mutex::new(LruCache::new( - NonZeroUsize::new(DEFAULT_CACHE_SIZE).unwrap(), - )), + ast_db: Mutex::default(), + plpgsql_db: Mutex::default(), } } + /// Drops the results of the statements for which `keep` returns `false`. + pub fn retain(&self, keep: impl Fn(&StatementId) -> bool) { + self.ast_db.lock().unwrap().retain(|id, _| keep(id)); + self.plpgsql_db.lock().unwrap().retain(|id, _| keep(id)); + } + pub fn get_or_cache_ast( &self, statement: &StatementId, @@ -47,7 +48,7 @@ impl PgQueryStore { }) }), ); - cache.put(statement.clone(), r.clone()); + cache.insert(statement.clone(), r.clone()); r } @@ -90,7 +91,7 @@ impl PgQueryStore { }) .map_err(|e| SyntaxDiagnostic::new(e.to_string(), Some(range))); - cache.put(statement.clone(), r.clone()); + cache.insert(statement.clone(), r.clone()); Some(r) } diff --git a/crates/pgls_workspace/src/workspace/server/tree_sitter.rs b/crates/pgls_workspace/src/workspace/server/tree_sitter.rs index 2a63971a7..ccbb89881 100644 --- a/crates/pgls_workspace/src/workspace/server/tree_sitter.rs +++ b/crates/pgls_workspace/src/workspace/server/tree_sitter.rs @@ -1,14 +1,13 @@ -use std::num::NonZeroUsize; use std::sync::{Arc, Mutex}; -use lru::LruCache; +use rustc_hash::FxHashMap; use super::statement_identifier::StatementId; -const DEFAULT_CACHE_SIZE: usize = 1000; - +/// The tree-sitter trees of the statements of a document. They are kept until the statement is +/// no longer in the document, see [`TreeSitterStore::retain`]. pub struct TreeSitterStore { - db: Mutex>>, + db: Mutex>>, parser: Mutex, } @@ -20,17 +19,20 @@ impl TreeSitterStore { .expect("Error loading sql language"); TreeSitterStore { - db: Mutex::new(LruCache::new( - NonZeroUsize::new(DEFAULT_CACHE_SIZE).unwrap(), - )), + db: Mutex::default(), parser: Mutex::new(parser), } } + /// Drops the trees of the statements for which `keep` returns `false`. + pub fn retain(&self, keep: impl Fn(&StatementId) -> bool) { + self.db.lock().unwrap().retain(|id, _| keep(id)); + } + pub fn get_or_cache_tree(&self, statement: &StatementId) -> Arc { // First check cache { - let mut cache = self.db.lock().unwrap(); + let cache = self.db.lock().unwrap(); if let Some(existing) = cache.get(statement) { return existing.clone(); } @@ -48,7 +50,7 @@ impl TreeSitterStore { if let Some(existing) = cache.get(statement) { return existing.clone(); } - cache.put(statement.clone(), tree.clone()); + cache.insert(statement.clone(), tree.clone()); } tree