minor #33236 Add return-types with help from DebugClassLoader in the …

…CI (nicolas-grekas) This PR was merged into the 4.4 branch. Discussion ---------- Add return-types with help from DebugClassLoader in the CI | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #33228 | License | MIT | Doc PR | - I've spent a great deal of time on this PR, experimenting with adding return types to the codebase. TL;DR: my conclusion is that we cannot make it for 5.0. There are two reasons for this: 1. The burden this will put on the community is immense, especially when considering that third party libs must also be updated for any apps to work at all on a return-typed 5.0. Symfony must add them last, not first. 2. We need return type covariance, yet this won't be available before PHP 7.4, while 5.0 supports 7.2. What's attached? - ~a draft patching logic in `DebugClassLoader` to add return-type where it discovers this should be done~ - return types added automatically thanks to #33283 - ~manual fixes for situations not handled (yet, if possible at all) by that logic in `DebugClassLoader`~ #33332 What's achieved? Tests are green \o/ At this stage, I think we have to acknowledge we won't add return-types in 5.0 but prepare a serious plan to add them in 6.0. This plan could be: - [x] make DebugClassLoader able to automate adding return types. - [x] in 4.4: add all possible return types that don't break BC, e.g. in `Tests` and in generated code - [x] spot and fix places where annotations aren't accurate, add more annotations where possible. - [x] ensure `DebugClassLoader` triggers the best possible deprecations that encourage ppl to add return-types in their libs/apps. This means we could decide to disable the current ones (see #33235) and to re-enable them in 5.1. This will also give us the time to fine-tune the tooling (item 1. on this list) Ideally, we could reach a point where we could test branch 4.4 *with* return-types: we'd use the tooling to add them automatically in the CI job, then we'd run tests and they should be green. Let's do this? Help Wanted, here is how: *With PHP 7.4*, run `php .github/patch-types.php`. This will add return types everywhere possible. Then run tests, e.g. `./phpunit src/Symfony/Component/HttpFoundation --exclude-group legacy,issue-32995` Here are the components that fail with return types added, please help me check them all with a PR on [my fork](https://github.com/nicolas-grekas/symfony/tree/eh-return-types): - [x] src/Symfony/Bridge/Doctrine - [x] src/Symfony/Bridge/Monolog - [x] src/Symfony/Bridge/PhpUnit - [x] src/Symfony/Bridge/ProxyManager - [x] src/Symfony/Bridge/Twig - [x] src/Symfony/Bundle/DebugBundle - [x] src/Symfony/Bundle/FrameworkBundle - [x] src/Symfony/Bundle/SecurityBundle - [x] src/Symfony/Bundle/TwigBundle - [x] src/Symfony/Bundle/WebProfilerBundle - [x] src/Symfony/Bundle/WebServerBundle - [x] src/Symfony/Component/Asset - [x] src/Symfony/Component/BrowserKit - [x] src/Symfony/Component/Cache - [x] nicolas-grekas#28 src/Symfony/Component/Config - [x] src/Symfony/Component/Console - [x] src/Symfony/Component/CssSelector - [x] src/Symfony/Component/Debug - [x] nicolas-grekas#28 src/Symfony/Component/DependencyInjection - [x] src/Symfony/Component/DomCrawler - [x] src/Symfony/Component/Dotenv - [x] src/Symfony/Component/ErrorHandler - [x] src/Symfony/Component/ErrorRenderer - [x] nicolas-grekas#24 src/Symfony/Component/EventDispatcher - [x] src/Symfony/Component/ExpressionLanguage - [x] src/Symfony/Component/Filesystem - [x] src/Symfony/Component/Finder - [x] src/Symfony/Component/Form - [x] src/Symfony/Component/HttpClient - [x] src/Symfony/Component/HttpFoundation - [x] src/Symfony/Component/HttpKernel - [x] src/Symfony/Component/Inflector - [x] src/Symfony/Component/Intl - [x] src/Symfony/Component/Ldap - [x] src/Symfony/Component/Lock - [x] src/Symfony/Component/Mailer - [x] src/Symfony/Component/Messenger - [x] src/Symfony/Component/Mime - [x] src/Symfony/Component/OptionsResolver - [x] src/Symfony/Component/Process - [x] src/Symfony/Component/PropertyAccess - [x] src/Symfony/Component/PropertyInfo - [x] nicolas-grekas#25 src/Symfony/Component/Routing - [x] nicolas-grekas#26 src/Symfony/Component/Security - [x] src/Symfony/Component/Security/Core - [x] src/Symfony/Component/Security/Guard - [x] src/Symfony/Component/Security/Http - [x] nicolas-grekas#29 src/Symfony/Component/Serializer - [x] src/Symfony/Component/Security/Csrf - [x] src/Symfony/Component/Stopwatch - [x] src/Symfony/Component/Templating - [x] nicolas-grekas#27 src/Symfony/Component/Translation - [x] src/Symfony/Component/Validator - [x] src/Symfony/Component/VarDumper - [x] src/Symfony/Component/VarExporter - [x] src/Symfony/Component/WebLink - [x] src/Symfony/Component/Workflow - [x] src/Symfony/Component/Yaml - [x] src/Symfony/Contracts Commits ------- 11149a1 Add return-types with help from DebugClassLoader in the CI
symfony · Aug 26, 2019 · 6dd9d24 · 6dd9d24
2 parents db9549d + 11149a1
commit 6dd9d24
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 7 deletions.
diff --git a/.github/patch-types.php b/.github/patch-types.php
@@ -0,0 +1,45 @@
+<?php
+
+if (false === getenv('SYMFONY_PATCH_TYPE_DECLARATIONS')) {
+    putenv('SYMFONY_PATCH_TYPE_DECLARATIONS=force=1&php71-compat=0');
+}
+
+require __DIR__.'/../.phpunit/phpunit-8.3-0/vendor/autoload.php';
+
+file_put_contents(__DIR__.'/../vendor/autoload.php', preg_replace('/^return (Composer.*);/m', <<<'EOTXT'
+$loader = \1;
+$loader->addClassMap(['Symfony\Component\Debug\Exception\FlattenException' => \dirname(__DIR__).'/src/Symfony/Component/Debug/Exception/FlattenException.php']);
+
+return $loader;
+
+EOTXT
+, file_get_contents(__DIR__.'/../vendor/autoload.php')));
+
+$loader = require __DIR__.'/../vendor/autoload.php';
+
+Symfony\Component\ErrorHandler\DebugClassLoader::enable();
+
+foreach ($loader->getClassMap() as $class => $file) {
+    switch (true) {
+        case false !== strpos(realpath($file), '/vendor/'):
+        case false !== strpos($file, '/src/Symfony/Bridge/PhpUnit/'):
+        case false !== strpos($file, '/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Validation/Article.php'):
+        case false !== strpos($file, '/src/Symfony/Component/Config/Tests/Fixtures/BadParent.php'):
+        case false !== strpos($file, '/src/Symfony/Component/Debug/Tests/Fixtures/'):
+        case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Compiler/OptionalServiceClass.php'):
+        case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/ParentNotExists.php'):
+        case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/BadClasses/MissingParent.php'):
+        case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/'):
+        case false !== strpos($file, '/src/Symfony/Component/ErrorHandler/Tests/Fixtures/'):
+        case false !== strpos($file, '/src/Symfony/Component/PropertyInfo/Tests/Fixtures/Dummy.php'):
+        case false !== strpos($file, '/src/Symfony/Component/PropertyInfo/Tests/Fixtures/ParentDummy.php'):
+        case false !== strpos($file, '/src/Symfony/Component/Serializer/Tests/Normalizer/Features/ObjectOuter.php'):
+        case false !== strpos($file, '/src/Symfony/Component/VarDumper/Tests/Fixtures/NotLoadableClass.php'):
+        case false !== strpos($file, '/src/Symfony/Component/VarDumper/Tests/Fixtures/Php74.php') && \PHP_VERSION_ID < 70400:
+            continue 2;
+    }
+
+    class_exists($class);
+}
+
+Symfony\Component\ErrorHandler\DebugClassLoader::disable();
diff --git a/.travis.yml b/.travis.yml
@@ -26,7 +26,7 @@ env:
 matrix:
     include:
         - php: 7.1
-          env: php_extra="7.2"
+          env: php_extra="7.2 7.4snapshot"
         - php: 7.3
           env: deps=high
         - php: 7.4snapshot
@@ -79,10 +79,8 @@ before_install:
       export COMPONENTS=$(find src/Symfony -mindepth 2 -type f -name phpunit.xml.dist -printf '%h\n')
       find ~/.phpenv -name xdebug.ini -delete
 
-      if [[ $TRAVIS_PHP_VERSION = 7.4* && $deps ]]; then
+      if [[ $TRAVIS_PHP_VERSION = 7.4* ]]; then
           export PHPUNIT_X="$PHPUNIT_X,issue-32995"
-      elif [[ $TRAVIS_PHP_VERSION = 7.4* ]]; then
-          export PHPUNIT_X="$PHPUNIT --group issue-32995"
       fi
 
       nanoseconds () {
@@ -150,7 +148,7 @@ before_install:
     - |
       # php.ini configuration
       for PHP in $TRAVIS_PHP_VERSION $php_extra; do
-          phpenv global $PHP 2>/dev/null || (cd / && wget https://s3.amazonaws.com/travis-php-archives/binaries/ubuntu/14.04/x86_64/php-$PHP.tar.bz2 -O - | tar -xj)
+          phpenv global $PHP 2>/dev/null || (cd / && wget https://storage.googleapis.com/travis-ci-language-archives/php/binaries/ubuntu/16.04/x86_64/php-$PHP.tar.bz2 -O - | tar -xj)
           INI=~/.phpenv/versions/$PHP/etc/conf.d/travis.ini
           echo date.timezone = Europe/Paris >> $INI
           echo memory_limit = -1 >> $INI
@@ -262,7 +260,7 @@ install:
       run_tests () {
           set -e
           export PHP=$1
-          if [[ $PHP != $TRAVIS_PHP_VERSION && $TRAVIS_PULL_REQUEST != false ]]; then
+          if [[ $PHP != 7.4* && $PHP != $TRAVIS_PHP_VERSION && $TRAVIS_PULL_REQUEST != false ]]; then
               echo -e "\\n\\e[1;34mIntermediate PHP version $PHP is skipped for pull requests.\\e[0m"
               break
           fi
@@ -279,6 +277,17 @@ install:
               echo "$COMPONENTS" | parallel --gnu "tfold {} 'cd {} && ([ -e composer.lock ] && ${COMPOSER_UP/update/install} || $COMPOSER_UP --prefer-lowest --prefer-stable) && $PHPUNIT_X'"
               echo "$COMPONENTS" | xargs -n1 -I{} tar --append -f ~/php-ext/composer-lowest.lock.tar {}/composer.lock
           else
+              if [[ $PHP = 7.4* ]]; then
+                  # add return types before running the test suite
+                  rm vendor/symfony/contracts -Rf
+                  ln -sd $(realpath src/Symfony/Contracts) vendor/symfony/contracts
+                  sed -i 's/"\*\*\/Tests\/"//' composer.json
+                  composer install --optimize-autoloader
+                  php .github/patch-types.php
+                  php .github/patch-types.php # ensure the script is idempotent
+                  export PHPUNIT_X="$PHPUNIT_X,issue-32995,legacy"
+              fi
+
               echo "$COMPONENTS" | parallel --gnu "tfold {} $PHPUNIT_X {}"
               tfold src/Symfony/Component/Console.tty $PHPUNIT src/Symfony/Component/Console --group tty
               if [[ $PHP = ${MIN_PHP%.*} ]]; then

diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php
@@ -100,7 +100,7 @@ protected function doDestroy($sessionId)
     }
 
     /**
-     * @return int
+     * @return bool
      */
     public function gc($maxlifetime)
     {