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

Refactored entire Reflector to be more flexible #3

Merged
merged 27 commits into from
Jul 2, 2015
Merged

Conversation

asgrim
Copy link
Member

@asgrim asgrim commented Jun 30, 2015

Initial Changes

  • Refactored classes into directories now
  • new Reflector now requires something implementing SourceLocator interface
  • SourceLocator interface has a single method that should return a value object LocatedSource
  • The LocatedSource VO is used to load the class requested internally by Reflector
  • The public API from Reflector is now much simpler, and only has:
    • $reflector->reflect($className)
    • $reflector->getClassesFromFile() (only works when FilenameSourceLocator)
  • Creates three SourceLocator-implementing classes:
    • ComposerSourceLocator - has a constructor dependency on Composer\Autoload\ClassLoader
    • FilenameSourceLocator - constructor dep is the filename as a string
    • StringSourceLocator - constructor dep is valid PHP code in a string
  • README.md is updated to show example usage of creating the ReflectionClass instances
  • Reflector::compileNodeExpression was moved to own class NodeCompiler::compile for easier refactoring later
  • Renamed MethodsTest fixture as PHPUnit was loading the file

Refactored changes

  • Created a new Symbol VO that identifies a symbol with a name and type (checks types are valid)
  • Refactored Reflector to use the Symbol to generically identify "symbols" (instead of just classes)
  • Replaced Reflector::getClassesFromFile with Reflector::getAllSymbols to be more generic and also fixes Replace getClassesFromFile with a better API for getAllClasses #4
  • Created a new BetterReflection\Reflection\Reflection interface that identifies symbols that can be reflected (currently only ReflectionClass)
  • Created a ClassReflector which simplfies the API to calling $reflector->reflect(..) so externally, users don't have to worry about creating Symbols etc.. In future, we can create FunctionReflector, TraitReflector and so on.

Todo

  • Refactor SourceLocator somehow to accomodate for other types of symbols:
    • classes
    • functions
    • traits
    • interfaces
    • constants

```php
<?php

$reflector = new Reflector(new FilenameSourceLocator('path/to/MyApp/MyClass.php'));
Copy link
Member

Choose a reason for hiding this comment

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

As per discussion, FilenameSourceLocator should probably be something like InSingleFileSourceLocator

@asgrim asgrim added this to the 1.0.0 milestone Jun 30, 2015
@asgrim asgrim changed the title Refactored Reflector to use a SourceLocator to load classes/files/code [WIP] Refactored Reflector to use a SourceLocator to load classes/files/code Jun 30, 2015
@asgrim asgrim assigned asgrim and unassigned Ocramius Jun 30, 2015
@asgrim asgrim mentioned this pull request Jul 2, 2015
10 tasks
@asgrim asgrim removed their assignment Jul 2, 2015
}

/**
* @param $className
Copy link
Member

Choose a reason for hiding this comment

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

string?

*/
public function getAllByIdentifierType(IdentifierType $identifierType)
{
$identifier = new Identifier('*', $identifierType);
Copy link
Member Author

Choose a reason for hiding this comment

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

@Ocramius I've created issue #21 to deal with this janky identifier stuff in getAllByIdentifierType

Ocramius added a commit that referenced this pull request Jul 2, 2015
Refactored entire Reflector to be more flexible
@Ocramius Ocramius merged commit 7933dac into master Jul 2, 2015
@Ocramius Ocramius deleted the refactor-loading branch July 2, 2015 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace getClassesFromFile with a better API for getAllClasses
2 participants