refactor: Improve ServiceMap configuration for application types (Base, Console, Web)
#45
Conversation
WalkthroughThis update introduces application type awareness for PHPStan analysis in Yii2 projects. It removes specialized PHPStan reflection extensions for request and response classes, replacing them with a stub files extension that selects appropriate stubs based on the configured application type. The configuration and tests are updated to support and validate this new approach, including new stub files and test fixtures. Changes
Sequence Diagram(s)sequenceDiagram
participant PHPStan
participant ServiceMap
participant StubFilesExtension
PHPStan->>ServiceMap: Load config (with phpstan.application_type)
ServiceMap->>ServiceMap: Validate and store application type
PHPStan->>StubFilesExtension: getFiles()
StubFilesExtension->>ServiceMap: getApplicationType()
StubFilesExtension->>StubFilesExtension: Resolve stub file for application type
StubFilesExtension-->>PHPStan: Return stub file path(s)
PHPStan->>StubFiles: Load stub for analysis
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (16)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #45 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 62 71 +9
===========================================
Files 1 2 +1
Lines 107 129 +22
===========================================
+ Hits 107 129 +22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…concurrency groups for jobs.
…n into fix-mini-23
…d add new test fixtures for invalid configurations.
…ed application type configurations.
…d unit tests for web application type resolution.
…nd enhance type inference documentation.
…roperties in `PHPStan` analysis.
…iveRecord`, and `Container` for `PHPStan` analysis.
…ryDynamicMethodReturnType` test.
…n into fix-mini-23
…ups in static analysis workflow.
…sts for Yii Console Application properties.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the ServiceMap configuration to improve how the application type is determined while updating test configuration paths and error message formats. Key changes include replacing fixture paths with configuration paths in tests, updating doc comments and exception messages across the codebase, and introducing a new mechanism to set the application type via the configuration.
Reviewed Changes
Copilot reviewed 74 out of 74 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ServiceMapComponentTest.php | Updated configuration file paths and doc comments for component tests. |
| tests/ServiceMapBehaviorTest.php | Updated configuration file paths and revised doc comments for behavior tests. |
| stub/*.stub | New stub files added for different Yii Application types. |
| src/type/*.php | Updated doc comments clarifying dynamic return type extension usage. |
| src/property/ApplicationPropertiesClassReflectionExtension.php | Refactored normalization logic using a new constant for supported application classes. |
| src/ServiceMap.php | Introduces application type extraction from config, updates error messaging, and refactors. |
| phpstan/neon, phpstan-console.neon, extension.neon | Updated configuration paths and service registrations to align with the refactored mapping. |
| .github/workflows/static.yml | Adjusted workflow configuration for concurrency and PHP version. |
Comments suppressed due to low confidence (2)
src/reflection/RequestMethodsClassReflectionExtension.php:1
- This file has been completely removed. Please ensure that any configuration or references to this reflection extension are updated accordingly to avoid broken dependencies.
<?php
src/ServiceMap.php:397
- Consider clarifying in either documentation or inline comments that the default application type (Application::class) references yii\web\Application, if this behavior is intentional for PHPStan analysis.
$this->applicationType = $config['phpstan']['application_type'] ?? Application::class;
terabytesoftw
changed the title
ServiceMap configuration for application types.ServiceMap configuration for application types (Base, Console, Web)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (23)
src/reflection/ComponentPropertyReflection.php (1)
44-44: Refine constructor PHPDoc for precision.The current description “Reflection of the class being analyzed.” is generic and may confuse the declaring‐class role. Consider restoring specificity:
- @param ClassReflection $declaringClass Reflection of the class being analyzed. + @param ClassReflection $declaringClass Class reflection of the class declaring the dynamic property.src/type/ContainerDynamicMethodReturnTypeExtension.php (1)
32-32: Doc comment clarity: The PHPDoc now states "application class resolution," but this extension usesServiceMapto resolve container service IDs to component class names. Consider rewording to reflect service/component resolution.Example:
- * @see ServiceMap for application class resolution. + * @see ServiceMap for component/service class resolution.tests/data/property/BehaviorPropertiesClassReflectionType.php (1)
36-79: Nit: keep array notation consistent with PHPStan conventions
assertType('array<mixed>', …)passes, but PHPStan’s preferred generic form isarray.
(Not blocking, purely cosmetic.)- assertType('array<mixed>', $component->arrayProperty); + assertType('array', $component->arrayProperty);tests/console/config/config.php (1)
12-16: Consider usingyii\web\Viewinstead ofyii\base\View
yii\base\Viewis an abstract base; console and web apps typically registeryii\web\View. Swapping to the concrete class avoids accidental instantiation errors if this config is reused outside tests.-use yii\base\View; +use yii\web\View;tests/type/ActiveRecordDynamicMethodReturnTypeExtensionTest.php (2)
40-42: Windows-compatibility: avoid hard-coding forward slashes in fixture path
gatherAssertTypes()will happily read the file, but the literal forward-slash will break the build on Windows runners.
UsingDIRECTORY_SEPARATOR(orsprintf('%s/data/type/…', $directory)) keeps the test platform-agnostic.- yield from self::gatherAssertTypes( - "{$directory}/data/type/ActiveRecordDynamicMethodReturnType.php", - ); + yield from self::gatherAssertTypes( + $directory . DIRECTORY_SEPARATOR . 'data' . DIRECTORY_SEPARATOR . 'type' . + DIRECTORY_SEPARATOR . 'ActiveRecordDynamicMethodReturnType.php', + );
45-48: Nit: dirname(DIR) repetitionThe same expression appears in every test class; consider extracting a private static helper (or a PHPUnit base class) to centralise the path construction and cut duplication.
tests/ServiceMapBehaviorTest.php (2)
23-24: Grammar nitpick in docblock“Ensures compatibility with based configuration files.” reads awkwardly after dropping “fixture-”. Suggest:
- * - Ensures compatibility with based configuration files. + * - Ensures compatibility with the provided configuration files.
39-42: Path construction duplicated across testsEvery test method builds
$configPaththe same way. Extracting a private helper will:
- Reduce 17+ identical concatenations,
- Prevent future typos,
- Make the path change-friendly.
Example:
private static function config(string $file): string { return __DIR__ . DIRECTORY_SEPARATOR . 'config' . DIRECTORY_SEPARATOR . $file; }Then:
-$configPath = __DIR__ . "{$ds}config{$ds}config.php"; +$configPath = self::config('config.php');tests/ServiceMapServiceTest.php (2)
27-28: Docblock wordingSame grammar tweak as in the behaviour test.
- * - Ensures compatibility with based configuration files. + * - Ensures compatibility with the provided configuration files.
60-62: Consider helper for repeated config-file pathsThe concatenation pattern is copy-pasted in ~30 places in this file. A small helper (see previous comment) would improve maintainability.
stub/BaseYiiWeb.stub (1)
11-18: Consider using a typed static property for better analysisSince PHP 7.4 allows typed static properties and this file exists solely for static analysis, declaring the type directly helps PHPStan without relying on the docblock.
- /** - * @var \yii\web\Application - */ - public static $app; + public static \yii\web\Application $app;Optional, but increases precision and removes the need for the phpDoc.
tests/ServiceMapComponentTest.php (1)
24-24: Doc-block grammar typoThe phrase “Ensures compatibility with based configuration files.” is missing a word – it should read “with configuration files” (or similar).
tests/console/property/ApplicationPropertiesClassReflectionExtensionTest.php (2)
38-40: PreferDIRECTORY_SEPARATOR-independent concatenationInterpolating a Unix separator directly (
"{$directory}/data/…") will break on Windows if the tests are ever executed there.
UseDIRECTORY_SEPARATORor, even simpler,sprintf('%s%s…', $directory, DIRECTORY_SEPARATOR)/$directory . '/data/…'because PHP resolves forward-slashes fine on Windows.- "{$directory}/data/property/ApplicationPropertiesClassReflectionType.php", + $directory . DIRECTORY_SEPARATOR . 'data' . DIRECTORY_SEPARATOR . 'property' . DIRECTORY_SEPARATOR + . 'ApplicationPropertiesClassReflectionType.php',
45-45: Hard-coded filename duplicates path logic
dirname(__DIR__) . '/extension-console-test.neon'repeats the forward-slash pattern called out above; consider refactoring to a small helper or reuse$directoryfor consistency & cross-platform safety.No functional bug, only maintainability.
tests/ServiceMapTest.php (3)
23-28: Minor wording typo in docblock“Ensures compatibility with based configuration files…” → “base configuration files…”.
Pure documentation; no runtime impact.
40-63: Path building repetitionEach test assembles paths manually with
$ds. This is verbose and easy to mistype. Consider extracting aprivate function fixture(string $relative): stringhelper to DRY these lines.Example:
- $ds = DIRECTORY_SEPARATOR; - $configPath = __DIR__ . "{$ds}console{$ds}config{$ds}config.php"; + $configPath = $this->fixture('console/config/config.php');
69-94: Exception messages are brittleHard-coding the full path in the expected message (
$configPathinterpolation) means the test will fail if it is executed from a different directory (e.g. vendor-dir inside another project).
Match only the stable part of the message or useexpectExceptionMessageMatches()with a regex.- $this->expectExceptionMessage("Configuration file '{$configPath}' must contain a valid 'phpstan' 'array'."); + $this->expectExceptionMessageMatches('/must contain a valid .*phpstan.*array/');src/StubFilesExtension.php (1)
81-86: Nit: Path assembly can be simplified
dirname(__DIR__) . '/stub'is cross-platform safe; the extra{$ds}indirection is unnecessary.Not a blocker.
tests/web/property/ApplicationPropertiesClassReflectionExtensionTest.php (1)
26-27: Update copyright year to reflect current year.The copyright notice shows 2023, but this is a new file being added in 2025.
Apply this diff to update the copyright year:
- * @copyright Copyright (C) 2023 Terabytesoftw. + * @copyright Copyright (C) 2025 Terabytesoftw.tests/console/data/property/ApplicationPropertiesClassReflectionType.php (1)
35-36: Update copyright year to reflect current year.The copyright notice shows 2023, but this is a new file being added in 2025.
Apply this diff to update the copyright year:
- * @copyright Copyright (C) 2023 Terabytesoftw. + * @copyright Copyright (C) 2025 Terabytesoftw.src/property/ApplicationPropertiesClassReflectionExtension.php (1)
44-46: Fix incorrect PHPDoc @see syntax.The
::classnotation is not valid in PHPDoc @see tags.Apply this diff to fix the PHPDoc syntax:
- * @see \yii\base\Application::class for Yii Base Application class. - * @see \yii\console\Application::class for Yii Console Application class. - * @see \yii\web\Application::class for Yii Web Application class. + * @see \yii\base\Application for Yii Base Application class. + * @see \yii\console\Application for Yii Console Application class. + * @see \yii\web\Application for Yii Web Application class.src/ServiceMap.php (1)
110-112: Consider adding @throws tag for potential exceptions.The constructor can throw
InvalidArgumentExceptionfrom the newprocessApplicationTypemethod if validation is added.Add the missing @throws tag to the constructor docblock after the suggested validation is implemented:
* @throws InvalidArgumentException if one or more arguments are invalid, of incorrect type or format. + * @throws InvalidArgumentException if the configured application type is not a valid Yii Application class. * @throws ReflectionException if the service definitions can't be resolved or are invalid.tests/web/data/property/ApplicationPropertiesClassReflectionType.php (1)
38-41: Docblock polish: replace stray period with a colonSlight wording tweak for consistency/readability.
- * Key features. + * Key features:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (55)
.github/workflows/static.yml(1 hunks)composer-require-checker.json(1 hunks)extension.neon(1 hunks)phpstan-console.neon(1 hunks)phpstan.neon(2 hunks)src/ServiceMap.php(22 hunks)src/StubFilesExtension.php(1 hunks)src/property/ApplicationPropertiesClassReflectionExtension.php(4 hunks)src/property/BehaviorPropertiesClassReflectionExtension.php(2 hunks)src/reflection/ComponentPropertyReflection.php(1 hunks)src/reflection/RequestMethodsClassReflectionExtension.php(0 hunks)src/reflection/RequestPropertiesClassReflectionExtension.php(0 hunks)src/reflection/ResponsePropertiesClassReflectionExtension.php(0 hunks)src/type/ActiveQueryDynamicMethodReturnTypeExtension.php(1 hunks)src/type/ContainerDynamicMethodReturnTypeExtension.php(1 hunks)stub/BaseYii.stub(1 hunks)stub/BaseYiiConsole.stub(1 hunks)stub/BaseYiiWeb.stub(1 hunks)tests/ServiceMapBehaviorTest.php(8 hunks)tests/ServiceMapComponentTest.php(10 hunks)tests/ServiceMapServiceTest.php(24 hunks)tests/ServiceMapTest.php(1 hunks)tests/StubFilesExtensionTest.php(1 hunks)tests/bootstrap.php(0 hunks)tests/config/config.php(2 hunks)tests/config/phpstan-unsupported-is-not-array.php(1 hunks)tests/config/phpstan-unsupported-type-array-invalid.php(1 hunks)tests/console/config/config.php(1 hunks)tests/console/data/property/ApplicationPropertiesClassReflectionType.php(1 hunks)tests/console/extension-console-test.neon(1 hunks)tests/console/property/ApplicationPropertiesClassReflectionExtensionTest.php(2 hunks)tests/data/method/BehaviorMethodsClassReflectionType.php(2 hunks)tests/data/property/BehaviorPropertiesClassReflectionType.php(2 hunks)tests/data/property/UserPropertiesClassReflectionType.php(2 hunks)tests/data/type/ActiveQueryDynamicMethodReturnType.php(7 hunks)tests/data/type/ActiveRecordDynamicMethodReturnType.php(2 hunks)tests/data/type/ActiveRecordDynamicStaticMethodReturnType.php(3 hunks)tests/data/type/ContainerDynamicMethodReturnType.php(13 hunks)tests/extension-test.neon(1 hunks)tests/extension-tests.neon(0 hunks)tests/fixture/data/property/ApplicationConsolePropertiesClassReflectionType.php(0 hunks)tests/fixture/data/property/ApplicationCustomPropertiesClassReflectionType.php(0 hunks)tests/fixture/data/property/ApplicationWebPropertiesClassReflectionType.php(0 hunks)tests/method/BehaviorMethodsClassReflectionExtensionTest.php(1 hunks)tests/property/BehaviorPropertiesClassReflectionExtensionTest.php(2 hunks)tests/property/UserPropertiesClassReflectionExtensionTest.php(1 hunks)tests/stub/MyActiveRecord.php(0 hunks)tests/type/ActiveQueryDynamicReturnTypeExtensionTest.php(1 hunks)tests/type/ActiveRecordDynamicMethodReturnTypeExtensionTest.php(1 hunks)tests/type/ActiveRecordDynamicStaticMethodReturnTypeExtensionTest.php(1 hunks)tests/type/ContainerDynamicMethodReturnTypeExtensionTest.php(1 hunks)tests/web/config/config.php(1 hunks)tests/web/data/property/ApplicationPropertiesClassReflectionType.php(1 hunks)tests/web/extension-test.neon(1 hunks)tests/web/property/ApplicationPropertiesClassReflectionExtensionTest.php(1 hunks)
💤 Files with no reviewable changes (9)
- tests/bootstrap.php
- tests/extension-tests.neon
- tests/stub/MyActiveRecord.php
- tests/fixture/data/property/ApplicationCustomPropertiesClassReflectionType.php
- src/reflection/RequestPropertiesClassReflectionExtension.php
- src/reflection/ResponsePropertiesClassReflectionExtension.php
- src/reflection/RequestMethodsClassReflectionExtension.php
- tests/fixture/data/property/ApplicationWebPropertiesClassReflectionType.php
- tests/fixture/data/property/ApplicationConsolePropertiesClassReflectionType.php
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/phpstan#40
File: src/ServiceMap.php:0-0
Timestamp: 2025-06-14T17:41:48.780Z
Learning: The yii2-extensions/phpstan repository contains a PHPStan extension specifically designed for static analysis of Yii framework applications.
phpstan.neon (1)
Learnt from: terabytesoftw
PR: yii2-extensions/phpstan#40
File: src/ServiceMap.php:0-0
Timestamp: 2025-06-14T17:41:48.780Z
Learning: The yii2-extensions/phpstan repository contains a PHPStan extension specifically designed for static analysis of Yii framework applications.
extension.neon (1)
Learnt from: terabytesoftw
PR: yii2-extensions/phpstan#40
File: src/ServiceMap.php:0-0
Timestamp: 2025-06-14T17:41:48.780Z
Learning: The yii2-extensions/phpstan repository contains a PHPStan extension specifically designed for static analysis of Yii framework applications.
src/StubFilesExtension.php (1)
Learnt from: terabytesoftw
PR: yii2-extensions/phpstan#40
File: src/ServiceMap.php:0-0
Timestamp: 2025-06-14T17:41:48.780Z
Learning: The yii2-extensions/phpstan repository contains a PHPStan extension specifically designed for static analysis of Yii framework applications.
🧬 Code Graph Analysis (19)
tests/type/ActiveRecordDynamicStaticMethodReturnTypeExtensionTest.php (3)
tests/type/ActiveQueryDynamicReturnTypeExtensionTest.php (1)
getAdditionalConfigFiles(45-48)tests/type/ActiveRecordDynamicMethodReturnTypeExtensionTest.php (1)
getAdditionalConfigFiles(45-48)tests/type/ContainerDynamicMethodReturnTypeExtensionTest.php (1)
getAdditionalConfigFiles(45-48)
tests/data/property/BehaviorPropertiesClassReflectionType.php (1)
tests/stub/MyComponent.php (1)
MyComponent(22-28)
tests/ServiceMapBehaviorTest.php (1)
src/ServiceMap.php (1)
ServiceMap(64-608)
tests/type/ActiveRecordDynamicMethodReturnTypeExtensionTest.php (3)
tests/type/ActiveQueryDynamicReturnTypeExtensionTest.php (1)
getAdditionalConfigFiles(45-48)tests/type/ActiveRecordDynamicStaticMethodReturnTypeExtensionTest.php (1)
getAdditionalConfigFiles(45-48)tests/type/ContainerDynamicMethodReturnTypeExtensionTest.php (1)
getAdditionalConfigFiles(45-48)
tests/data/type/ActiveQueryDynamicMethodReturnType.php (3)
tests/stub/MyActiveRecord.php (1)
MyActiveRecord(23-48)tests/stub/Post.php (1)
find(18-21)tests/stub/PostQuery.php (1)
published(18-21)
tests/data/type/ActiveRecordDynamicStaticMethodReturnType.php (3)
tests/stub/Category.php (1)
Category(31-46)tests/stub/MyActiveRecord.php (1)
MyActiveRecord(23-48)tests/stub/User.php (1)
User(26-66)
tests/property/BehaviorPropertiesClassReflectionExtensionTest.php (2)
tests/method/BehaviorMethodsClassReflectionExtensionTest.php (1)
getAdditionalConfigFiles(43-46)tests/property/UserPropertiesClassReflectionExtensionTest.php (1)
getAdditionalConfigFiles(43-46)
tests/web/config/config.php (5)
tests/stub/BehaviorOne.php (1)
BehaviorOne(22-56)tests/stub/BehaviorTwo.php (1)
BehaviorTwo(22-40)tests/stub/MyActiveRecord.php (1)
MyActiveRecord(23-48)tests/stub/MyComponent.php (1)
MyComponent(22-28)tests/stub/User.php (1)
User(26-66)
tests/data/type/ActiveRecordDynamicMethodReturnType.php (3)
tests/stub/Category.php (1)
Category(31-46)tests/stub/MyActiveRecord.php (1)
MyActiveRecord(23-48)tests/stub/User.php (1)
User(26-66)
tests/type/ActiveQueryDynamicReturnTypeExtensionTest.php (3)
tests/type/ActiveRecordDynamicMethodReturnTypeExtensionTest.php (1)
getAdditionalConfigFiles(45-48)tests/type/ActiveRecordDynamicStaticMethodReturnTypeExtensionTest.php (1)
getAdditionalConfigFiles(45-48)tests/type/ContainerDynamicMethodReturnTypeExtensionTest.php (1)
getAdditionalConfigFiles(45-48)
tests/property/UserPropertiesClassReflectionExtensionTest.php (1)
tests/property/BehaviorPropertiesClassReflectionExtensionTest.php (1)
getAdditionalConfigFiles(43-46)
tests/console/property/ApplicationPropertiesClassReflectionExtensionTest.php (1)
tests/web/property/ApplicationPropertiesClassReflectionExtensionTest.php (2)
DataProvider(48-52)getAdditionalConfigFiles(43-46)
tests/data/type/ContainerDynamicMethodReturnType.php (1)
tests/stub/MyActiveRecord.php (1)
MyActiveRecord(23-48)
tests/method/BehaviorMethodsClassReflectionExtensionTest.php (1)
tests/property/BehaviorPropertiesClassReflectionExtensionTest.php (1)
getAdditionalConfigFiles(43-46)
tests/ServiceMapTest.php (1)
src/ServiceMap.php (2)
ServiceMap(64-608)getApplicationType(150-153)
tests/console/data/property/ApplicationPropertiesClassReflectionType.php (1)
tests/web/data/property/ApplicationPropertiesClassReflectionType.php (21)
ApplicationPropertiesClassReflectionType(47-163)testReturnApplicationInstanceFromYiiApp(49-52)testReturnAssetManagerFromComponent(54-57)testReturnCacheFromComponent(59-62)testReturnStringFromCharsetProperty(64-67)testReturnControllerFromProperty(69-72)testReturnDbFromComponent(74-77)testReturnStringFromDefaultRouteProperty(79-82)testReturnErrorHandlerFromComponent(84-87)testReturnFormatterFromComponent(89-92)testReturnI18nFromComponent(99-102)testReturnStringFromLanguageProperty(104-107)testReturnLogFromComponent(109-112)testReturnMailerFromComponent(114-117)testReturnRequestFromComponent(119-122)testReturnResponseFromComponent(124-127)testReturnStringFromSourceLanguageProperty(134-137)testReturnStringFromTimeZoneProperty(139-142)testReturnStringFromUniqueIdProperty(144-147)testReturnUrlManagerFromComponent(149-152)testReturnViewFromComponent(159-162)
tests/type/ContainerDynamicMethodReturnTypeExtensionTest.php (3)
tests/type/ActiveQueryDynamicReturnTypeExtensionTest.php (1)
getAdditionalConfigFiles(45-48)tests/type/ActiveRecordDynamicMethodReturnTypeExtensionTest.php (1)
getAdditionalConfigFiles(45-48)tests/type/ActiveRecordDynamicStaticMethodReturnTypeExtensionTest.php (1)
getAdditionalConfigFiles(45-48)
src/StubFilesExtension.php (2)
src/type/ContainerDynamicMethodReturnTypeExtension.php (1)
__construct(45-48)src/ServiceMap.php (3)
__construct(118-138)ServiceMap(64-608)getApplicationType(150-153)
tests/web/property/ApplicationPropertiesClassReflectionExtensionTest.php (1)
tests/console/property/ApplicationPropertiesClassReflectionExtensionTest.php (4)
DataProvider(48-52)ApplicationPropertiesClassReflectionExtensionTest(29-53)dataFileAsserts(34-41)getAdditionalConfigFiles(43-46)
🔇 Additional comments (46)
src/type/ActiveQueryDynamicMethodReturnTypeExtension.php (1)
258-258: Approve PHPDoc refinement for clarity.The updated description “Reflection of the class being analyzed.” accurately conveys the role of
$classReflectionin this context and aligns with similar doc improvements elsewhere.tests/data/property/UserPropertiesClassReflectionType.php (3)
5-5: Verify test namespace mapping.You’ve updated the namespace to
yii2\extensions\phpstan\tests\data\property; ensure yourcomposer.json(autoload-dev PSR-4) reflects this path change so PHPUnit and PHPStan can locate the fixtures.
12-12: Approve docblock wording enhancement.The refined class‐level description is clearer and aligns with overall test fixture restructuring.
22-22: Approve list item clarification.The improved phrasing of the second bullet enhances readability without altering intent.
composer-require-checker.json (1)
5-5: Approve symbol-whitelist update.Adding
"PHPStan\\PhpDoc\\StubFilesExtension"matches your new stub extension implementation and keeps the dependency-checker in sync.phpstan.neon (2)
18-18: Verify updated analysis paths.You’ve narrowed
pathstotests/web; confirm thattests/webexists and contains all intended web tests, and thatphpstan-console.neoncovers console fixtures.
31-31: Verifyconfig_pathalignment.Ensure
tests/web/config/config.phpis present (and updated from the oldfixturedirectory), so PHPStan can bootstrap the correct Yii application context.tests/config/config.php (2)
5-5: Approve addition ofViewimport. The import ofyii\web\Viewis required for the newviewcomponent configuration.
30-32: Approveviewcomponent configuration. Including theviewcomponent withView::classaligns the web application test setup with the new application type-aware PHPStan enhancements.tests/console/extension-console-test.neon (1)
1-7: Approve new console PHPStan test configuration. This NEON file correctly includesextension.neonand setsyii2.config_pathto the console config, mirroring the web setup for application type separation.tests/extension-test.neon (1)
1-7: Approve new web PHPStan extension test configuration. Includingextension.neonand pointingyii2.config_pathto the web config file aligns with the application type-specific testing strategy.tests/config/phpstan-unsupported-is-not-array.php (1)
1-8: Approve edge-case PHPStan config test. Returning'phpstan' => 1intentionally validates how the system handles non-array config values, bolstering robustness.tests/data/method/BehaviorMethodsClassReflectionType.php (1)
5-6: Namespace update looks correctThe new namespace matches the relocated fixtures directory and keeps PSR-4 consistency. No issues spotted.
tests/config/phpstan-unsupported-type-array-invalid.php (1)
5-9: Good negative-case fixtureExplicitly supplying an invalid non-string
application_typewill help ensureServiceMapthrows as expected. No changes required.tests/data/property/BehaviorPropertiesClassReflectionType.php (1)
5-6: Namespace realignment confirmedDirectory and namespace now align; this prevents autoloading issues in the test suite.
tests/web/extension-test.neon (2)
1-2: Set up base PHPStan extension config.
Including../../extension.neoncorrectly pulls in the main extension configuration for web tests.
4-6: Define web application config path.
yii2.config_pathpoints to the web application fixture at%rootDir%/../../../tests/web/config/config.php. Confirm this matches the test fixture layout.tests/type/ActiveQueryDynamicReturnTypeExtensionTest.php (2)
40-42: Update fixture path indataFileAsserts.
The intermediatefixturedirectory has been removed; the path now correctly points todata/type/ActiveQueryDynamicMethodReturnType.php.
47-48: Correct configuration filename.
Usingextension-test.neonensures consistency with the new naming convention across the test suite.tests/type/ActiveRecordDynamicStaticMethodReturnTypeExtensionTest.php (2)
40-42: Update fixture path indataFileAsserts.
The path now points todata/type/ActiveRecordDynamicStaticMethodReturnType.php, reflecting the removal of thefixturefolder.
47-48: Align config file reference.
Switched toextension-test.neonto match the unified test configuration naming.tests/type/ContainerDynamicMethodReturnTypeExtensionTest.php (2)
40-42: Update fixture path indataFileAsserts.
The path now correctly targetsdata/type/ContainerDynamicMethodReturnType.phpafter removing thefixturedirectory.
47-48: Unify config file naming.
Returningextension-test.neonaligns this class with the other tests’ configuration references.tests/property/UserPropertiesClassReflectionExtensionTest.php (2)
38-40: Standardize fixture path.
Now correctly pointing todata/property/UserPropertiesClassReflectionType.php, reflecting the simplified directory structure.
45-46: Correct configuration file reference.
Usingextension-test.neonhere matches the convention used across all test suites.src/property/BehaviorPropertiesClassReflectionExtension.php (2)
100-101: Docblock wording only – no action requiredParameter description was unified; implementation untouched. ✅
126-127: Docblock consistency looks goodThe same clarification was applied to
findPropertyInBehaviors(). No further comments.tests/ServiceMapBehaviorTest.php (1)
134-136: Updated exception message matches source implementation – LGTMMessage now mirrors
ServiceMap::throwErrorWhenIsNotString(). ✅tests/ServiceMapServiceTest.php (1)
216-218: Dynamic path in expected messageEmbedding
$configPathdirectly inexpectExceptionMessage()is great; it avoids brittle hard-coding. 👍tests/data/type/ActiveRecordDynamicStaticMethodReturnType.php (2)
5-5: Namespace update matches new directory structureThe change aligns the namespace with the moved file. No issues spotted.
38-145: Refactored assertions are clearerInlining the expressions into
assertType()removes unnecessary variables and keeps the tests succinct. Tests still exercise the same scenarios – good refactor.tests/property/BehaviorPropertiesClassReflectionExtensionTest.php (1)
38-40: Confirm updated fixture & config paths work at runtimeThe
fixture/segment was dropped and the config file renamed toextension-test.neon.
Both look consistent with the other tests, but please double-check that:
tests/data/property/BehaviorPropertiesClassReflectionType.phpreally exists, and- the new
extension-test.neonis picked up byphpstan analyse(it replaces the old pluralised file).A quick grep/script will prevent a silent “file not found” during CI.
#!/bin/bash fd BehaviorPropertiesClassReflectionType.php tests/data | head fd extension-test.neon tests | headAlso applies to: 45-45
tests/data/type/ContainerDynamicMethodReturnType.php (2)
5-5: Namespace realignment LGTMThe namespace switch to
tests\data\typematches the new folder layout – nothing else to flag.
88-92: Minor style change only – fineInlining the
get()call and using the trailing-comma style is purely cosmetic and does not affect PHPStan inference.tests/data/type/ActiveRecordDynamicMethodReturnType.php (1)
40-44: Assertions updated – no functional changeAll modified
assertType()calls simply collapse temporary variables and adopt trailing-comma formatting.
Type strings remain identical; the refactor is safe.Also applies to: 50-54, 60-64, 70-74, 80-84, 90-94, 100-104, 110-114, 120-124, 130-136, 143-147, 153-156
tests/ServiceMapComponentTest.php (1)
41-43: ```shell
#!/usr/bin/env bash
echo "Occurrences of default config path in tests/ServiceMapComponentTest.php:"
rg -n '{$ds}config{$ds}config.php' tests/ServiceMapComponentTest.php || true
echo
echo "Total count:"
rg -c '{$ds}config{$ds}config.php' tests/ServiceMapComponentTest.php</details> <details> <summary>tests/data/type/ActiveQueryDynamicMethodReturnType.php (2)</summary> `5-8`: **Namespace/import update looks correct** The namespace realignment and addition of `yii\db\Exception` keep the file consistent with the new directory layout. --- `43-46`: **Updated `assertType()` expectations align with new generics** All revised `assertType()` invocations correctly express the expected union/generic types after the refactor. No issues spotted. Also applies to: 51-51, 63-64, 82-84, 111-113, 164-168, 174-181, 208-213, 217-221 </details> <details> <summary>stub/BaseYiiConsole.stub (1)</summary> `1-23`: **Minimal console-app stub is sound** The stub mirrors Yii’s real class hierarchy and gives PHPStan the `$app` property typed to `\yii\console\Application`. LGTM. </details> <details> <summary>tests/web/config/config.php (1)</summary> `5-66`: **Comprehensive test config – no blocking issues** Configuration is syntactically valid, uses strict types, and covers a rich set of scenarios (behaviors, components, DI container). 👍 </details> <details> <summary>tests/method/BehaviorMethodsClassReflectionExtensionTest.php (2)</summary> `38-40`: **Path correction matches new fixture layout** Switching to `data/method/...` is consistent with the flattened test structure. --- `45-46`: **Config filename update aligns with other tests** `extension-test.neon` path mirrors the change made across the suite. All good. </details> <details> <summary>extension.neon (1)</summary> `47-50`: **Service registration LGTM** `StubFilesExtension` is correctly tagged with `phpstan.stubFilesExtension`; PHPStan will autowire the `ServiceMap` dependency. No changes required. </details> <details> <summary>phpstan-console.neon (1)</summary> `1-32`: **Configuration baseline looks solid** The console-specific config cleanly isolates console tests, enables strict rules, and wires the bootstrap file. Nothing blocking here. </details> <details> <summary>tests/console/data/property/ApplicationPropertiesClassReflectionType.php (1)</summary> `45-48`: **Verify the necessity of AssetManager in console application context.** `AssetManager` is typically a web-specific component used for managing CSS and JavaScript assets. Its presence in a console application test is unusual and might indicate a configuration issue or unnecessary component. ```shell #!/bin/bash # Description: Check if AssetManager is commonly used in console applications within the codebase # Search for AssetManager usage in console-related files echo "=== Searching for AssetManager in console contexts ===" rg -A 5 "AssetManager" --glob "*console*" --glob "!*test*" # Check if AssetManager is configured in console config files echo -e "\n=== Checking console configuration files ===" fd -e php -e neon "console.*config" --exec grep -l "assetManager" {} \; # Look for the console application configuration used in tests echo -e "\n=== Checking test console config ===" cat tests/console/config/config.php 2>/dev/null || echo "Console config file not found"src/property/ApplicationPropertiesClassReflectionExtension.php (1)
110-120: Verify the impact of reordering property resolution checks.The order of checks has been changed to prioritize service map component lookup before annotation properties. This could affect behavior if a property exists in both the service map and annotations.
#!/bin/bash # Description: Check for potential conflicts between service map components and annotation properties # Search for properties that might be defined in both annotations and service map echo "=== Searching for @property annotations in Application classes ===" ast-grep --pattern '@property $_ $$PROP_NAME' --lang php | grep -E "(Application|BaseApplication)" -A 2 # Check if any common property names exist in both component definitions and annotations echo -e "\n=== Common Yii component names that might conflict ===" rg -o "'\w+' =>" extension.neon | cut -d"'" -f2 | sort | uniq | while read comp; do echo "Checking for @property.*$comp in Application classes:" rg "@property.*\$$comp" --glob "*Application*.php" done
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores