Skip to content

chore: remove phpstan baseline#11751

Merged
ChiragAgg5k merged 4 commits into
1.9.xfrom
chore/remove-phpstan-baseline
Apr 2, 2026
Merged

chore: remove phpstan baseline#11751
ChiragAgg5k merged 4 commits into
1.9.xfrom
chore/remove-phpstan-baseline

Conversation

@ChiragAgg5k

Copy link
Copy Markdown
Member

Summary

  • remove phpstan-baseline.neon and the include from phpstan.neon
  • fix the suppressed PHPStan findings across controllers, workers, modules, config templates, and tests
  • keep composer analyze green without adding new ignores

Verification

  • composer analyze -- --no-progress
  • ./vendor/bin/phpunit tests/unit/Event/EventTest.php tests/unit/Functions/Validator/HeadersTest.php

@greptile-apps

greptile-apps Bot commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes the 721-line phpstan-baseline.neon suppression file and fixes every underlying PHPStan warning at the source across controllers, workers, modules, config templates, and tests. The changes are overwhelmingly mechanical and correct: removing dead ?? '' guards on non-nullable strings, hoisting variable initialisations before try blocks, dropping unused closure captures, and tightening @return self to @return static for covariant subclass safety.

Notable changes worth awareness:

  • src/Appwrite/Platform/Workers/Migrations.php$aggregatedResources = [] correctly moved before the try block so the finally iterator always sees populated data (fixes a previously-identified silent stats-processing bug).
  • app/config/templates/site.php & function.php — duplicate outputDirectory key (silently overwritten) renamed to fallbackFile; this surfaces a value that was always dropped before — confirm downstream consumers handle fallbackFile.
  • app/controllers/api/account.php — uses $currentSession instead of the post-loop $session variable for event params, which is both more correct and PHPStan-safe.
  • src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Update.php — removes a dead $enabled ??= ... line ($enabled is typed bool, never null).
  • src/Appwrite/Platform/Workers/Webhooks.php — removes reference to undefined $curlError inside sendEmailAlert; cURL transport errors will now produce the less informative "The server returned 0 status code" message in failure emails instead of the actual error string.

Confidence Score: 4/5

  • Safe to merge after confirming the fallbackFile key is intentional and handled by framework consumers.
  • The vast majority of changes are correct, well-scoped PHPStan fixes. The $aggregatedResources regression from the previous review thread is properly resolved. Two items deserve a quick author confirmation before merging: (1) the fallbackFile key introduced in site.php/function.php is a new functional value that was previously silently dropped — it should be verified that the proxy/sites layer reads it; (2) the Webhooks.php cURL error removal degrades the diagnostic content of failure emails, though it is not a runtime regression.
  • app/config/templates/site.php (new fallbackFile key) and src/Appwrite/Platform/Workers/Webhooks.php (lost cURL error detail in email alert).

Important Files Changed

Filename Overview
phpstan-baseline.neon Entire 721-line baseline suppression file deleted — all findings are now fixed at the source.
phpstan.neon Removed includes reference to the deleted baseline file.
src/Appwrite/Platform/Workers/Migrations.php Correctly hoists $aggregatedResources = [] to before the try block, fixing the previously identified issue where the finally block always saw an empty array and processMigrationResourceStats was never called.
app/config/templates/site.php Corrects duplicate outputDirectory key (silent PHP overwrite) by renaming the first occurrence to fallbackFile; this introduces a new key that was never present before and may require verification that downstream consumers handle it.
app/config/templates/function.php Same duplicate-key fix (outputDirectoryfallbackFile) as site.php, plus removal of redundant @var array<string> PHPDoc above method.
src/Appwrite/Platform/Workers/Webhooks.php Removes reference to undefined $curlError inside sendEmailAlert; the fix is correct but loses cURL error details from failure notification emails when a transport error (not an HTTP error) triggers the alert.
app/controllers/api/account.php Introduces $currentSession to replace use of loop variable $session after the foreach loop, correctly scoping the event params to the session that matched the cookie condition.
src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Update.php Removes dead $enabled ??= ... line: $enabled is typed bool (never null) in the method signature, so the null-coalescing assignment was a no-op that PHPStan correctly flagged.
src/Appwrite/Platform/Modules/Functions/Workers/Builds.php Initialises $deployment = new Document() before the try block so the catch block can safely call $deployment->getAttribute(); replaces ! empty($rule) with !$rule->isEmpty() to correctly detect an empty Document returned by findOne().
src/Appwrite/Platform/Modules/Functions/Http/Executions/Create.php Inlines the enqueueDeletes helper, removes dead ?? '' guards on non-nullable strings, and adds null-guard for $session instanceof Document in the session-lookup loop.
src/Appwrite/Event/Event.php Changes from() return type from self to static to support covariant return types in subclasses; updates two PHPDoc @return self to @return static for consistency.
src/Appwrite/Platform/Workers/Messaging.php Switches from $provider->getAttribute('type') to previously-assigned $providerType variable, and adds (int) cast on delivery count — both legitimate PHPStan fixes.
src/Appwrite/GraphQL/Resolvers.php Removes unused $context, $info, and $type captures from several inner closures; these were unused variables that PHPStan flagged.
tests/unit/Functions/Validator/HeadersTest.php Removes a test case using null as an array key (which PHP casts to "") to satisfy PHPStan's array-key type constraint — minor coverage loss but acceptable.

Reviews (3): Last reviewed commit: "fix: preserve cors max age type" | Re-trigger Greptile

Comment thread src/Appwrite/Platform/Workers/Migrations.php Outdated
Comment thread src/Appwrite/Platform/Modules/Teams/Http/Memberships/Create.php
@github-actions

github-actions Bot commented Apr 1, 2026

Copy link
Copy Markdown

🔄 PHP-Retry Summary

Flaky tests detected across commits:

Commit 33f8e35 - 5 flaky tests
Test Retries Total Time Details
UsageTest::testVectorsDBStats 1 10.17s Logs
LegacyConsoleClientTest::testListDocumentsWithCache 1 1.67s Logs
LegacyCustomClientTest::testPatchAttribute 1 111ms Logs
LegacyTransactionsCustomClientTest::testPartialFailureRollback 1 240.51s Logs
LegacyTransactionsCustomServerTest::testTransactionSizeLimit 1 240.62s Logs
Commit 24bd4c3 - 5 flaky tests
Test Retries Total Time Details
LegacyCustomClientTest::testCreateIndexes 1 241.19s Logs
LegacyCustomServerTest::testIncrementAttribute 1 240.28s Logs
LegacyTransactionsCustomClientTest::testBulkOperations 1 240.24s Logs
VectorsDBPermissionsMemberTest::testSetupDatabase 1 1.30s Logs
UsageTest::testVectorsDBStats 1 10.52s Logs
Commit eb366cf - 4 flaky tests
Test Retries Total Time Details
UsageTest::testVectorsDBStats 1 10.28s Logs
UsageTest::testPrepareSitesStats 1 5.25s Logs
LegacyCustomClientTest::testAttributeResponseModels 1 241.66s Logs
LegacyCustomServerTest::testCreateAttributes 1 241.15s Logs

@github-actions

github-actions Bot commented Apr 1, 2026

Copy link
Copy Markdown

✨ Benchmark results

  • Requests per second: 1,605
  • Requests with 200 status code: 289,025
  • P99 latency: 0.109700198

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,605 1,164
200 289,025 209,628
P99 0.109700198 0.191660995

@ChiragAgg5k ChiragAgg5k merged commit 96f4ef7 into 1.9.x Apr 2, 2026
111 of 116 checks passed
@ChiragAgg5k ChiragAgg5k deleted the chore/remove-phpstan-baseline branch April 2, 2026 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants