Skip to content

Conversation

@davidperezgar
Copy link
Member

Fixes #1146

Description

This PR improves the direct_file_access check to use AST parsing with nikic/php-parser to 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

  • Implemented AST parsing using nikic/php-parser to analyze PHP files for structural code only
  • Removed deprecated imports - Replaced PhpParser\Node\Scalar\* class imports with string class name comparisons to avoid deprecation warnings
  • Enhanced detection logic:
    • Files with only class/interface/trait/enum definitions are now correctly identified as safe
    • Files with namespace declarations and use statements are handled recursively
    • Asset files (files with only assignments and return statements) are properly allowed
    • Safe if statements (with class_exists, function_exists, etc.) are now recognized as safe
    • Top-level function declarations are correctly flagged as unsafe (procedural code)
  • Refactored code complexity - Split is_safe_expression() into smaller, focused methods to reduce cyclomatic complexity from 28 to below threshold
  • Added PHPStan ignore rules for property access on dynamically typed PhpParser nodes
  • Maintained backward compatibility - Falls back to regex-based checking if AST parsing fails

Credits

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

  • Reduces false positives - Class-only files no longer trigger unnecessary warnings
  • Better developer experience - Improves signal-to-noise ratio of Plugin Check
  • More accurate detection - AST parsing provides precise analysis of file structure
  • Aligns with modern practices - Better support for PSR-style and autoloaded plugin architectures
  • Maintains security - Still correctly flags files with procedural code that need guards

Testing Instructions

  1. Test class-only files:

    • Create a PHP file with only a class definition (no ABSPATH guard)
    • Run Plugin Check on the plugin
    • Expected: No error should be reported for the class-only file
  2. Test interface/trait-only files:

    • Create files with only interface or trait definitions
    • Run Plugin Check
    • Expected: No errors for these files
  3. Test procedural code:

    • Create a PHP file with a top-level function declaration (no ABSPATH guard)
    • Run Plugin Check
    • Expected: Error should be reported requiring ABSPATH guard
  4. Test asset files:

    • Create a file with only variable assignments and return statements (like build asset files)
    • Run Plugin Check
    • Expected: No error should be reported
  5. Test safe function calls:

    • Create a file with if statements checking class_exists() or function_exists() with return statements
    • Run Plugin Check
    • Expected: No error should be reported
  6. Run PHPUnit tests:

    npm run test-php -- --filter Direct_File_Access_Check_Tests
    • Expected: All tests should pass

Checklist

  • Code follows WordPress Coding Standards
  • Self-reviewed the code
  • Added necessary comments
  • No new linter errors (PHPStan ignores added for expected dynamic property access)
  • PHPMD complexity issues resolved
  • Tests updated/verified to work with new implementation
  • Backward compatibility maintained with regex fallback

@github-actions
Copy link

github-actions bot commented Jan 3, 2026

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: davidperezgar <davidperez@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@davidperezgar davidperezgar added this to the 1.9.0 milestone Jan 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve direct_file_access check to ignore class-only files

2 participants