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 <[email protected]>
Gerrit-Reviewer: Brian Wolff <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
