Skip to content

Pretty much compatible ReflectionMethod #58

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

Merged
Ocramius merged 4 commits into from
Jul 10, 2015
Merged

Pretty much compatible ReflectionMethod #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
df44960
Added ReflectionMethod->getModifiers()
asgrim Jul 10, 2015
d003214
Added incomplete implementation of ReflectionMethod->getPrototype()
asgrim Jul 10, 2015
d16e79a
Updated compatibility.md
asgrim Jul 10, 2015
236fe8a
Use constants to refer to flags in tests so BC does not break if valu…
asgrim Jul 10, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 26 additions & 26 deletions docs/compatibility.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ The progress of compatibility can also be tracked in issue [#7](https://github.c
| getDefaultProperties | todo |
| getDocComment | todo |
| getEndLine | todo |
| getExtension | :x: No - extensions are not supported (#15) |
| getExtension | :x: No - extensions are not supported ([#15](https://github.com/Roave/BetterReflection/issues/15)) |
| getFileName | :heavy_check_mark: Yes |
| getInterfaceNames | todo |
| getInterfaces | todo |
Expand All @@ -26,8 +26,8 @@ The progress of compatibility can also be tracked in issue [#7](https://github.c
| getProperty | :heavy_check_mark: Yes |
| getShortName | :heavy_check_mark: Yes |
| getStartLine | todo |
| getStaticProperties | :x: No - would require loading (#14) |
| getStaticPropertyValue | :x: No - would require loading (#14) |
| getStaticProperties | :x: No - would require loading ([#14](https://github.com/Roave/BetterReflection/issues/14)) |
| getStaticPropertyValue | :x: No - would require loading ([#14](https://github.com/Roave/BetterReflection/issues/14)) |
| getTraitAliases | todo |
| getTraitNames | todo |
| getTraits | todo |
Expand All @@ -50,18 +50,18 @@ The progress of compatibility can also be tracked in issue [#7](https://github.c
| newInstance | todo |
| newInstanceArgs | todo |
| newInstanceWithoutConstructor | todo |
| setStaticPropertyValue | :x: No - would require loading (#14) |
| setStaticPropertyValue | :x: No - would require loading ([#14](https://github.com/Roave/BetterReflection/issues/14)) |

## ReflectionFunctionAbstract

| Method | Supported |
|--------|-----------|
| getClosureScopeClass | :x: No - would require loading of the method itself (#14) |
| getClosureThis | :x: No - would require loading of the method itself (#14) |
| getClosureScopeClass | :x: No - would require loading of the method itself ([#14](https://github.com/Roave/BetterReflection/issues/14)) |
| getClosureThis | :x: No - would require loading of the method itself ([#14](https://github.com/Roave/BetterReflection/issues/14)) |
| getDocComment | :heavy_check_mark: Yes |
| getEndLine | :heavy_check_mark: Yes |
| getExtension | :x: No - extensions are not supported (#15) |
| getExtensionName | :x: No - extensions are not supported (#15) |
| getExtension | :x: No - extensions are not supported ([#15](https://github.com/Roave/BetterReflection/issues/15)) |
| getExtensionName | :x: No - extensions are not supported ([#15](https://github.com/Roave/BetterReflection/issues/15)) |
| getFileName | :heavy_check_mark: Yes |
| getName | :heavy_check_mark: Yes |
| getNamespaceName | :heavy_check_mark: Yes |
Expand All @@ -70,26 +70,26 @@ The progress of compatibility can also be tracked in issue [#7](https://github.c
| getParameters | :heavy_check_mark: Yes |
| getShortName | :heavy_check_mark: Yes |
| getStartLine | :heavy_check_mark: Yes |
| getStaticVariables | :x: No - would require loading (#14) |
| getStaticVariables | :x: No - would require loading ([#14](https://github.com/Roave/BetterReflection/issues/14)) |
| inNamespace | :heavy_check_mark: Yes |
| isClosure | :heavy_check_mark: Yes - but see issue (#37) |
| isDeprecated | :heavy_check_mark: Yes - but see issue (#38) |
| isClosure | :heavy_check_mark: Yes - but see issue ([#37](https://github.com/Roave/BetterReflection/issues/37)) |
| isDeprecated | :heavy_check_mark: Yes - but see issue ([#38](https://github.com/Roave/BetterReflection/issues/38)) |
| isGenerator | :heavy_check_mark: Yes |
| isInternal | :heavy_check_mark: Yes - but see issue (#38) |
| isInternal | :heavy_check_mark: Yes - but see issue ([#38](https://github.com/Roave/BetterReflection/issues/38)) |
| isUserDefined | :heavy_check_mark: Yes |
| isVariadic | :heavy_check_mark: Yes |
| returnsReference | :heavy_check_mark: Ye |
| returnsReference | :heavy_check_mark: Yes |

## ReflectionMethod

| Method | Supported |
|--------|-----------|
| getClosure | :x: No - would require loading of the method itself (#14) |
| getClosure | :x: No - would require loading of the method itself ([#14](https://github.com/Roave/BetterReflection/issues/14)) |
| getDeclaringClass | :heavy_check_mark: Yes |
| getModifiers | todo |
| getPrototype | todo |
| invoke | :x: No - would require loading of the method itself (#14) |
| invokeArgs | :x: No - would require loading of the method itself (#14) |
| getModifiers | :heavy_check_mark: Yes |
| getPrototype | todo - [#57](https://github.com/Roave/BetterReflection/issues/57) |
| invoke | :x: No - would require loading of the method itself ([#14](https://github.com/Roave/BetterReflection/issues/14)) |
| invokeArgs | :x: No - would require loading of the method itself ([#14](https://github.com/Roave/BetterReflection/issues/14)) |
| isAbstract | :heavy_check_mark: Yes |
| isConstructor | :heavy_check_mark: Yes |
| isDestructor | :heavy_check_mark: Yes |
Expand All @@ -98,7 +98,7 @@ The progress of compatibility can also be tracked in issue [#7](https://github.c
| isProtected | :heavy_check_mark: Yes |
| isPublic | :heavy_check_mark: Yes |
| isStatic | :heavy_check_mark: Yes |
| setAccessible | :x: No - would require loading of the method itself (#14) |
| setAccessible | :x: No - would require loading of the method itself ([#14](https://github.com/Roave/BetterReflection/issues/14)) |
| _inherited methods_ | see `ReflectionFunctionAbstract` |

## ReflectionParameter
Expand Down Expand Up @@ -126,10 +126,10 @@ The progress of compatibility can also be tracked in issue [#7](https://github.c

| Method | Supported |
|--------|-----------|
| getClosure | :x: No - would require actual compilation of the AST (#14) |
| invoke | :x: No - would require loading of the function itself (#14) |
| invokeArgs | :x: No - would require loading of the function itself (#14) |
| isDisabled | :heavy_check_mark: Yes - but see issue (#38) |
| getClosure | :x: No - would require actual compilation of the AST ([#14](https://github.com/Roave/BetterReflection/issues/14)) |
| invoke | :x: No - would require loading of the function itself ([#14](https://github.com/Roave/BetterReflection/issues/14)) |
| invokeArgs | :x: No - would require loading of the function itself ([#14](https://github.com/Roave/BetterReflection/issues/14)) |
| isDisabled | :heavy_check_mark: Yes - but see issue ([#38](https://github.com/Roave/BetterReflection/issues/38)) |
| _inherited methods_ | see `ReflectionFunctionAbstract` |

## ReflectionProperty
Expand All @@ -140,14 +140,14 @@ The progress of compatibility can also be tracked in issue [#7](https://github.c
| getDocComment | :heavy_check_mark: Yes |
| getModifiers | :heavy_check_mark: Yes |
| getName | :heavy_check_mark: Yes |
| getValue | :x: No - would require an instance of an object (#14) |
| getValue | :x: No - would require an instance of an object ([#14](https://github.com/Roave/BetterReflection/issues/14)) |
| isDefault | :heavy_check_mark: Yes |
| isPrivate | :heavy_check_mark: Yes |
| isProtected | :heavy_check_mark: Yes |
| isPublic | :heavy_check_mark: Yes |
| isStatic | :heavy_check_mark: Yes |
| setAccessible | :x: No - would require an instance of an object (#14) |
| setValue | :x: No - would require an instance of an object (#14) |
| setAccessible | :x: No - would require an instance of an object ([#14](https://github.com/Roave/BetterReflection/issues/14)) |
| setValue | :x: No - would require an instance of an object ([#14](https://github.com/Roave/BetterReflection/issues/14)) |

## ReflectionExtension

Expand Down
7 changes: 7 additions & 0 deletions src/Reflection/Exception/MethodPrototypeNotFound.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

namespace BetterReflection\Reflection\Exception;

class MethodPrototypeNotFound extends \ReflectionException
{
}
35 changes: 35 additions & 0 deletions src/Reflection/ReflectionMethod.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,41 @@ public static function createFromNode(
return $method;
}

/**
* Find the prototype for this method, if it exists. If it does not exist
* it will throw a MethodPrototypeNotFound exception.
*
* @return ReflectionMethod
* @throws Exception\MethodPrototypeNotFound
*/
public function getPrototype()
{
// @todo complete this implementation
/* @see https://github.com/Roave/BetterReflection/issues/57 */
throw new Exception\MethodPrototypeNotFound(sprintf(
'Method %s::%s does not have a prototype',
$this->getDeclaringClass()->getName(),
$this->getName()
));
}

/**
* Get the core-reflection-compatible modifier values
*
* @return int
*/
public function getModifiers()
{
$val = 0;
$val += $this->isStatic() ? \ReflectionMethod::IS_STATIC : 0;
Copy link
Copy Markdown
Member

@Ocramius Ocramius Jul 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

$val = ($this->isStatic() & \ReflectionMethod::IS_STATIC)
    | ($this->isPublic() & \ReflectionMethod::IS_PUBLIC)
    | ($this->isProtected() & \ReflectionMethod::IS_PROTECTED)
    | ($this->isPrivate() & \ReflectionMethod::IS_PRIVATE)
    | ($this->isAbstract() & \ReflectionMethod::IS_ABSTRACT)
    | ($this->isFinal() & \ReflectionMethod::IS_FINAL)
    | ($this->isStatic() & \ReflectionMethod::IS_STATIC)
    | ($this->isStatic() & \ReflectionMethod::IS_STATIC);

Copy link
Copy Markdown
Member

@Ocramius Ocramius Jul 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example is broken btw. true & 1 === 1, true & 2 === 0

Copy link
Copy Markdown
Member

@Ocramius Ocramius Jul 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

$val = ($this->isStatic() ? 0 : \ReflectionMethod::IS_STATIC)
    | ($this->isPublic() ? 0 : \ReflectionMethod::IS_PUBLIC)
    | ($this->isProtected() ? 0 : \ReflectionMethod::IS_PROTECTED)
    | ($this->isPrivate() ? 0 : \ReflectionMethod::IS_PRIVATE)
    | ($this->isAbstract() ? 0 : \ReflectionMethod::IS_ABSTRACT)
    | ($this->isFinal() ? 0 : \ReflectionMethod::IS_FINAL)
    | ($this->isStatic() ? 0 : \ReflectionMethod::IS_STATIC)
    | ($this->isStatic() ? 0 : \ReflectionMethod::IS_STATIC);

So yeah, ternaries -.-

Copy link
Copy Markdown
Member Author

@asgrim asgrim Jul 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That.. also doesn't work.

Copy link
Copy Markdown
Member

@Ocramius Ocramius Jul 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, guess I'll just be ok with the current approach.

$val += $this->isPublic() ? \ReflectionMethod::IS_PUBLIC : 0;
$val += $this->isProtected() ? \ReflectionMethod::IS_PROTECTED : 0;
$val += $this->isPrivate() ? \ReflectionMethod::IS_PRIVATE : 0;
$val += $this->isAbstract() ? \ReflectionMethod::IS_ABSTRACT : 0;
$val += $this->isFinal() ? \ReflectionMethod::IS_FINAL : 0;
return $val;
}

/**
* Check to see if a flag is set on this method
*
Expand Down
31 changes: 31 additions & 0 deletions test/unit/Fixture/PrototypeTree.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

interface FooInterface {
public function foo($a, $b);
}

abstract class ClassA implements FooInterface {
abstract public function foo($a, $b);
}

class ClassB extends ClassA {
public function foo($a, $b = 123) {}
}

class ClassC implements FooInterface {
public function foo($a, $b = 123) {}
}

interface BarInterface {
public function bar();
}

trait MyTrait {
abstract public function bar();
}

class ClassT {
use MyTrait;

public function bar() {}
}
43 changes: 43 additions & 0 deletions test/unit/Reflection/ReflectionMethodTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@

namespace BetterReflectionTest\Reflection;

use BetterReflection\Reflection\Exception\MethodPrototypeNotFound;
use BetterReflection\Reflection\ReflectionMethod;
use BetterReflection\Reflector\ClassReflector;
use BetterReflection\Reflection\ReflectionParameter;
use BetterReflection\SourceLocator\ComposerSourceLocator;
use BetterReflection\SourceLocator\SingleFileSourceLocator;

/**
* @covers \BetterReflection\Reflection\ReflectionMethod
Expand Down Expand Up @@ -128,4 +131,44 @@ public function testMethodNameWithNamespace()
$this->assertSame('', $methodInfo->getNamespaceName());
$this->assertSame('someMethod', $methodInfo->getShortName());
}

public function modifierProvider()
{
return [
['publicMethod', \ReflectionMethod::IS_PUBLIC, ['public']],
['privateMethod', \ReflectionMethod::IS_PRIVATE, ['private']],
['protectedMethod', \ReflectionMethod::IS_PROTECTED, ['protected']],
['finalPublicMethod', \ReflectionMethod::IS_FINAL | \ReflectionMethod::IS_PUBLIC, ['final', 'public']],
['abstractPublicMethod', \ReflectionMethod::IS_ABSTRACT | \ReflectionMethod::IS_PUBLIC, ['abstract', 'public']],
['staticPublicMethod', \ReflectionMethod::IS_STATIC | \ReflectionMethod::IS_PUBLIC, ['public', 'static']],
['noVisibility', \ReflectionMethod::IS_PUBLIC, ['public']],
];
}

/**
* @param string $methodName
* @param int $expectedModifier
* @param string[] $expectedModifierNames
* @dataProvider modifierProvider
*/
public function testGetModifiers($methodName, $expectedModifier, array $expectedModifierNames)
{
$classInfo = $this->reflector->reflect('\BetterReflectionTest\Fixture\Methods');
$method = $classInfo->getMethod($methodName);

$this->assertSame($expectedModifier, $method->getModifiers());
$this->assertSame(
$expectedModifierNames,
\Reflection::getModifierNames($method->getModifiers())
);
}

public function testGetPrototype()
{
$fixture = __DIR__ . '/../Fixture/PrototypeTree.php';
$reflector = new ClassReflector(new SingleFileSourceLocator($fixture));

$this->setExpectedException(MethodPrototypeNotFound::class);
$reflector->reflect('ClassB')->getMethod('foo')->getPrototype();
}
}
8 changes: 4 additions & 4 deletions test/unit/Reflection/ReflectionPropertyTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,10 @@ public function testExportThrowsException()
public function modifierProvider()
{
return [
['publicProperty', 256, ['public']],
['protectedProperty', 512, ['protected']],
['privateProperty', 1024, ['private']],
['publicStaticProperty', 257, ['public', 'static']],
['publicProperty', \ReflectionProperty::IS_PUBLIC, ['public']],
['protectedProperty', \ReflectionProperty::IS_PROTECTED, ['protected']],
['privateProperty', \ReflectionProperty::IS_PRIVATE, ['private']],
['publicStaticProperty', \ReflectionProperty::IS_PUBLIC | \ReflectionProperty::IS_STATIC, ['public', 'static']],
];
}

Expand Down