#42 Discover and load dependencies via composer only after having discovered all source symbols #48
Conversation
composer.json
Outdated
| @@ -21,7 +21,8 @@ | |||
| "phpunit/phpunit": "^7.0", | |||
| "doctrine/coding-standard": "^4.0", | |||
| "squizlabs/php_codesniffer": "^3.2", | |||
| "phpstan/phpstan": "^0.9.2" | |||
| "phpstan/phpstan": "^0.9.2", | |||
| "roave/security-advisories": "dev-master" | |||
|
|
||
| namespace Composer\Autoload; | ||
|
|
||
| class ComposerStaticInitaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa |
src/Comparator.php
Outdated
| public function compare(ClassReflector $oldApi, ClassReflector $newApi) : Changes | ||
| { | ||
| public function compare( | ||
| ClassReflector $definedApi, |
There was a problem hiding this comment.
Think this needs a comment; it's not immediately obvious why (it now looks like) we're now comparing three APIs...
Just something like:
$definedApi the original API without any dependencies used as a base
$oldApi the original API but with dependencies included to allow type analysis
$newApi the new API to compare against the old
There was a problem hiding this comment.
I will likely rename the variables to something obnoxiously clear :-)
src/Comparator.php
Outdated
| public function compare( | ||
| ClassReflector $definedApi, | ||
| ClassReflector $oldApi, | ||
| ClassReflector $newApi |
There was a problem hiding this comment.
Do we not need the $newApi without dependencies? At some point we may define adding a class as a warning or something, how would we determine this?
There was a problem hiding this comment.
No, we need it with dependencies.
$definedApi => old classes, no dependencies - only defined symbols
$oldApi => old classes with dependencies - needed to analyse inheritance, covariance and such
$newApi => new classes with dependencies - needed to analyse inheritance, covariance and such
To make it more simple: you can't do any operations on the ReflectionClass instances without having all the dependencies. Even a simple ReflectionClass#getParentClasses() call can lead to a crash.
|
Also needs a rebase 🥃 |
…smap to find classes
…erate when using --classmap-authoritative
…ser install` in it via the composer installer
…oser install` without relying on unsafe CLI
…sses to actually reflect by first scanning for "known API"
…for reflected symbols after the defined symbols list is known
…ons nor unknown classes
…otherwise PHPStan reports nullability issues
…otherwise PHPStan reports nullability issues
…ty of `ReflectionClass#getProperty()` results that were asserted upon
… `$oldApi` We are reflecting on these in order: 1. the existing classes (`$definedApi`) 2. the old classes (`$oldApi`) 3. The new classes (`$newApi`)
…ding the dependency to consumers
Ocramius
force-pushed
the
feature/dependency-installation
branch
from
f756850 to
a46097e
Compare
…)` as per @asgrim's review Ref: #48 (comment)
|
@asgrim rebased/fixed/etc - if you merge this I'll try focusing on bells and whistles this evening, to make this nice and shiny for 🚢 |
Fixes #42
Fixes #47 (done it since I was touching
composer.jsonanyway)