Skip to content

Commit 424edc0

Browse files
committed
A project with its own phpcs.xml runs phpcs, even when
`squizlabs/php_codesniffer` is only a transitive dependency
1 parent bc6c58b commit 424edc0

4 files changed

Lines changed: 104 additions & 14 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
110110

111111
- **A Laravel project that requires Larastan gets PHPStan diagnostics too.** PHPStan auto-detection looked only for a direct `phpstan/phpstan` dependency in `composer.json`, so a project that requires `larastan/larastan` and lets it pull `phpstan/phpstan` in transitively never had `vendor/bin/phpstan` recognised, even though the binary was right there. A Laravel project is now recognised through a direct dependency on `larastan/larastan`, or a fork of it such as `calebdw/larastan`, instead: plain PHPStan does not understand Eloquent magic, facades, or container bindings, so a Laravel project that depends on `phpstan/phpstan` directly but has not installed Larastan is still left alone rather than run through an analyser that would misread its own framework.
112112
- **A project with its own `phpstan.neon` gets PHPStan diagnostics regardless of what `composer.json` declares.** PHPStan auto-detection depended entirely on a `composer.json` dependency, so a project that hand-authors a `phpstan.neon` or `phpstan.neon.dist` config, whether it depends on `phpstan/phpstan` transitively, installs a Larastan fork the dependency check does not otherwise certify, or wires PHPStan up some other way entirely, never had `vendor/bin/phpstan` recognised. A `phpstan.neon`/`phpstan.neon.dist` file at the workspace root is now itself enough to certify PHPStan on a project, including a Laravel one that has not installed Larastan.
113+
- **A project with its own `phpcs.xml` gets phpcbf as its formatter, even when `squizlabs/php_codesniffer` is only a transitive dependency.** Formatter auto-detection looked only for a direct `squizlabs/php_codesniffer` entry in `require-dev`, so a project that instead depends on a rules package like `slevomat/coding-standard` or `cakephp/cakephp-codesniffer`, which pull PHP_CodeSniffer in transitively, never had `vendor/bin/phpcbf` recognised even though the binary was right there. A `phpcs.xml`, `.phpcs.xml`, `phpcs.xml.dist`, or `.phpcs.xml.dist` file at the workspace root now certifies phpcbf on its own. Closes #374.
113114
- **A custom Eloquent builder keeps the model it was built for.** `SiteCertificate::query()->whereKey($id)->firstOrFail()` resolved to the base `Model`, or reported `subject type 'TModel' could not be resolved`, whenever the model routed its queries through a custom builder. PHP has no generics, so almost nobody writes `@template`/`@extends` on a builder subclass: `class SiteCertificateBuilder extends Builder {}` is the whole class, and the model was lost at whichever method on the chain returns it. The model the query was started from now travels through the builder to the end of the chain, so the result of `firstOrFail()`, `first()`, `get()`, and the rest completes, hovers, and is checked as the concrete model, whether the builder is declared with generics or without. A builder specialised this way also keeps everything the ordinary resolution gives it, including the query-builder methods it reaches through `@mixin`. Closes #362.
114115
- **`array_filter()` reports the type the filter leaves behind.** A callback that tests the value it is handed proves something about every entry that survives, but the result kept whatever element type went in, so `array_filter($values, fn ($v) => $v !== null)` still looked like it could hold `null` and returning it from a function declared `int[]` was reported as a type error. The surviving values are now narrowed the way the body of an `if` narrows the variable it guards, whether the test is written as an `is_…()` call, a comparison against `null`, an `instanceof` check, or a callable string such as `'is_int'`. The keys were already narrowed this way in the modes that hand the callback a key, and both halves narrow together under `ARRAY_FILTER_USE_BOTH`. Closes #376.
115116
- **A deprecation warning belongs to the variable it is written on.** The deprecated-usage check typed its subject from a cache keyed by variable name and class, so two methods of the same class that reuse a parameter name shared one entry and whichever type was bound first won. A Laravel controller with a `Request $request` method above a `PendingRequest $request` method reported `Illuminate\Http\Request::get is deprecated` on the HTTP client call, where `get()` is the ordinary way to make a request, and renaming either parameter made the warning vanish. Every subject is now typed in the scope it is written in, so each method, closure, and `instanceof` branch gets its own answer, and a genuine deprecation is still reported no matter which order the methods appear in.

docs/configuration.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ The file is optional. Unknown keys are silently ignored, so the file is forward-
143143
PHPantom ships a built-in PHP formatter (mago-formatter) that works out of the box, so `textDocument/formatting` requests are answered without any setup. The formatter is chosen per project in this order:
144144

145145
1. **Explicit config wins.** A tool path set under `[formatting]` in `.phpantom.toml` (`pint`, `php-cs-fixer`, or `phpcbf`) is always used. Setting a tool to `""` disables it.
146-
2. **Composer `require-dev` wins over the built-in formatter.** If `composer.json` lists `laravel/pint`, `friendsofphp/php-cs-fixer`, or `squizlabs/php_codesniffer` in `require-dev`, PHPantom resolves the binary through Composer's bin-dir and runs it as a subprocess. These tools discover their own project config (`pint.json`, `.php-cs-fixer.php`, `.phpcs.xml`, etc.) as they normally would.
146+
2. **Composer `require-dev` wins over the built-in formatter.** If `composer.json` lists `laravel/pint`, `friendsofphp/php-cs-fixer`, or `squizlabs/php_codesniffer` in `require-dev`, PHPantom resolves the binary through Composer's bin-dir and runs it as a subprocess. A `phpcs.xml`, `.phpcs.xml`, `phpcs.xml.dist`, or `.phpcs.xml.dist` file at the workspace root certifies phpcbf the same way, so a project that only pulls `squizlabs/php_codesniffer` in transitively (e.g. through `slevomat/coding-standard`) is still detected. These tools discover their own project config (`pint.json`, `.php-cs-fixer.php`, `.phpcs.xml`, etc.) as they normally would.
147147
3. **Otherwise, the built-in formatter is used.**
148148

149149
The built-in formatter defaults to the PER-CS 2.0 style. If a `mago.toml` is present at the workspace root, its `[formatter]` table is honoured instead, so PHPantom formats with the same preset and settings your project already uses with the Mago CLI:

src/diagnostics/mod.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,9 @@
133133
//!
134134
//! - **PHPCS proxy diagnostics** — run PHP_CodeSniffer via
135135
//! `phpcs --report=json` and surface coding standard violations as
136-
//! LSP diagnostics. Auto-detected when `squizlabs/php_codesniffer`
137-
//! is in `require-dev`; configurable under `[phpcs]`.
136+
//! LSP diagnostics. Auto-detected via `vendor/bin/phpcs` or `$PATH`,
137+
//! independent of what `composer.json` declares; configurable under
138+
//! `[phpcs]`.
138139
//!
139140
//! PHPCS runs in its own **dedicated worker task**, following the
140141
//! same pattern as the PHPStan worker. At most one PHPCS process

src/formatting.rs

Lines changed: 99 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@
1212
//! `.phpantom.toml`, use that tool. If they set it to `""`, that
1313
//! tool is disabled.
1414
//! 2. **Composer `require-dev` wins over built-in.** If
15-
//! `composer.json` lists `laravel/pint`,
16-
//! `friendsofphp/php-cs-fixer`, or `squizlabs/php_codesniffer` in
17-
//! `require-dev`, resolve the binary via Composer's bin-dir and run
18-
//! it as a subprocess.
15+
//! `composer.json` lists `laravel/pint` or `friendsofphp/php-cs-fixer`
16+
//! in `require-dev`, resolve the binary via Composer's bin-dir and
17+
//! run it as a subprocess. `squizlabs/php_codesniffer` does the same,
18+
//! or, if the project pulls it in only transitively (e.g. through
19+
//! `slevomat/coding-standard`), a `phpcs.xml`/`.phpcs.xml` config file
20+
//! at the workspace root certifies phpcbf just as well.
1921
//! 3. **Otherwise, use mago-formatter.** No subprocess, no temp files,
2022
//! no external dependencies. Uses PER-CS 2.0 defaults or if present `mago.toml`.
2123
//!
@@ -93,8 +95,11 @@ pub(crate) enum FormattingStrategy {
9395
/// - If `config.is_disabled()` (both tools set to `""`) → `Disabled`.
9496
/// - If either tool has an explicit non-empty path in config →
9597
/// `External` with those tools.
96-
/// - If `composer_json` has `friendsofphp/php-cs-fixer` or
97-
/// `squizlabs/php_codesniffer` in `require-dev` → `External`,
98+
/// - If `composer_json` has `laravel/pint`, `friendsofphp/php-cs-fixer`,
99+
/// or `squizlabs/php_codesniffer` in `require-dev`, or the workspace
100+
/// root has a `phpcs.xml`/`.phpcs.xml`/`phpcs.xml.dist`/
101+
/// `.phpcs.xml.dist` config file (which certifies phpcbf even when
102+
/// PHP_CodeSniffer is pulled in only transitively) → `External`,
98103
/// resolving paths via the Composer bin-dir.
99104
/// - Otherwise → `BuiltIn`.
100105
pub(crate) fn resolve_strategy(
@@ -143,8 +148,11 @@ pub(crate) fn resolve_strategy(
143148
return FormattingStrategy::External(tools);
144149
}
145150

146-
// No explicit config — check composer.json require-dev.
147-
if let Some(package) = composer_json {
151+
// No explicit config — check composer.json require-dev, or a
152+
// hand-authored phpcs config file for phpcbf.
153+
let has_phpcs_config = workspace_root.is_some_and(crate::phpcs::has_project_config);
154+
155+
if composer_json.is_some() || has_phpcs_config {
148156
let mut tools = Vec::new();
149157
let bin = bin_dir.unwrap_or("vendor/bin");
150158

@@ -155,21 +163,32 @@ pub(crate) fn resolve_strategy(
155163
let pint_disabled = config.pint.as_deref() == Some("");
156164

157165
if !pint_disabled
158-
&& crate::composer::has_require_dev(package, "laravel/pint")
166+
&& composer_json
167+
.is_some_and(|package| crate::composer::has_require_dev(package, "laravel/pint"))
159168
&& let Some(tool) = resolve_from_bin_dir("pint", workspace_root, bin)
160169
{
161170
tools.push(tool);
162171
}
163172

164173
if !fixer_disabled
165-
&& crate::composer::has_require_dev(package, "friendsofphp/php-cs-fixer")
174+
&& composer_json.is_some_and(|package| {
175+
crate::composer::has_require_dev(package, "friendsofphp/php-cs-fixer")
176+
})
166177
&& let Some(tool) = resolve_from_bin_dir("php-cs-fixer", workspace_root, bin)
167178
{
168179
tools.push(tool);
169180
}
170181

182+
// A phpcs config file certifies phpcbf on its own: a project
183+
// that pulls squizlabs/php_codesniffer in only transitively
184+
// (e.g. through slevomat/coding-standard) never lists it in
185+
// require-dev directly, but a phpcs.xml is still deliberate
186+
// evidence the project uses it.
171187
if !phpcbf_disabled
172-
&& crate::composer::has_require_dev(package, "squizlabs/php_codesniffer")
188+
&& (has_phpcs_config
189+
|| composer_json.is_some_and(|package| {
190+
crate::composer::has_require_dev(package, "squizlabs/php_codesniffer")
191+
}))
173192
&& let Some(tool) = resolve_from_bin_dir("phpcbf", workspace_root, bin)
174193
{
175194
tools.push(tool);
@@ -877,6 +896,75 @@ mod tests {
877896
}
878897
}
879898

899+
#[test]
900+
fn strategy_phpcs_config_file_without_composer_dependency() {
901+
let dir = tempfile::tempdir().unwrap();
902+
let vendor_bin = dir.path().join("vendor/bin");
903+
std::fs::create_dir_all(&vendor_bin).unwrap();
904+
905+
let p = vendor_bin.join("phpcbf");
906+
std::fs::write(&p, "#!/bin/sh\n").unwrap();
907+
#[cfg(unix)]
908+
{
909+
use std::os::unix::fs::PermissionsExt;
910+
std::fs::set_permissions(&p, std::fs::Permissions::from_mode(0o755)).unwrap();
911+
}
912+
913+
// squizlabs/php_codesniffer is pulled in only transitively (e.g.
914+
// via slevomat/coding-standard), so it never appears in
915+
// require-dev directly. A hand-authored phpcs.xml certifies
916+
// phpcbf on its own, even with no composer.json at all.
917+
std::fs::write(dir.path().join("phpcs.xml"), "").unwrap();
918+
919+
let config = FormattingConfig::default();
920+
let strategy = resolve_strategy(Some(dir.path()), &config, None, None);
921+
match &strategy {
922+
FormattingStrategy::External(tools) => {
923+
assert_eq!(tools.len(), 1);
924+
assert_eq!(tools[0].name, "phpcbf");
925+
assert_eq!(tools[0].path, vendor_bin.join("phpcbf"));
926+
}
927+
other => panic!("Expected External, got {:?}", other),
928+
}
929+
}
930+
931+
#[test]
932+
fn strategy_phpcs_xml_dist_certifies_phpcbf_over_transitive_dependency() {
933+
let dir = tempfile::tempdir().unwrap();
934+
let vendor_bin = dir.path().join("vendor/bin");
935+
std::fs::create_dir_all(&vendor_bin).unwrap();
936+
937+
let p = vendor_bin.join("phpcbf");
938+
std::fs::write(&p, "#!/bin/sh\n").unwrap();
939+
#[cfg(unix)]
940+
{
941+
use std::os::unix::fs::PermissionsExt;
942+
std::fs::set_permissions(&p, std::fs::Permissions::from_mode(0o755)).unwrap();
943+
}
944+
945+
std::fs::write(dir.path().join("phpcs.xml.dist"), "").unwrap();
946+
947+
// Only a coding-standard package that pulls in squizlabs/php_codesniffer
948+
// transitively; the direct dependency check alone would miss this.
949+
let composer: crate::composer::ComposerPackage =
950+
serde_json::from_value(serde_json::json!({
951+
"require-dev": {
952+
"slevomat/coding-standard": "^8.0"
953+
}
954+
}))
955+
.unwrap();
956+
957+
let config = FormattingConfig::default();
958+
let strategy = resolve_strategy(Some(dir.path()), &config, Some(&composer), None);
959+
match &strategy {
960+
FormattingStrategy::External(tools) => {
961+
assert_eq!(tools.len(), 1);
962+
assert_eq!(tools[0].name, "phpcbf");
963+
}
964+
other => panic!("Expected External, got {:?}", other),
965+
}
966+
}
967+
880968
#[test]
881969
fn strategy_require_dev_both_tools() {
882970
let dir = tempfile::tempdir().unwrap();

0 commit comments

Comments
 (0)