Brian Wolff has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/393542 )
Change subject: Keep track of hook registrations ...................................................................... Keep track of hook registrations As of yet, it doesn't do anything with the information. Also, so far only Hooks::register is implemented. $wgHooks and extension.json is not yet implemented. Change-Id: I1bc7c7996dbe5684d6aeefbd7c4ee6abb0f3e992 --- M src/MWVisitor.php A tests/integration/registerhook/Hooks.php A tests/integration/registerhook/expectedResults.txt A tests/integration/registerhook/test.php 4 files changed, 240 insertions(+), 30 deletions(-) diff --git a/src/MWVisitor.php b/src/MWVisitor.php index 27bf1aa..833dd98 100644 --- a/src/MWVisitor.php +++ b/src/MWVisitor.php @@ -3,7 +3,9 @@ use Phan\Language\Context; use Phan\Language\FQSEN\FullyQualifiedMethodName; use Phan\Language\FQSEN\FullyQualifiedFunctionName; +use Phan\Language\FQSEN\FullyQualifiedFunctionLikeName; use Phan\Language\FQSEN; +use Phan\Language\Type\CallableType; use Phan\Plugin; use Phan\CodeBase; use ast\Node; @@ -37,7 +39,19 @@ } /** - * Try and recognize + * Check static calls for hook registration + * + * Forwards to method call handler. + * @param Node $node + */ + public function visitStaticCall( Node $node ) { + $this->visitMethodCall( $node ); + } + + /** + * Try and recognize hook registration + * + * Also handles static calls * @param Node $node */ public function visitMethodCall( Node $node ) { @@ -46,7 +60,7 @@ $methodName = $node->children['method']; $method = $ctx->getMethod( $methodName, - false /* not a static call */ + $node->kind === \ast\AST_STATIC_CALL ); // Should this be getDefiningFQSEN() instead? $methodName = (string)$method->getFQSEN(); @@ -57,7 +71,10 @@ case '\Parser::setTransparentTagHook': $type = $this->getHookTypeForRegistrationMethod( $methodName ); // $this->debug( __METHOD__, "registering $methodName as $type" ); - $this->handleHookRegistration( $node, $type ); + $this->handleParserHookRegistration( $node, $type ); + break; + case '\Hooks::register': + $this->handleNormalHookRegistration( $node ); break; } } catch ( Exception $e ) { @@ -82,44 +99,75 @@ } /** - * When someone calls $parser->setFunctionHook() + * Handle registering a normal hook from Hooks::register (Not from $wgHooks) + * + * @param Node $node The node representing the AST_STATIC_CALL + */ + private function handleNormalHookRegistration( Node $node ) { + assert( $node->kind === \ast\AST_STATIC_CALL ); + $params = $node->children['args']->children; + if ( count( $params ) < 2 ) { + $this->debug( __METHOD__, "Could not understand Hooks::register" ); + return; + } + $hookName = $params[0]; + if ( !is_string( $params[0] ) ) { + $this->debug( __METHOD__, "Could not register hook. Name is complex" ); + return; + } + $cb = $this->getCallableFromHookRegistration( $params[1], $hookName ); + if ( $cb ) { + $this->debug( __METHOD__, "registering $cb as handling $hookName" ); + $this->registerHook( $hookName, $cb ); + } else { + $this->debug( __METHOD__, "Could not register $hookName hook due to complex callback" ); + } + } + + /** + * When someone calls $parser->setFunctionHook() or setTagHook() * * @note Causes phan to error out if given non-existent class - * @param Node $node + * @param Node $node The AST_METHOD_CALL node * @param string $hookType The name of the hook */ - private function handleHookRegistration( Node $node, string $hookType ) { + private function handleParserHookRegistration( Node $node, string $hookType ) { $args = $node->children['args']->children; if ( count( $args ) < 2 ) { return; } $callback = $this->getFQSENFromCallable( $args[1] ); if ( $callback ) { - $alreadyRegistered = $this->plugin->registerHook( $hookType, $callback ); - if ( !$alreadyRegistered ) { - // If this is the first time seeing this, re-analyze the - // node, just in case we had already passed it by. - if ( $callback->isClosure() ) { - // For closures we have to reanalyze the parent - // function, as we can't reanalyze the closure, and - // we definitely need to since the closure would - // have already been analyzed at this point since - // we are operating in post-order. - $func = $this->context->getFunctionLikeInScope( $this->code_base ); - } elseif ( $callback instanceof FullyQualifiedMethodName ) { - $func = $this->code_base->getMethodByFQSEN( $callback ); - } else { - assert( $callback instanceof FullyQualifiedFunctionName ); - $func = $this->code_base->getFunctionByFQSEN( $callback ); - } - // Make sure we reanalyze the hook function now that - // we know what it is, in case its already been - // analyzed. - $func->analyze( - $func->getContext(), - $this->code_base - ); + $this->registerHook( $hookType, $callback ); + } + } + + private function registerHook( string $hookType, FullyQualifiedFunctionLikeName $callback ) { + $alreadyRegistered = $this->plugin->registerHook( $hookType, $callback ); + $this->debug( __METHOD__, "registering $callback for hook $hookType" ); + if ( !$alreadyRegistered ) { + // If this is the first time seeing this, re-analyze the + // node, just in case we had already passed it by. + if ( $callback->isClosure() ) { + // For closures we have to reanalyze the parent + // function, as we can't reanalyze the closure, and + // we definitely need to since the closure would + // have already been analyzed at this point since + // we are operating in post-order. + $func = $this->context->getFunctionLikeInScope( $this->code_base ); + } elseif ( $callback instanceof FullyQualifiedMethodName ) { + $func = $this->code_base->getMethodByFQSEN( $callback ); + } else { + assert( $callback instanceof FullyQualifiedFunctionName ); + $func = $this->code_base->getFunctionByFQSEN( $callback ); } + // Make sure we reanalyze the hook function now that + // we know what it is, in case its already been + // analyzed. + $func->analyze( + $func->getContext(), + $this->code_base + ); } } @@ -207,4 +255,120 @@ ); } } + + /** + * Given a MediaWiki hook registration, find the callback + * + * @note This is a different format than Parser hooks use. + * + * Valid examples of callbacks: + * "wfSomeFunction" + * "SomeClass::SomeStaticMethod" + * A Closure + * $instanceOfSomeObject (With implied method name based on hook name) + * new SomeClass + * [ <one of the above>, $extraArgsForCallback, ...] + * [ [<one of the above>], $extraArgsForCallback, ...] + * [ $instanceOfObj, 'methodName', $optionalArgForCallback, ... ] + * [ [ $instanceOfObj, 'methodName' ], $optionalArgForCallback, ...] + * + * Oddly enough, [ 'NameOfClass', 'NameOfStaticMethod' ] does not appear + * to be valid, despite that being a valid callable. + * + * @param Node|string $node + * @param string $hookName + * @return FullyQualifiedFunctionLikeName|null The corresponding FQSEN + */ + private function getCallableFromHookRegistration( $node, $hookName ) { + // "wfSomething", "Class::Method", closure + if ( !is_object( $node ) || $node->kind === \ast\AST_CLOSURE ) { + return $this->getFQSENFromCallable( $node ); + } + + // FIXME: This doesn't support syntax like: + // $wgHooks['foo'][] = new HookHandler; + // which is valid. + if ( $node->kind === \ast\AST_VAR && is_string( $node->children['name'] ) ) { + return $this->getCallbackForVar( $node, 'on' . $hookName ); + } elseif ( + $node->kind === \ast\AST_NEW && + is_string( $node->children['class']->children['name'] ) + ) { + $className = $node->children['class']->children['name']; + return FullyQualifiedMethodName::fromStringInContext( + $className . '::' . 'on' . $hookName, + $this->context + ); + } + + if ( $node->kind === \ast\AST_ARRAY ) { + if ( count( $node->children ) === 0 ) { + return null; + } + $firstChild = $node->children[0]; + if ( + ( $firstChild instanceof Node + && $firstChild->kind === \ast\AST_ARRAY ) || + !( $firstChild instanceof Node ) || + count( $node->children ) === 1 + ) { + // One of: + // [ [ <callback> ], $optionalArgs, ... ] + // [ 'SomeClass::method', $optionalArgs, ... ] + // [ <callback> ] + // Important to note, this is safe because the + // [ 'SomeClass', 'MethodToCallStatically' ] + // syntax isn't supported by hooks. + return $this->getCallableFromHookRegistration( $firstChild, $hookName ); + } + // Remaining case is: [ $someObject, 'methodToCall', 'arg', ... ] + if ( !is_string( $node->children[1] ) ) { + return null; + } + $methodName = $node->children[1]; + if ( $firstChild->kind === \ast\AST_VAR && is_string( $firstChild->children['name'] ) ) { + return $this->getCallbackForVar( $node, $methodName ); + + } elseif ( + $firstChild->kind === \ast\AST_NEW && + is_string( $firstChild->children['class']->children['name'] ) + ) { + // FIXME does this work right with namespaces + $className = $firstChild->children['class']->children['name']; + return FullyQualifiedMethodName::fromFullyQualifiedString( + $className . '::' . $methodName + ); + } + } + return null; + } + + /** + * Given an AST_VAR node, figure out what it represents as callback + * + * @note This doesn't handle classes implementing __invoke, but its + * unclear if hooks support that either. + * @param Node $node The variable + * @param string $defaultMethod If the var is an object, what method to use + * @return FullyQualifiedFunctionLikeName|null The corresponding FQSEN + */ + private function getCallbackForVar( Node $node, $defaultMethod = '' ) { + $cnode = $this->getCtxN( $node ); + $var = $cnode->getVariable(); + $types = $var->getUnionType()->getTypeSet(); + foreach ( $types as $type ) { + if ( $type instanceof CallableType ) { + return $this->getFQSENFromCallable( $node ); + } + if ( $type->isNativeType() ) { + return null; + } + if ( $defaultMethod ) { + return FullyQualifiedMethodName::fromFullyQualifiedString( + $type->asFQSEN() . '::' . $defaultMethod + ); + } + } + return null; + } } diff --git a/tests/integration/registerhook/Hooks.php b/tests/integration/registerhook/Hooks.php new file mode 100644 index 0000000..e4f7e2d --- /dev/null +++ b/tests/integration/registerhook/Hooks.php @@ -0,0 +1,5 @@ +<?php +class Hooks { + public function register( $name, $callback ) { + } +} diff --git a/tests/integration/registerhook/expectedResults.txt b/tests/integration/registerhook/expectedResults.txt new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/tests/integration/registerhook/expectedResults.txt diff --git a/tests/integration/registerhook/test.php b/tests/integration/registerhook/test.php new file mode 100644 index 0000000..586b1f3 --- /dev/null +++ b/tests/integration/registerhook/test.php @@ -0,0 +1,41 @@ +<?php + +namespace foo; + +use \Hooks; + +class SomeClass { + public function register() { + Hooks::register( 'Something', 'SecondClass::hook1' ); + Hooks::register( 'Something', [ 'SecondClass::hook2' ] ); + Hooks::register( 'Something', [ [ 'SecondClass::hook3' ] ] ); + Hooks::register( 'Something', [ 'SecondClass::hook4', 'someArg' ] ); + Hooks::register( 'Something', [ '\foo\wfSomeGlobal', 'someArg' ] ); + + $something = new SecondClass; + Hooks::register( 'Something', [ $something, 'hook5' ] ); + Hooks::register( 'Something', [ new SecondClass, 'hook6' ] ); + Hooks::register( 'Something', new SecondClass ); + } + +} + +function wfSomeGlobal( $arg1, &$arg2, $extraArg = '' ) { +} + +class SecondClass { + public static function hook1( $arg1, &$arg2, $extraArg = '' ) { + } + public static function hook2( $arg1, &$arg2, $extraArg = '' ) { + } + public static function hook3( $arg1, &$arg2, $extraArg = '' ) { + } + public static function hook4( $arg1, &$arg2, $extraArg = '' ) { + } + public static function hook5( $arg1, &$arg2, $extraArg = '' ) { + } + public static function hook6( $arg1, &$arg2, $extraArg = '' ) { + } + public function onSomething( $arg1, &$arg2, $extraArg = '' ) { + } +} -- To view, visit https://gerrit.wikimedia.org/r/393542 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1bc7c7996dbe5684d6aeefbd7c4ee6abb0f3e992 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/tools/phan/SecurityCheckPlugin Gerrit-Branch: master Gerrit-Owner: Brian Wolff <bawolff...@gmail.com> Gerrit-Reviewer: Brian Wolff <bawolff...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits