Improve direct_file_access check to ignore class-only files#1147
Open
davidperezgar wants to merge 2 commits intotrunkfrom
Open
Improve direct_file_access check to ignore class-only files#1147davidperezgar wants to merge 2 commits intotrunkfrom
direct_file_access check to ignore class-only files#1147davidperezgar wants to merge 2 commits intotrunkfrom
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
ernilambar
approved these changes
Jan 9, 2026
marekdedic
reviewed
Jan 10, 2026
| */ | ||
| private function is_ast_valid_for_direct_access( array $ast ) { | ||
| $safe_node_types = array( | ||
| Stmt\Nop::class, |
There was a problem hiding this comment.
I'm not that familiar with the AST, but probably there should be Stmt\Declare_::class as well - if I'm reading it correctly, this would also allow
declare(strict_types=1);
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1146
Description
This PR improves the
direct_file_accesscheck to use AST parsing withnikic/php-parserto accurately detect files that only contain structural code (classes, interfaces, traits, namespaces) and skip them from requiring direct access guards. This reduces false positives for modern, OOP-based plugins that follow PSR-style and autoloaded architectures.The implementation uses Abstract Syntax Tree (AST) parsing to detect whether a file contains only structural code without executable statements, making the check more accurate than the previous regex-based approach.
Changes
nikic/php-parserto analyze PHP files for structural code onlyPhpParser\Node\Scalar\*class imports with string class name comparisons to avoid deprecation warningsclass_exists,function_exists, etc.) are now recognized as safeis_safe_expression()into smaller, focused methods to reduce cyclomatic complexity from 28 to below thresholdCredits
This implementation is based on the AST parsing approach used in the internal plugin review scanner, originally developed by @frantorres. The logic for detecting safe structural code has been adapted and integrated into the public Plugin Check tool.
Benefits
Testing Instructions
Test class-only files:
Test interface/trait-only files:
Test procedural code:
Test asset files:
Test safe function calls:
class_exists()orfunction_exists()with return statementsRun PHPUnit tests:
Checklist