Skip to content

Commit 573ff14

Browse files
authored
refactor(core): consolidate complete_package_names, seal Ecosystem, remove once_cell (#71)
* refactor(core): consolidate complete_package_names, seal Ecosystem, remove once_cell Consolidate complete_package_names across Cargo, npm, PyPI, Bundler, Dart ecosystems to use complete_package_names_generic from deps-core. Rename ranges_overlap to position_in_range for semantic clarity. Replace once_cell::sync::Lazy with std::sync::LazyLock, remove once_cell dependency. Seal Ecosystem trait with private::Sealed supertrait to prevent external implementations. Rename format_version_for_code_action to format_version_for_text_edit to better reflect its TextEdit context. Closes #68 * refactor(core): unify DependencySource enum across all ecosystems Consolidate 4 duplicate DependencySource enums (Cargo, PyPI, Dart, Bundler) into a single 7-variant enum in deps-core: Registry, Git, Path, Url, Sdk, Workspace, CustomRegistry. Eliminates lossy conversions at trait boundaries: PyPI Url and Dart Sdk variants are now preserved through the Dependency trait. Cargo workspace_inherited bool replaced by DependencySource::Workspace. Add #[non_exhaustive] for forward compatibility. Add is_registry() and is_version_resolvable() convenience methods. Simplify UnifiedDependency::is_registry() to delegate to core enum. Ref #68
1 parent a9ac193 commit 573ff14

38 files changed

+348
-577
lines changed

Cargo.lock

Lines changed: 0 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ futures = "0.3"
2929
insta = "1"
3030
mockito = "1"
3131
node-semver = "2.2"
32-
once_cell = "1.21"
3332
pep440_rs = "0.7"
3433
pep508_rs = "0.9"
3534
bytes = "1"

crates/deps-bundler/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ urlencoding = { workspace = true }
2727
[dev-dependencies]
2828
criterion = { workspace = true, features = ["html_reports"] }
2929
insta = { workspace = true, features = ["json"] }
30-
once_cell = { workspace = true }
3130
tempfile = { workspace = true }
3231
tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
3332
tokio-test = { workspace = true }

crates/deps-bundler/src/ecosystem.rs

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -34,32 +34,9 @@ impl BundlerEcosystem {
3434
}
3535
}
3636

37-
/// Completes gem names by searching rubygems.org.
3837
async fn complete_package_names(&self, prefix: &str) -> Vec<CompletionItem> {
39-
use deps_core::completion::build_package_completion;
40-
41-
// Require at least 2 characters, max 100 for security
42-
if prefix.len() < 2 || prefix.len() > 100 {
43-
return vec![];
44-
}
45-
46-
let results = match self.registry.search(prefix, 20).await {
47-
Ok(r) => r,
48-
Err(e) => {
49-
tracing::warn!("Gem search failed for '{}': {}", prefix, e);
50-
return vec![];
51-
}
52-
};
53-
54-
let insert_range = tower_lsp_server::ls_types::Range::default();
55-
56-
results
57-
.into_iter()
58-
.map(|metadata| {
59-
let boxed: Box<dyn deps_core::Metadata> = Box::new(metadata);
60-
build_package_completion(boxed.as_ref(), insert_range)
61-
})
62-
.collect()
38+
deps_core::completion::complete_package_names_generic(self.registry.as_ref(), prefix, 20)
39+
.await
6340
}
6441

6542
async fn complete_versions(&self, package_name: &str, prefix: &str) -> Vec<CompletionItem> {
@@ -73,6 +50,8 @@ impl BundlerEcosystem {
7350
}
7451
}
7552

53+
impl deps_core::ecosystem::private::Sealed for BundlerEcosystem {}
54+
7655
impl Ecosystem for BundlerEcosystem {
7756
fn id(&self) -> &'static str {
7857
"bundler"
@@ -201,8 +180,8 @@ mod tests {
201180
let cache = Arc::new(deps_core::HttpCache::new());
202181
let ecosystem = BundlerEcosystem::new(cache);
203182

204-
// Prefix longer than 100 chars should return empty
205-
let long_prefix = "a".repeat(101);
183+
// Prefix longer than 200 chars should return empty
184+
let long_prefix = "a".repeat(201);
206185
let results = ecosystem.complete_package_names(&long_prefix).await;
207186
assert!(results.is_empty());
208187
}

crates/deps-bundler/src/formatter.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use deps_core::lsp_helpers::EcosystemFormatter;
77
pub struct BundlerFormatter;
88

99
impl EcosystemFormatter for BundlerFormatter {
10-
fn format_version_for_code_action(&self, version: &str) -> String {
10+
fn format_version_for_text_edit(&self, version: &str) -> String {
1111
format!("'{version}'")
1212
}
1313

@@ -27,8 +27,8 @@ mod tests {
2727
#[test]
2828
fn test_format_version() {
2929
let formatter = BundlerFormatter;
30-
assert_eq!(formatter.format_version_for_code_action("7.0.8"), "'7.0.8'");
31-
assert_eq!(formatter.format_version_for_code_action("1.0.0"), "'1.0.0'");
30+
assert_eq!(formatter.format_version_for_text_edit("7.0.8"), "'7.0.8'");
31+
assert_eq!(formatter.format_version_for_text_edit("1.0.0"), "'1.0.0'");
3232
}
3333

3434
#[test]

crates/deps-bundler/src/parser.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -231,16 +231,14 @@ fn extract_source(line: &str) -> DependencySource {
231231
if let Some(caps) = GIT_OPTION.captures(line) {
232232
return DependencySource::Git {
233233
url: caps[1].to_string(),
234-
branch: None,
235-
tag: None,
236-
ref_: None,
234+
rev: None,
237235
};
238236
}
239237

240238
if let Some(caps) = GITHUB_OPTION.captures(line) {
241-
return DependencySource::Github {
242-
repo: caps[1].to_string(),
243-
branch: None,
239+
return DependencySource::Git {
240+
url: format!("https://github.com/{}", &caps[1]),
241+
rev: None,
244242
};
245243
}
246244

@@ -421,10 +419,12 @@ gem 'rails', git: 'https://github.com/rails/rails.git'";
421419
let gemfile = r"source 'https://rubygems.org'
422420
gem 'rails', github: 'rails/rails'";
423421
let result = parse_gemfile(gemfile, &test_uri()).unwrap();
424-
assert!(matches!(
425-
result.dependencies[0].source,
426-
DependencySource::Github { .. }
427-
));
422+
match &result.dependencies[0].source {
423+
DependencySource::Git { url, .. } => {
424+
assert!(url.contains("github.com/rails/rails"));
425+
}
426+
_ => panic!("Expected Git source"),
427+
}
428428
}
429429

430430
#[test]

crates/deps-bundler/src/types.rs

Lines changed: 15 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -16,28 +16,7 @@ pub struct BundlerDependency {
1616
pub require: Option<String>,
1717
}
1818

19-
/// Source location of a dependency.
20-
#[derive(Debug, Clone, PartialEq, Eq)]
21-
pub enum DependencySource {
22-
/// Default rubygems.org registry
23-
Registry,
24-
/// Git repository
25-
Git {
26-
url: String,
27-
branch: Option<String>,
28-
tag: Option<String>,
29-
ref_: Option<String>,
30-
},
31-
/// Local filesystem path
32-
Path { path: String },
33-
/// GitHub shorthand (e.g., "rails/rails")
34-
Github {
35-
repo: String,
36-
branch: Option<String>,
37-
},
38-
/// Custom gem source
39-
Source { name: String, url: String },
40-
}
19+
pub use deps_core::parser::DependencySource;
4120

4221
/// Gem group classification.
4322
#[derive(Debug, Clone, PartialEq, Eq, Default)]
@@ -106,21 +85,7 @@ impl deps_core::DependencyInfo for BundlerDependency {
10685
}
10786

10887
fn source(&self) -> deps_core::parser::DependencySource {
109-
match &self.source {
110-
DependencySource::Registry => deps_core::parser::DependencySource::Registry,
111-
DependencySource::Git { url, ref_, .. } => deps_core::parser::DependencySource::Git {
112-
url: url.clone(),
113-
rev: ref_.clone(),
114-
},
115-
DependencySource::Path { path } => {
116-
deps_core::parser::DependencySource::Path { path: path.clone() }
117-
}
118-
DependencySource::Github { repo, .. } => deps_core::parser::DependencySource::Git {
119-
url: format!("https://github.com/{repo}"),
120-
rev: None,
121-
},
122-
DependencySource::Source { .. } => deps_core::parser::DependencySource::Registry,
123-
}
88+
self.source.clone()
12489
}
12590

12691
fn features(&self) -> &[String] {
@@ -146,21 +111,7 @@ impl deps_core::Dependency for BundlerDependency {
146111
}
147112

148113
fn source(&self) -> deps_core::parser::DependencySource {
149-
match &self.source {
150-
DependencySource::Registry => deps_core::parser::DependencySource::Registry,
151-
DependencySource::Git { url, ref_, .. } => deps_core::parser::DependencySource::Git {
152-
url: url.clone(),
153-
rev: ref_.clone(),
154-
},
155-
DependencySource::Path { path } => {
156-
deps_core::parser::DependencySource::Path { path: path.clone() }
157-
}
158-
DependencySource::Github { repo, .. } => deps_core::parser::DependencySource::Git {
159-
url: format!("https://github.com/{repo}"),
160-
rev: None,
161-
},
162-
DependencySource::Source { .. } => deps_core::parser::DependencySource::Registry,
163-
}
114+
self.source.clone()
164115
}
165116

166117
fn features(&self) -> &[String] {
@@ -195,27 +146,19 @@ mod tests {
195146
let registry = DependencySource::Registry;
196147
let git = DependencySource::Git {
197148
url: "https://github.com/rails/rails".into(),
198-
branch: Some("main".into()),
199-
tag: None,
200-
ref_: None,
149+
rev: Some("main".into()),
201150
};
202151
let path = DependencySource::Path {
203152
path: "../local".into(),
204153
};
205-
let github = DependencySource::Github {
206-
repo: "rails/rails".into(),
207-
branch: None,
208-
};
209-
let source = DependencySource::Source {
210-
name: "custom".into(),
154+
let custom = DependencySource::CustomRegistry {
211155
url: "https://custom.gem.source".into(),
212156
};
213157

214-
assert!(matches!(registry, DependencySource::Registry));
215-
assert!(matches!(git, DependencySource::Git { .. }));
216-
assert!(matches!(path, DependencySource::Path { .. }));
217-
assert!(matches!(github, DependencySource::Github { .. }));
218-
assert!(matches!(source, DependencySource::Source { .. }));
158+
assert!(registry.is_registry());
159+
assert!(!git.is_registry());
160+
assert!(!path.is_registry());
161+
assert!(custom.is_registry());
219162
}
220163

221164
#[test]
@@ -311,10 +254,7 @@ mod tests {
311254
assert_eq!(dep.version_requirement(), Some("~> 1.0"));
312255
assert!(dep.version_range().is_some());
313256
assert!(dep.features().is_empty());
314-
assert!(matches!(
315-
dep.source(),
316-
deps_core::parser::DependencySource::Registry
317-
));
257+
assert!(dep.source().is_registry());
318258
}
319259

320260
#[test]
@@ -323,9 +263,7 @@ mod tests {
323263

324264
let dep = create_test_dependency(DependencySource::Git {
325265
url: "https://github.com/rails/rails".into(),
326-
branch: Some("main".into()),
327-
tag: None,
328-
ref_: Some("abc123".into()),
266+
rev: Some("abc123".into()),
329267
});
330268

331269
match dep.source() {
@@ -353,37 +291,15 @@ mod tests {
353291
}
354292
}
355293

356-
#[test]
357-
fn test_dependency_info_trait_github() {
358-
use deps_core::DependencyInfo;
359-
360-
let dep = create_test_dependency(DependencySource::Github {
361-
repo: "rails/rails".into(),
362-
branch: Some("main".into()),
363-
});
364-
365-
match dep.source() {
366-
deps_core::parser::DependencySource::Git { url, rev } => {
367-
assert_eq!(url, "https://github.com/rails/rails");
368-
assert_eq!(rev, None);
369-
}
370-
_ => panic!("Expected Git source"),
371-
}
372-
}
373-
374294
#[test]
375295
fn test_dependency_info_trait_custom_source() {
376296
use deps_core::DependencyInfo;
377297

378-
let dep = create_test_dependency(DependencySource::Source {
379-
name: "private".into(),
298+
let dep = create_test_dependency(DependencySource::CustomRegistry {
380299
url: "https://gems.example.com".into(),
381300
});
382301

383-
assert!(matches!(
384-
dep.source(),
385-
deps_core::parser::DependencySource::Registry
386-
));
302+
assert!(dep.source().is_registry());
387303
}
388304

389305
#[test]
@@ -404,19 +320,12 @@ mod tests {
404320
DependencySource::Registry,
405321
DependencySource::Git {
406322
url: "https://github.com/test/repo".into(),
407-
branch: None,
408-
tag: Some("v1.0".into()),
409-
ref_: None,
323+
rev: Some("v1.0".into()),
410324
},
411325
DependencySource::Path {
412326
path: "./local".into(),
413327
},
414-
DependencySource::Github {
415-
repo: "test/repo".into(),
416-
branch: None,
417-
},
418-
DependencySource::Source {
419-
name: "custom".into(),
328+
DependencySource::CustomRegistry {
420329
url: "https://custom.example.com".into(),
421330
},
422331
];

crates/deps-cargo/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ urlencoding = { workspace = true }
2828
[dev-dependencies]
2929
criterion = { workspace = true, features = ["html_reports"] }
3030
insta = { workspace = true, features = ["json"] }
31-
once_cell = { workspace = true }
3231
tempfile = { workspace = true }
3332
tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
3433
tokio-test = { workspace = true }

0 commit comments

Comments
 (0)