Skip to content

Commit a9ac193

Browse files
authored
refactor(core): remove legacy traits, add Ecosystem defaults, replace async_trait (#70)
Remove ~2000 lines of deprecated handler.rs and legacy trait system (PackageRegistry, VersionInfo, PackageMetadata, EcosystemHandler). Add default method implementations to Ecosystem trait for generate_inlay_hints, generate_hover, generate_code_actions, generate_diagnostics — eliminating ~400 lines of identical boilerplate across 8 ecosystem crates. Replace async_trait proc-macro with explicit BoxFuture pattern across Ecosystem, Registry, and LockFileProvider traits and all implementations. Centralize test mock types (MockDep, MockParseResult) in deps-core. Closes #68
1 parent 97b7783 commit a9ac193

39 files changed

+1294
-4805
lines changed

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2525
- Remove `toml_edit` from workspace dependencies (all TOML parsers now use `toml-span`)
2626
- Extract `LineOffsetTable` and `position_in_range` to deps-core for reuse across ecosystems
2727
- Extract `complete_package_names_generic` to deps-core completion module
28+
- **Architectural refactoring** — Remove legacy trait system and eliminate code duplication across 8 ecosystem crates (closes #68):
29+
- Delete `handler.rs` and legacy `PackageRegistry`, `VersionInfo`, `PackageMetadata` traits from deps-core
30+
- Add `fn formatter(&self)` required method to `Ecosystem` trait; default LSP handler implementations for `generate_inlay_hints`, `generate_hover`, `generate_code_actions`, `generate_diagnostics` — eliminating ~400 duplicate lines across ecosystems
31+
- Replace `#[async_trait]` with `BoxFuture` pattern in `Ecosystem`, `Registry`, and `LockFileProvider` traits (dyn-safe, no async_trait allocations)
32+
- Centralize `MockDep`/`MockParseResult` test helpers in `deps-core::lsp_helpers` tests (was duplicated 11 times)
33+
- Remove conflicting duplicate `Version`/`Metadata` impls from `deps-dart` and `deps-bundler` types modules
2834

2935
## [0.7.1] - 2026-02-22
3036

crates/deps-bundler/src/ecosystem.rs

Lines changed: 34 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,11 @@
11
//! Bundler ecosystem implementation for deps-lsp.
22
3-
use async_trait::async_trait;
43
use std::any::Any;
5-
use std::collections::HashMap;
64
use std::sync::Arc;
7-
use tower_lsp_server::ls_types::{
8-
CodeAction, CompletionItem, Diagnostic, Hover, InlayHint, Position, Uri,
9-
};
5+
use tower_lsp_server::ls_types::{CompletionItem, Position, Uri};
106

117
use deps_core::{
12-
Ecosystem, EcosystemConfig, ParseResult as ParseResultTrait, Registry, Result, lsp_helpers,
8+
Ecosystem, ParseResult as ParseResultTrait, Registry, Result, lsp_helpers::EcosystemFormatter,
139
};
1410

1511
use crate::formatter::BundlerFormatter;
@@ -77,7 +73,6 @@ impl BundlerEcosystem {
7773
}
7874
}
7975

80-
#[async_trait]
8176
impl Ecosystem for BundlerEcosystem {
8277
fn id(&self) -> &'static str {
8378
"bundler"
@@ -95,9 +90,15 @@ impl Ecosystem for BundlerEcosystem {
9590
&["Gemfile.lock"]
9691
}
9792

98-
async fn parse_manifest(&self, content: &str, uri: &Uri) -> Result<Box<dyn ParseResultTrait>> {
99-
let result = crate::parser::parse_gemfile(content, uri)?;
100-
Ok(Box::new(result))
93+
fn parse_manifest<'a>(
94+
&'a self,
95+
content: &'a str,
96+
uri: &'a Uri,
97+
) -> deps_core::ecosystem::BoxFuture<'a, Result<Box<dyn ParseResultTrait>>> {
98+
Box::pin(async move {
99+
let result = crate::parser::parse_gemfile(content, uri)?;
100+
Ok(Box::new(result) as Box<dyn ParseResultTrait>)
101+
})
101102
}
102103

103104
fn registry(&self) -> Arc<dyn Registry> {
@@ -108,92 +109,32 @@ impl Ecosystem for BundlerEcosystem {
108109
Some(Arc::new(crate::lockfile::GemfileLockParser))
109110
}
110111

111-
async fn generate_inlay_hints(
112-
&self,
113-
parse_result: &dyn ParseResultTrait,
114-
cached_versions: &HashMap<String, String>,
115-
resolved_versions: &HashMap<String, String>,
116-
loading_state: deps_core::LoadingState,
117-
config: &EcosystemConfig,
118-
) -> Vec<InlayHint> {
119-
lsp_helpers::generate_inlay_hints(
120-
parse_result,
121-
cached_versions,
122-
resolved_versions,
123-
loading_state,
124-
config,
125-
&self.formatter,
126-
)
127-
}
128-
129-
async fn generate_hover(
130-
&self,
131-
parse_result: &dyn ParseResultTrait,
132-
position: Position,
133-
cached_versions: &HashMap<String, String>,
134-
resolved_versions: &HashMap<String, String>,
135-
) -> Option<Hover> {
136-
lsp_helpers::generate_hover(
137-
parse_result,
138-
position,
139-
cached_versions,
140-
resolved_versions,
141-
self.registry.as_ref(),
142-
&self.formatter,
143-
)
144-
.await
112+
fn formatter(&self) -> &dyn EcosystemFormatter {
113+
&self.formatter
145114
}
146115

147-
async fn generate_code_actions(
148-
&self,
149-
parse_result: &dyn ParseResultTrait,
116+
fn generate_completions<'a>(
117+
&'a self,
118+
parse_result: &'a dyn ParseResultTrait,
150119
position: Position,
151-
_cached_versions: &HashMap<String, String>,
152-
uri: &Uri,
153-
) -> Vec<CodeAction> {
154-
lsp_helpers::generate_code_actions(
155-
parse_result,
156-
position,
157-
uri,
158-
self.registry.as_ref(),
159-
&self.formatter,
160-
)
161-
.await
162-
}
163-
164-
async fn generate_diagnostics(
165-
&self,
166-
parse_result: &dyn ParseResultTrait,
167-
cached_versions: &HashMap<String, String>,
168-
resolved_versions: &HashMap<String, String>,
169-
_uri: &Uri,
170-
) -> Vec<Diagnostic> {
171-
lsp_helpers::generate_diagnostics_from_cache(
172-
parse_result,
173-
cached_versions,
174-
resolved_versions,
175-
&self.formatter,
176-
)
177-
}
178-
179-
async fn generate_completions(
180-
&self,
181-
parse_result: &dyn ParseResultTrait,
182-
position: Position,
183-
content: &str,
184-
) -> Vec<CompletionItem> {
185-
use deps_core::completion::{CompletionContext, detect_completion_context};
186-
187-
let context = detect_completion_context(parse_result, position, content);
188-
189-
match context {
190-
CompletionContext::PackageName { prefix } => self.complete_package_names(&prefix).await,
191-
CompletionContext::Version {
192-
package_name,
193-
prefix,
194-
} => self.complete_versions(&package_name, &prefix).await,
195-
CompletionContext::Feature { .. } | CompletionContext::None => vec![],
196-
}
120+
content: &'a str,
121+
) -> deps_core::ecosystem::BoxFuture<'a, Vec<CompletionItem>> {
122+
Box::pin(async move {
123+
use deps_core::completion::{CompletionContext, detect_completion_context};
124+
125+
let context = detect_completion_context(parse_result, position, content);
126+
127+
match context {
128+
CompletionContext::PackageName { prefix } => {
129+
self.complete_package_names(&prefix).await
130+
}
131+
CompletionContext::Version {
132+
package_name,
133+
prefix,
134+
} => self.complete_versions(&package_name, &prefix).await,
135+
CompletionContext::Feature { .. } | CompletionContext::None => vec![],
136+
}
137+
})
197138
}
198139

199140
fn as_any(&self) -> &dyn Any {

crates/deps-bundler/src/lockfile.rs

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
//!
33
//! Parses Gemfile.lock files to extract resolved dependency versions.
44
5-
use async_trait::async_trait;
65
use deps_core::error::{DepsError, Result};
76
use deps_core::lockfile::{
87
LockFileProvider, ResolvedPackage, ResolvedPackages, ResolvedSource,
@@ -36,23 +35,28 @@ enum Section {
3635
RubyVersion,
3736
}
3837

39-
#[async_trait]
4038
impl LockFileProvider for GemfileLockParser {
4139
fn locate_lockfile(&self, manifest_uri: &Uri) -> Option<PathBuf> {
4240
locate_lockfile_for_manifest(manifest_uri, Self::LOCKFILE_NAMES)
4341
}
4442

45-
async fn parse_lockfile(&self, lockfile_path: &Path) -> Result<ResolvedPackages> {
46-
tracing::debug!("Parsing Gemfile.lock: {}", lockfile_path.display());
47-
48-
let content = tokio::fs::read_to_string(lockfile_path)
49-
.await
50-
.map_err(|e| DepsError::ParseError {
51-
file_type: format!("Gemfile.lock at {}", lockfile_path.display()),
52-
source: Box::new(e),
53-
})?;
54-
55-
parse_gemfile_lock(&content)
43+
fn parse_lockfile<'a>(
44+
&'a self,
45+
lockfile_path: &'a Path,
46+
) -> std::pin::Pin<Box<dyn std::future::Future<Output = Result<ResolvedPackages>> + Send + 'a>>
47+
{
48+
Box::pin(async move {
49+
tracing::debug!("Parsing Gemfile.lock: {}", lockfile_path.display());
50+
51+
let content = tokio::fs::read_to_string(lockfile_path)
52+
.await
53+
.map_err(|e| DepsError::ParseError {
54+
file_type: format!("Gemfile.lock at {}", lockfile_path.display()),
55+
source: Box::new(e),
56+
})?;
57+
58+
parse_gemfile_lock(&content)
59+
})
5660
}
5761
}
5862

crates/deps-bundler/src/registry.rs

Lines changed: 46 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -165,32 +165,7 @@ fn parse_gem_info(data: &[u8]) -> Result<GemInfo> {
165165
})
166166
}
167167

168-
// Implement PackageRegistry trait
169-
#[async_trait::async_trait]
170-
impl deps_core::PackageRegistry for RubyGemsRegistry {
171-
type Version = BundlerVersion;
172-
type Metadata = GemInfo;
173-
type VersionReq = String;
174-
175-
async fn get_versions(&self, name: &str) -> Result<Vec<Self::Version>> {
176-
self.get_versions(name).await
177-
}
178-
179-
async fn get_latest_matching(
180-
&self,
181-
name: &str,
182-
req: &Self::VersionReq,
183-
) -> Result<Option<Self::Version>> {
184-
self.get_latest_matching(name, req).await
185-
}
186-
187-
async fn search(&self, query: &str, limit: usize) -> Result<Vec<Self::Metadata>> {
188-
self.search(query, limit).await
189-
}
190-
}
191-
192-
// Implement VersionInfo trait
193-
impl deps_core::VersionInfo for BundlerVersion {
168+
impl deps_core::Version for BundlerVersion {
194169
fn version_string(&self) -> &str {
195170
&self.number
196171
}
@@ -199,13 +174,12 @@ impl deps_core::VersionInfo for BundlerVersion {
199174
self.yanked
200175
}
201176

202-
fn features(&self) -> Vec<String> {
203-
vec![]
177+
fn as_any(&self) -> &dyn std::any::Any {
178+
self
204179
}
205180
}
206181

207-
// Implement PackageMetadata trait
208-
impl deps_core::PackageMetadata for GemInfo {
182+
impl deps_core::Metadata for GemInfo {
209183
fn name(&self) -> &str {
210184
&self.name
211185
}
@@ -225,34 +199,50 @@ impl deps_core::PackageMetadata for GemInfo {
225199
fn latest_version(&self) -> &str {
226200
&self.version
227201
}
202+
203+
fn as_any(&self) -> &dyn std::any::Any {
204+
self
205+
}
228206
}
229207

230208
// Implement Registry trait for trait object support
231-
#[async_trait::async_trait]
232209
impl deps_core::Registry for RubyGemsRegistry {
233-
async fn get_versions(&self, name: &str) -> Result<Vec<Box<dyn deps_core::Version>>> {
234-
let versions = self.get_versions(name).await?;
235-
Ok(versions
236-
.into_iter()
237-
.map(|v| Box::new(v) as Box<dyn deps_core::Version>)
238-
.collect())
210+
fn get_versions<'a>(
211+
&'a self,
212+
name: &'a str,
213+
) -> deps_core::ecosystem::BoxFuture<'a, Result<Vec<Box<dyn deps_core::Version>>>> {
214+
Box::pin(async move {
215+
let versions = self.get_versions(name).await?;
216+
Ok(versions
217+
.into_iter()
218+
.map(|v| Box::new(v) as Box<dyn deps_core::Version>)
219+
.collect())
220+
})
239221
}
240222

241-
async fn get_latest_matching(
242-
&self,
243-
name: &str,
244-
req: &str,
245-
) -> Result<Option<Box<dyn deps_core::Version>>> {
246-
let version = self.get_latest_matching(name, req).await?;
247-
Ok(version.map(|v| Box::new(v) as Box<dyn deps_core::Version>))
223+
fn get_latest_matching<'a>(
224+
&'a self,
225+
name: &'a str,
226+
req: &'a str,
227+
) -> deps_core::ecosystem::BoxFuture<'a, Result<Option<Box<dyn deps_core::Version>>>> {
228+
Box::pin(async move {
229+
let version = self.get_latest_matching(name, req).await?;
230+
Ok(version.map(|v| Box::new(v) as Box<dyn deps_core::Version>))
231+
})
248232
}
249233

250-
async fn search(&self, query: &str, limit: usize) -> Result<Vec<Box<dyn deps_core::Metadata>>> {
251-
let results = self.search(query, limit).await?;
252-
Ok(results
253-
.into_iter()
254-
.map(|m| Box::new(m) as Box<dyn deps_core::Metadata>)
255-
.collect())
234+
fn search<'a>(
235+
&'a self,
236+
query: &'a str,
237+
limit: usize,
238+
) -> deps_core::ecosystem::BoxFuture<'a, Result<Vec<Box<dyn deps_core::Metadata>>>> {
239+
Box::pin(async move {
240+
let results = self.search(query, limit).await?;
241+
Ok(results
242+
.into_iter()
243+
.map(|m| Box::new(m) as Box<dyn deps_core::Metadata>)
244+
.collect())
245+
})
256246
}
257247

258248
fn package_url(&self, name: &str) -> String {
@@ -471,8 +461,8 @@ mod tests {
471461
}
472462

473463
#[test]
474-
fn test_version_info_trait() {
475-
use deps_core::VersionInfo;
464+
fn test_version_trait() {
465+
use deps_core::Version;
476466

477467
let version = BundlerVersion {
478468
number: "1.0.0".into(),
@@ -488,8 +478,8 @@ mod tests {
488478
}
489479

490480
#[test]
491-
fn test_package_metadata_trait() {
492-
use deps_core::PackageMetadata;
481+
fn test_metadata_trait() {
482+
use deps_core::Metadata;
493483

494484
let gem = GemInfo {
495485
name: "test".into(),
@@ -511,8 +501,8 @@ mod tests {
511501
}
512502

513503
#[test]
514-
fn test_package_metadata_trait_empty_optionals() {
515-
use deps_core::PackageMetadata;
504+
fn test_metadata_trait_empty_optionals() {
505+
use deps_core::Metadata;
516506

517507
let gem = GemInfo {
518508
name: "empty".into(),

0 commit comments

Comments
 (0)