Replace static stub files with dynamic stub generation for Yii::$app type inference, adding support for custom application types.#83
Conversation
…` type inference, adding support for custom application types.
📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaces packaged static Yii stub files with runtime-generated PHPStan stubs in StubFilesExtension, caches generated stubs by content hash, removes three static stub files, updates PHPStan config to ignore tests/custom/, and adds tests/configs for custom application-type inference. Changes
Sequence Diagram(s)sequenceDiagram
participant PHPStan as PHPStan Runner
participant StubExt as StubFilesExtension
participant ServiceMap as ServiceMap
participant FS as Filesystem (tmp)
PHPStan->>StubExt: request getFiles()
StubExt->>ServiceMap: getApplicationType()
ServiceMap-->>StubExt: ApplicationType string
StubExt->>FS: compute md5(of generated stub content) -> target path
alt stub exists
FS-->>StubExt: return existing stub path
else stub missing
StubExt->>FS: write PID-suffixed temp file with content (atomic rename)
FS-->>StubExt: write success / file exists / error
alt write success
FS-->>StubExt: rename -> publish stub path
else name collision
FS-->>StubExt: return existing published path
else write error
StubExt-->>PHPStan: throw RuntimeException
end
end
StubExt-->>PHPStan: return [stub path]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #83 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 71 81 +10
===========================================
Files 2 2
Lines 126 156 +30
===========================================
+ Hits 126 156 +30 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/StubFilesExtension.php`:
- Around line 118-151: The current stub cache key is derived only from
$applicationType and can return a half-written file or stale content; instead
build the $content (using buildApplicationTypeDeclaration($escapedType)) first
and compute the cache key from the content (or include an explicit generator
version), then check for existence of the final stubPath; if missing, write to a
temporary file (e.g., $stubPath . '.tmp.' . uniqid()) and rename() it to
$stubPath to publish atomically; replace the direct file_put_contents($stubPath,
$content) call with this temp-write+rename flow and ensure you throw the same
RuntimeException if the rename/write fails.
In `@tests/custom/data/property/ApplicationPropertiesClassReflectionType.php`:
- Around line 23-40: Add an assertion that accesses the custom-only property
defined in ApplicationCustom (named $virtualProperty) to ensure PHPStan sees
PHPDoc-backed members on the custom app class; update the test suite by adding
an assertType('string' or the correct type, Yii::$app->virtualProperty) (e.g.,
in testReturnCustomApplicationInstanceFromYiiApp or as a new test method) so the
custom ApplicationCustom class's virtualProperty is verified rather than only
inherited yii\web\Application properties.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 49451754-e6a9-4c33-8694-a26c06a2afc0
📒 Files selected for processing (11)
CHANGELOG.mdphpstan.neonsrc/StubFilesExtension.phpstub/BaseYii.stubstub/BaseYiiConsole.stubstub/BaseYiiWeb.stubtests/StubFilesExtensionTest.phptests/config/phpstan-custom-app-config.phptests/custom/data/property/ApplicationPropertiesClassReflectionType.phptests/custom/extension-custom-test.neontests/custom/property/ApplicationPropertiesClassReflectionExtensionTest.php
💤 Files with no reviewable changes (3)
- stub/BaseYii.stub
- stub/BaseYiiWeb.stub
- stub/BaseYiiConsole.stub
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: phpunit-compatibility / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit-compatibility / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit-compatibility / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.5-windows-2022
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit-compatibility / PHP 8.1-windows-2022
- GitHub Check: phpunit-compatibility / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit-compatibility / PHP 8.4-windows-2022
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit-compatibility / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: phpunit-compatibility / PHP 8.3-windows-2022
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: terabytesoftw
Repo: yii2-extensions/phpstan PR: 40
File: src/ServiceMap.php:0-0
Timestamp: 2025-06-14T17:41:48.820Z
Learning: The yii2-extensions/phpstan repository contains a PHPStan extension specifically designed for static analysis of Yii framework applications.
📚 Learning: 2025-10-10T12:54:50.152Z
Learnt from: terabytesoftw
Repo: yii2-extensions/phpstan PR: 77
File: .github/linters/actionlint.yml:5-7
Timestamp: 2025-10-10T12:54:50.152Z
Learning: In the yii2-extensions/phpstan repository, the maintainer prefers to keep the actionlint.yml configuration with the generic ignore entry `"section is alias node but mapping node is expected"` as it works correctly for their workflows, and previous suggestions to change it have failed in other repositories.
Applied to files:
phpstan.neontests/custom/extension-custom-test.neon
📚 Learning: 2025-06-14T17:41:48.820Z
Learnt from: terabytesoftw
Repo: yii2-extensions/phpstan PR: 40
File: src/ServiceMap.php:0-0
Timestamp: 2025-06-14T17:41:48.820Z
Learning: The yii2-extensions/phpstan repository contains a PHPStan extension specifically designed for static analysis of Yii framework applications.
Applied to files:
tests/config/phpstan-custom-app-config.phptests/custom/extension-custom-test.neontests/custom/data/property/ApplicationPropertiesClassReflectionType.phptests/custom/property/ApplicationPropertiesClassReflectionExtensionTest.phpsrc/StubFilesExtension.phptests/StubFilesExtensionTest.php
📚 Learning: 2025-06-17T22:22:55.915Z
Learnt from: terabytesoftw
Repo: yii2-extensions/phpstan PR: 47
File: src/type/ActiveRecordGetAttributeDynamicMethodReturnTypeExtension.php:107-110
Timestamp: 2025-06-17T22:22:55.915Z
Learning: In PHPStan extensions for this repository, `instanceof` must not be used in PHPStan runtime mode; instead, use `::class` comparisons for reliable type checks (e.g., `$obj::class === SomeClass::class` instead of `$obj instanceof SomeClass`).
Applied to files:
tests/custom/property/ApplicationPropertiesClassReflectionExtensionTest.php
📚 Learning: 2025-06-12T16:41:20.240Z
Learnt from: terabytesoftw
Repo: yii2-extensions/phpstan PR: 36
File: src/type/ActiveQueryDynamicMethodReturnTypeExtension.php:153-157
Timestamp: 2025-06-12T16:41:20.240Z
Learning: In PHPStan extensions for this repository, `instanceof` must not be used to compare `PHPStan\Type\*` objects (e.g., `ThisType`) because it is not compatible with PHPStan’s runtime mode; instead, use `$type::class === SomeType::class` for reliable checks.
Applied to files:
tests/custom/property/ApplicationPropertiesClassReflectionExtensionTest.php
🪛 GitHub Actions: ecs
src/StubFilesExtension.php
[error] 1-1: ECS code style check failed for src/StubFilesExtension.php (diff indicates formatting changes; also shows \ No newline at end of file).
🪛 PHPMD (2.15.0)
tests/custom/property/ApplicationPropertiesClassReflectionExtensionTest.php
[warning] 19-43: Avoid excessively long class names like ApplicationPropertiesClassReflectionExtensionTest. Keep class name length under 40. (undefined)
(LongClassName)
src/StubFilesExtension.php
[error] 84-87: The method buildApplicationTypeDeclaration uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
[warning] 116-116: Avoid variables with short names like $ds. Configured minimum length is 3. (undefined)
(ShortVariable)
tests/StubFilesExtensionTest.php
[warning] 37-37: Avoid variables with short names like $ds. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 69-69: Avoid variables with short names like $ds. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 131-131: Avoid variables with short names like $ds. Configured minimum length is 3. (undefined)
(ShortVariable)
🔇 Additional comments (1)
tests/StubFilesExtensionTest.php (1)
21-33: Nice coverage expansion for generated stubs.The new assertions check existence, reuse, and emitted
@vartypes across default, web, console, and custom application configurations, which is a solid safety net for this refactor.Also applies to: 35-65, 67-98, 100-127, 129-158
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/StubFilesExtensionTest.php`:
- Around line 50-51: Replace the short local variable $ds that holds
DIRECTORY_SEPARATOR with a private class constant (e.g., add private const DS =
DIRECTORY_SEPARATOR; to the StubFilesExtensionTest class) and update all
occurrences where $ds is used (for example in the expressions that build
$configPath and other path variables) to use self::DS instead (e.g., __DIR__ .
self::DS . 'config' . self::DS . 'phpstan-base-app-config.php'), which will
silence PHPMD short-name warnings and keep path construction consistent across
methods.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 34f3a4f9-cf75-4125-a818-15f63e5ce78e
📒 Files selected for processing (5)
src/StubFilesExtension.phptests/StubFilesExtensionTest.phptests/config/phpstan-base-app-config.phptests/config/phpstan-global-class-app-config.phptests/custom/data/property/ApplicationPropertiesClassReflectionType.php
📜 Review details
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: terabytesoftw
Repo: yii2-extensions/phpstan PR: 40
File: src/ServiceMap.php:0-0
Timestamp: 2025-06-14T17:41:48.820Z
Learning: The yii2-extensions/phpstan repository contains a PHPStan extension specifically designed for static analysis of Yii framework applications.
📚 Learning: 2025-06-14T17:41:48.820Z
Learnt from: terabytesoftw
Repo: yii2-extensions/phpstan PR: 40
File: src/ServiceMap.php:0-0
Timestamp: 2025-06-14T17:41:48.820Z
Learning: The yii2-extensions/phpstan repository contains a PHPStan extension specifically designed for static analysis of Yii framework applications.
Applied to files:
tests/config/phpstan-base-app-config.phptests/config/phpstan-global-class-app-config.phptests/custom/data/property/ApplicationPropertiesClassReflectionType.phpsrc/StubFilesExtension.phptests/StubFilesExtensionTest.php
📚 Learning: 2025-10-10T12:54:50.152Z
Learnt from: terabytesoftw
Repo: yii2-extensions/phpstan PR: 77
File: .github/linters/actionlint.yml:5-7
Timestamp: 2025-10-10T12:54:50.152Z
Learning: In the yii2-extensions/phpstan repository, the maintainer prefers to keep the actionlint.yml configuration with the generic ignore entry `"section is alias node but mapping node is expected"` as it works correctly for their workflows, and previous suggestions to change it have failed in other repositories.
Applied to files:
tests/config/phpstan-base-app-config.php
📚 Learning: 2025-06-12T16:41:20.240Z
Learnt from: terabytesoftw
Repo: yii2-extensions/phpstan PR: 36
File: src/type/ActiveQueryDynamicMethodReturnTypeExtension.php:153-157
Timestamp: 2025-06-12T16:41:20.240Z
Learning: In PHPStan extensions for this repository, `instanceof` must not be used to compare `PHPStan\Type\*` objects (e.g., `ThisType`) because it is not compatible with PHPStan’s runtime mode; instead, use `$type::class === SomeType::class` for reliable checks.
Applied to files:
tests/config/phpstan-base-app-config.phptests/custom/data/property/ApplicationPropertiesClassReflectionType.php
🪛 PHPMD (2.15.0)
src/StubFilesExtension.php
[error] 94-97: The method buildApplicationTypeDeclaration uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
tests/StubFilesExtensionTest.php
[warning] 50-50: Avoid variables with short names like $ds. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 94-94: Avoid variables with short names like $ds. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 130-130: Avoid variables with short names like $ds. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 200-200: Avoid variables with short names like $ds. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 244-244: Avoid variables with short names like $ds. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 278-278: Avoid variables with short names like $ds. Configured minimum length is 3. (undefined)
(ShortVariable)
[warning] 305-305: Avoid variables with short names like $ds. Configured minimum length is 3. (undefined)
(ShortVariable)
🔇 Additional comments (10)
src/StubFilesExtension.php (3)
46-49: LGTM! Clean constructor with optional stub directory.The constructor properly accepts an optional
$stubDirectoryparameter with an empty string default, deferring tosys_get_temp_dir()ingenerateStub(). This design allows for both production use and testability (e.g., testing non-writable directories).
77-108: Namespace extraction logic is correct; else clause is acceptable here.The static analysis hint about the else clause is a style preference. In this context, the if/else structure clearly expresses the two distinct cases (global namespace vs. namespaced class) and is readable. The early return alternative would require duplicating the heredoc template.
The logic correctly handles:
- Base
\yii\base\Applicationreturns only the base declaration- Classes with namespaces extract namespace and class name properly
- Global namespace classes (no backslash) use
namespace { }syntax
160-198: Well-implemented atomic write pattern with content-based caching.The implementation correctly:
- Derives the cache key from
md5($content)to prevent stale files when generator logic changes- Uses PID-suffixed temp file to avoid collisions between concurrent processes
- Applies
LOCK_EXfor exclusive write lock- Uses atomic
rename()for POSIX systems- Handles Windows/non-POSIX edge cases where
rename()fails if target existsThe
@codeCoverageIgnoreStartblock appropriately marks the race-condition fallback path that's difficult to test deterministically.tests/config/phpstan-global-class-app-config.php (1)
1-9: LGTM! Valid test configuration for global namespace application type.This configuration correctly tests the edge case where an application class exists in the global namespace (no backslash prefix). The
StubFilesExtension::buildApplicationTypeDeclaration()handles this by emittingnamespace { class GlobalApplication extends \yii\base\Application {} }.tests/config/phpstan-base-app-config.php (1)
1-11: LGTM! Proper test configuration for base application type.Using
Application::classconstant ensures type safety and IDE support. This configuration tests the special case inbuildApplicationTypeDeclaration()where the application type equals\yii\base\Application, returning only the base declaration without a child class.tests/custom/data/property/ApplicationPropertiesClassReflectionType.php (1)
21-47: LGTM! Comprehensive type assertion coverage for custom application.The test methods properly validate:
Yii::$appresolves to the configuredApplicationCustomtype- Inherited properties (
charset,defaultRoute,language) resolve tostring- The custom
@property string $virtualPropertyfromApplicationCustomis correctly inferredThis ensures the dynamic stub generation correctly exposes both inherited and PHPDoc-defined properties on custom application classes. Based on learnings, this aligns with the PHPStan extension purpose for Yii framework static analysis.
tests/StubFilesExtensionTest.php (4)
32-44: LGTM! Good caching behavior test.This test correctly validates that consecutive calls to
getFiles()return the same cached stub path, verifying the content-basedmd5($content)caching mechanism works as expected.
50-88: Solid test for base application type with negative assertion.The test correctly validates both the positive case (
@var \yii\base\Applicationis present) and the negative case (no childclass Application extendsdeclaration). This ensures the special-case handling inbuildApplicationTypeDeclaration()works correctly for the base type.
276-287: Good error handling test with appropriate expectations.The test correctly validates that a
RuntimeExceptionis thrown when the stub directory is non-writable. Using an absolute non-existent path (/nonexistent/directory/path) ensures the test works cross-platform.
289-313: Proper test lifecycle management.The
tearDown()method correctly cleans up only tracked stubs, whilecleanGeneratedStubs()provides isolation between tests by removing all matching stubs before each test runs. Thefile_exists()guard intearDown()is good defensive coding against double-deletion attempts.
|
@coderabbitai approve? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me review the key changes before considering approval. 🏁 Script executed: Length of output: 16316
No issues found. I'm approving the PR. ✅ [approve] 🧠 Learnings used |
1 participant
Pull Request