Skip to content

Commit 0de10f4

Browse files
committed
A project that uses Mago only for formatting no longer gets Mago's lint
and analyze reports
1 parent 6618d29 commit 0de10f4

7 files changed

Lines changed: 738 additions & 98 deletions

File tree

docs/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
467467
- **`int ** int` carries the same benevolent `int|float` union as division.** PHP promotes exponentiation to a `float` on overflow (`2 ** 64`) or a negative exponent (`2 ** -1`), a property of the operand values rather than their types, so `takes_int($base ** $exp)` and `$base **= $exp` under `declare(strict_types=1)` were reported the same way plain division was before the fix above. The result is now treated with the same benevolence: one branch fitting the target is enough, and a declared `int|float` or an exponentiation of one still has to fit whole.
468468
- **A reopened file no longer shows diagnostics from before it was closed.** Each open file's pull `resultId` counted up from zero, and closing a file dropped that counter along with everything else, so reopening it started counting from zero again while the editor could still be holding a higher id from before the close. Once the reopened file's recomputes climbed back to that number, a pull carrying the stale id matched the current one and the editor kept showing the diagnostics the file had before it was closed. Result ids are now drawn from a single sequence for the whole session, the way `workspace/diagnostic`'s ids already were, so a reopened file can never land on an id the editor saw before.
469469
- **PHPStan auto-detection no longer picks up a `vendor/bin/phpstan` the project doesn't actually depend on.** A `phpstan` binary from a transitive dependency of something else, was proxied as though the project itself used PHPStan. Auto-detection under `vendor/bin` now only fires when `composer.json` requires `phpstan/phpstan` directly, in `require` or `require-dev`. A `phpstan` found on `$PATH` is unaffected, since installing it globally is a deliberate choice, and an explicit `command` in `.phpantom.toml` still overrides detection entirely, which is how a manually managed install (a versioned `.phar` outside the Composer bin dir, as OpenCart ships one) is wired up.
470+
- **A project that uses Mago only for formatting no longer gets Mago's lint and analyze reports.** Any `mago.toml` at the workspace root was treated as a request for every Mago diagnostic, so a project that had written one to pick a formatting style, which is what PHPantom's own documentation suggests for controlling the built-in formatter, suddenly saw a wall of problems from a checker it never ran. Which of Mago's checkers run is now decided by what the project configures: a `[linter]` table turns on `mago lint`, an `[analyzer]` table turns on `mago analyze`, and a file that carries neither turns on neither. `lint` and `analyze` under `[mago]` in `.phpantom.toml` override the detection in either direction, so a project that runs a checker without configuring it can still ask for the reports.
471+
- **`mago analyze` no longer reports Laravel code it has no way to understand.** Mago's analyser has no built-in Laravel support, so it cannot follow the Eloquent and facade indirection PHPantom models; the gap is meant to be closed by an extension, a mechanism Mago only grew in 1.47, and no Laravel extension exists yet. On a Laravel project, `mago analyze` is therefore proxied only when the `mago.toml` wires one up, either an enabled `[extension-hosts.*]` entry or a namespaced plugin such as `plugins = ["acme/laravel"]`. Mago's own plugins (`stdlib`, `psl`, `flow-php`, `psr-container`) do not count, since none of them carries that knowledge. `mago lint` keeps running, as its linter does have a Laravel integration, and `analyze` under `[mago]` still forces the issue either way.
472+
- **Mago auto-detection no longer picks up a `vendor/bin/mago` the project doesn't actually depend on.** As with PHPStan above, a binary installed as somebody else's transitive dependency was proxied as though the project itself used Mago. Auto-detection under `vendor/bin` now only fires when `composer.json` requires `carthage-software/mago` directly, and a global install or an explicit `command` is unaffected.
470473

471474
## [0.9.0] - 2026-07-20
472475

docs/configuration.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,18 @@ Mago is only activated when `mago.toml` exists at the workspace root.
108108

109109
| Key | Type | Default | Description |
110110
| ----------------- | ------- | ------- | ----------- |
111-
| `command` | string | unset | Command or path for Mago. Unset: auto-detect via `vendor/bin/mago` then `$PATH`. `""`: disable. |
111+
| `command` | string | unset | Command or path for Mago. Unset: auto-detect via `vendor/bin/mago` (only when `composer.json` requires `carthage-software/mago` directly) then `$PATH`. `""`: disable. |
112+
| `lint` | bool | unset | Proxy `mago lint` diagnostics. Unset: only when `mago.toml` has a `[linter]` table. |
113+
| `analyze` | bool | unset | Proxy `mago analyze` diagnostics. Unset: only when `mago.toml` has an `[analyzer]` table, and on Laravel only when it also wires up an extension. |
112114
| `lint-timeout` | integer | `30000` | Max runtime in milliseconds before `mago lint` is killed. |
113115
| `analyze-timeout` | integer | `60000` | Max runtime in milliseconds before `mago analyze` is killed. |
114116

117+
Which of Mago's two diagnostic commands run follows the workspace `mago.toml`, since a project that uses Mago for one thing rarely wants the others. A `mago.toml` holding a `[formatter]` table and nothing else belongs to a project that formats with Mago and checks its code with something else, so neither `mago lint` nor `mago analyze` is proxied for it.
118+
119+
On a Laravel project, `mago analyze` additionally needs the `mago.toml` to wire up an extension, either an enabled `[extension-hosts.*]` entry or a namespaced plugin such as `plugins = ["acme/laravel"]`. Mago's analyser has no built-in Laravel support, so without one it cannot see through Eloquent or the facades and reports correct code in bulk. Mago's own plugins (`stdlib`, `psl`, `flow-php`, `psr-container`) do not count, since none of them supplies that knowledge. `mago lint` is unaffected, as its linter does have a Laravel integration.
120+
121+
Set `lint` or `analyze` explicitly to override all of this in either direction.
122+
115123
### `[laravel]`
116124

117125
#### `[laravel.schema]`

src/config.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,17 @@ pub struct MagoConfig {
397397
/// - `""` — disable Mago.
398398
/// - Any other value — use as the command.
399399
pub command: Option<String>,
400+
/// Whether to proxy `mago lint` diagnostics.
401+
///
402+
/// - `None` (default) — proxy them when the workspace `mago.toml`
403+
/// configures the linter (it carries a `[linter]` table).
404+
/// - `true` / `false` — always / never, whatever `mago.toml` says.
405+
pub lint: Option<bool>,
406+
/// Whether to proxy `mago analyze` diagnostics.
407+
///
408+
/// Same three states as [`lint`](Self::lint), keyed on an
409+
/// `[analyzer]` table in `mago.toml` when unset.
410+
pub analyze: Option<bool>,
400411
/// Maximum runtime in milliseconds before `mago lint` is killed.
401412
/// Defaults to 30 000 ms (30 seconds).
402413
#[serde(rename = "lint-timeout")]
@@ -745,6 +756,9 @@ mod tests {
745756
assert!(config.phpcs.timeout.is_none());
746757
assert_eq!(config.phpcs.timeout_ms(), 30_000);
747758
assert!(config.mago.command.is_none());
759+
// Unset means "follow mago.toml", not on or off.
760+
assert!(config.mago.lint.is_none());
761+
assert!(config.mago.analyze.is_none());
748762
assert!(config.mago.lint_timeout.is_none());
749763
assert!(config.mago.analyze_timeout.is_none());
750764
assert_eq!(config.mago.lint_timeout_ms(), 30_000);
@@ -1052,6 +1066,8 @@ timeout = 15000
10521066
10531067
[mago]
10541068
command = "/usr/local/bin/mago"
1069+
lint = true
1070+
analyze = false
10551071
lint-timeout = 15000
10561072
analyze-timeout = 45000
10571073
"#,
@@ -1092,6 +1108,8 @@ analyze-timeout = 45000
10921108
assert_eq!(config.phpcs.standard.as_deref(), Some("PSR12"));
10931109
assert_eq!(config.phpcs.timeout_ms(), 15_000);
10941110
assert_eq!(config.mago.command.as_deref(), Some("/usr/local/bin/mago"));
1111+
assert_eq!(config.mago.lint, Some(true));
1112+
assert_eq!(config.mago.analyze, Some(false));
10951113
assert_eq!(config.mago.lint_timeout_ms(), 15_000);
10961114
assert_eq!(config.mago.analyze_timeout_ms(), 45_000);
10971115
assert!(!config.mago.is_disabled());

src/diagnostics/external/mago.rs

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,14 @@ impl Backend {
7575
None => continue,
7676
};
7777

78-
// Mago requires mago.toml to operate.
79-
if !mago::has_mago_config(&workspace_root) {
78+
let composer_pkg = crate::composer::read_composer_package(&workspace_root);
79+
let laravel = composer_pkg
80+
.as_ref()
81+
.is_some_and(crate::composer::is_laravel_project);
82+
83+
// Mago requires mago.toml to operate, and its tables decide
84+
// whether the project uses `mago lint` at all.
85+
if !mago::enabled_services(&workspace_root, &config.mago, laravel).lint {
8086
continue;
8187
}
8288

@@ -85,14 +91,17 @@ impl Backend {
8591
None => continue,
8692
};
8793

88-
let bin_dir: Option<String> = crate::composer::read_composer_package(&workspace_root)
89-
.map(|pkg| crate::composer::get_bin_dir(&pkg));
94+
let bin_dir: Option<String> = composer_pkg.as_ref().map(crate::composer::get_bin_dir);
9095

91-
let resolved =
92-
match mago::resolve_mago(Some(&workspace_root), &config.mago, bin_dir.as_deref()) {
93-
Some(r) => r,
94-
None => continue,
95-
};
96+
let resolved = match mago::resolve_mago(
97+
Some(&workspace_root),
98+
&config.mago,
99+
bin_dir.as_deref(),
100+
composer_pkg.as_ref(),
101+
) {
102+
Some(r) => r,
103+
None => continue,
104+
};
96105

97106
// ── Step 4: run mago lint (the slow part) ───────────────
98107
let mago_config = config.mago.clone();
@@ -201,8 +210,14 @@ impl Backend {
201210
None => continue,
202211
};
203212

204-
// Mago requires mago.toml to operate.
205-
if !mago::has_mago_config(&workspace_root) {
213+
let composer_pkg = crate::composer::read_composer_package(&workspace_root);
214+
let laravel = composer_pkg
215+
.as_ref()
216+
.is_some_and(crate::composer::is_laravel_project);
217+
218+
// Mago requires mago.toml to operate, and its tables decide
219+
// whether the project uses `mago analyze` at all.
220+
if !mago::enabled_services(&workspace_root, &config.mago, laravel).analyze {
206221
continue;
207222
}
208223

@@ -211,14 +226,17 @@ impl Backend {
211226
None => continue,
212227
};
213228

214-
let bin_dir: Option<String> = crate::composer::read_composer_package(&workspace_root)
215-
.map(|pkg| crate::composer::get_bin_dir(&pkg));
229+
let bin_dir: Option<String> = composer_pkg.as_ref().map(crate::composer::get_bin_dir);
216230

217-
let resolved =
218-
match mago::resolve_mago(Some(&workspace_root), &config.mago, bin_dir.as_deref()) {
219-
Some(r) => r,
220-
None => continue,
221-
};
231+
let resolved = match mago::resolve_mago(
232+
Some(&workspace_root),
233+
&config.mago,
234+
bin_dir.as_deref(),
235+
composer_pkg.as_ref(),
236+
) {
237+
Some(r) => r,
238+
None => continue,
239+
};
222240

223241
// ── Step 4: run mago analyze (the slow part) ────────────
224242
let mago_config = config.mago.clone();

src/diagnostics/subject_cache.rs

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
//! Cache key for per-pass member-access subject resolution.
2+
//!
3+
//! A single file can contain hundreds of member-access spans that share
4+
//! the same subject text (e.g. 60 occurrences of `$this->assertEquals`,
5+
//! `$this->assertTrue`, …). Without caching, each span triggers the
6+
//! full resolution pipeline including `resolve_variable_types`, which
7+
//! re-parses the entire file via `with_parsed_program`.
8+
//!
9+
//! Every diagnostic that caches subject resolutions must key that cache
10+
//! the same way, because the key is what decides whether two accesses
11+
//! are guaranteed to see the same type. A key that is too coarse leaks
12+
//! one access's type into another: keying only by `(variable_name,
13+
//! class_name)` reports a deprecation against a same-named parameter in
14+
//! a *different* method of the same class. [`SubjectCacheKey::build`]
15+
//! is the one place that decision lives, so all consumers share it.
16+
//!
17+
//! The key deliberately omits per-access byte offsets so the cache stays
18+
//! effective — a service file with 200 accesses to `$model->` resolves
19+
//! the variable once, not 200 times. Expression-level narrowing
20+
//! (ternary `instanceof`, inline `&&` chains) can refine a type at a
21+
//! single byte offset without creating a narrowing block; consumers that
22+
//! care handle it with an uncached re-resolution fallback rather than by
23+
//! making the key finer.
24+
25+
use crate::symbol_map::SymbolMap;
26+
use crate::types::{AccessKind, ClassInfo};
27+
28+
/// Scope identifier for the subject resolution cache.
29+
///
30+
/// Two member accesses share the same scope when they are inside the
31+
/// same class body (identified by class name and byte offset of the
32+
/// opening brace) **and** the same function/method/closure body
33+
/// (identified by its start offset). This prevents two methods in
34+
/// the same class from sharing a cache entry when a same-named
35+
/// variable has a different type in each method.
36+
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
37+
pub(crate) enum ScopeKey {
38+
/// Inside a class at the given byte offset, within a specific
39+
/// function/method/closure scope. `fn_scope_start` is the byte
40+
/// offset of the enclosing function body (from
41+
/// [`SymbolMap::find_enclosing_scope`]), or `0` for class-level
42+
/// code outside any method.
43+
Class {
44+
name: String,
45+
start_offset: u32,
46+
fn_scope_start: u32,
47+
},
48+
/// Top-level code outside any class, within a specific
49+
/// function scope (`0` when truly top-level).
50+
TopLevel { fn_scope_start: u32 },
51+
}
52+
53+
/// Cache key combining the subject text, access kind, and scope.
54+
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
55+
pub(crate) struct SubjectCacheKey {
56+
subject_text: String,
57+
access_kind: AccessKind,
58+
scope: ScopeKey,
59+
/// The `effective_from` offset of the active variable definition at
60+
/// the point of access, or `0` for non-variable subjects. This
61+
/// ensures that accesses before and after a reassignment get
62+
/// separate cache entries.
63+
var_def_offset: u32,
64+
/// The innermost narrowing block containing the access for variable
65+
/// subjects (excluding `$this`), or `0` for non-variable subjects.
66+
/// This ensures that accesses inside different instanceof-narrowing
67+
/// contexts (e.g. different if-bodies) get independent cache
68+
/// entries. Without this, the first access caches a narrowed type
69+
/// and subsequent accesses in a different narrowing context reuse
70+
/// the wrong result.
71+
narrowing_offset: u32,
72+
/// The offset of the most recent `assert($var instanceof …)`
73+
/// statement preceding this access, or `0` if there is none.
74+
/// Assert-instanceof statements act as sequential narrowing
75+
/// boundaries: they change the variable's resolved type without
76+
/// creating a block scope, so accesses before and after the
77+
/// assert must get separate cache entries.
78+
assert_offset: u32,
79+
}
80+
81+
impl SubjectCacheKey {
82+
/// Build the cache key for a member access on `subject_text` at
83+
/// `access_offset`.
84+
pub(crate) fn build(
85+
symbol_map: &SymbolMap,
86+
current_class: Option<&ClassInfo>,
87+
subject_text: &str,
88+
access_kind: AccessKind,
89+
access_offset: u32,
90+
) -> Self {
91+
let fn_scope_start = symbol_map.find_enclosing_scope(access_offset);
92+
93+
// For variable subjects (excluding $this), compute the active
94+
// definition offset so that accesses before and after a
95+
// reassignment get separate cache entries.
96+
let var_def_offset = if subject_text.starts_with('$')
97+
&& subject_text != "$this"
98+
&& !subject_text.starts_with("$this->")
99+
{
100+
// Extract the bare variable name (e.g. "$file" from "$file"
101+
// or from a chain like "$file->foo()").
102+
let var_name = subject_text
103+
.find("->")
104+
.map(|i| &subject_text[..i])
105+
.unwrap_or(subject_text);
106+
symbol_map.active_var_def_offset(
107+
&var_name[1..], // strip leading '$'
108+
access_offset,
109+
)
110+
} else {
111+
0
112+
};
113+
114+
// Narrowing discrimination applies to regular variables ($var)
115+
// AND property chains on $this ($this->prop), because instanceof
116+
// checks and assert() calls can narrow property types just like
117+
// local variables. Bare $this is excluded because its type
118+
// never changes within a method.
119+
let needs_narrowing_discriminator =
120+
subject_text.starts_with('$') && subject_text != "$this";
121+
let (narrowing_offset, assert_offset) = if needs_narrowing_discriminator {
122+
(
123+
symbol_map.find_narrowing_block(access_offset),
124+
symbol_map.find_preceding_assert_offset(access_offset),
125+
)
126+
} else {
127+
(0, 0)
128+
};
129+
130+
SubjectCacheKey {
131+
subject_text: subject_text.to_string(),
132+
access_kind,
133+
scope: scope_key_for(current_class, fn_scope_start),
134+
var_def_offset,
135+
narrowing_offset,
136+
assert_offset,
137+
}
138+
}
139+
}
140+
141+
/// Build a [`ScopeKey`] from the innermost enclosing class (if any)
142+
/// and the enclosing function/method/closure scope start offset.
143+
fn scope_key_for(current_class: Option<&ClassInfo>, fn_scope_start: u32) -> ScopeKey {
144+
match current_class {
145+
Some(cc) => ScopeKey::Class {
146+
name: cc.name.to_string(),
147+
start_offset: cc.start_offset,
148+
fn_scope_start,
149+
},
150+
None => ScopeKey::TopLevel { fn_scope_start },
151+
}
152+
}

0 commit comments

Comments
 (0)