Comments (13)
@panosru thanks for the explanation. Seems like a real bummer 😉. Will try to look into that.
from framework.
@icanhazstring Indeed you are correct, I by mistake replaced the @inheritdoc
with @Contract\Inherit
but forgot to remove the { }
.
I run composer update
and removed the { }
around the @Contract\Inherit
:
/**
* @Contract\Inherit()
*/
public function getBalance()
{
// return $this->balance;
return 10;
}
and now it seems to work:
Fatal error: Uncaught DomainException: Invalid return value received from the assertion body, only boolean or void can be returned in /demo/Account.php on line 48
PhpDeal\Exception\ContractViolation: Contract $__result == $this->balance violated for Demo\Account->getBalance in /demo/Account.php on line 48
Thanks for the hint! That debug repo was created only for debug purposes of current issue, it does not reflect the way I'm handling my production or development projects, but in any case thanks!
Have a great day and thanks for the implementation!
from framework.
@panosru just released a new version 0.5
with the new feature.
Closing this.
from framework.
Hi @panosru thanks for reaching out. As far as I can see you removed the contract annotation. Without it the underlying aop framework can't predict where to check for violations.
I hope I could help you.
from framework.
@panosru did you expect the @inheritdoc
to use the @Contract\Ensure
from the AccountContractInterface
? If so and this does not work this might be a bug.
@lisachenko is this supposed to work?
from framework.
@icanhazstring I'm sorry for my late reply, I had a flight to catch, yes I expected the @inheritdoc
to use the @Contract\Ensure
from the parent.
from what I read here
class Foo extends FooParent
{
/**
* @param int $amount
* @Contract\Verify("$amount != 1")
* {@inheritdoc}
*/
public function bar($amount)
{
...
}
}
class FooParent
{
/**
* @param int $amount
* @Contract\Verify("$amount != 2")
*/
public function bar($amount)
{
...
}
}
Foo::bar does not accept '1' and '2' literals as a parameter.
That tells me that Foo::bar
inherits the @Contract\Verify
from FooParent::bar
because of @inheritdoc
, based on the example on the docs that should work:
class Foo extends FooParent
{
/**
* @param int $amount
* {@inheritdoc}
*/
public function bar($amount)
{
...
}
}
class FooParent
{
/**
* @param int $amount
* @Contract\Verify("$amount != 2")
*/
public function bar($amount)
{
...
}
}
but it seems that @inheritdoc
works only when you have a @Contract\*
in the method you add the @inheritdoc
, otherwise is ignored.
The "problem" here is that @inheritdoc
works only when you have specified in that docblock a @Contract\*
, but @inheritdoc
is widely used and having the library detecting @inheritdoc
would bring more issues rather than solving the problem.
I suppose that @inheritdoc
could work there where you use @Contract\*
but if in my case you just want to inherit the parent contracts a separate annotation should be introduced @Contract\Inherit
for instance.
from framework.
Just cross referencing #30 with this, since I realized the exception you expect seems not correct.
This should be changed, and might be in the next major version.
Anyways, I dug into your problem and can confirm it. The problem with that is, the aspects are only working for methods with an registered annotation. I will try to figure out a good way to scan for inherited contracts.
Edit: Ok so now I only came up with one solution.
Problem is @inheritdoc
is, as you mentioned, used in a wide range of files. If we make GoAop (the underlying framework) to make a proxy for all classes with this annotation, everything and everyone will freak out 😉
The only thing that came to my mind is a new annotation @InheritContracts
on class level.
So you will need to add this annotation on the class you are writing, which will make sure every method with @inheritdoc
will be checked for existing contracts in a parent class recursively. This might help you solve your problem.
Maybe @lisachenko has an opinion on this as well 😉
from framework.
@icanhazstring So, to visualise it, based on the solution you described, it would look like this:
/**
* @InheritContracts
*/
class Foo extends FooParent
{
/**
* @param int $amount
* {@inheritdoc}
*/
public function bar($amount)
{
...
}
}
class FooParent
{
/**
* @param int $amount
* @Contract\Verify("$amount != 2")
*/
public function bar($amount)
{
...
}
}
Tagging a class will then seek its methods for potential @inheritdoc
in their dockblock, am I right?
from framework.
@panosru exactly. That's the plan 😉
from framework.
@panosru I have made a branch where I implemented an new annotation.
@Inherit
as you proposed earlier.
You can try it out by using the dev branch:
Add this repo to your composer.json
{
"repositories": [
{"type":"vcs", "url":"https://github.com/php-deal/framework"}
]
}
Require the needed dev branch.
composer require phpdeal/framework:dev-feature/inherit-contracts
The functionality is as follows:
@Inherit
works for both Class and Method annotation.
- since there is no
@inheritdoc
on class level, theInvariant
annotation already has an auto inheritance - for methods all existing contracts will work with
@inheritdoc
and look in the inheritance tree - if you skip the
@inheritdoc
annotation and useInherit
annotation, you will get the same result- this also works without an existing annotation (
Ensure
,Verify
) on the current method.
- this also works without an existing annotation (
To illustrate:
Given this interface
interface A
{
/*
* @Contract\Ensure(...)
*/
public function fu();
}
class B implements A
{
/*
* @Contract\Inherit
*/
public function fu() {}
}
If this basically works for you, let me now, and I will clean up the branch, add tests, as well update the readme.
from framework.
@icanhazstring Thanks for the branch! I cloned it and I'm testing it but it seems to not working for me.
I have created a debug repo which you can clone and run composer install
then php test.php
will return 10
instead of error as it supposed to...
Also you had a typo on your previous message, it should be composer require php-deal/framework:dev-feature/inherit-contracts
(with dash)
from framework.
@panosru will give it a try and see what is wrong.
And back. Ok found the problem.
There are two problems.
- You need to avoid adding
{}
around annotations. If they are there, the annotation reader can't find them - Currently you need to supply the
@inheritdoc
annotation as well. I will change this, so you don't have to add this.
Also as a hint: If you need minimum-stability
as dev
, add prefer-stable: true
as well. This will make sure that every other package that is not needed as dev, will be installed in stable version 😉
from framework.
@panosru as you can see I just provided a new PR solving your issue.
Feel free to look it up. Can you please try again if this implementation works for you.
If so I will merge this an tag a new release.
from framework.
Related Issues (20)
- [Feature] PhpStorm integration HOT 1
- Invariant for magic getters HOT 4
- ZF2 Module HOT 7
- Contract propagation for interfaces HOT 4
- Contributing guide HOT 3
- Library name in Scrutinizer and Packagist HOT 3
- Project status? HOT 4
- Issue with exception display on command line HOT 4
- Issue running PHPUnit with composer HOT 1
- PathResolver' not found HOT 2
- Violating contracts should throw ContractViolation HOT 1
- Bootstrap from class and run on self HOT 2
- Setup with symfony and better doc HOT 1
- Feature : automatic unit test generation based from contracts HOT 2
- [Optimization] Switch from eval to create_function HOT 1
- Request for Documentation/Help to use this library HOT 4
- Defining Expression Handlers HOT 2
- using with PHPUnit HOT 6
- Contract propagation HOT 8
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from framework.