implement extends

Author: psteinroe

crates/pgt_lsp/tests/server.rs

@@ -3,6 +3,7 @@ use anyhow::Error;
 use anyhow::Result;
 use anyhow::bail;
 use biome_deserialize::Merge;
+use biome_deserialize::StringSet;
 use futures::Sink;
 use futures::SinkExt;
 use futures::Stream;
@@ -1326,7 +1327,6 @@ async fn multiple_projects() -> Result<()> {
         })
         .await?
         .unwrap();
-    println!("{:?}", res_ws_two);
 
     // only the first one has a db connection and should return completion items
     assert!(!match res_ws_one {
@@ -1343,3 +1343,203 @@ async fn multiple_projects() -> Result<()> {
 
     Ok(())
 }
+
+#[tokio::test]
+async fn extends_config() -> Result<()> {
+    let factory = ServerFactory::default();
+    let mut fs = MemoryFileSystem::default();
+    let test_db = get_new_test_db().await;
+
+    let setup = r#"
+            create table public.extends_config_test (
+                id serial primary key,
+                name varchar(255) not null
+            );
+        "#;
+
+    test_db
+        .execute(setup)
+        .await
+        .expect("Failed to setup test database");
+
+    // shared config with default db connection
+    let conf_with_db = PartialConfiguration::init();
+    fs.insert(
+        url!("postgrestools.jsonc").to_file_path().unwrap(),
+        serde_json::to_string_pretty(&conf_with_db).unwrap(),
+    );
+
+    // test_one extends the shared config but sets our test db
+    let mut conf_with_db = PartialConfiguration::init();
+    conf_with_db.merge_with(PartialConfiguration {
+        extends: Some(StringSet::from_iter(["../postgrestools.jsonc".to_string()])),
+        db: Some(PartialDatabaseConfiguration {
+            database: Some(
+                test_db
+                    .connect_options()
+                    .get_database()
+                    .unwrap()
+                    .to_string(),
+            ),
+            ..Default::default()
+        }),
+        ..Default::default()
+    });
+    fs.insert(
+        url!("test_one/postgrestools.jsonc").to_file_path().unwrap(),
+        serde_json::to_string_pretty(&conf_with_db).unwrap(),
+    );
+
+    // test_two extends it but keeps the default one
+    let mut conf_without_db = PartialConfiguration::init();
+    conf_without_db.merge_with(PartialConfiguration {
+        extends: Some(StringSet::from_iter(["../postgrestools.jsonc".to_string()])),
+        ..Default::default()
+    });
+    fs.insert(
+        url!("test_two/postgrestools.jsonc").to_file_path().unwrap(),
+        serde_json::to_string_pretty(&conf_without_db).unwrap(),
+    );
+
+    let (service, client) = factory
+        .create_with_fs(None, DynRef::Owned(Box::new(fs)))
+        .into_inner();
+
+    let (stream, sink) = client.split();
+    let mut server = Server::new(service);
+
+    let (sender, _) = channel(CHANNEL_BUFFER_SIZE);
+    let reader = tokio::spawn(client_handler(stream, sink, sender));
+
+    server.initialize_workspaces().await?;
+    server.initialized().await?;
+
+    server.load_configuration().await?;
+
+    server
+        .open_named_document(
+            "select  from public.extends_config_test;\n",
+            url!("test_one/document.sql"),
+            "sql",
+        )
+        .await?;
+
+    server
+        .change_named_document(
+            url!("test_one/document.sql"),
+            3,
+            vec![TextDocumentContentChangeEvent {
+                range: Some(Range {
+                    start: Position {
+                        line: 0,
+                        character: 7,
+                    },
+                    end: Position {
+                        line: 0,
+                        character: 7,
+                    },
+                }),
+                range_length: Some(0),
+                text: " ".to_string(),
+            }],
+        )
+        .await?;
+
+    let res_ws_one = server
+        .get_completion(CompletionParams {
+            work_done_progress_params: WorkDoneProgressParams::default(),
+            partial_result_params: PartialResultParams::default(),
+            context: None,
+            text_document_position: TextDocumentPositionParams {
+                text_document: TextDocumentIdentifier {
+                    uri: url!("test_one/document.sql"),
+                },
+                position: Position {
+                    line: 0,
+                    character: 8,
+                },
+            },
+        })
+        .await?
+        .unwrap();
+
+    server
+        .open_named_document(
+            "select  from public.users;\n",
+            url!("test_two/document.sql"),
+            "sql",
+        )
+        .await?;
+
+    server
+        .change_named_document(
+            url!("test_two/document.sql"),
+            3,
+            vec![TextDocumentContentChangeEvent {
+                range: Some(Range {
+                    start: Position {
+                        line: 0,
+                        character: 7,
+                    },
+                    end: Position {
+                        line: 0,
+                        character: 7,
+                    },
+                }),
+                range_length: Some(0),
+                text: " ".to_string(),
+            }],
+        )
+        .await?;
+
+    let res_ws_two = server
+        .get_completion(CompletionParams {
+            work_done_progress_params: WorkDoneProgressParams::default(),
+            partial_result_params: PartialResultParams::default(),
+            context: None,
+            text_document_position: TextDocumentPositionParams {
+                text_document: TextDocumentIdentifier {
+                    uri: url!("test_two/document.sql"),
+                },
+                position: Position {
+                    line: 0,
+                    character: 8,
+                },
+            },
+        })
+        .await?
+        .unwrap();
+
+    let items_one = match res_ws_one {
+        CompletionResponse::Array(ref a) => a,
+        CompletionResponse::List(ref l) => &l.items,
+    };
+
+    // test one should have our test db connection and should return the completion items for the `extends_config_test` table
+    assert!(items_one.iter().any(|item| {
+        item.label_details.clone().is_some_and(|details| {
+            details
+                .description
+                .is_some_and(|desc| desc.contains("public.extends_config_test"))
+        })
+    }));
+
+    let items_two = match res_ws_two {
+        CompletionResponse::Array(ref a) => a,
+        CompletionResponse::List(ref l) => &l.items,
+    };
+
+    // test two should not have a db connection and should not return the completion items for the `extends_config_test` table
+    assert!(!items_two.iter().any(|item| {
+        item.label_details.clone().is_some_and(|details| {
+            details
+                .description
+                .is_some_and(|desc| desc.contains("public.extends_config_test"))
+        })
+    }));
+
+    server.shutdown().await?;
+    reader.abort();
+
+    Ok(())
+}

crates/pgt_workspace/src/configuration.rs

@@ -31,34 +31,41 @@ pub struct LoadedConfiguration {
 }
 
 impl LoadedConfiguration {
-    /// Return the path of the **directory** where the configuration is
-    pub fn directory_path(&self) -> Option<&Path> {
-        self.directory_path.as_deref()
-    }
-
-    /// Return the path of the **file** where the configuration is
-    pub fn file_path(&self) -> Option<&Path> {
-        self.file_path.as_deref()
-    }
-}
-
-impl From<Option<ConfigurationPayload>> for LoadedConfiguration {
-    fn from(value: Option<ConfigurationPayload>) -> Self {
+    fn try_from_payload(
+        value: Option<ConfigurationPayload>,
+        fs: &DynRef<'_, dyn FileSystem>,
+    ) -> Result<Self, WorkspaceError> {
         let Some(value) = value else {
-            return LoadedConfiguration::default();
+            return Ok(LoadedConfiguration::default());
         };
 
         let ConfigurationPayload {
+            external_resolution_base_path,
             configuration_file_path,
-            deserialized: partial_configuration,
-            ..
+            deserialized: mut partial_configuration,
         } = value;
 
-        LoadedConfiguration {
+        partial_configuration.apply_extends(
+            fs,
+            &configuration_file_path,
+            &external_resolution_base_path,
+        )?;
+
+        Ok(Self {
             configuration: partial_configuration,
             directory_path: configuration_file_path.parent().map(PathBuf::from),
             file_path: Some(configuration_file_path),
-        }
+        })
+    }
+
+    /// Return the path of the **directory** where the configuration is
+    pub fn directory_path(&self) -> Option<&Path> {
+        self.directory_path.as_deref()
+    }
+
+    /// Return the path of the **file** where the configuration is
+    pub fn file_path(&self) -> Option<&Path> {
+        self.file_path.as_deref()
     }
 }
 
@@ -68,7 +75,7 @@ pub fn load_configuration(
     config_path: ConfigurationPathHint,
 ) -> Result<LoadedConfiguration, WorkspaceError> {
     let config = load_config(fs, config_path)?;
-    Ok(LoadedConfiguration::from(config))
+    LoadedConfiguration::try_from_payload(config, fs)
 }
 
 /// - [Result]: if an error occurred while loading the configuration file.
@@ -123,7 +130,7 @@ fn load_config(
         ConfigurationPathHint::None => file_system.working_directory().unwrap_or_default(),
     };
 
-    // We first search for `postgrestools.jsonc`
+    // We first search for `postgrestools.jsonc` files
     if let Some(auto_search_result) = file_system.auto_search(
         &configuration_directory,
         ConfigName::file_names().as_slice(),
@@ -344,9 +351,10 @@ impl PartialConfigurationExt for PartialConfiguration {
                     extend_entry_as_path
                         .extension()
                         .map(OsStr::as_encoded_bytes),
-                    Some(b"json" | b"jsonc")
+                    Some(b"jsonc")
                 ) {
-                relative_resolution_base_path.join(extend_entry)
+                // Normalize the path to handle relative segments like "../"
+                normalize_path(&relative_resolution_base_path.join(extend_entry))
             } else {
                 fs.resolve_configuration(extend_entry.as_str(), external_resolution_base_path)
                     .map_err(|error| {
@@ -369,7 +377,7 @@ impl PartialConfigurationExt for PartialConfiguration {
                         err.to_string(),
                     )
                     .with_verbose_advice(markup! {
-                        "Biome tried to load the configuration file \""<Emphasis>{
+                        "Postgres Tools tried to load the configuration file \""<Emphasis>{
                             extend_configuration_file_path.display().to_string()
                         }</Emphasis>"\" in \"extends\" using \""<Emphasis>{
                             external_resolution_base_path.display().to_string()
@@ -438,6 +446,36 @@ impl PartialConfigurationExt for PartialConfiguration {
     }
 }
 
+/// Normalizes a path, resolving '..' and '.' segments without requiring the path to exist
+fn normalize_path(path: &Path) -> PathBuf {
+    let mut components = Vec::new();
+    for component in path.components() {
+        match component {
+            std::path::Component::ParentDir => {
+                if !components.is_empty() {
+                    components.pop();
+                }
+            }
+            std::path::Component::Normal(c) => components.push(c),
+            std::path::Component::CurDir => {}
+            c @ std::path::Component::RootDir | c @ std::path::Component::Prefix(_) => {
+                components.clear();
+                components.push(c.as_os_str());
+            }
+        }
+    }
+
+    if components.is_empty() {
+        PathBuf::from("/")
+    } else {
+        let mut result = PathBuf::new();
+        for component in components {
+            result.push(component);
+        }
+        result
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;