Skip to content

#67 stub out dependencies that cannot be located by using empty classes #74

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 27, 2018
Merged

#67 stub out dependencies that cannot be located by using empty classes #74

merged 1 commit into from
May 27, 2018

Conversation

Ocramius
Copy link
Member

@Ocramius Ocramius commented May 27, 2018 •
edited
Loading

Fixes #67

@bendavies can I ask for a review here please, as well as checking if this allows you to integrate this tool with your project?

@Ocramius Ocramius added the bug label May 27, 2018
@Ocramius Ocramius added this to the 1.0.2 milestone May 27, 2018
@Ocramius Ocramius requested a review from asgrim May 27, 2018 14:23
@Ocramius Ocramius self-assigned this May 27, 2018
@Ocramius
Copy link
Member Author

Btw, thanks @michaelcullum for the idea!

@Ocramius
Copy link
Member Author

Shipping/releasing here, as I won't have time to do it later today or tomorrow, and it is indeed a blocker...

@Ocramius Ocramius merged commit 9e95a60 into master May 27, 2018
@Ocramius Ocramius deleted the fix/#67-ignore-not-found-classes-by-stubbing-them-to-empty-classes branch May 27, 2018 14:57
@bendavies
Copy link
Contributor

bendavies commented May 27, 2018 via email

@bendavies
Copy link
Contributor

After this PR i'm receiving this running on API platform.

PHP Fatal error:  Uncaught Error: Call to a member function getAst() on null in /Users/bendavies/Code/BackwardCompatibilityCheck/vendor/roave/better-reflection/src/NodeCompiler/CompileNodeToValue.php:121
Stack trace:
#0 /Users/bendavies/Code/BackwardCompatibilityCheck/vendor/roave/better-reflection/src/NodeCompiler/CompileNodeToValue.php(44): Roave\BetterReflection\NodeCompiler\CompileNodeToValue->compileClassConstFetch(Object(PhpParser\Node\Expr\ClassConstFetch), Object(Roave\BetterReflection\NodeCompiler\CompilerContext))
#1 /Users/bendavies/Code/BackwardCompatibilityCheck/vendor/nikic/php-parser/lib/PhpParser/ConstExprEvaluator.php(145): Roave\BetterReflection\NodeCompiler\CompileNodeToValue->Roave\BetterReflection\NodeCompiler\{closure}(Object(PhpParser\Node\Expr\ClassConstFetch))
#2 /Users/bendavies/Code/BackwardCompatibilityCheck/vendor/nikic/php-parser/lib/PhpParser/ConstExprEvaluator.php(152): PhpParser\ConstExprEvaluator->evaluate(Object(PhpParser\Node\Expr\ClassConstFetch))
#3 /Users/bendavies/Code/BackwardCompatibil in /Users/bendavies/Code/BackwardCompatibilityCheck/vendor/roave/better-reflection/src/NodeCompiler/CompileNodeToValue.php on line 121

reproduction:

git clone git://github.com/api-platform/core.git
git checkout 2.2
../BackwardCompatibilityCheck/bin/roave-backward-compatibility-check --from=v2.2.6 --to=HEAD

@Ocramius
Copy link
Member Author

This usually happens when a constant cannot be located. If you have a reference to a non-existing class and then attempt to retrieve a constant from it, it will:

  1. Be stubbed out as an empty class
  2. The constant lookup will fail

I just reproduced this in zendframework/zend-mvc when comparing 2.x and master, but couldn't pin it down to a single constant. I will try to isolate the issue, but it looks like the scenario I described above, and we won't be able to do much about it besides turning it into a failed assertion. It is possible that without some core classes or extensions the sources would lead to an autoload-time error also in your project.

@Ocramius
Copy link
Member Author

@bendavies see Roave/BetterReflection#425 - that should at least lead to better exception messages for these scenarios. Not something the tool can fix though :-\

@Ocramius Ocramius mentioned this pull request May 29, 2018
Open
@Ocramius
Copy link
Member Author

Also reported #77, since stubbing drags in some more issues, and is also incomplete (can't stub interface, class and trait separately)

@Ocramius Ocramius mentioned this pull request Aug 21, 2018
Closed
Ocramius added a commit that referenced this pull request Aug 21, 2018
@Ocramius
#98 remove StubClassSourceLocator from default setup
ab29456 
This effectively reverts #74 without introducing a BC break: the class
is now deprecated, but still exists in the codebase.

The idea of the `StubClassSourceLocator` was that it should be possible
to analyse code even if dependency holes are present, but that is now
solved by skipping BC break checks for any reflection-based exception.

This massively simplifies things, as any missing dependency will simply
lead to a skipped error and a non-zero exit code, much like it happens
in `maglnet/composer-require-checker` (https://github.com/maglnet/ComposerRequireChecker).

This also fixes weird scenarios where a pre-existing class is being
looked up in a new codebase that doesn't contain it. Assuming that
following was deleted across two breaking versions:

```php
namespace Foo;
interface Bar {}
```

Before this change, this breakage would not be located, because the
`StubClassSourceLocator` would create an implicit `Foo\Bar` stub, which
would therefore be considered as "not a change".
@Ocramius Ocramius mentioned this pull request Aug 21, 2018
Merged
Ocramius added a commit that referenced this pull request Aug 21, 2018
@Ocramius
This effectively reverts #74 without introducing a BC break: the class
8944487 
is now deprecated, but still exists in the codebase.

The idea of the `StubClassSourceLocator` was that it should be possible
to analyse code even if dependency holes are present, but that is now
solved by skipping BC break checks for any reflection-based exception.

This massively simplifies things, as any missing dependency will simply
lead to a skipped error and a non-zero exit code, much like it happens
in `maglnet/composer-require-checker` (https://github.com/maglnet/ComposerRequireChecker).

This also fixes weird scenarios where a pre-existing class is being
looked up in a new codebase that doesn't contain it. Assuming that
following was deleted across two breaking versions:

```php
namespace Foo;
interface Bar {}
```

Before this change, this breakage would not be located, because the
`StubClassSourceLocator` would create an implicit `Foo\Bar` stub, which
would therefore be considered as "not a change".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asgrim asgrim Awaiting requested review from asgrim

Assignees

@Ocramius Ocramius

Labels
bug
Projects
None yet
Milestone
1.0.2
Development

Successfully merging this pull request may close these issues.
2 participants
@Ocramius @bendavies