Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Whitelist for PHAR Scoping #3750

Closed
sebastianbergmann opened this issue Jul 7, 2019 · 6 comments
Closed

Whitelist for PHAR Scoping #3750

sebastianbergmann opened this issue Jul 7, 2019 · 6 comments

Comments

@sebastianbergmann
Copy link
Owner

sebastianbergmann commented Jul 7, 2019

Currently, we exclude

  • PHPUnit\*,
  • SebastianBergmann\CodeCoverage\*,
  • PharIo\*,
  • PHP_Token*,
  • Prophecy\*,

from the prefixing of all code bundled in PHAR distribution with random/unique namespace.

I would like to reduce the amount of code that does not get prefixed.

When it comes to PHPUnit\*, only classes, interfaces, and traits that are not annotated with @internal should be excluded. I will investigate whether this works by manually creating a list of these code units. In the long run, it would be great if php-scoper supported this natively.

I am not sure why SebastianBergmann\CodeCoverage\*, PharIo\*, and PHP_Token* are excluded. Maybe @sebastianfeldmann can provide some insight here? Excluding PharIo\*, for instance, means that @theseer and @sebastianheuer cannot use the scoped phpunit.phar for testing their PharIo code as intended.

With regard to Prophecy\* I am sure that some classes, interfaces, or traits of Prophecy need to be excluded (so that code complete and/our type annotations in test code continue to work).

@sebastianbergmann
Copy link
Owner Author

sebastianbergmann commented Jul 7, 2019

PharIo\*

diff --git a/build/scoper.inc.php b/build/scoper.inc.php
index 9dd770d2a..827dbbece 100644
--- a/build/scoper.inc.php
+++ b/build/scoper.inc.php
@@ -12,7 +12,6 @@
     'whitelist' => [
         'PHPUnit\*',
         'SebastianBergmann\CodeCoverage\*',
-        'PharIo\*',
         'PHP_Token*',
         'Prophecy\*',
     ],
$ ant phar
.
.
.
    [phpab] phar archive '/usr/local/src/phpunit/build/phpunit-8.3-g20665d017.phar' generated.
$ cd /tmp
$ git clone git@github.com:phar-io/manifest.git
$ cd manifest
diff --git a/composer.json b/composer.json
index 9888b5a..4a4d92b 100644
--- a/composer.json
+++ b/composer.json
@@ -38,8 +38,5 @@
     "branch-alias": {
         "dev-master": "1.0.x-dev"
     }
-  },
-  "require-dev": {
-    "phpunit/phpunit": "^8.2"
   }
 }
$ composer update
$ /usr/local/src/phpunit/build/phpunit-8.3-g20665d017.phar
PHPUnit 8.3-dev by Sebastian Bergmann and contributors.

Runtime:       PHP 7.3.7
Configuration: /usr/local/src/manifest/phpunit.xml

...............................................................  63 / 135 ( 46%)
............................................................... 126 / 135 ( 93%)
.........                                                       135 / 135 (100%)

Time: 98 ms, Memory: 14.00 MB

OK (135 tests, 139 assertions)

Conclusion: PharIo\* does not need to be excluded.

@sebastianbergmann
Copy link
Owner Author

sebastianbergmann commented Jul 7, 2019

While investigating whether SebastianBergmann\CodeCoverage\* needs to be excluded, I came across #3751. The problem covered in that issue occurs with and without SebastianBergmann\CodeCoverage\* being excluded.

@sebastianfeldmann
Copy link
Sponsor

It seems that PharIO is a remains of some early phpscoper errors I encountered. Great if it works now.

PHP_Token was not working prefixed, because of dynamic class names and aliasing at that time had some issues with the way the phpunit phar gets created.

The rest I assumed is public API, but prefixing it based on internal or not would be nice.

@sebastianbergmann
Copy link
Owner Author

Thank you, @sebastianfeldmann for your feedback. I removed PharIo\* from the whitelist in 92b6044.

@sebastianbergmann
Copy link
Owner Author

Interestingly enough I was able to remove both SebastianBergmann\CodeCoverage\* and PHP_Token* from the whitelist and was able to generate a correct HTML code coverage report for phar-io/manifest using the resulting phpunit.phar.

I have not committed these changes yet as I want to test them a little further.

@sebastianbergmann sebastianbergmann removed this from the PHPUnit 8.3 milestone Aug 1, 2019
@sebastianfeldmann
Copy link
Sponsor

sebastianfeldmann commented Sep 6, 2019

All my tests so far indicate that SebastianBergmann\CodeCoverage\* and PHP_Token* can safely be removed from the whitelist.

sebastianfeldmann pushed a commit to sebastianfeldmann/phpunit that referenced this issue Sep 6, 2019
Since CodeCoverage and PHP_Token are not part of the public API of
PHPUnit they can safely be removed from the whitelist.

Issue sebastianbergmann#3750
sebastianfeldmann pushed a commit to sebastianfeldmann/phpunit that referenced this issue Sep 6, 2019
Since CodeCoverage and PHP_Token are not part of the public API of
PHPUnit they can safely be removed from the whitelist.

Issue sebastianbergmann#3750
sebastianbergmann pushed a commit that referenced this issue Sep 6, 2019
Since CodeCoverage and PHP_Token are not part of the public API of
PHPUnit they can safely be removed from the whitelist.

Issue #3750
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

No branches or pull requests

2 participants