diff --git a/lib/class-command.php b/lib/class-command.php index ee93eb41..4d9ca02d 100644 --- a/lib/class-command.php +++ b/lib/class-command.php @@ -152,7 +152,14 @@ protected function _get_phpdoc_data( $path, $format = 'json' ) { $output = parse_files( $files, $path ); if ( 'json' == $format ) { - return json_encode( $output, JSON_PRETTY_PRINT ); + $json = json_encode( $output, JSON_PRETTY_PRINT ); + + if ( false === $json ) { + WP_CLI::error( sprintf( 'Problem encoding the data from %1$s as JSON: %2$s', $path, json_last_error_msg() ) ); + exit; + } + + return $json; } return $output; diff --git a/lib/class-hook-reflector.php b/lib/class-hook-reflector.php index 6b979b93..29036e5e 100644 --- a/lib/class-hook-reflector.php +++ b/lib/class-hook-reflector.php @@ -10,10 +10,16 @@ class Hook_Reflector extends BaseReflector { /** + * Get the hook name as it is spelled in the source. + * + * The name is printed from the source expression instead of read from the + * interpreted string value. Interpreting escape sequences may produce bytes + * that are not valid UTF-8, which cannot be encoded as JSON. + * * @return string */ public function getName() { - $printer = new \PhpParser\PrettyPrinter\Standard(); + $printer = new Pretty_Printer(); return $this->cleanupName( $printer->prettyPrintExpr( $this->node->args[0]->value ) ); } @@ -26,8 +32,10 @@ private function cleanupName( $name ) { $matches = array(); // quotes on both ends of a string - if ( preg_match( '/^[\'"]([^\'"]*)[\'"]$/', $name, $matches ) ) { - return $matches[1]; + // The quoted body may contain the other quote character, as in "it's", + // or an escaped copy of the quote that delimits it, as in 'it\'s'. + if ( preg_match( '/^([\'"])((?:(?!\1)[^\\\\]|\\\\.)*)\1$/s', $name, $matches ) ) { + return $matches[2]; } // two concatenated things, last one of them a variable diff --git a/lib/class-pretty-printer.php b/lib/class-pretty-printer.php index 7cfd0c55..b4ccf2fb 100644 --- a/lib/class-pretty-printer.php +++ b/lib/class-pretty-printer.php @@ -5,7 +5,57 @@ /** * Extends default printer for arguments. */ -class Pretty_Printer extends \PhpParser\PrettyPrinter\Standard { +class Pretty_Printer extends \phpDocumentor\Reflection\PrettyPrinter { + /** + * Print names as they appeared before PHP-Parser's name resolution. + * + * PHP-Parser represents resolved global names as fully-qualified names. The + * leading namespace separator is useful in an AST, but adding it to exported + * source expressions changes the established JSON output. + * + * Single-segment fully-qualified names are therefore printed without the + * leading backslash. Inside a namespaced file the printed form denotes a + * namespaced symbol rather than the global one, for example `\Foo::BAR` is + * printed as `Foo::BAR`, which in a namespaced file would resolve to + * `Vendor\Foo::BAR`. This is an accepted limitation because the parser + * targets global-namespace WordPress core code. + * + * @param \PhpParser\Node\Name\FullyQualified $node Fully-qualified name. + * + * @return string Printed name. + */ + protected function pName_FullyQualified( \PhpParser\Node\Name\FullyQualified $node ): string { + $name = $node->toString(); + + return false === strpos( $name, '\\' ) ? $name : '\\' . $name; + } + + /** + * Print heredoc and nowdoc strings with their delimiters. + * + * The parent printer returns PHP-Parser's `rawValue` attribute so that + * escape sequences are not interpreted. For heredoc and nowdoc strings that + * attribute holds the body only, without the `<<getAttribute( 'kind' ); + + if ( + \PhpParser\Node\Scalar\String_::KIND_HEREDOC === $kind || + \PhpParser\Node\Scalar\String_::KIND_NOWDOC === $kind + ) { + return \PhpParser\PrettyPrinter\Standard::pScalar_String( $node ); + } + + return parent::pScalar_String( $node ); + } + /** * Pretty prints an argument. * diff --git a/lib/runner.php b/lib/runner.php index 70e77d92..11ebbbd1 100644 --- a/lib/runner.php +++ b/lib/runner.php @@ -90,7 +90,7 @@ function parse_files( $files, $root ) { $out['constants'][] = array( 'name' => $constant->getShortName(), 'line' => $constant->getLineNumber(), - 'value' => $constant->getValue(), + 'value' => export_expression( $constant->getNode()->value ), ); } @@ -102,7 +102,7 @@ function parse_files( $files, $root ) { $func = array( 'name' => $function->getShortName(), 'namespace' => $function->getNamespace(), - 'aliases' => $function->getNamespaceAliases(), + 'aliases' => strip_global_namespace_prefixes( $function->getNamespaceAliases() ), 'line' => $function->getLineNumber(), 'end_line' => $function->getNode()->getAttribute( 'endLine' ), 'arguments' => export_arguments( $function->getArguments() ), @@ -132,8 +132,8 @@ function parse_files( $files, $root ) { 'end_line' => $class->getNode()->getAttribute( 'endLine' ), 'final' => $class->isFinal(), 'abstract' => $class->isAbstract(), - 'extends' => $class->getParentClass(), - 'implements' => $class->getInterfaces(), + 'extends' => strip_global_namespace_prefix( $class->getParentClass() ), + 'implements' => strip_global_namespace_prefixes( $class->getInterfaces() ), 'properties' => export_properties( $class->getProperties(), $class_setup_blueprints, $path ), 'methods' => export_methods( $class->getMethods(), $class_setup_blueprints, $path ), 'doc' => $class_doc, @@ -149,33 +149,62 @@ function parse_files( $files, $root ) { throw $e; } - /* - * nikic/php-parser in version 3 started adding a namespace prefix - * at the start of global names, but this is different than how the - * documentation was previously generated. this removes those prefixes - * by removing a leading reverse solidus (\) when no other reverse - * solidus appears before the end of a sequence of PHP identifier - * characters. - */ - array_walk_recursive( - $output, - static function( &$value ) { - if ( is_string( $value ) ) { - // "\wp_kses()" -> "wp_kses()" - $without_global_namespace = preg_replace( - '~(^|\p{Z})\\\\([A-Z_a-z\x80-\xFF][0-9A-Z_a-z\x80-\xFF]*)([:(\p{Z}]|->|$)~', - '$1$2$3', - $value, - ); + return $output; +} - if ( $value !== $without_global_namespace ) { - $value = $without_global_namespace; - } - } - } +/** + * Remove a synthetic leading namespace prefix from a global name. + * + * @param mixed $name Name to normalize. + * + * @return mixed + */ +function strip_global_namespace_prefix( $name ) { + if ( ! is_string( $name ) ) { + return $name; + } + + return preg_replace( + '~^\\\\([A-Z_a-z\x80-\xFF][0-9A-Z_a-z\x80-\xFF]*)([:(\p{Z}]|->|$)~', + '$1$2', + $name ); +} - return $output; +/** + * Remove synthetic leading namespace prefixes from global names. + * + * @param array $names Names to normalize. + * + * @return array + */ +function strip_global_namespace_prefixes( array $names ) { + foreach ( $names as $key => $name ) { + $names[ $key ] = strip_global_namespace_prefix( $name ); + } + + return $names; +} + +/** + * Export an expression without PHP-Parser's synthetic global namespace prefixes. + * + * @param null|\PhpParser\Node\Expr $expression Expression to export. + * + * @return null|string + */ +function export_expression( $expression ) { + if ( null === $expression ) { + return null; + } + + static $printer = null; + + if ( null === $printer ) { + $printer = new Pretty_Printer(); + } + + return $printer->prettyPrintExpr( $expression ); } /** @@ -530,7 +559,7 @@ function export_docblock( $element, array $inherited_setup_blueprints = array(), 'content' => preg_replace( '/[\n\r]+/', ' ', format_description( $tag->getDescription() ) ), ); if ( method_exists( $tag, 'getTypes' ) ) { - $tag_data['types'] = $tag->getTypes(); + $tag_data['types'] = strip_global_namespace_prefixes( $tag->getTypes() ); } if ( method_exists( $tag, 'getLink' ) ) { $tag_data['link'] = $tag->getLink(); @@ -697,8 +726,8 @@ function export_arguments( array $arguments ) { foreach ( $arguments as $argument ) { $output[] = array( 'name' => $argument->getName(), - 'default' => $argument->getDefault(), - 'type' => $argument->getType(), + 'default' => export_expression( $argument->getNode()->default ), + 'type' => strip_global_namespace_prefix( $argument->getType() ), ); } @@ -720,7 +749,7 @@ function export_properties( array $properties, array $inherited_setup_blueprints 'name' => $property->getName(), 'line' => $property->getLineNumber(), 'end_line' => $property->getNode()->getAttribute( 'endLine' ), - 'default' => $property->getDefault(), + 'default' => export_expression( $property->getNode()->default ), // 'final' => $property->isFinal(), 'static' => $property->isStatic(), 'visibility' => $property->getVisibility(), @@ -746,7 +775,7 @@ function export_methods( array $methods, array $inherited_setup_blueprints = arr $method_data = array( 'name' => $method->getShortName(), 'namespace' => $method->getNamespace(), - 'aliases' => $method->getNamespaceAliases(), + 'aliases' => strip_global_namespace_prefixes( $method->getNamespaceAliases() ), 'line' => $method->getLineNumber(), 'end_line' => $method->getNode()->getAttribute( 'endLine' ), 'final' => $method->isFinal(), @@ -1283,7 +1312,7 @@ function export_uses( array $uses ) { case 'methods': $out[ $type ][] = array( 'name' => $name[1], - 'class' => $name[0], + 'class' => strip_global_namespace_prefix( $name[0] ), 'static' => $element->isStatic(), 'line' => $element->getLineNumber(), 'end_line' => $element->getNode()->getAttribute( 'endLine' ), @@ -1292,6 +1321,7 @@ function export_uses( array $uses ) { default: case 'functions': + $name = strip_global_namespace_prefix( $name ); $out[ $type ][] = array( 'name' => $name, 'line' => $element->getLineNumber(), diff --git a/tests/phpunit/tests/export/docblocks.inc b/tests/phpunit/tests/export/docblocks.inc index 27624632..e691bb3d 100644 --- a/tests/phpunit/tests/export/docblocks.inc +++ b/tests/phpunit/tests/export/docblocks.inc @@ -40,6 +40,21 @@ function test_func( $var, $num ) { return true; } +/** + * Tests special characters in documentation. + * + * ```php + * true === wp_is_valid_utf8( '✏' ); + * false === wp_is_valid_utf8( "just \xC0 test" ); + * ``` + */ +function test_special_characters() {} + +/** + * \xC0 starts this description. + */ +function test_leading_escape_sequence() {} + /** * This is a class docblock. * diff --git a/tests/phpunit/tests/export/docblocks.php b/tests/phpunit/tests/export/docblocks.php index ba73e3dc..dc902f05 100644 --- a/tests/phpunit/tests/export/docblocks.php +++ b/tests/phpunit/tests/export/docblocks.php @@ -230,6 +230,31 @@ public function test_function_docblocks() { ); } + /** + * Test that special characters in documentation are preserved. + */ + public function test_special_characters_are_preserved() { + $this->assertFunctionHasDocs( + 'test_special_characters', + array( + 'long_description' => '
true === wp_is_valid_utf8( \'✏\' );' . "\n"
+					. 'false === wp_is_valid_utf8( "just \\xC0 test" );
', + ) + ); + } + + /** + * Test that a leading escape sequence in documentation is preserved. + */ + public function test_leading_escape_sequence_is_preserved() { + $this->assertFunctionHasDocs( + 'test_leading_escape_sequence', + array( + 'description' => '\\xC0 starts this description.', + ) + ); + } + /** * Test that class docs are exported. */ diff --git a/tests/phpunit/tests/export/global-names.inc b/tests/phpunit/tests/export/global-names.inc new file mode 100644 index 00000000..10948ae9 --- /dev/null +++ b/tests/phpunit/tests/export/global-names.inc @@ -0,0 +1,53 @@ +global_method(); + } + + public function create_namespaced() { + return ( new \Vendor\Global_Class() )->global_method(); + } +} + +/** + * Documents inline references. + * + * Calls {@see \Global_Doc_Function()} and {@see \Vendor\Thing::m()} in prose. + * + * Spells `{@see \Global_Inline::method()}` in an inline code span. + * + * ```php + * // Renders {@see \Global_Widget::render()}. + * $widget->render(); + * ``` + * + * @see \Global_See_Target() Compares prefixed references. + * @see Global_See_Plain() Compares plain references. + * @link https://example.com/reference Explains the behavior. + * @param \Global_Prefixed_Type $ignored Unused. + */ +function documented_inline_references() {} diff --git a/tests/phpunit/tests/export/global-names.php b/tests/phpunit/tests/export/global-names.php new file mode 100644 index 00000000..434b2582 --- /dev/null +++ b/tests/phpunit/tests/export/global-names.php @@ -0,0 +1,176 @@ +export_data['functions'][0]; + + $this->assertEquals( + array( 'Global_Alias' => 'Global_Alias_Source' ), + $function['aliases'] + ); + $this->assertEquals( 'Global_Parameter', $function['arguments'][0]['type'] ); + $this->assertSame( 'true', $function['arguments'][1]['default'] ); + $this->assertSame( 'null', $function['arguments'][2]['default'] ); + $this->assertSame( 'GLOBAL_MODE', $function['arguments'][3]['default'] ); + $this->assertSame( "'\\xC0'", $function['arguments'][4]['default'] ); + $this->assertSame( '\\Vendor\\GLOBAL_MODE', $function['arguments'][5]['default'] ); + $this->assertStringContainsString( + '{@see Global_Doc_Function()}', + $function['doc']['long_description'] + ); + $this->assertStringContainsString( + '\\xC0 as documentation', + $function['doc']['long_description'] + ); + $this->assertEquals( + array( 'Global_Doc_Type' ), + $function['doc']['tags'][0]['types'] + ); + } + + /** + * Test that inline references in prose are preserved as authored. + */ + public function test_prefixed_inline_reference_metadata() { + $function = $this->export_data['functions'][1]; + + $this->assertStringContainsString( + '{@see \\Global_Doc_Function()}', + $function['doc']['long_description'] + ); + } + + /** + * Test that see tag references are preserved as authored. + */ + public function test_see_tag_reference_metadata() { + $tags = $this->export_data['functions'][1]['doc']['tags']; + + $this->assertSame( '\\Global_See_Target()', $tags[0]['refers'] ); + $this->assertSame( 'Global_See_Plain()', $tags[1]['refers'] ); + } + + /** + * Test that link tag targets are preserved as authored. + */ + public function test_link_tag_metadata() { + $tags = $this->export_data['functions'][1]['doc']['tags']; + + $this->assertSame( 'https://example.com/reference', $tags[2]['link'] ); + } + + /** + * Test that prefixes synthesized by type resolution stay stripped. + */ + public function test_prefixed_tag_type_metadata() { + $tags = $this->export_data['functions'][1]['doc']['tags']; + + $this->assertEquals( array( 'Global_Prefixed_Type' ), $tags[3]['types'] ); + } + + /** + * Test that namespaced inline references keep their prefix. + */ + public function test_namespaced_inline_reference_metadata() { + $function = $this->export_data['functions'][1]; + + $this->assertStringContainsString( + '{@see \\Vendor\\Thing::m()}', + $function['doc']['long_description'] + ); + } + + /** + * Test that inline references in code samples are preserved. + */ + public function test_code_sample_inline_reference_metadata() { + $function = $this->export_data['functions'][1]; + + $this->assertStringContainsString( + '// Renders {@see \\Global_Widget::render()}.', + $function['doc']['long_description'] + ); + } + + /** + * Test that inline references in inline code spans are preserved. + */ + public function test_inline_code_span_inline_reference_metadata() { + $function = $this->export_data['functions'][1]; + + $this->assertStringContainsString( + '{@see \\Global_Inline::method()}', + $function['doc']['long_description'] + ); + } + + /** + * Test class metadata. + */ + public function test_class_metadata() { + $class = $this->export_data['classes'][0]; + + $this->assertEquals( 'Global_Parent', $class['extends'] ); + $this->assertEquals( array( 'Global_Interface' ), $class['implements'] ); + } + + /** + * Test expression metadata. + */ + public function test_expression_metadata() { + $this->assertSame( 'GLOBAL_VALUE', $this->export_data['constants'][0]['value'] ); + $this->assertSame( + 'global_default(GLOBAL_VALUE)', + $this->export_data['constants'][1]['value'] + ); + $this->assertSame( + '\\Vendor\\global_default(\\Vendor\\GLOBAL_VALUE)', + $this->export_data['constants'][2]['value'] + ); + + $class = $this->export_data['classes'][1]; + $this->assertSame( 'false', $class['properties'][0]['default'] ); + $this->assertSame( 'GLOBAL_MODE', $class['properties'][1]['default'] ); + + $method = $class['methods'][0]; + $this->assertSame( + 'new Global_Class()', + $method['uses']['methods'][0]['class'] + ); + + $method = $class['methods'][1]; + $this->assertSame( + 'new \\Vendor\\Global_Class()', + $method['uses']['methods'][0]['class'] + ); + } + + /** + * Test method-use metadata. + */ + public function test_method_use_metadata() { + $this->assertFileUsesMethod( + array( + 'name' => 'global_method', + 'line' => 16, + 'end_line' => 16, + 'class' => 'Global_Class', + 'static' => true, + ) + ); + } +} diff --git a/tests/phpunit/tests/export/hooks.inc b/tests/phpunit/tests/export/hooks.inc index 54cf8f81..6a8b4663 100644 --- a/tests/phpunit/tests/export/hooks.inc +++ b/tests/phpunit/tests/export/hooks.inc @@ -6,3 +6,15 @@ do_action( $variable . '-action' ); do_action( "another-{$variable}-action" ); do_action( 'hook_' . $object->property . '_pre' ); apply_filters( 'plain_filter', $variable, $filter_context ); +do_action( '\xC0 hook' ); +do_action( "\x09tab" ); +do_action( '\x09tab' ); +do_action( "\xC0 bad" ); +apply_filters( 'heredoc_filter', << '$filter_context' ) ); + + $this->assertFileContainsHook( + array( 'name' => '\\xC0 hook', 'line' => 9 ) + ); + + $this->assertFileContainsHook( + array( 'name' => '\\x09tab', 'line' => 10 ) + ); + + $this->assertFileContainsHook( + array( 'name' => '\\x09tab', 'line' => 11 ) + ); + } + + /** + * Test that hook names keep escapes that are invalid UTF-8 once interpreted. + */ + public function test_hook_names_keep_invalid_utf8_escapes() { + + $this->assertFileContainsHook( + array( 'name' => '\\xC0 bad', 'line' => 12 ) + ); + } + + /** + * Test that heredoc and nowdoc arguments keep their delimiters. + */ + public function test_hook_arguments_keep_doc_string_delimiters() { + + $this->assertFileContainsHook( + array( + 'type' => 'filter', + 'name' => 'heredoc_filter', + 'line' => 13, + 'arguments.0' => "<< '2', + ) + ); + + $this->assertFileContainsHook( + array( + 'type' => 'filter', + 'name' => 'nowdoc_filter', + 'line' => 16, + 'arguments.0' => "<<<'EOT'\nnowdoc \$body\nEOT", + 'arguments.1' => '2', + ) + ); + } + + /** + * Test that hook names containing a quote character lose the quotes that + * surround them in the source. + */ + public function test_hook_names_containing_quotes() { + + $this->assertFileContainsHook( + array( 'name' => "it's", 'line' => 19 ) + ); + + $this->assertFileContainsHook( + array( 'name' => "it\\'s", 'line' => 20 ) + ); + } + + /** + * Test that the exported data can be encoded as JSON. + */ + public function test_export_data_is_json_encodable() { + + $this->assertNotFalse( + json_encode( $this->export_data, JSON_PRETTY_PRINT ), + json_last_error_msg() + ); } } diff --git a/tests/phpunit/tests/export/uses/class-mapping.inc b/tests/phpunit/tests/export/uses/class-mapping.inc new file mode 100644 index 00000000..08d422c4 --- /dev/null +++ b/tests/phpunit/tests/export/uses/class-mapping.inc @@ -0,0 +1,5 @@ +add_help_tab( array() ); +} diff --git a/tests/phpunit/tests/export/uses/class-mapping.php b/tests/phpunit/tests/export/uses/class-mapping.php new file mode 100644 index 00000000..e306117f --- /dev/null +++ b/tests/phpunit/tests/export/uses/class-mapping.php @@ -0,0 +1,30 @@ +assertFunctionUsesMethod( + 'show_help' + , array( + 'name' => 'add_help_tab', + 'line' => 4, + 'end_line' => 4, + 'class' => 'WP_Screen', + 'static' => false, + ) + ); + } +}