Remove usage of static vars or properties#59002

Open

come-nc wants to merge 23 commits into

fix/remove-static-vars

Contributor

come-nc commented Mar 17, 2026 •

edited

Loading

Summary

Static properties, and even worse static vars in methods are hard to reset from outside, and become a major problem when wanting to make use of the worker mode of FrankenPHP or any similar project.

So this PR removes such declarations and adds a psalm custom plugin to forbid their use.

Rebase after merge of Remove static vars, first batch #59159

Checklist

Code is properly formatted
Sign-off message is added to all commits
Tests (unit, integration, api and/or acceptance) are included
Screenshots before/after for front-end changes
Documentation (manuals or wiki) has been updated or is not required
Backports requested where applicable (ex: critical bugfixes)
Labels added where applicable (ex: bug/enhancement, 3. to review, feature component)
Milestone added for target branch/version (ex: 32.x for stable32)

AI (if applicable)

The content of this PR was partly or fully generated using AI

come-nc added this to the Nextcloud 34 milestone

come-nc self-assigned this

come-nc added the 2. developing label

This comment was marked as resolved.

Sign in to view

come-nc commented

View reviewed changes

lib/private/Memcache/Redis.php Outdated

come-nc commented

View reviewed changes

apps/files_external/lib/Service/BackendService.php Outdated

come-nc mentioned this pull request

Remove static vars, first batch #59159

Merged

7 tasks

come-nc force-pushed the fix/remove-static-vars branch from 4620c0f to 91ba91a Compare

March 24, 2026 13:23

joshtrichards added the technical debt label

come-nc force-pushed the fix/remove-static-vars branch 2 times, most recently from 068218c to 2a29e8e Compare

April 28, 2026 14:43

come-nc force-pushed the fix/remove-static-vars branch from 2a29e8e to 93cbab6 Compare

May 11, 2026 16:45

Contributor Author

come-nc commented May 12, 2026

fix(files_external): Move MountConfig to non-static services #60318 is fixing one of the issues

come-nc force-pushed the fix/remove-static-vars branch 2 times, most recently from 5ac54b5 to af33409 Compare

May 21, 2026 09:27

come-nc added 15 commits

June 1, 2026 14:08


          fix: Remove static var in Memcache/Redis

58bc96a

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          fix: Remove static vars in TaskProcessing, TextProcessing, TextToImage

e4e1c46

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          chore: Remove types from const properties

7f3d2f8

Support was added in PHP 8.3 and we need to support 8.2

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          feat: Add a psalm plugin to forbid static properties and variables

9cd72b7

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          fix: Remove static vars in trashbin, versions and storages

8248c73

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          chore: Move Util static property to top of the file

db9128f

It’s not easy to remove this one but at least make it visible

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          chore: Remove static vars in TemplateLayout

82b06c7

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          fix: Add a factory for Memcached object instead of a static var

84c33b2

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          fix: Fix TemplateLayout tests

ed35b1a

The behaviour of getAppNameFromPath is really different from what was
 mocked, so I’m not sure whether the class behaves as initially
 intended. I adapted the test to match the class behavior for now.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          fix: Remove static vars in preview Generator

3c99734

There is only one instance so caching in a property is enough.
There were two levels of caching, removed one.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          fix: Remove static var is Access class

69a670d

It’s actually more correct to cache this per-instance.
What’s less clear is whether this can always fit in memory.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          fix: Use a CappedMemoryCache instead of an array to cache stuff in us…

361e925

…er_ldap

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          fix: Turn static var into standard one in Tags class

b95495f

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          fix: Remove static property use in SeekableHttpStream

cca08ec

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          fix: Fix suppressing ImpureStaticProperty and suppress it in a few pl…

ce50b1b

…aces

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

come-nc added 5 commits

June 1, 2026 14:09


          chore(user_ldap): Move static var to static property and silence warning

27c8d42

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          chore: Silence psalm false-positives

7391c49

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          fix(user_ldap): Move accesses to AccessFactory instead of static var

702f9ea

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          chore(files_versions): Be extra-cautious to please psalm

dd80bc2

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>


          chore: Suppress last known impure static properties

19a85a9

We will keep these legacy ones for now. We can search for the
 ImpureStaticProperty suppression and add special treatement for them in
 the frankenphp PR if needed.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

come-nc force-pushed the fix/remove-static-vars branch from af33409 to 19a85a9 Compare

June 1, 2026 12:10

come-nc added 3. to review and removed 2. developing labels

come-nc modified the milestones: Nextcloud 34, Nextcloud 35

come-nc marked this pull request as ready for review

June 1, 2026 12:40

come-nc requested a review from a team as a code owner

June 1, 2026 12:40

come-nc requested review from ArtificialOwl, icewind1991, leftybournes and salmart-dev and removed request for a team

June 1, 2026 12:40

CarlSchwan approved these changes

View reviewed changes

build/psalm/StaticVarsChecker.php


          chore: Get cache lazily in OC\Memcache\Redis

ab6109b

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

come-nc marked this pull request as draft

June 1, 2026 14:58

come-nc added 2. developing and removed 3. to review labels

This comment was marked as resolved.

Sign in to view


          fix: Make sure getNumConcurrentPreviews never returns 0

a1036fe

Tests were stuck forever waiting for a semaphore because concurrency was
 returning 0. It was previously working by chance because cache of the
 value in a static var was leaking through tests.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>

come-nc force-pushed the fix/remove-static-vars branch from bba8e07 to a1036fe Compare

June 1, 2026 16:05

CarlSchwan marked this pull request as ready for review

June 1, 2026 16:09

come-nc added 3. to review and removed 2. developing labels

Member

provokateurin commented Jun 1, 2026

Can you add an explanation for all @psalm-supress ImpureStaticProperty, so can understand it in the review and anyone else in the future?

Contributor Author

come-nc commented Jun 1, 2026

Can you add an explanation for all @psalm-supress ImpureStaticProperty, so can understand it in the review and anyone else in the future?

I’ve put an explanation where I could.
The one without explanation are ones which are really impure and should be fixed, but cannot really right now.
Some of them will get a cleanup method like in 4d08ffe later.

I’d like to avoid putting a generic psalm-supress ImpureStaticProperty TODO remove later. The idea is to avoid adding more static properties and have an easy way to search for current ones (search for ImpureStaticProperty or run psalm in info mode)

But if there are some of them specifically where you want an explanation comment on them and I’ll try to add one.
For stuff like Filesystem and versions/trashbin static classes, it’s just bad design technical debt. It’s not easy to remove as it’s used all-over.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

CarlSchwan CarlSchwan approved these changes

ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl is a code owner automatically assigned from nextcloud/server-backend

icewind1991 Awaiting requested review from icewind1991 icewind1991 is a code owner automatically assigned from nextcloud/server-backend

salmart-dev Awaiting requested review from salmart-dev salmart-dev is a code owner automatically assigned from nextcloud/server-backend

leftybournes Awaiting requested review from leftybournes leftybournes is a code owner automatically assigned from nextcloud/server-backend

At least 2 approving reviews are required to merge this pull request.

Labels

3. to review technical debt