Complete dynamic properties for tests/Zend/** #289

hungtrinh · 2022-11-22T10:26:40Z

Fix bug PHP 8.2: Dynamic Properties are deprecated' for test suite tests/Zend/**

Work Done

Removed #[AllowDynamicProperties] attribute added by me in tests/Zend/** at PR GitHub actions phpunit on php 8.2 & Fix bug on php 8.2 #275

Used rector to add real properties needed by test case class tests/Zend/** because i concerns #AllowDynamicProperties will removed in php >= 8.3

$ ./bin/rector --no-diffs tests/

This config help complete dynamic properties to class

zf1-future/rector.php

Lines 16 to 17 in 1f1cc46

    
           // https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#completedynamicpropertiesrector 
        
           $rectorConfig->rule(CompleteDynamicPropertiesRector::class);

What next

After this PR merged into master, i will do the same thing for library/Zend to eliminate issue Creation of dynamic property xxx is deprecated in library/Zend

For Reviewer

Please keep my git commit signing by merge branch, please don't use rebase ^^

glensc · 2022-11-22T10:42:05Z

For Reviewer

Please keep my git commit signing by merge branch, please don't use rebase ^^

Then you have to move those unrelated commits to separate PR. Remove rector changes from this PR and submit as a separate PR. You could submit them after this is merged if that makes a more logical order.

also noticed some commits have same title, these probably could be joined or the title is wrong? PHP 8.2: Add missing properties to classes tests/Zend/Queue/*

Also, remove "PHP 8.2: "prefix from messages to make them shorter (some are still wrapped over 72 chars), if someone wants to know if they are PHP 8.2 related they can see that from PR or from merge commit.

hungtrinh · 2022-11-22T13:37:54Z

Firstly Mr @glensc please review #290

After that i will rebase and fix this PR according to your advice

The logical order is:

Add rector tool Composer add dev tool Rector #290
Use rector tool to do Complete dynamic properties for tests/Zend/** #289

glensc · 2022-11-22T14:26:57Z

Don't worry about this PR make mistake on tests/Zend/** suite, everything verified by github action task phpunit for php 8.2 , php 8.1, php 8.0, php 8.4 (Everything success)

what does this sentence mean? what mistake?

hungtrinh · 2022-11-22T14:44:22Z

I mean something bad like

syntax error
change test spec but don't have anything to verify

glensc · 2022-11-22T14:51:22Z

Syntax errors will be caught by CI:

zf1-future/.github/workflows/lint.yml

Line 12 in 29a05ff

uses: prestashop/github-action-php-lint/7.1@v1.1

still, that sentence doesn't make any sense to me. remove it?

glensc · 2022-11-22T14:53:47Z

btw, all properties were added as public. probably doesn't matter at all for tests what their visibility is. so not worth putting time into fixing t hem, unless you have clever way of doing that.

hungtrinh · 2022-11-22T15:08:49Z

btw, all properties were added as public. probably doesn't matter at all for tests what their visibility is. so not worth putting time into fixing t hem, unless you have clever way of doing that.

Oh it's done by run ./bin/rector --no-diffs tests/ not take much time

glensc · 2022-11-22T15:22:09Z

Oh it's done by run ./bin/rector --no-diffs tests/ not take much time

so, rector can figure out which property needs to be public, which one can be private/protected?

hungtrinh · 2022-11-22T15:34:24Z

Oh it's done by run ./bin/rector --no-diffs tests/ not take much time

so, rector can figure out which property needs to be public, which one can be private/protected?

I think we can't do that at this time

Reference: https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#completedynamicpropertiesrector

glensc · 2022-11-22T15:46:36Z

that's why I said for tests/*.php they can all stay public.

hungtrinh · 2022-11-22T15:55:45Z

Mr. @glensc I followed your advice

Square 2 commit PHP 8.2: Add missing properties to classes tests/Zend/Queue/*
Removed "PHP 8.2: "prefix from messages

Jimbolino · 2022-11-22T22:08:44Z

so, rector can figure out which property needs to be public, which one can be private/protected?

I think we can't do that at this time

I actually opened a bug, after trying this too
rectorphp/rector#7574

Currently they hardcoded the missing properties to "MODIFIER_PUBLIC"
https://github.com/rectorphp/rector-src/blob/30afbb8da236ab0935851b91a7fb4b5cf26aa2e8/rules/CodeQuality/NodeFactory/MissingPropertiesFactory.php#L32

So before i ran rector, i manually edited the line to MODIFIER_PROTECTED :)

hungtrinh · 2022-11-23T01:03:46Z

@Jimbolino thanks you, a good trick for test suite ^^

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Amf/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Application/Bootstrap/BootstrapTest.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Cache/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Captcha/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Config/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Controller/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Crypt/Rsa/RsaTest.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Date/DateObjectTest.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Dojo/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Dom/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/EventManager/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Feed/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Mail/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Measure/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Mobile/Push/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Oauth/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/OpenId/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Queue/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Stdlib/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Validate/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/XmlRpc/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/View/Helper/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/CurrencyTest.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/DateTest.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/FilterTest.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/LoaderTest.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/ValidateTest.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Gdata/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Service/AllTests.php

PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated Verified by: (not working right now, need migrate code Zend_Test_PHPUnit_ControllerTestCase support phpunit 9) docker run -u "$(id -u):$(id -g)" -v $(pwd):$(pwd) -w $(pwd) -t --rm php:8.2-rc-cli-alpine -d memory_limit=-1 ./bin/phpunit -v --bootstrap tests/TestHelper.php tests/Zend/Test/PHPUnit/AllTests.php

glensc · 2022-11-23T12:29:39Z

Too many changes to manually review. But as tests pass, this should be good enough.

Ported from Shardj/zf1-future#289 PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated

hungtrinh marked this pull request as draft November 22, 2022 12:56

hungtrinh changed the title PHP 8.2 complete dynamic properties for tests/Zend/** Complete dynamic properties for tests/Zend/** Nov 22, 2022

hungtrinh force-pushed the php82-complete-dynamic-properties-for-tests branch from b8fcc06 to f89ef2e Compare November 22, 2022 15:48

hungtrinh marked this pull request as ready for review November 22, 2022 15:55

hungtrinh marked this pull request as draft November 23, 2022 03:30

hungtrinh added 12 commits November 23, 2022 11:15

hungtrinh added 18 commits November 23, 2022 11:15

hungtrinh force-pushed the php82-complete-dynamic-properties-for-tests branch from f89ef2e to e352dd1 Compare November 23, 2022 04:22

hungtrinh marked this pull request as ready for review November 23, 2022 04:24

glensc merged commit 713e3f4 into Shardj:master Nov 23, 2022

hungtrinh mentioned this pull request Nov 27, 2022

PHP 8.2 - complete dynamic properties for library/Zend #291

Merged

falkenhawk pushed a commit to zf1s/zf1 that referenced this pull request May 18, 2023

Add missing properties to test classes

21c5f6d

Ported from Shardj/zf1-future#289 PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated

falkenhawk mentioned this pull request May 18, 2023

php8.2 compat: add missing class properties zf1s/zf1#170

Merged

falkenhawk pushed a commit to zf1s/zf1 that referenced this pull request May 18, 2023

Add missing properties to test classes

c9016ec

Ported from Shardj/zf1-future#289 PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated

falkenhawk pushed a commit to zf1s/zf1 that referenced this pull request May 18, 2023

Add missing properties to test classes

3cfe169

Ported from Shardj/zf1-future#289 PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated

falkenhawk pushed a commit to zf1s/zf1 that referenced this pull request May 18, 2023

Add missing properties to test classes

3f8be99

Ported from Shardj/zf1-future#289 PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated

falkenhawk pushed a commit to zf1s/zf1 that referenced this pull request May 18, 2023

Add missing properties to test classes

1579f00

Ported from Shardj/zf1-future#289 PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated

falkenhawk pushed a commit to zf1s/zf1 that referenced this pull request Jan 3, 2024

Add missing properties to test classes

5214ecb

Ported from Shardj/zf1-future#289 PHP 8.2: Dynamic Properties are deprecated fix Reference: https://php.watch/versions/8.2/dynamic-properties-deprecated

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Complete dynamic properties for tests/Zend/** #289

Complete dynamic properties for tests/Zend/** #289

hungtrinh commented Nov 22, 2022 •

edited

Loading

glensc commented Nov 22, 2022

hungtrinh commented Nov 22, 2022

glensc commented Nov 22, 2022

hungtrinh commented Nov 22, 2022

glensc commented Nov 22, 2022

glensc commented Nov 22, 2022

hungtrinh commented Nov 22, 2022

glensc commented Nov 22, 2022

hungtrinh commented Nov 22, 2022

glensc commented Nov 22, 2022

hungtrinh commented Nov 22, 2022

Jimbolino commented Nov 22, 2022

hungtrinh commented Nov 23, 2022

glensc commented Nov 23, 2022

	// https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#completedynamicpropertiesrector
	$rectorConfig->rule(CompleteDynamicPropertiesRector::class);

Complete dynamic properties for tests/Zend/** #289

Complete dynamic properties for tests/Zend/** #289

Conversation

hungtrinh commented Nov 22, 2022 • edited Loading

Work Done

What next

For Reviewer

glensc commented Nov 22, 2022

hungtrinh commented Nov 22, 2022

glensc commented Nov 22, 2022

hungtrinh commented Nov 22, 2022

glensc commented Nov 22, 2022

glensc commented Nov 22, 2022

hungtrinh commented Nov 22, 2022

glensc commented Nov 22, 2022

hungtrinh commented Nov 22, 2022

glensc commented Nov 22, 2022

hungtrinh commented Nov 22, 2022

Jimbolino commented Nov 22, 2022

hungtrinh commented Nov 23, 2022

glensc commented Nov 23, 2022

hungtrinh commented Nov 22, 2022 •

edited

Loading