Skip to content

Commit 0fdff4e

Browse files
kadamwhitekadamwhite
authored
Merge pull request #347 from humanmade/register_block_type-sniff
Introduce sniff to avoid doing_it_wrong usage of register_block_type
2 parents 3b83d48 + 7826ed4 commit 0fdff4e

5 files changed

Lines changed: 309 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
<!-- When submitting a PR for a feature, add the appropriate prerelease heading here
44
and fill in the contents as you go. This simplifies later release management. -->
55

6+
## 2.4.0
7+
8+
- Add `HM.Functions.RegisterBlockTypePath` sniff recommending `register_block_type_from_metadata()` when a file path is passed to `register_block_type()`
9+
610
## 2.3.0
711

812
- Do not lint PHP within `build/` and `dist/` build directories

‎HM/Sniffs/Functions/RegisterBlockTypePathSniff.php‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
<?php
2+
3+
namespace HM\Sniffs\Functions;
4+
5+
use PHP_CodeSniffer\Util\Tokens;
6+
use PHPCSUtils\Utils\PassedParameters;
7+
use PHPCSUtils\Utils\TextStrings;
8+
use WordPressCS\WordPress\AbstractFunctionParameterSniff;
9+
10+
/**
11+
* Recommend register_block_type_from_metadata() when a path is passed to register_block_type().
12+
*
13+
* register_block_type() only delegates to register_block_type_from_metadata() if the
14+
* path it is given exists at call time. A missing path (unbuilt assets, a typo) falls
15+
* through to WP_Block_Type_Registry::register(), which tries to parse the path as a
16+
* "namespace/block-name" string and triggers a confusing _doing_it_wrong() notice.
17+
* Calling register_block_type_from_metadata() directly states the intent and fails
18+
* predictably when the path is wrong.
19+
*/
20+
class RegisterBlockTypePathSniff extends AbstractFunctionParameterSniff {
21+
22+
/**
23+
* Valid block name pattern, per WP_Block_Type_Registry::register().
24+
*/
25+
const VALID_BLOCK_NAME = '`^[a-z][a-z0-9-]*/[a-z][a-z0-9-]*$`';
26+
27+
/**
28+
* Warning message shared by both report sites.
29+
*/
30+
const MESSAGE = 'Argument 1 of register_block_type() appears to be a path; use register_block_type_from_metadata() instead for predictable file handling.';
31+
32+
/**
33+
* Group name for this group of functions.
34+
*
35+
* @var string
36+
*/
37+
protected $group_name = 'register_block_type';
38+
39+
/**
40+
* Functions this sniff is looking for.
41+
*
42+
* @var array
43+
*/
44+
protected $target_functions = [
45+
'register_block_type' => true,
46+
];
47+
48+
/**
49+
* Functions which build filesystem paths. A call to any of these within the
50+
* first argument marks it as a path.
51+
*
52+
* @var array
53+
*/
54+
protected $path_functions = [
55+
'dirname',
56+
'realpath',
57+
'plugin_dir_path',
58+
'get_template_directory',
59+
'get_stylesheet_directory',
60+
'get_theme_file_path',
61+
'get_parent_theme_file_path',
62+
'trailingslashit',
63+
'untrailingslashit',
64+
'path_join',
65+
];
66+
67+
/**
68+
* Process the parameters of a matched register_block_type() call.
69+
*
70+
* @param int $stackPtr The position of the function name token.
71+
* @param string $group_name The name of the matched group.
72+
* @param string $matched_content The matched function name in lowercase.
73+
* @param array $parameters The parameters passed to the call.
74+
* @return void
75+
*/
76+
public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) {
77+
$block_type = PassedParameters::getParameterFromStack( $parameters, 1, 'block_type' );
78+
if ( $block_type === false ) {
79+
return;
80+
}
81+
82+
if ( ! $this->parameter_looks_like_path( $block_type ) ) {
83+
return;
84+
}
85+
86+
/*
87+
* register_block_type_from_metadata() names its first parameter $file_or_folder,
88+
* so the rename fixer is only safe when the argument is passed positionally.
89+
*/
90+
if ( isset( $block_type['name'] ) ) {
91+
$this->phpcsFile->addWarning( self::MESSAGE, $stackPtr, 'PathDetected' );
92+
return;
93+
}
94+
95+
$fix = $this->phpcsFile->addFixableWarning( self::MESSAGE, $stackPtr, 'PathDetected' );
96+
if ( $fix ) {
97+
$this->phpcsFile->fixer->replaceToken( $stackPtr, 'register_block_type_from_metadata' );
98+
}
99+
}
100+
101+
/**
102+
* Determine whether a parameter appears to contain a filesystem path.
103+
*
104+
* Signals, in order of our confidence they indicate a path string:
105+
* - __DIR__ or __FILE__ magic constants.
106+
* - Call(s) to known path-building function (dirname(), plugin_dir_path(), etc).
107+
* - A constant named like a path (FOO_DIR, FOO_PATH, FOO_FILE).
108+
* - A string literal containing ".json", a backslash, multiple slashes, a leading
109+
* "/" or leading "." (none of which can appear in valid block names).
110+
*
111+
* We skip flagging bare variables or single-slash literals to avoid false positives.
112+
*
113+
* @param array $param Parameter info array from PassedParameters.
114+
* @return bool Whether the parameter looks like a path.
115+
*/
116+
protected function parameter_looks_like_path( array $param ) {
117+
$string_literals = [];
118+
$non_empty_count = 0;
119+
120+
for ( $i = $param['start']; $i <= $param['end']; $i++ ) {
121+
$token = $this->tokens[ $i ];
122+
123+
if ( isset( Tokens::$emptyTokens[ $token['code'] ] ) ) {
124+
continue;
125+
}
126+
$non_empty_count++;
127+
128+
if ( $token['code'] === T_DIR || $token['code'] === T_FILE ) {
129+
return true;
130+
}
131+
132+
if ( $token['code'] === T_STRING ) {
133+
$next = $this->phpcsFile->findNext( Tokens::$emptyTokens, $i + 1, $param['end'] + 1, true );
134+
$is_function_call = $next !== false && $this->tokens[ $next ]['code'] === T_OPEN_PARENTHESIS;
135+
136+
if ( $is_function_call && in_array( strtolower( $token['content'] ), $this->path_functions, true ) ) {
137+
return true;
138+
}
139+
140+
if ( ! $is_function_call && preg_match( '`_(DIR|PATH|FILE)$`i', $token['content'] ) === 1 ) {
141+
return true;
142+
}
143+
}
144+
145+
if ( $token['code'] === T_CONSTANT_ENCAPSED_STRING || $token['code'] === T_DOUBLE_QUOTED_STRING ) {
146+
$string_literals[] = TextStrings::stripQuotes( $token['content'] );
147+
}
148+
}
149+
150+
// The whole parameter is a single string literal.
151+
if ( $non_empty_count === 1 && count( $string_literals ) === 1 ) {
152+
$text = $string_literals[0];
153+
if ( preg_match( self::VALID_BLOCK_NAME, $text ) === 1 ) {
154+
return false;
155+
}
156+
157+
return stripos( $text, '.json' ) !== false
158+
|| strpos( $text, '\\' ) !== false
159+
|| substr_count( $text, '/' ) >= 2
160+
|| ( isset( $text[0] ) && ( $text[0] === '/' || $text[0] === '.' ) );
161+
}
162+
163+
/*
164+
* String fragments within a larger expression (usually concatenation).
165+
* A single-slash fragment such as '/my-block' could be completing a block
166+
* name from a namespace prefix, so we look only for unambiguous signals.
167+
*/
168+
foreach ( $string_literals as $text ) {
169+
if ( stripos( $text, '.json' ) !== false || substr_count( $text, '/' ) >= 2 ) {
170+
return true;
171+
}
172+
}
173+
174+
return false;
175+
}
176+
}

‎HM/Tests/Functions/RegisterBlockTypePathUnitTest.inc‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
// OK: valid block names.
4+
register_block_type( 'my-plugin/my-block' );
5+
register_block_type( 'my-plugin/my-block', [ 'render_callback' => 'my_render' ] );
6+
7+
// OK: no signal we can confidently act on.
8+
register_block_type( $block );
9+
register_block_type( $path );
10+
register_block_type( $path . '/my-block' );
11+
register_block_type( new WP_Block_Type( 'my-plugin/my-block' ) );
12+
register_block_type( MY_NAMESPACE . '/my-block' );
13+
14+
// Flagged as paths: magic constants.
15+
register_block_type( __DIR__ . '/build/my-block' );
16+
register_block_type( __DIR__ . '/build/my-block/block.json' );
17+
register_block_type( __FILE__ );
18+
19+
// Flagged as paths: path-building functions.
20+
register_block_type( dirname( __FILE__ ) . '/blocks/my-block' );
21+
register_block_type( plugin_dir_path( __FILE__ ) . 'blocks/my-block' );
22+
register_block_type( trailingslashit( $base ) . 'my-block' );
23+
24+
// Flagged as paths: constants named like paths.
25+
register_block_type( MY_PLUGIN_DIR . '/blocks/my-block' );
26+
register_block_type( MY_PLUGIN_PATH . '/my-block' );
27+
28+
// Flagged as paths: path-like string literals.
29+
register_block_type( 'blocks/my-block/block.json' );
30+
register_block_type( '/var/www/html/blocks/my-block' );
31+
register_block_type( './blocks/my-block' );
32+
register_block_type( "{$dir}/build/my-block" );
33+
34+
// Flagged as paths: fully-qualified call.
35+
\register_block_type( __DIR__ . '/build/my-block' );
36+
37+
// Flagged, but not auto-fixable: named argument doesn't exist on replacement.
38+
register_block_type( block_type: __DIR__ . '/build/my-block' );
39+
40+
// OK: second parameter never holds the path.
41+
register_block_type( 'my-plugin/my-block', [ 'style' => 'file:./style.css' ] );

‎HM/Tests/Functions/RegisterBlockTypePathUnitTest.inc.fixed‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
// OK: valid block names.
4+
register_block_type( 'my-plugin/my-block' );
5+
register_block_type( 'my-plugin/my-block', [ 'render_callback' => 'my_render' ] );
6+
7+
// OK: no signal we can confidently act on.
8+
register_block_type( $block );
9+
register_block_type( $path );
10+
register_block_type( $path . '/my-block' );
11+
register_block_type( new WP_Block_Type( 'my-plugin/my-block' ) );
12+
register_block_type( MY_NAMESPACE . '/my-block' );
13+
14+
// Flagged as paths: magic constants.
15+
register_block_type_from_metadata( __DIR__ . '/build/my-block' );
16+
register_block_type_from_metadata( __DIR__ . '/build/my-block/block.json' );
17+
register_block_type_from_metadata( __FILE__ );
18+
19+
// Flagged as paths: path-building functions.
20+
register_block_type_from_metadata( dirname( __FILE__ ) . '/blocks/my-block' );
21+
register_block_type_from_metadata( plugin_dir_path( __FILE__ ) . 'blocks/my-block' );
22+
register_block_type_from_metadata( trailingslashit( $base ) . 'my-block' );
23+
24+
// Flagged as paths: constants named like paths.
25+
register_block_type_from_metadata( MY_PLUGIN_DIR . '/blocks/my-block' );
26+
register_block_type_from_metadata( MY_PLUGIN_PATH . '/my-block' );
27+
28+
// Flagged as paths: path-like string literals.
29+
register_block_type_from_metadata( 'blocks/my-block/block.json' );
30+
register_block_type_from_metadata( '/var/www/html/blocks/my-block' );
31+
register_block_type_from_metadata( './blocks/my-block' );
32+
register_block_type_from_metadata( "{$dir}/build/my-block" );
33+
34+
// Flagged as paths: fully-qualified call.
35+
\register_block_type_from_metadata( __DIR__ . '/build/my-block' );
36+
37+
// Flagged, but not auto-fixable: named argument doesn't exist on replacement.
38+
register_block_type( block_type: __DIR__ . '/build/my-block' );
39+
40+
// OK: second parameter never holds the path.
41+
register_block_type( 'my-plugin/my-block', [ 'style' => 'file:./style.css' ] );

‎HM/Tests/Functions/RegisterBlockTypePathUnitTest.php‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
namespace HM\Tests\Functions;
4+
5+
use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
6+
7+
/**
8+
* Test class for RegisterBlockTypePath sniff.
9+
*
10+
* @group hm-sniffs
11+
*/
12+
class RegisterBlockTypePathUnitTest extends AbstractSniffUnitTest {
13+
14+
/**
15+
* Returns the lines where errors should occur.
16+
*
17+
* @return array <int line number> => <int number of errors>
18+
*/
19+
public function getErrorList() {
20+
return [];
21+
}
22+
23+
/**
24+
* Returns the lines where warnings should occur.
25+
*
26+
* @return array <int line number> => <int number of warnings>
27+
*/
28+
public function getWarningList() {
29+
return [
30+
15 => 1,
31+
16 => 1,
32+
17 => 1,
33+
20 => 1,
34+
21 => 1,
35+
22 => 1,
36+
25 => 1,
37+
26 => 1,
38+
29 => 1,
39+
30 => 1,
40+
31 => 1,
41+
32 => 1,
42+
35 => 1,
43+
38 => 1,
44+
];
45+
}
46+
47+
}

0 commit comments

Comments
 (0)