Skip to content

Commit 717652b

Browse files
committed
Suppress false positives for Laravel Eloquent scope methods on Builder
1 parent be5d7eb commit 717652b

7 files changed

Lines changed: 396 additions & 47 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
6767
- **Diagnostics now work for vendor files open in the editor.** Projects using `--prefer-source` or monorepo setups where libraries are edited from the `vendor/` directory no longer have diagnostics suppressed. Previously all vendor files were unconditionally skipped, hiding syntax errors, deprecation warnings, and all other diagnostics. The `analyze` command also accepts vendor paths as explicit overrides (e.g. `phpantom_lsp analyze vendor/some-package/src/`).
6868
- **Full-line diagnostic suppression no longer requires a code mapping.** Full-line diagnostics (from tools like PHPStan that only report line numbers) are now suppressed whenever any precise diagnostic exists on the same line, regardless of error codes. Previously suppression relied on a hand-maintained mapping between error codes from different tools, which was incomplete and allowed redundant full-line underlines to obscure the precise location of the issue.
6969

70+
- **Scope methods on Eloquent Builder no longer produce false-positive diagnostics.** When a scope method's return type was a bare `Builder` (without generic arguments), subsequent calls in the chain lost the model type and reported scope methods as not found. Bare `Builder` return types on scope methods are now automatically wrapped as `Builder<ConcreteModel>` to preserve the chain. Additionally, unknown method diagnostics on the Eloquent Builder are suppressed when `__call` is present, since Builder dispatches scope methods and Query\Builder calls through it at runtime.
7071
- **Hover on closure parameters lost inferred generic arguments.** When a closure parameter had an explicit bare class type hint (e.g. `function (Builder $q)`) and the callable signature inferred the same class with generic arguments (e.g. `Builder<Article>`), hover displayed the bare class name instead of the generic version. The inferred type string is now preserved through the resolution pipeline so that hover, completion, and diagnostics all see the same type.
7172
- **Hover on variables assigned from chained methods lost generic parameters.** When a method returned `static`, `$this`, or `self` and the receiver carried generic parameters (e.g. `Builder<Article>`), hover on a variable assigned from the chain showed the bare class name (`Builder`) instead of the generic version. The receiver's type string is now carried through method call resolution so that self-referencing return types preserve generic parameters across chains.
7273
- **Hover on the `$` sign of a variable at its assignment site showed no type.** Hovering on the `$` of `$order = new Order()` displayed a bare `$order` with no type information, while hovering one character to the right on `o` worked correctly. The variable's byte offset coincided with the assignment statement's start offset, causing the resolver to skip the assignment entirely. The cursor offset is now nudged past the statement boundary so the assignment is included.

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ within the same impact tier.
2323

2424
| # | Item | Impact | Effort |
2525
| --- | ------------------------------------------------------------------------------------------------------------------------------------------------ | ------ | ------ |
26-
| B6 | [Scope methods not found on Builder in analyzer chains](todo/bugs.md#b6-scope-methods-not-found-on-builder-in-analyzer-chains) | High | Medium |
2726
| B7 | [PHPDoc `@param` generic array type not merged with native `array` hint](todo/bugs.md#b7-phpdoc-param-generic-array-type-not-merged-with-native-array-hint) | Low | Medium |
2827
| H10 | [`return.unusedType` — remove unused type from return union](todo/phpstan-actions.md#h10-returnunusedtype--remove-unused-type-from-return-union) | Medium | Medium |
2928
| H6 | `return.type` — update return type to match actual returns | Medium | Medium |

docs/todo/bugs.md

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,5 @@
11
# PHPantom — Bug Fixes
22

3-
## B6: Scope methods not found on Builder in analyzer chains
4-
5-
PHPantom's completion engine correctly injects scope methods onto
6-
`Builder<ConcreteModel>` via `try_inject_builder_scopes` in
7-
`resolve_named_type`. However, the analyzer's `check_member_on_resolved_classes`
8-
uses `resolve_class_fully_cached` which is keyed by bare FQN without
9-
generic args. A prior cache entry for `Builder` (without model-specific
10-
scopes) is returned, and the scope method is reported as not found.
11-
12-
The analyzer does check `base_classes` first (before the cache) to avoid
13-
this, but in method chains like
14-
`ArticleCategoryTranslation::whereHas(...)->whereLanguage(...)`, the
15-
intermediate `Builder<ArticleCategoryTranslation>` type produced by
16-
`whereHas()` may not carry the scope-injected methods in `base_classes`.
17-
18-
Affected diagnostics (5 direct + 2 cascading):
19-
20-
Direct `unknown_member` — scope method exists on model but not found on
21-
Builder:
22-
- `ArticleRepository:69``whereLanguage` (scope on
23-
`ArticleCategoryTranslation`)
24-
- `ProductRepository:271``whereIsLuxury` (scope on `Product`)
25-
- `ProductRepository:272``whereIsDerma` (scope on `Product`)
26-
- `ProductRepository:273``whereIsProHairCare` (scope on `Product`)
27-
- `ProductRepository:369``whereIsLuxury` (scope on `Product`)
28-
29-
Cascading `unresolved_member_access`:
30-
- `EventRepository:23``pluck` after broken
31-
`whereIsBlackFriday()->whereIsVisible()` chain
32-
33-
Note: `EventRepository:22` reports `whereIsVisible` not found on Builder.
34-
Product has `scopeIsVisibleIn` (takes a `Country` parameter) but no
35-
`scopeWhereIsVisible` and no `is_visible` column. This may be a genuine
36-
code bug in the project rather than an LSP issue.
37-
38-
**Impact:** 5–6 direct `unknown_member` diagnostics plus 1–2 cascading.
39-
403
## B7: PHPDoc `@param` generic array type not merged with native `array` hint
414

425
When a method has a native type hint `array` and a PHPDoc `@param` with
@@ -68,7 +31,4 @@ to `array`. The `@param` says the array case is `list<Request>`, so
6831
native `array` with the docblock's `list<Request>`.
6932

7033
**Impact:** 1 diagnostic in the shared project
71-
(`MobilePayConnection:76`).
72-
73-
74-
34+
(`MobilePayConnection:76`).

src/diagnostics/unknown_members.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ use crate::Backend;
5959
use crate::completion::resolver::{ResolutionCtx, SubjectOutcome, resolve_subject_outcome};
6060
use crate::symbol_map::SymbolKind;
6161
use crate::types::{AccessKind, ClassInfo, ClassLikeKind};
62+
use crate::virtual_members::laravel::ELOQUENT_BUILDER_FQN;
6263
use crate::virtual_members::resolve_class_fully_cached;
6364

6465
use super::helpers::{find_innermost_enclosing_class, make_diagnostic};
@@ -634,6 +635,27 @@ impl Backend {
634635
.iter()
635636
.any(|c| has_magic_method_for_access(c, is_static, true)));
636637

638+
// ── Suppress diagnostics on Eloquent Builder with __call ────
639+
// Eloquent Builder's `__call` specifically dispatches scope
640+
// methods defined on the concrete model and forwards unknown
641+
// calls to Query\Builder. When the Builder is used without
642+
// generic args (e.g. a closure parameter typed as bare
643+
// `Builder`), scope methods cannot be injected because the
644+
// model type is unknown. Emitting a warning for every scope
645+
// call on a bare Builder creates pervasive false positives in
646+
// real Laravel projects. Suppress the diagnostic entirely
647+
// when any resolved branch is an Eloquent Builder that has
648+
// `__call`, since the method WILL succeed at runtime.
649+
if has_magic_call
650+
&& resolved_classes.iter().any(|c| {
651+
c.name == "Builder"
652+
&& c.file_namespace.as_deref() == Some("Illuminate\\Database\\Eloquent")
653+
|| c.name == ELOQUENT_BUILDER_FQN
654+
})
655+
{
656+
return MemberCheckResult::Ok;
657+
}
658+
637659
// ── Member is unresolved on ALL branches — emit diagnostic ──
638660
let range = match offset_range_to_lsp_range(content, start as usize, end as usize) {
639661
Some(r) => r,

src/virtual_members/laravel/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -382,11 +382,11 @@ fn inject_scopes_and_model_methods(
382382
// 1. Inject scope methods.
383383
let scope_methods = build_scope_methods_for_builder(model_arg, class_loader);
384384
for method in scope_methods {
385-
if !result
385+
let already_exists = result
386386
.methods
387387
.iter()
388-
.any(|m| m.name == method.name && m.is_static == method.is_static)
389-
{
388+
.any(|m| m.name == method.name && m.is_static == method.is_static);
389+
if !already_exists {
390390
result.methods.push(method);
391391
}
392392
}

src/virtual_members/laravel/scopes.rs

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ use std::sync::Arc;
1111

1212
use crate::php_type::PhpType;
1313
use crate::types::{ClassInfo, MethodInfo};
14+
use crate::util::short_name;
1415

16+
use super::ELOQUENT_BUILDER_FQN;
1517
use super::helpers::extends_eloquent_model;
1618

1719
/// Build the default return type for scope methods that don't declare a return
@@ -138,7 +140,9 @@ pub fn build_scope_methods_for_builder(
138140
) -> Vec<MethodInfo> {
139141
let model_class = match class_loader(model_name) {
140142
Some(c) => c,
141-
None => return Vec::new(),
143+
None => {
144+
return Vec::new();
145+
}
142146
};
143147

144148
// Only synthesize scopes for actual Eloquent models.
@@ -153,7 +157,6 @@ pub fn build_scope_methods_for_builder(
153157
// Using the pre-provider resolution preserves the raw methods.
154158
let resolved_model =
155159
crate::inheritance::resolve_class_with_inheritance(&model_class, class_loader);
156-
157160
// Build a substitution map so that `static`, `$this`, and `self`
158161
// in scope return types resolve to the concrete model name.
159162
// The default scope return type is `\...\Builder<static>` where
@@ -183,6 +186,20 @@ pub fn build_scope_methods_for_builder(
183186
// Apply substitutions to the return type.
184187
if let Some(ref mut ret) = m.return_type {
185188
*ret = ret.substitute(&subs);
189+
190+
// When a scope method declares a bare `Builder` return type
191+
// (without generic args), the chain loses track of the
192+
// concrete model. Subsequent calls on the returned Builder
193+
// would not find model-specific scope methods because
194+
// `type_hint_to_classes_typed` only injects scopes when
195+
// generic args are present. Wrap bare Builder return types
196+
// as `Builder<ModelName>` to preserve the chain.
197+
if is_bare_builder_type(ret) {
198+
*ret = PhpType::Generic(
199+
ELOQUENT_BUILDER_FQN.to_string(),
200+
vec![PhpType::Named(model_name.to_string())],
201+
);
202+
}
186203
}
187204

188205
methods.push(m);
@@ -191,6 +208,19 @@ pub fn build_scope_methods_for_builder(
191208
methods
192209
}
193210

211+
/// Check whether a `PhpType` is a bare Eloquent Builder reference
212+
/// without generic arguments.
213+
///
214+
/// Matches both the FQN (`Illuminate\Database\Eloquent\Builder`) and
215+
/// the short name (`Builder`) since scope methods in user code
216+
/// typically use the imported short name.
217+
fn is_bare_builder_type(ty: &PhpType) -> bool {
218+
match ty {
219+
PhpType::Named(name) => name == ELOQUENT_BUILDER_FQN || short_name(name) == "Builder",
220+
_ => false,
221+
}
222+
}
223+
194224
#[cfg(test)]
195225
#[path = "scopes_tests.rs"]
196226
mod tests;

0 commit comments

Comments
 (0)