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

[WIP] Variable collector #224

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
language: php

php:
- 5.6
# - 5.6
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have switched to using PHP7 at least for development - guessing that you are going to switch to PHP7 in the near-future anyway?

Copy link
Member

Choose a reason for hiding this comment

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

Yes - that's the plan :) #221

- 7.0
- hhvm

Expand Down
10 changes: 10 additions & 0 deletions src/Reflection/ReflectionFunctionAbstract.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use PhpParser\PrettyPrinter\Standard as StandardPrettyPrinter;
use PhpParser\PrettyPrinterAbstract;
use SuperClosure\Analyzer\AstAnalyzer;
use BetterReflection\Util\Visitor\VariableCollectionVisitor;

abstract class ReflectionFunctionAbstract implements \Reflector
{
Expand Down Expand Up @@ -619,4 +620,13 @@ public function getReturnStatementsAst()

return $visitor->getReturnNodes();
}

public function getVariables()
{
$nodeTraverser = new NodeTraverser();
$nodeTraverser->addVisitor($visitor = new VariableCollectionVisitor($this->getDeclaringClass(), $this->reflector));
$nodeTraverser->traverse([$this->getNode()]);

return $visitor->getVariables();
}
}
2 changes: 2 additions & 0 deletions src/Reflection/ReflectionMethod.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

use BetterReflection\Reflector\Reflector;
use PhpParser\Node\Stmt\ClassMethod as MethodNode;
use PhpParser\NodeTraverser;
use BetterReflection\Util\Visitor\VariableCollectionVisitor;

class ReflectionMethod extends ReflectionFunctionAbstract
{
Expand Down
81 changes: 81 additions & 0 deletions src/Reflection/ReflectionVariable.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php

namespace BetterReflection\Reflection;

use phpDocumentor\Reflection\Type;
use phpDocumentor\Reflection\Types;

class ReflectionVariable
{
/**
* @var string
*/
private $name;

/**
* @var int
*/
private $declaredAt;

/**
* @var $type
*/
private $type;

/**
* @param string $name
* @param int $declaredAt
Copy link
Member

Choose a reason for hiding this comment

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

Unsure about just having the line number.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, ideally there would be the range of (characters?) that the variable is available for, but the PHP parser just gives us line numbers ..

* @param string $type
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't say much - need some sort of more restrictive hint here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its actually a ReflectionType ...

* @return ReflectionType
*/
public static function createFromName($name, ReflectionType $type = null, $declaredAt)
{
$reflectionType = new self();
$reflectionType->name = $name;
$reflectionType->declaredAt = $declaredAt;
$reflectionType->type = $type;
return $reflectionType;
}

public function getName()
{
return $this->name;
}

public function getDeclaredAt()
{
return $this->declaredAt;
}

/**
* Get a PhpDocumentor type object for this type
Copy link
Member

Choose a reason for hiding this comment

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

What if the type mutated during execution? Two variables with same name, but different type and declaration line, then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We return all declarations, so you could get two with the same name, but covering different parts of the code.

*
* @return Type
*/
public function getTypeObject()
{
return $this->type;
}

/**
* Checks if it is a built-in type (i.e., it's not an object...)
*
* @see http://php.net/manual/en/reflectiontype.isbuiltin.php
* @return bool
*/
public function isBuiltin()
{
return (!$this->type instanceof Types\Object_);
}

/**
* Convert this string type to a string
*
* @see https://github.com/php/php-src/blob/master/ext/reflection/php_reflection.c#L2993
* @return string
*/
public function __toString()
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't add this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found it useful for testing, but mainly I added it to be consistent with the other Reflection* classes.

Copy link
Member

Choose a reason for hiding this comment

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

Unless it actually is required API, I wouldn't add it. __toString() is often abused, so let's not give users more rope to hang themselves :)

Copy link
Contributor Author

@dantleech dantleech Dec 4, 2016

Choose a reason for hiding this comment

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

ok, will remove it. I can only guess that the __toString functionality in the other classes is useful for dumping a representation of a class as a whole, and that Variables would not be part of the representation /cc @asgrim

{
return sprintf('@var $%s (%s): %s', $this->name, $this->type, $this->declaredAt);
}
}
Loading