Skip to content

Commit 24045c8

Browse files
committed
chore: apply PR review feedback (formatting and clippy)
- Fix formatting across multiple files using 'cargo fmt'. - Collapse nested 'if let' statements into single chains with 'and_then' or 'let_chains'. - Remove redundant closures. - Fix unused variable warning in 'src/references/mod.rs'.
1 parent b31841e commit 24045c8

5 files changed

Lines changed: 160 additions & 80 deletions

File tree

src/definition/member/mod.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -811,13 +811,13 @@ impl Backend {
811811
let mut current = Some(class.clone());
812812
for _ in 0..MAX_INHERITANCE_DEPTH {
813813
let Some(curr) = current else { break };
814-
if let Some(laravel) = curr.laravel() {
815-
if let Some(builder) = &laravel.custom_builder {
816-
if let Some(name) = builder.base_name() {
817-
builder_fqn = name.to_string();
818-
break;
819-
}
820-
}
814+
if let Some(name) = curr
815+
.laravel()
816+
.and_then(|l| l.custom_builder.as_ref())
817+
.and_then(|b| b.base_name())
818+
{
819+
builder_fqn = name.to_string();
820+
break;
821821
}
822822
current = curr
823823
.parent_class

src/references/mod.rs

Lines changed: 52 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,8 @@ impl Backend {
144144
});
145145

146146
// Resolve the enclosing class to scope the search.
147-
let hierarchy = self.resolve_member_declaration_hierarchy(uri, span_start, name, is_static);
147+
let hierarchy =
148+
self.resolve_member_declaration_hierarchy(uri, span_start, name, is_static);
148149
return self.find_member_references(
149150
name,
150151
is_static,
@@ -176,8 +177,13 @@ impl Backend {
176177
} => {
177178
// Resolve the subject to determine the class hierarchy
178179
// so we only return references on related classes.
179-
let hierarchy =
180-
self.resolve_member_access_hierarchy(uri, subject_text, *is_static, span_start, member_name);
180+
let hierarchy = self.resolve_member_access_hierarchy(
181+
uri,
182+
subject_text,
183+
*is_static,
184+
span_start,
185+
member_name,
186+
);
181187

182188
self.find_member_references(
183189
member_name,
@@ -196,7 +202,8 @@ impl Backend {
196202
}
197203
SymbolKind::MemberDeclaration { name, is_static } => {
198204
// Resolve the enclosing class to scope the search.
199-
let hierarchy = self.resolve_member_declaration_hierarchy(uri, span_start, name, *is_static);
205+
let hierarchy =
206+
self.resolve_member_declaration_hierarchy(uri, span_start, name, *is_static);
200207
self.find_member_references(
201208
name,
202209
*is_static,
@@ -1334,14 +1341,13 @@ impl Backend {
13341341
// add that builder to the hierarchy.
13351342
let mut extensions = Vec::new();
13361343
for fqn in &hierarchy {
1337-
if let Some(cls) = class_loader(fqn) {
1338-
if let Some(laravel) = cls.laravel() {
1339-
if let Some(builder) = &laravel.custom_builder {
1340-
if let Some(builder_fqn) = builder.base_name() {
1341-
extensions.push(normalize_fqn(builder_fqn).to_string());
1342-
}
1343-
}
1344-
}
1344+
if let Some(cls) = class_loader(fqn)
1345+
&& let Some(builder_fqn) = cls
1346+
.laravel()
1347+
.and_then(|l| l.custom_builder.as_ref())
1348+
.and_then(|b| b.base_name())
1349+
{
1350+
extensions.push(normalize_fqn(builder_fqn).to_string());
13451351
}
13461352
}
13471353
for ext_fqn in extensions {
@@ -1358,14 +1364,18 @@ impl Backend {
13581364
let class_index = self.fqn_class_index.read();
13591365
for (class_fqn, class_info) in class_index.iter() {
13601366
if let Some(laravel) = class_info.laravel() {
1361-
if let Some(builder) = &laravel.custom_builder {
1362-
if let Some(builder_fqn) = builder.base_name() {
1363-
let normalized = normalize_fqn(builder_fqn);
1364-
if hierarchy.contains(normalized.as_str()) {
1365-
model_seeds.push(class_fqn.clone());
1366-
}
1367+
if let Some(normalized) = laravel
1368+
.custom_builder
1369+
.as_ref()
1370+
.and_then(|b| b.base_name())
1371+
.map(normalize_fqn)
1372+
{
1373+
if hierarchy.contains(normalized.as_str()) {
1374+
model_seeds.push(class_fqn.clone());
13671375
}
1368-
} else if hierarchy.contains(crate::virtual_members::laravel::ELOQUENT_BUILDER_FQN) {
1376+
} else if hierarchy
1377+
.contains(crate::virtual_members::laravel::ELOQUENT_BUILDER_FQN)
1378+
{
13691379
// All models use the base Eloquent Builder by default.
13701380
model_seeds.push(class_fqn.clone());
13711381
}
@@ -1429,26 +1439,32 @@ impl Backend {
14291439

14301440
// Laravel forwarded methods
14311441
if let Some(laravel) = cls.laravel() {
1432-
if let Some(builder) = &laravel.custom_builder {
1433-
if let Some(builder_fqn) = builder.base_name() {
1434-
if let Some(builder_cls) = class_loader(builder_fqn) {
1435-
// Forwarded methods are instance methods on the builder
1436-
// but called statically on the model.
1437-
if builder_cls.methods.iter().any(|m| {
1438-
m.name.eq_ignore_ascii_case(name) && (!is_static || !m.is_static)
1439-
}) {
1440-
return true;
1441-
}
1442-
}
1442+
if let Some(builder_cls) = laravel
1443+
.custom_builder
1444+
.as_ref()
1445+
.and_then(|b| b.base_name())
1446+
.and_then(class_loader)
1447+
{
1448+
// Forwarded methods are instance methods on the builder
1449+
// but called statically on the model.
1450+
if builder_cls
1451+
.methods
1452+
.iter()
1453+
.any(|m| m.name.eq_ignore_ascii_case(name) && (!is_static || !m.is_static))
1454+
{
1455+
return true;
14431456
}
14441457
}
14451458
// Standard builder forwarding
1446-
if let Some(builder_cls) = class_loader(crate::virtual_members::laravel::ELOQUENT_BUILDER_FQN) {
1447-
if builder_cls.methods.iter().any(|m| {
1448-
m.name.eq_ignore_ascii_case(name) && (!is_static || !m.is_static)
1449-
}) {
1450-
return true;
1451-
}
1459+
if class_loader(crate::virtual_members::laravel::ELOQUENT_BUILDER_FQN)
1460+
.filter(|bc| {
1461+
bc.methods
1462+
.iter()
1463+
.any(|m| m.name.eq_ignore_ascii_case(name) && (!is_static || !m.is_static))
1464+
})
1465+
.is_some()
1466+
{
1467+
return true;
14521468
}
14531469
}
14541470

src/virtual_members/laravel/builder.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,13 @@ pub(super) fn build_builder_forwarded_methods(
9999
let mut current = Some(class.clone());
100100
for _ in 0..MAX_INHERITANCE_DEPTH {
101101
let Some(curr) = current else { break };
102-
if let Some(laravel) = curr.laravel() {
103-
if let Some(builder) = &laravel.custom_builder {
104-
if let Some(name) = builder.base_name() {
105-
requested_builder_fqn = name.to_string();
106-
break;
107-
}
108-
}
102+
if let Some(name) = curr
103+
.laravel()
104+
.and_then(|l| l.custom_builder.as_ref())
105+
.and_then(|b| b.base_name())
106+
{
107+
requested_builder_fqn = name.to_string();
108+
break;
109109
}
110110
current = curr
111111
.parent_class

tests/integration/laravel_custom_builder.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -820,7 +820,6 @@ class User extends Model {}
820820
);
821821
}
822822

823-
824823
#[tokio::test]
825824
async fn test_goto_definition_custom_builder_inherited_attribute() {
826825
let (backend, dir) = create_psr4_workspace(

tests/integration/laravel_references.rs

Lines changed: 94 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ User::query()->active();
100100
}
101101

102102
let builder_uri = Url::from_file_path(dir.path().join("src/Models/UserBuilder.php")).unwrap();
103-
103+
104104
// Find references for UserBuilder::active() declaration (line 5, col 21)
105105
let params = ReferenceParams {
106106
text_document_position: TextDocumentPositionParams {
@@ -113,18 +113,39 @@ User::query()->active();
113113
include_declaration: true,
114114
},
115115
};
116-
117-
let locations = backend.references(params).await.unwrap().unwrap_or_default();
118-
116+
117+
let locations = backend
118+
.references(params)
119+
.await
120+
.unwrap()
121+
.unwrap_or_default();
122+
119123
let usage_uri = Url::from_file_path(dir.path().join("usage.php")).unwrap();
120124
let usage_locs: Vec<_> = locations.iter().filter(|l| l.uri == usage_uri).collect();
121-
125+
122126
let lines: Vec<_> = usage_locs.iter().map(|l| l.range.start.line).collect();
123-
124-
assert!(lines.contains(&4), "Should find User::active(). Found at lines: {:?}", lines);
125-
assert!(lines.contains(&6), "Should find User::query()->active(). Found at lines: {:?}", lines);
126-
assert!(!lines.contains(&5), "Should NOT find Post::active(). Found at lines: {:?}", lines);
127-
assert_eq!(usage_locs.len(), 2, "Should find exactly 2 references in usage.php, but found: {:?}", lines);
127+
128+
assert!(
129+
lines.contains(&4),
130+
"Should find User::active(). Found at lines: {:?}",
131+
lines
132+
);
133+
assert!(
134+
lines.contains(&6),
135+
"Should find User::query()->active(). Found at lines: {:?}",
136+
lines
137+
);
138+
assert!(
139+
!lines.contains(&5),
140+
"Should NOT find Post::active(). Found at lines: {:?}",
141+
lines
142+
);
143+
assert_eq!(
144+
usage_locs.len(),
145+
2,
146+
"Should find exactly 2 references in usage.php, but found: {:?}",
147+
lines
148+
);
128149
}
129150

130151
#[tokio::test]
@@ -225,7 +246,7 @@ User::query()->active();
225246
}
226247

227248
let usage_uri = Url::from_file_path(dir.path().join("usage.php")).unwrap();
228-
249+
229250
// Find references for User::active() usage (line 4, col 6)
230251
let params = ReferenceParams {
231252
text_document_position: TextDocumentPositionParams {
@@ -238,20 +259,48 @@ User::query()->active();
238259
include_declaration: true,
239260
},
240261
};
241-
242-
let locations = backend.references(params).await.unwrap().unwrap_or_default();
243-
262+
263+
let locations = backend
264+
.references(params)
265+
.await
266+
.unwrap()
267+
.unwrap_or_default();
268+
244269
let usage_locs: Vec<_> = locations.iter().filter(|l| l.uri == usage_uri).collect();
245270
let lines: Vec<_> = usage_locs.iter().map(|l| l.range.start.line).collect();
246-
247-
assert!(lines.contains(&4), "Should find User::active(). Found at lines: {:?}", lines);
248-
assert!(lines.contains(&6), "Should find User::query()->active(). Found at lines: {:?}", lines);
249-
assert!(!lines.contains(&5), "Should NOT find Post::active(). Found at lines: {:?}", lines);
250-
assert_eq!(usage_locs.len(), 2, "Should find exactly 2 references in usage.php, but found: {:?}", lines);
271+
272+
assert!(
273+
lines.contains(&4),
274+
"Should find User::active(). Found at lines: {:?}",
275+
lines
276+
);
277+
assert!(
278+
lines.contains(&6),
279+
"Should find User::query()->active(). Found at lines: {:?}",
280+
lines
281+
);
282+
assert!(
283+
!lines.contains(&5),
284+
"Should NOT find Post::active(). Found at lines: {:?}",
285+
lines
286+
);
287+
assert_eq!(
288+
usage_locs.len(),
289+
2,
290+
"Should find exactly 2 references in usage.php, but found: {:?}",
291+
lines
292+
);
251293

252294
// Also should find the declaration in UserBuilder.php
253-
let builder_locs: Vec<_> = locations.iter().filter(|l| l.uri.to_string().contains("UserBuilder.php")).collect();
254-
assert_eq!(builder_locs.len(), 1, "Should find declaration in UserBuilder.php");
295+
let builder_locs: Vec<_> = locations
296+
.iter()
297+
.filter(|l| l.uri.to_string().contains("UserBuilder.php"))
298+
.collect();
299+
assert_eq!(
300+
builder_locs.len(),
301+
1,
302+
"Should find declaration in UserBuilder.php"
303+
);
255304
}
256305

257306
#[tokio::test]
@@ -335,7 +384,7 @@ Member::query()->active();
335384
}
336385

337386
let builder_uri = Url::from_file_path(dir.path().join("src/Models/UserBuilder.php")).unwrap();
338-
387+
339388
// Find references for UserBuilder::active() declaration (line 5, col 21)
340389
let params = ReferenceParams {
341390
text_document_position: TextDocumentPositionParams {
@@ -348,14 +397,30 @@ Member::query()->active();
348397
include_declaration: true,
349398
},
350399
};
351-
352-
let locations = backend.references(params).await.unwrap().unwrap_or_default();
353-
400+
401+
let locations = backend
402+
.references(params)
403+
.await
404+
.unwrap()
405+
.unwrap_or_default();
406+
354407
let usage_uri = Url::from_file_path(dir.path().join("usage.php")).unwrap();
355408
let usage_locs: Vec<_> = locations.iter().filter(|l| l.uri == usage_uri).collect();
356409
let lines: Vec<_> = usage_locs.iter().map(|l| l.range.start.line).collect();
357-
358-
assert!(lines.contains(&3), "Should find Member::active(). Found at lines: {:?}", lines);
359-
assert!(lines.contains(&4), "Should find Member::query()->active(). Found at lines: {:?}", lines);
360-
assert_eq!(usage_locs.len(), 2, "Should find exactly 2 references in usage.php");
410+
411+
assert!(
412+
lines.contains(&3),
413+
"Should find Member::active(). Found at lines: {:?}",
414+
lines
415+
);
416+
assert!(
417+
lines.contains(&4),
418+
"Should find Member::query()->active(). Found at lines: {:?}",
419+
lines
420+
);
421+
assert_eq!(
422+
usage_locs.len(),
423+
2,
424+
"Should find exactly 2 references in usage.php"
425+
);
361426
}

0 commit comments

Comments
 (0)