Fix 'Access to an undefined property' PHPStan L0 errors

#3280882: KernelTestBase::tearDown() cleanup prevents good typehinting practices is in.
Planning to review tomorrow, if nobody beats me to it.

Going through the changes here.

In the baseline changes, only thing removed are 'Acces to undefined property' mentions, so nothing 'out of scope touched.

Going through the code changes: 80% of changes are indeed just adding the property as a class property.

Noticing here that in some cases those are typehinted, and in some cases they are not explicitly typehinted, but only with a comment. (I don't think that's really a problem for now, and phpstan on higher level will point that out.) But was this now on purpose, or more random inconsistency? (nothing to blame off course in this huge MR)

One thing i'm thinking about here, since this issue is also about php8.2 compatibility (with the dynamic property deprecation). Is how far adding those typehints, makes this also committable for D9 (with the lower php requirement, and less typehinting possibility) which we would also be php8.2 compatible (probably not php9). So not sure if we should remove the typehinting here for now, and add that again for D10 only?

Nice changes BTW, by cleaning up some small stuff. (Removing the property, when not using, and introducing the admin_user adminUser cleanup).

Other thing i'm seeing is that in some tests we use a UserInterface and sometimes the User class. Not sure we can make this consistent (i think that needs to be solved on the createUser function level (and thus in an other issue)).

I do see in the baseline 34 lines left of undefined properties. Do we want those fixed as well? (Checking some of them they are also straightforward). On the other hand, this MR is already massive and fixes almost 25% of the baseline, so might be good to get this in, and cleanup further. So only blocking thing IMO is the typehinting question.

Parenting to #3275851: [META] Fix PHP 8.2 dynamic property deprecations, hopefully that is correct?

Needs to be rebased again.

Merged in 10.0.x.

@mondrake is there a [policy, no patch] issue for typed properties, or should we open one?

Opened #3291517: [policy, no patch] Use type declarations for new class properties

Two little notes and one actual question.

Thanks, this all looks good to me now.

Can we can this one in two parts. Can we can one where all the public properties are added and then another issue where you are proposing to not use a public property. Because the non public properties require much more consideration.

+1 to RTBC, it will be helpful to minimize amount of failures at CI

Related somehow is not caught by this #3295813: ViewsEntitySchemaSubscriber access undefined property of View

Probably it makes sense to split the issue further - many properties are added without descriptions and not clear are they required or not

For example - it makes sense as separate issue

fail: [Other] Line 0 of sites/default/files/simpletest/phpunit-465.xml:
PHPUnit Test failed to complete; Error: PHPUnit 9.5.20 �[44m#StandWith�[0m�[43mUkraine�[0m

Testing Drupal\Tests\file\Kernel\ValidateTest
..                                                                  2 / 2 (100%)

Time: 00:03.189, Memory: 4.00 MB

OK (2 tests, 27 assertions)

Unsilenced deprecation notices (44)

  40x: Creation of dynamic property Drupal\Core\Config\Entity\Query\Query::$conjunction is deprecated
    20x in ValidateTest::testCallerValidation from Drupal\Tests\file\Kernel
    20x in ValidateTest::testInsecureExtensions from Drupal\Tests\file\Kernel

  2x: Creation of dynamic property Drupal\Core\Config\Entity\Query\QueryFactory::$keyValueFactory is deprecated
    1x in ValidateTest::testCallerValidation from Drupal\Tests\file\Kernel
    1x in ValidateTest::testInsecureExtensions from Drupal\Tests\file\Kernel

  2x: Creation of dynamic property Drupal\Core\Config\Entity\Query\QueryFactory::$configManager is deprecated
    1x in ValidateTest::testCallerValidation from Drupal\Tests\file\Kernel
    1x in ValidateTest::testInsecureExtensions from Drupal\Tests\file\Kernel

PHP 8.2 reveals 3 the most common reasons for failed tests https://dispatcher.drupalci.org/job/drupal_patches/138678/testReport/jun...

Testing Drupal\Tests\action\Functional\ActionListTest
E                                                                   1 / 1 (100%)

Time: 00:05.175, Memory: 4.00 MB

There was 1 error:

1) Drupal\Tests\action\Functional\ActionListTest::testEmptyActionList
Exception: Deprecated function: Use of "static" in callables is deprecated
Drupal\user\Entity\Role::postLoad()() (Line: 172)


/var/www/html/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:49
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:204
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:153
/var/www/html/vendor/guzzlehttp/promises/src/TaskQueue.php:48
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:248
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:224
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:269
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:226
/var/www/html/vendor/guzzlehttp/promises/src/Promise.php:62
/var/www/html/vendor/guzzlehttp/guzzle/src/Client.php:187
/var/www/html/core/tests/Drupal/Tests/DrupalTestBrowser.php:137
/var/www/html/vendor/symfony/browser-kit/AbstractBrowser.php:370
/var/www/html/vendor/symfony/browser-kit/AbstractBrowser.php:568
/var/www/html/vendor/symfony/browser-kit/AbstractBrowser.php:388
/var/www/html/vendor/behat/mink-browserkit-driver/src/BrowserKitDriver.php:111
/var/www/html/vendor/behat/mink/src/Session.php:148
/var/www/html/core/tests/Drupal/Tests/BrowserTestBase.php:292
/var/www/html/core/tests/Drupal/Tests/BrowserTestBase.php:262
/var/www/html/core/tests/Drupal/Tests/BrowserTestBase.php:379
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:726

ERRORS!
Tests: 1, Assertions: 1, Errors: 1.

Unsilenced deprecation notices (2)

  1x: Creation of dynamic property Drupal\Core\Session\UserSession::$pass_raw is deprecated
    1x in ActionListTest::testEmptyActionList from Drupal\Tests\action\Functional

  1x: Creation of dynamic property Drupal\Core\Session\UserSession::$passRaw is deprecated
    1x in ActionListTest::testEmptyActionList from Drupal\Tests\action\Functional

Still needs more review for Berdir's comments, and added a couple on the MR too.

There's a lot more revealed via #3295821: Ignore: patch testing issue for PHP 8.2 attributes and it looks like makes sense to split the issue, at least for migrations, view and tests

@mondrake: If you're willing to do one more reroll, I'm willing to do a review and by what I've seen so far in the MR that would almost certainly lead to an RTBC from me.

@mondrake:
Damn, you're quick :)

Left three (per usual slightly(?) annoying) nitpicks about comments. You can ignore them if you want, they are most definitely not blocking this otherwise very nice clean-up.

I would suggest either changing them or closing them, afterwards put it back on NR, and if TestBot agrees, this is RTBC for me.

All my (feeble) nitpicks were addressed. For me this is RTBC if TestBot also agrees.

Thank you!
It will have collision with #3308744: Fix magic connection property access from \Drupal\Core\FileTransfer\FileTransfer::__get() but nitpicks could be polished later

Committed/pushed to 10.1.x and cherry-picked to 10.0.x.

Moving to 9.5.x for backport, but not sure what to do about the type hints here - @alexpott suggested specifying the properties without the type hints, maybe that's the best we can do?

Well, that was a "bit of a grind"...

Attached interdiff between 10.0/10.1 MR and the 9.5 MR.

diff -u b/core/modules/views/src/Plugin/views/query/QueryPluginBase.php b/core/modules/views/src/Plugin/views/query/QueryPluginBase.php
--- b/core/modules/views/src/Plugin/views/query/QueryPluginBase.php
+++ b/core/modules/views/src/Plugin/views/query/QueryPluginBase.php
@@ -48,11 +48,6 @@
   protected $limit;
 
   /**
-   * Generate a query and a countquery from all of the information supplied
-   * to the object.
-  public int $offset;
-
-  /**
    * Controls how the WHERE and HAVING groups are put together.
    *
    * @var string
@@ -63,2 +58,7 @@
    * Generate a query and a countquery from all of the information supplied
+   * to the object.
+  public int $offset;
+
+  /**
+   * Generate a query and a countquery from all of the information supplied
    * to the object.

This part of the interdiff looks weird, not sure what happened here. Everything in the MR and everything else in the interdiff looks correct so this is RTBC.

Thanks @longwave, noticed that "weirdness" in the interdiff as well, but, after comparing the visual representations of the changes in both MRs, I couldn't see a problem. Good to hear that confirmed.

Well that was quick.

Committed/pushed to 9.5.x, thanks!

(Thanks @mondrake for kicking off a PHP7.3 test. Should have thought of that myself)

We have a report in #3321338: Multiple deprecation notices on installing Drupal 9.5 on php 8.2 and mysql 8 about Role.php not being fixed in 9.5.x.