Address review feedback

MichaReiser · MichaReiser · commit fd81fb1b7adc · 2024-11-15T18:55:54.000+01:00
diff --git a/crates/red_knot_python_semantic/src/module_resolver/resolver.rs b/crates/red_knot_python_semantic/src/module_resolver/resolver.rs
@@ -159,7 +159,7 @@ impl SearchPaths {
 
         let SearchPathSettings {
             extra_paths,
-            workspace_root: src_root,
+            src_root,
             custom_typeshed,
             site_packages: site_packages_paths,
         } = settings;
@@ -1293,7 +1293,7 @@ mod tests {
                 target_version: PythonVersion::PY38,
                 search_paths: SearchPathSettings {
                     extra_paths: vec![],
-                    workspace_root: src.clone(),
+                    src_root: src.clone(),
                     custom_typeshed: Some(custom_typeshed),
                     site_packages: SitePackages::Known(vec![site_packages]),
                 },
@@ -1798,7 +1798,7 @@ not_a_directory
                 target_version: PythonVersion::default(),
                 search_paths: SearchPathSettings {
                     extra_paths: vec![],
-                    workspace_root: SystemPathBuf::from("/src"),
+                    src_root: SystemPathBuf::from("/src"),
                     custom_typeshed: None,
                     site_packages: SitePackages::Known(vec![
                         venv_site_packages,
diff --git a/crates/red_knot_python_semantic/src/module_resolver/testing.rs b/crates/red_knot_python_semantic/src/module_resolver/testing.rs
@@ -229,7 +229,7 @@ impl TestCaseBuilder<MockedTypeshed> {
                 target_version,
                 search_paths: SearchPathSettings {
                     extra_paths: vec![],
-                    workspace_root: src.clone(),
+                    src_root: src.clone(),
                     custom_typeshed: Some(typeshed.clone()),
                     site_packages: SitePackages::Known(vec![site_packages.clone()]),
                 },
diff --git a/crates/red_knot_python_semantic/src/program.rs b/crates/red_knot_python_semantic/src/program.rs
@@ -70,7 +70,7 @@ pub struct SearchPathSettings {
     pub extra_paths: Vec<SystemPathBuf>,
 
     /// The root of the workspace, used for finding first-party modules.
-    pub workspace_root: SystemPathBuf,
+    pub src_root: SystemPathBuf,
 
     /// Optional path to a "custom typeshed" directory on disk for us to use for standard-library types.
     /// If this is not provided, we will fallback to our vendored typeshed stubs for the stdlib,
@@ -84,7 +84,7 @@ pub struct SearchPathSettings {
 impl SearchPathSettings {
     pub fn new(src_root: SystemPathBuf) -> Self {
         Self {
-            workspace_root: src_root,
+            src_root,
             extra_paths: vec![],
             custom_typeshed: None,
             site_packages: SitePackages::Known(vec![]),
diff --git a/crates/red_knot_workspace/src/workspace/metadata.rs b/crates/red_knot_workspace/src/workspace/metadata.rs
@@ -90,7 +90,7 @@ impl WorkspaceMetadata {
                     pyproject,
                     ancestor.to_path_buf(),
                     base_configuration,
-                )?;
+                );
 
                 if let Some(workspace_table) = workspace_table {
                     let workspace_root = ancestor;
@@ -168,7 +168,7 @@ impl WorkspaceMetadata {
 
             // Create a package with a default configuration
             PackageMetadata {
-                name: path.file_name().unwrap_or("<virtual>").into(),
+                name: path.file_name().unwrap_or("root").into(),
                 root: path.to_path_buf(),
                 // TODO create the configuration from the pyproject toml
                 configuration: base_configuration.cloned().unwrap_or_default(),
@@ -206,14 +206,11 @@ impl PackageMetadata {
         pyproject: PyProject,
         root: SystemPathBuf,
         base_configuration: Option<&Configuration>,
-    ) -> Result<Self, WorkspaceDiscoveryError> {
-        let Some(project) = pyproject.project else {
-            return Err(WorkspaceDiscoveryError::MissingProjectTable { package_path: root });
-        };
-
-        let Some(name) = project.name.as_ref() else {
-            return Err(WorkspaceDiscoveryError::MissingPackageName { package_path: root });
-        };
+    ) -> Self {
+        let name = pyproject.project.and_then(|project| project.name);
+        let name = name
+            .map(|name| Name::new(&*name))
+            .unwrap_or_else(|| Name::new(root.file_name().unwrap_or("root")));
 
         // TODO: load configuration from pyrpoject.toml
         let mut configuration = Configuration::default();
@@ -222,11 +219,11 @@ impl PackageMetadata {
             configuration.extend(base_configuration.clone());
         }
 
-        Ok(PackageMetadata {
-            name: Name::from(name.as_str()),
+        PackageMetadata {
+            name,
             root,
             configuration,
-        })
+        }
     }
 
     pub fn name(&self) -> &Name {
@@ -341,7 +338,7 @@ fn collect_packages(
             });
         }
 
-        let package = PackageMetadata::from_pyproject(pyproject, member_path, base_configuration)?;
+        let package = PackageMetadata::from_pyproject(pyproject, member_path, base_configuration);
 
         tracing::debug!(
             "Adding package '{}' at '{}'",
@@ -407,12 +404,6 @@ pub enum WorkspaceDiscoveryError {
         #[from]
         error: GlobError,
     },
-
-    #[error("the `[project]` is missing in the `pyproject.toml` for package '{package_path}'`")]
-    MissingProjectTable { package_path: SystemPathBuf },
-
-    #[error("the `project.name` is missing in the `pyproject.toml` for package '{package_path}'`")]
-    MissingPackageName { package_path: SystemPathBuf },
 }
 
 #[cfg(test)]
@@ -433,7 +424,7 @@ mod tests {
 
         system
             .memory_file_system()
-            .write_files([(root.join("src/foo.py"), ""), (root.join("src/bar.py"), "")])
+            .write_files([(root.join("foo.py"), ""), (root.join("bar.py"), "")])
             .context("Failed to write files")?;
 
         let workspace = WorkspaceMetadata::discover(&root, &system, None)
@@ -454,15 +445,14 @@ mod tests {
         system
             .memory_file_system()
             .write_files([
-                (root.join("src/foo.py"), ""),
-                (root.join("src/bar.py"), ""),
                 (
                     root.join("pyproject.toml"),
                     r#"
                     [project]
                     name = "backend"
                     "#,
                 ),
+                (root.join("db/__init__.py"), ""),
             ])
             .context("Failed to write files")?;
 
@@ -473,7 +463,7 @@ mod tests {
         snapshot_workspace!(workspace);
 
         // Discovering the same package from a subdirectory should give the same result
-        let from_src = WorkspaceMetadata::discover(&root.join("src"), &system, None)
+        let from_src = WorkspaceMetadata::discover(&root.join("db"), &system, None)
             .context("Failed to discover workspace from src sub-directory")?;
 
         assert_eq!(from_src, workspace);
@@ -489,8 +479,6 @@ mod tests {
         system
             .memory_file_system()
             .write_files([
-                (root.join("src/foo.py"), ""),
-                (root.join("src/bar.py"), ""),
                 (
                     root.join("pyproject.toml"),
                     r#"
@@ -543,8 +531,6 @@ mod tests {
         system
             .memory_file_system()
             .write_files([
-                (root.join("src/foo.py"), ""),
-                (root.join("src/bar.py"), ""),
                 (
                     root.join("pyproject.toml"),
                     r#"
@@ -597,8 +583,6 @@ mod tests {
         system
             .memory_file_system()
             .write_files([
-                (root.join("src/foo.py"), ""),
-                (root.join("src/bar.py"), ""),
                 (
                     root.join("pyproject.toml"),
                     r#"
@@ -643,8 +627,6 @@ mod tests {
         system
             .memory_file_system()
             .write_files([
-                (root.join("src/foo.py"), ""),
-                (root.join("src/bar.py"), ""),
                 (
                     root.join("pyproject.toml"),
                     r#"
@@ -685,8 +667,6 @@ mod tests {
         system
             .memory_file_system()
             .write_files([
-                (root.join("src/foo.py"), ""),
-                (root.join("src/bar.py"), ""),
                 (
                     root.join("pyproject.toml"),
                     r#"
@@ -721,8 +701,6 @@ mod tests {
         system
             .memory_file_system()
             .write_files([
-                (root.join("src/foo.py"), ""),
-                (root.join("src/bar.py"), ""),
                 (
                     root.join("pyproject.toml"),
                     r#"
@@ -752,8 +730,6 @@ mod tests {
         system
             .memory_file_system()
             .write_files([
-                (root.join("src/foo.py"), ""),
-                (root.join("src/bar.py"), ""),
                 (
                     root.join("pyproject.toml"),
                     r#"
@@ -783,8 +759,6 @@ mod tests {
         system
             .memory_file_system()
             .write_files([
-                (root.join("src/foo.py"), ""),
-                (root.join("src/bar.py"), ""),
                 (
                     root.join("pyproject.toml"),
                     r#"
diff --git a/crates/red_knot_workspace/src/workspace/settings.rs b/crates/red_knot_workspace/src/workspace/settings.rs
@@ -74,12 +74,12 @@ impl SearchPathConfiguration {
             .unwrap_or(SitePackages::Known(vec![]));
 
         SearchPathSettings {
-            extra_paths: self.clone().extra_paths.unwrap_or_default(),
-            workspace_root: self
+            extra_paths: self.extra_paths.clone().unwrap_or_default(),
+            src_root: self
                 .clone()
                 .src_root
                 .unwrap_or_else(|| workspace_root.to_path_buf()),
-            custom_typeshed: self.clone().custom_typeshed,
+            custom_typeshed: self.custom_typeshed.clone(),
             site_packages,
         }
     }
diff --git a/crates/red_knot_workspace/src/workspace/snapshots/red_knot_workspace__workspace__metadata__tests__member_pattern_matching_file.snap b/crates/red_knot_workspace/src/workspace/snapshots/red_knot_workspace__workspace__metadata__tests__member_pattern_matching_file.snap
@@ -27,7 +27,7 @@ WorkspaceMetadata(
       ),
       search_paths: SearchPathSettings(
         extra_paths: [],
-        workspace_root: "/app",
+        src_root: "/app",
         custom_typeshed: None,
         site_packages: Known([]),
       ),
diff --git a/crates/red_knot_workspace/src/workspace/snapshots/red_knot_workspace__workspace__metadata__tests__member_pattern_matching_hidden_folder.snap b/crates/red_knot_workspace/src/workspace/snapshots/red_knot_workspace__workspace__metadata__tests__member_pattern_matching_hidden_folder.snap
@@ -1,6 +1,6 @@
 ---
 source: crates/red_knot_workspace/src/workspace/metadata.rs
-expression: "&workspace"
+expression: workspace
 ---
 WorkspaceMetadata(
   root: "/app",
@@ -27,7 +27,7 @@ WorkspaceMetadata(
       ),
       search_paths: SearchPathSettings(
         extra_paths: [],
-        workspace_root: "/app",
+        src_root: "/app",
         custom_typeshed: None,
         site_packages: Known([]),
       ),
diff --git a/crates/red_knot_workspace/src/workspace/snapshots/red_knot_workspace__workspace__metadata__tests__package_without_pyproject.snap b/crates/red_knot_workspace/src/workspace/snapshots/red_knot_workspace__workspace__metadata__tests__package_without_pyproject.snap
@@ -1,6 +1,6 @@
 ---
 source: crates/red_knot_workspace/src/workspace/metadata.rs
-expression: "&workspace"
+expression: workspace
 ---
 WorkspaceMetadata(
   root: "/app",
@@ -27,7 +27,7 @@ WorkspaceMetadata(
       ),
       search_paths: SearchPathSettings(
         extra_paths: [],
-        workspace_root: "/app",
+        src_root: "/app",
         custom_typeshed: None,
         site_packages: Known([]),
       ),
diff --git a/crates/red_knot_workspace/src/workspace/snapshots/red_knot_workspace__workspace__metadata__tests__single_package.snap b/crates/red_knot_workspace/src/workspace/snapshots/red_knot_workspace__workspace__metadata__tests__single_package.snap
@@ -1,6 +1,6 @@
 ---
 source: crates/red_knot_workspace/src/workspace/metadata.rs
-expression: "&workspace"
+expression: workspace
 ---
 WorkspaceMetadata(
   root: "/app",
@@ -27,7 +27,7 @@ WorkspaceMetadata(
       ),
       search_paths: SearchPathSettings(
         extra_paths: [],
-        workspace_root: "/app",
+        src_root: "/app",
         custom_typeshed: None,
         site_packages: Known([]),
       ),
diff --git a/crates/red_knot_workspace/src/workspace/snapshots/red_knot_workspace__workspace__metadata__tests__workspace_excluded.snap b/crates/red_knot_workspace/src/workspace/snapshots/red_knot_workspace__workspace__metadata__tests__workspace_excluded.snap
@@ -40,7 +40,7 @@ WorkspaceMetadata(
       ),
       search_paths: SearchPathSettings(
         extra_paths: [],
-        workspace_root: "/app",
+        src_root: "/app",
         custom_typeshed: None,
         site_packages: Known([]),
       ),
diff --git a/crates/red_knot_workspace/src/workspace/snapshots/red_knot_workspace__workspace__metadata__tests__workspace_members.snap b/crates/red_knot_workspace/src/workspace/snapshots/red_knot_workspace__workspace__metadata__tests__workspace_members.snap
@@ -53,7 +53,7 @@ WorkspaceMetadata(
       ),
       search_paths: SearchPathSettings(
         extra_paths: [],
-        workspace_root: "/app",
+        src_root: "/app",
         custom_typeshed: None,
         site_packages: Known([]),
       ),
