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.

Sign up for GitHub

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

Parser: Fix JS/PHP inconsistencies with empty attributes #11434

Merged
merged 3 commits into from
Nov 10, 2018
Merged

Parser: Fix JS/PHP inconsistencies with empty attributes #11434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e71c1ee
squash and rebase
dmsnell Nov 9, 2018
41d1a47
add test for making sure we can parse empty attrs
dmsnell Nov 9, 2018
d1acdf6
retrigger tests
dmsnell Nov 9, 2018
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
22 changes: 17 additions & 5 deletions lib/parser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ private function peg_f2($blockName, $a) { return $a; }
private function peg_f3($blockName, $attrs) {
return array(
'blockName' => $blockName,
'attrs' => isset( $attrs ) ? $attrs : array(),
'attrs' => empty( $attrs ) ? peg_empty_attrs() : $attrs,
'innerBlocks' => array(),
'innerHTML' => '',
'innerContent' => array(),
Expand All @@ -271,7 +271,7 @@ private function peg_f4($s, $children, $e) {

return array(
'blockName' => $s['blockName'],
'attrs' => $s['attrs'],
'attrs' => empty( $s['attrs'] ) ? peg_empty_attrs() : $s['attrs'],
'innerBlocks' => $innerBlocks,
'innerHTML' => $innerHTML,
'innerContent' => $innerContent,
Expand Down Expand Up @@ -1582,6 +1582,18 @@ public function parse($input) {
// The `maybeJSON` function is not needed in PHP because its return semantics
// are the same as `json_decode`

if ( ! function_exists( 'peg_empty_attrs' ) ) {
function peg_empty_attrs() {
static $empty_attrs = null;

if ( null === $empty_attrs ) {
$empty_attrs = json_decode( '{}', true );
}

return $empty_attrs;
}
}

// array arguments are backwards because of PHP
if ( ! function_exists( 'peg_process_inner_content' ) ) {
function peg_process_inner_content( $array ) {
Expand Down Expand Up @@ -1610,7 +1622,7 @@ function peg_join_blocks( $pre, $tokens, $post ) {
if ( ! empty( $pre ) ) {
$blocks[] = array(
'blockName' => null,
'attrs' => array(),
'attrs' => peg_empty_attrs(),
'innerBlocks' => array(),
'innerHTML' => $pre,
'innerContent' => array( $pre ),
Expand All @@ -1625,7 +1637,7 @@ function peg_join_blocks( $pre, $tokens, $post ) {
if ( ! empty( $html ) ) {
$blocks[] = array(
'blockName' => null,
'attrs' => array(),
'attrs' => peg_empty_attrs(),
'innerBlocks' => array(),
'innerHTML' => $html,
'innerContent' => array( $html ),
Expand All @@ -1636,7 +1648,7 @@ function peg_join_blocks( $pre, $tokens, $post ) {
if ( ! empty( $post ) ) {
$blocks[] = array(
'blockName' => null,
'attrs' => array(),
'attrs' => peg_empty_attrs(),
'innerBlocks' => array(),
'innerHTML' => $post,
'innerContent' => array( $post ),
Expand Down
25 changes: 17 additions & 8 deletions packages/block-serialization-default-parser/parser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,14 @@ class WP_Block_Parser {
*/
public $stack;

/**
* Empty associative array, here due to PHP quirks
*
* @since 4.4.0
* @var array empty associative array
*/
public $empty_attrs;

/**
* Parses a document and returns a list of block structures
*
Expand All @@ -186,10 +194,11 @@ class WP_Block_Parser {
* @return WP_Block_Parser_Block[]
*/
function parse( $document ) {
$this->document = $document;
$this->offset = 0;
$this->output = array();
$this->stack = array();
$this->document = $document;
$this->offset = 0;
$this->output = array();
$this->stack = array();
$this->empty_attrs = json_decode( '{}', true );

do {
// twiddle our thumbs
Expand Down Expand Up @@ -364,7 +373,7 @@ function next_token() {
* match back in PHP to see which one it was.
*/
$has_match = preg_match(
'/<!--\s+(?<closer>\/)?wp:(?<namespace>[a-z][a-z0-9_-]*\/)?(?<name>[a-z][a-z0-9_-]*)\s+(?<attrs>{(?:[^}]+|}+(?=})|(?!}\s+-->).)+?}\s+)?(?<void>\/)?-->/s',
'/<!--\s+(?<closer>\/)?wp:(?<namespace>[a-z][a-z0-9_-]*\/)?(?<name>[a-z][a-z0-9_-]*)\s+(?<attrs>{(?:[^}]+|}+(?=})|(?!}\s+-->).)*?}\s+)?(?<void>\/)?-->/s',
$this->document,
$matches,
PREG_OFFSET_CAPTURE,
Expand Down Expand Up @@ -392,7 +401,7 @@ function next_token() {
*/
$attrs = $has_attrs
? json_decode( $matches[ 'attrs' ][ 0 ], /* as-associative */ true )
: json_decode( '{}', /* don't ask why, just verify in PHP */ false );
: $this->empty_attrs;

/*
* This state isn't allowed
Expand Down Expand Up @@ -422,8 +431,8 @@ function next_token() {
* @param string $innerHTML HTML content of block
* @return WP_Block_Parser_Block freeform block object
*/
static function freeform( $innerHTML ) {
return new WP_Block_Parser_Block( null, array(), array(), $innerHTML, array( $innerHTML ) );
function freeform( $innerHTML ) {
return new WP_Block_Parser_Block( null, $this->empty_attrs, array(), $innerHTML, array( $innerHTML ) );
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/block-serialization-default-parser/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ let document;
let offset;
let output;
let stack;
const tokenizer = /<!--\s+(\/)?wp:([a-z][a-z0-9_-]*\/)?([a-z][a-z0-9_-]*)\s+({(?:[^}]+|}+(?=})|(?!}\s+-->)[^])+?}\s+)?(\/)?-->/g;
const tokenizer = /<!--\s+(\/)?wp:([a-z][a-z0-9_-]*\/)?([a-z][a-z0-9_-]*)\s+({(?:[^}]+|}+(?=})|(?!}\s+-->)[^])*?}\s+)?(\/)?-->/g;

function Block( blockName, attrs, innerBlocks, innerHTML, innerContent ) {
return {
Expand Down
29 changes: 0 additions & 29 deletions packages/block-serialization-default-parser/test/__snapshots__/index.js.snap
View file
Open in desktop

This file was deleted.

22 changes: 17 additions & 5 deletions packages/block-serialization-spec-parser/grammar.pegjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@
// The `maybeJSON` function is not needed in PHP because its return semantics
// are the same as `json_decode`

if ( ! function_exists( 'peg_empty_attrs' ) ) {
function peg_empty_attrs() {
static $empty_attrs = null;

if ( null === $empty_attrs ) {
$empty_attrs = json_decode( '{}', true );
}

return $empty_attrs;
}
}

// array arguments are backwards because of PHP
if ( ! function_exists( 'peg_process_inner_content' ) ) {
function peg_process_inner_content( $array ) {
Expand Down Expand Up @@ -78,7 +90,7 @@ if ( ! function_exists( 'peg_join_blocks' ) ) {
if ( ! empty( $pre ) ) {
$blocks[] = array(
'blockName' => null,
'attrs' => array(),
'attrs' => peg_empty_attrs(),
'innerBlocks' => array(),
'innerHTML' => $pre,
'innerContent' => array( $pre ),
Expand All @@ -93,7 +105,7 @@ if ( ! function_exists( 'peg_join_blocks' ) ) {
if ( ! empty( $html ) ) {
$blocks[] = array(
'blockName' => null,
'attrs' => array(),
'attrs' => peg_empty_attrs(),
'innerBlocks' => array(),
'innerHTML' => $html,
'innerContent' => array( $html ),
Expand All @@ -104,7 +116,7 @@ if ( ! function_exists( 'peg_join_blocks' ) ) {
if ( ! empty( $post ) ) {
$blocks[] = array(
'blockName' => null,
'attrs' => array(),
'attrs' => peg_empty_attrs(),
'innerBlocks' => array(),
'innerHTML' => $post,
'innerContent' => array( $post ),
Expand Down Expand Up @@ -212,7 +224,7 @@ Block_Void
/** <?php
return array(
'blockName' => $blockName,
'attrs' => isset( $attrs ) ? $attrs : array(),
'attrs' => empty( $attrs ) ? peg_empty_attrs() : $attrs,
'innerBlocks' => array(),
'innerHTML' => '',
'innerContent' => array(),
Expand All @@ -236,7 +248,7 @@ Block_Balanced

return array(
'blockName' => $s['blockName'],
'attrs' => $s['attrs'],
'attrs' => empty( $s['attrs'] ) ? peg_empty_attrs() : $s['attrs'],
'innerBlocks' => $innerBlocks,
'innerHTML' => $innerHTML,
'innerContent' => $innerContent,
Expand Down
22 changes: 17 additions & 5 deletions packages/block-serialization-spec-parser/parser.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

71 changes: 67 additions & 4 deletions packages/block-serialization-spec-parser/shared-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,64 @@
export const jsTester = ( parse ) => () => {
describe( 'basic parsing', () => {
test( 'parse() works properly', () => {
expect( parse( '<!-- wp:core/more --><!--more--><!-- /wp:core/more -->' ) ).toMatchSnapshot();
describe( 'output structure', () => {
test( 'output is an array', () => {
expect( parse( '' ) ).toEqual( expect.any( Array ) );
expect( parse( 'test' ) ).toEqual( expect.any( Array ) );
expect( parse( '<!-- wp:void /-->' ) ).toEqual( expect.any( Array ) );
expect( parse( '<!-- wp:block --><!-- wp:inner /--><!-- /wp:block -->' ) ).toEqual( expect.any( Array ) );
expect( parse( '<!-- wp:first /--><!-- wp:second /-->' ) ).toEqual( expect.any( Array ) );
} );

test( 'parses blocks of various types', () => {
expect( parse( '<!-- wp:void /-->' )[ 0 ] ).toHaveProperty( 'blockName', 'core/void' );
expect( parse( '<!-- wp:void {} /-->' )[ 0 ] ).toHaveProperty( 'blockName', 'core/void' );
expect( parse( '<!-- wp:void {"value":true} /-->' )[ 0 ] ).toHaveProperty( 'blockName', 'core/void' );
expect( parse( '<!-- wp:void {"a":{}} /-->' )[ 0 ] ).toHaveProperty( 'blockName', 'core/void' );
expect( parse( '<!-- wp:void { "value" : true } /-->' )[ 0 ] ).toHaveProperty( 'blockName', 'core/void' );
expect( parse( '<!-- wp:void {\n\t"value" : true\n} /-->' )[ 0 ] ).toHaveProperty( 'blockName', 'core/void' );
expect( parse( '<!-- wp:block --><!-- /wp:block -->' )[ 0 ] ).toHaveProperty( 'blockName', 'core/block' );
expect( parse( '<!-- wp:block {} --><!-- /wp:block -->' )[ 0 ] ).toHaveProperty( 'blockName', 'core/block' );
expect( parse( '<!-- wp:block {"value":true} --><!-- /wp:block -->' )[ 0 ] ).toHaveProperty( 'blockName', 'core/block' );
expect( parse( '<!-- wp:block {} -->inner<!-- /wp:block -->' )[ 0 ] ).toHaveProperty( 'blockName', 'core/block' );
expect( parse( '<!-- wp:block {"value":{"a" : "true"}} -->inner<!-- /wp:block -->' )[ 0 ] ).toHaveProperty( 'blockName', 'core/block' );
} );

test( 'blockName is namespaced string (except freeform)', () => {
expect( parse( 'freeform has null name' )[ 0 ] ).toHaveProperty( 'blockName', null );
expect( parse( '<!-- wp:more /-->' )[ 0 ] ).toHaveProperty( 'blockName', 'core/more' );
expect( parse( '<!-- wp:core/more /-->' )[ 0 ] ).toHaveProperty( 'blockName', 'core/more' );
expect( parse( '<!-- wp:my/more /-->' )[ 0 ] ).toHaveProperty( 'blockName', 'my/more' );
} );

test( 'JSON attributes are key/value object', () => {
expect( parse( 'freeform has empty attrs' )[ 0 ] ).toHaveProperty( 'attrs', {} );
expect( parse( '<!-- wp:void /-->' )[ 0 ] ).toHaveProperty( 'attrs', {} );
expect( parse( '<!-- wp:void {} /-->' )[ 0 ] ).toHaveProperty( 'blockName', 'core/void' );
expect( parse( '<!-- wp:void {} /-->' )[ 0 ] ).toHaveProperty( 'attrs', {} );
expect( parse( '<!-- wp:void {"key": "value"} /-->' )[ 0 ] ).toHaveProperty( 'attrs', { key: 'value' } );
expect( parse( '<!-- wp:block --><!-- /wp:block -->' )[ 0 ] ).toHaveProperty( 'attrs', {} );
expect( parse( '<!-- wp:block {} --><!-- /wp:block -->' )[ 0 ] ).toHaveProperty( 'attrs', {} );
expect( parse( '<!-- wp:block {"key": "value"} --><!-- /wp:block -->' )[ 0 ] ).toHaveProperty( 'attrs', { key: 'value' } );
} );

test( 'innerBlocks is a list', () => {
expect( parse( 'freeform has empty innerBlocks' )[ 0 ] ).toHaveProperty( 'innerBlocks', [] );
expect( parse( '<!-- wp:void /-->' )[ 0 ] ).toHaveProperty( 'innerBlocks', [] );
expect( parse( '<!-- wp:block --><!-- /wp:block -->' )[ 0 ] ).toHaveProperty( 'innerBlocks', [] );

const withInner = parse( '<!-- wp:block --><!-- wp:inner /--><!-- /wp:block -->' )[ 0 ];
expect( withInner ).toHaveProperty( 'innerBlocks', expect.any( Array ) );
expect( withInner.innerBlocks ).toHaveLength( 1 );

const withTwoInner = parse( '<!-- wp:block -->a<!-- wp:first /-->b<!-- wp:second /-->c<!-- /wp:block -->' )[ 0 ];
expect( withTwoInner ).toHaveProperty( 'innerBlocks', expect.any( Array ) );
expect( withTwoInner.innerBlocks ).toHaveLength( 2 );
} );

test( 'innerHTML is a string', () => {
expect( parse( 'test' )[ 0 ] ).toHaveProperty( 'innerHTML', expect.any( String ) );
expect( parse( '<!-- wp:test /-->' )[ 0 ] ).toHaveProperty( 'innerHTML', expect.any( String ) );
expect( parse( '<!-- wp:test --><!-- /wp:test -->' )[ 0 ] ).toHaveProperty( 'innerHTML', expect.any( String ) );
expect( parse( '<!-- wp:test -->test<!-- /wp:test -->' )[ 0 ] ).toHaveProperty( 'innerHTML', expect.any( String ) );
} );
} );

Expand Down Expand Up @@ -131,7 +188,13 @@ export const phpTester = ( name, filename ) => makeTest(
}

try {
return JSON.parse( process.stdout );
/*
* Due to an issue with PHP's json_encode() serializing an empty associative array
* as an empty list `[]` we're manually replacing the already-encoded bit here.
*
* This is an issue with the test runner, not with the parser.
*/
return JSON.parse( process.stdout.replace( /"attrs":\s*\[\]/g, '"attrs":{}' ) );
} catch ( e ) {
throw new Error( 'failed to parse JSON:\n' + process.stdout );
}
Expand Down
Loading