From 7826ed405ad8ff7d4aa09243c9dc329184f1672b Mon Sep 17 00:00:00 2001 From: "K. Adam White" Date: Mon, 6 Jul 2026 21:36:57 -0400 Subject: [PATCH] Introduce sniff to avoid doing_it_wrong usage of register_block_type register_block_type internally delegates to register_block_type_from_metadata if a readable file path is passed, but if the file does not exist (usually from not having run a build) it throws a confusing doing_it_wrong message. We should be using register_block_type_from_metadata directly whenever we are not passing the name of a block as the first argument, since this can lead to wasted debugging time if the error is not understood correctly. This sniff attempts to put in place some heuristic detection for path-y first arguments passed to register_block_type, to catch this in PRs / CI. --- CHANGELOG.md | 4 + .../Functions/RegisterBlockTypePathSniff.php | 176 ++++++++++++++++++ .../RegisterBlockTypePathUnitTest.inc | 41 ++++ .../RegisterBlockTypePathUnitTest.inc.fixed | 41 ++++ .../RegisterBlockTypePathUnitTest.php | 47 +++++ 5 files changed, 309 insertions(+) create mode 100644 HM/Sniffs/Functions/RegisterBlockTypePathSniff.php create mode 100644 HM/Tests/Functions/RegisterBlockTypePathUnitTest.inc create mode 100644 HM/Tests/Functions/RegisterBlockTypePathUnitTest.inc.fixed create mode 100644 HM/Tests/Functions/RegisterBlockTypePathUnitTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index ebc83d42..f1a014ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ +## 2.4.0 + +- Add `HM.Functions.RegisterBlockTypePath` sniff recommending `register_block_type_from_metadata()` when a file path is passed to `register_block_type()` + ## 2.3.0 - Do not lint PHP within `build/` and `dist/` build directories diff --git a/HM/Sniffs/Functions/RegisterBlockTypePathSniff.php b/HM/Sniffs/Functions/RegisterBlockTypePathSniff.php new file mode 100644 index 00000000..4bdd0e5b --- /dev/null +++ b/HM/Sniffs/Functions/RegisterBlockTypePathSniff.php @@ -0,0 +1,176 @@ + true, + ]; + + /** + * Functions which build filesystem paths. A call to any of these within the + * first argument marks it as a path. + * + * @var array + */ + protected $path_functions = [ + 'dirname', + 'realpath', + 'plugin_dir_path', + 'get_template_directory', + 'get_stylesheet_directory', + 'get_theme_file_path', + 'get_parent_theme_file_path', + 'trailingslashit', + 'untrailingslashit', + 'path_join', + ]; + + /** + * Process the parameters of a matched register_block_type() call. + * + * @param int $stackPtr The position of the function name token. + * @param string $group_name The name of the matched group. + * @param string $matched_content The matched function name in lowercase. + * @param array $parameters The parameters passed to the call. + * @return void + */ + public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { + $block_type = PassedParameters::getParameterFromStack( $parameters, 1, 'block_type' ); + if ( $block_type === false ) { + return; + } + + if ( ! $this->parameter_looks_like_path( $block_type ) ) { + return; + } + + /* + * register_block_type_from_metadata() names its first parameter $file_or_folder, + * so the rename fixer is only safe when the argument is passed positionally. + */ + if ( isset( $block_type['name'] ) ) { + $this->phpcsFile->addWarning( self::MESSAGE, $stackPtr, 'PathDetected' ); + return; + } + + $fix = $this->phpcsFile->addFixableWarning( self::MESSAGE, $stackPtr, 'PathDetected' ); + if ( $fix ) { + $this->phpcsFile->fixer->replaceToken( $stackPtr, 'register_block_type_from_metadata' ); + } + } + + /** + * Determine whether a parameter appears to contain a filesystem path. + * + * Signals, in order of our confidence they indicate a path string: + * - __DIR__ or __FILE__ magic constants. + * - Call(s) to known path-building function (dirname(), plugin_dir_path(), etc). + * - A constant named like a path (FOO_DIR, FOO_PATH, FOO_FILE). + * - A string literal containing ".json", a backslash, multiple slashes, a leading + * "/" or leading "." (none of which can appear in valid block names). + * + * We skip flagging bare variables or single-slash literals to avoid false positives. + * + * @param array $param Parameter info array from PassedParameters. + * @return bool Whether the parameter looks like a path. + */ + protected function parameter_looks_like_path( array $param ) { + $string_literals = []; + $non_empty_count = 0; + + for ( $i = $param['start']; $i <= $param['end']; $i++ ) { + $token = $this->tokens[ $i ]; + + if ( isset( Tokens::$emptyTokens[ $token['code'] ] ) ) { + continue; + } + $non_empty_count++; + + if ( $token['code'] === T_DIR || $token['code'] === T_FILE ) { + return true; + } + + if ( $token['code'] === T_STRING ) { + $next = $this->phpcsFile->findNext( Tokens::$emptyTokens, $i + 1, $param['end'] + 1, true ); + $is_function_call = $next !== false && $this->tokens[ $next ]['code'] === T_OPEN_PARENTHESIS; + + if ( $is_function_call && in_array( strtolower( $token['content'] ), $this->path_functions, true ) ) { + return true; + } + + if ( ! $is_function_call && preg_match( '`_(DIR|PATH|FILE)$`i', $token['content'] ) === 1 ) { + return true; + } + } + + if ( $token['code'] === T_CONSTANT_ENCAPSED_STRING || $token['code'] === T_DOUBLE_QUOTED_STRING ) { + $string_literals[] = TextStrings::stripQuotes( $token['content'] ); + } + } + + // The whole parameter is a single string literal. + if ( $non_empty_count === 1 && count( $string_literals ) === 1 ) { + $text = $string_literals[0]; + if ( preg_match( self::VALID_BLOCK_NAME, $text ) === 1 ) { + return false; + } + + return stripos( $text, '.json' ) !== false + || strpos( $text, '\\' ) !== false + || substr_count( $text, '/' ) >= 2 + || ( isset( $text[0] ) && ( $text[0] === '/' || $text[0] === '.' ) ); + } + + /* + * String fragments within a larger expression (usually concatenation). + * A single-slash fragment such as '/my-block' could be completing a block + * name from a namespace prefix, so we look only for unambiguous signals. + */ + foreach ( $string_literals as $text ) { + if ( stripos( $text, '.json' ) !== false || substr_count( $text, '/' ) >= 2 ) { + return true; + } + } + + return false; + } +} diff --git a/HM/Tests/Functions/RegisterBlockTypePathUnitTest.inc b/HM/Tests/Functions/RegisterBlockTypePathUnitTest.inc new file mode 100644 index 00000000..9db7a40b --- /dev/null +++ b/HM/Tests/Functions/RegisterBlockTypePathUnitTest.inc @@ -0,0 +1,41 @@ + 'my_render' ] ); + +// OK: no signal we can confidently act on. +register_block_type( $block ); +register_block_type( $path ); +register_block_type( $path . '/my-block' ); +register_block_type( new WP_Block_Type( 'my-plugin/my-block' ) ); +register_block_type( MY_NAMESPACE . '/my-block' ); + +// Flagged as paths: magic constants. +register_block_type( __DIR__ . '/build/my-block' ); +register_block_type( __DIR__ . '/build/my-block/block.json' ); +register_block_type( __FILE__ ); + +// Flagged as paths: path-building functions. +register_block_type( dirname( __FILE__ ) . '/blocks/my-block' ); +register_block_type( plugin_dir_path( __FILE__ ) . 'blocks/my-block' ); +register_block_type( trailingslashit( $base ) . 'my-block' ); + +// Flagged as paths: constants named like paths. +register_block_type( MY_PLUGIN_DIR . '/blocks/my-block' ); +register_block_type( MY_PLUGIN_PATH . '/my-block' ); + +// Flagged as paths: path-like string literals. +register_block_type( 'blocks/my-block/block.json' ); +register_block_type( '/var/www/html/blocks/my-block' ); +register_block_type( './blocks/my-block' ); +register_block_type( "{$dir}/build/my-block" ); + +// Flagged as paths: fully-qualified call. +\register_block_type( __DIR__ . '/build/my-block' ); + +// Flagged, but not auto-fixable: named argument doesn't exist on replacement. +register_block_type( block_type: __DIR__ . '/build/my-block' ); + +// OK: second parameter never holds the path. +register_block_type( 'my-plugin/my-block', [ 'style' => 'file:./style.css' ] ); diff --git a/HM/Tests/Functions/RegisterBlockTypePathUnitTest.inc.fixed b/HM/Tests/Functions/RegisterBlockTypePathUnitTest.inc.fixed new file mode 100644 index 00000000..4f61e408 --- /dev/null +++ b/HM/Tests/Functions/RegisterBlockTypePathUnitTest.inc.fixed @@ -0,0 +1,41 @@ + 'my_render' ] ); + +// OK: no signal we can confidently act on. +register_block_type( $block ); +register_block_type( $path ); +register_block_type( $path . '/my-block' ); +register_block_type( new WP_Block_Type( 'my-plugin/my-block' ) ); +register_block_type( MY_NAMESPACE . '/my-block' ); + +// Flagged as paths: magic constants. +register_block_type_from_metadata( __DIR__ . '/build/my-block' ); +register_block_type_from_metadata( __DIR__ . '/build/my-block/block.json' ); +register_block_type_from_metadata( __FILE__ ); + +// Flagged as paths: path-building functions. +register_block_type_from_metadata( dirname( __FILE__ ) . '/blocks/my-block' ); +register_block_type_from_metadata( plugin_dir_path( __FILE__ ) . 'blocks/my-block' ); +register_block_type_from_metadata( trailingslashit( $base ) . 'my-block' ); + +// Flagged as paths: constants named like paths. +register_block_type_from_metadata( MY_PLUGIN_DIR . '/blocks/my-block' ); +register_block_type_from_metadata( MY_PLUGIN_PATH . '/my-block' ); + +// Flagged as paths: path-like string literals. +register_block_type_from_metadata( 'blocks/my-block/block.json' ); +register_block_type_from_metadata( '/var/www/html/blocks/my-block' ); +register_block_type_from_metadata( './blocks/my-block' ); +register_block_type_from_metadata( "{$dir}/build/my-block" ); + +// Flagged as paths: fully-qualified call. +\register_block_type_from_metadata( __DIR__ . '/build/my-block' ); + +// Flagged, but not auto-fixable: named argument doesn't exist on replacement. +register_block_type( block_type: __DIR__ . '/build/my-block' ); + +// OK: second parameter never holds the path. +register_block_type( 'my-plugin/my-block', [ 'style' => 'file:./style.css' ] ); diff --git a/HM/Tests/Functions/RegisterBlockTypePathUnitTest.php b/HM/Tests/Functions/RegisterBlockTypePathUnitTest.php new file mode 100644 index 00000000..99c5db0b --- /dev/null +++ b/HM/Tests/Functions/RegisterBlockTypePathUnitTest.php @@ -0,0 +1,47 @@ + => + */ + public function getErrorList() { + return []; + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() { + return [ + 15 => 1, + 16 => 1, + 17 => 1, + 20 => 1, + 21 => 1, + 22 => 1, + 25 => 1, + 26 => 1, + 29 => 1, + 30 => 1, + 31 => 1, + 32 => 1, + 35 => 1, + 38 => 1, + ]; + } + +}