PHP Code Review Checklist: Modern PHP Standards and Security

Published on
Written byChristoffer Artmann
PHP Code Review Checklist: Modern PHP Standards and Security

PHP's flexibility makes it easy to write code that looks correct but harbors serious security vulnerabilities. A loose comparison instead of strict can bypass authentication. Missing output escaping exposes users to XSS. User input concatenated into SQL queries creates injection vulnerabilities. These issues hide in code that passes casual review.

This checklist helps catch what matters: security vulnerabilities, outdated patterns that modern PHP handles better, and architectural decisions that create maintenance problems.

Quick Reference Checklist

Use this checklist as a quick reference during code review. Each item links to detailed explanations below.

Type Safety & Modern PHP

  • Files declare strict types at the top: declare(strict_types=1)
  • All comparisons use strict equality === and !== instead of loose == and !=
  • Function parameters have type declarations: function createUser(string $name, int $age): User
  • Return types are declared on all functions
  • Class properties use type declarations (PHP 7.4+)
  • Nullable types ?User are intentional, not design uncertainty
  • Union types int|float express clear intent (PHP 8.0+)
  • Functions returning objects never return null unless declared nullable

Security Fundamentals

  • All database queries use prepared statements with parameter binding: $stmt->execute([$userId])
  • No user input concatenates directly into SQL strings: "WHERE id = $userId" is vulnerable
  • Raw query methods DB::raw() and executeQuery() avoid user input
  • All user output uses context-appropriate escaping: htmlspecialchars($userInput, ENT_QUOTES, 'UTF-8')
  • Template auto-escaping is enabled; raw output {{ var|raw }} and {!! $var !!} is intentional and safe
  • File uploads validate actual content type with finfo_file(), not just extensions
  • Uploaded files stored outside web root or with non-executable permissions
  • Passwords use password_hash() with PASSWORD_BCRYPT or PASSWORD_ARGON2ID, never md5() or sha1()
  • Session regeneration session_regenerate_id(true) called after login to prevent session fixation
  • CSRF tokens present on all state-changing forms
  • Sensitive configuration (API keys, credentials) not committed to version control

Modern PHP Patterns

  • Constructor property promotion used for simple value objects: public function __construct(public string $name) (PHP 8.0+)
  • Named arguments improve clarity: createUser(name: $name, email: $email, role: 'admin')
  • Match expressions used for simple value mapping instead of switch
  • Dependencies injected through constructors, not instantiated with new
  • Dependency injection uses interfaces UserRepositoryInterface for flexibility
  • Code follows PSR-12 style standards
  • Namespaces follow PSR-4 autoloading conventions
  • Namespace organization reflects domain concepts App\Domain\User\UserRepository, not technical roles App\Repositories

Error Handling

  • Try-catch blocks either recover gracefully or fail explicitly, not silent catch (Exception $e) { }
  • No empty catch blocks that swallow exceptions silently
  • Error suppression operator @ absent or justified
  • Logging happens where errors occur or are handled, not both
  • User-facing errors show generic messages, not system details like $e->getMessage()
  • Detailed errors logged for debugging
  • External service calls have fallback strategies
  • Failures degrade gracefully rather than breaking entire features

Dependencies & Architecture

  • Composer version constraints specific "^8.0", not wildcards "*"
  • Development dependencies in require-dev, not require
  • Production dependencies not in require-dev
  • Framework conventions followed (routing, ORM, templates)
  • Business logic independent of framework-specific classes
  • Autoloading uses PSR-4 for application code
  • Namespace structure matches directory structure
  • No references to framework Request/Response objects in domain logic

Type Safety in Modern PHP

PHP's evolution toward stricter typing represents one of the most significant improvements in the language's history, yet many codebases mix modern type declarations with legacy patterns that undermine those benefits. When we review PHP code, the presence or absence of strict types fundamentally changes what we need to verify. Code that declares declare(strict_types=1) at the top of files operates under different rules than code without this declaration, and that difference has profound implications for reliability.

Consider authentication code that compares user input against stored credentials. Without strict types, PHP performs type juggling that creates security holes. A comparison like if ($userInput == $storedValue) becomes dangerous because PHP coerces types during comparison. The string "0e123" equals the string "0e456" under loose comparison because PHP interprets both as scientific notation representing zero. An attacker who discovers this can bypass authentication by providing specially crafted input. With strict comparison using ===, this vulnerability disappears, but strict comparison requires conscious choice in every comparison. With declare(strict_types=1), the entire file operates under stricter rules that prevent many type-related bugs.

The presence of type declarations on function parameters and return values signals modern PHP usage, but we need to verify those declarations are meaningful and complete. A function that declares string as a parameter type but then checks if (empty($param)) reveals uncertain design—empty string is a valid string, so why does the function treat it specially? This suggests the developer wants a non-empty string but lacks the type system to express that requirement. Better design either uses a value object that enforces non-empty constraints or documents the validation explicitly rather than hiding it in runtime checks.

Property type declarations, introduced in PHP 7.4 and improved in PHP 8.0, deserve similar scrutiny. A class with some properties typed and others untyped usually indicates incomplete migration from older PHP versions. This inconsistency matters because typed properties provide documentation and prevent entire classes of bugs where wrong types get assigned. When we see untyped properties, we should ask whether there's a reason they can't be typed or whether the developer simply hasn't updated them yet.

Union types and nullable types, available in PHP 8.0+, make intentions explicit in ways older PHP couldn't. A parameter declared as ?string explicitly allows null, while string does not. This clarity helps, but we should verify that nullable types are intentional rather than artifacts of uncertain design. A function accepting ?array as a parameter but immediately calling $array = $array ?? [] suggests the function really wants to guarantee an array internally. Making the parameter non-nullable with a default value of [] expresses this intent more clearly and pushes the decision to the caller where it belongs.

The interaction between return types and exception handling particularly matters in code review. A function declared to return User should return a User object or throw an exception—returning null violates the type contract. Yet we often see code that declares return types but then returns null in error cases, creating runtime type errors when callers expect the declared type. This usually indicates the function should either return ?User to allow null, throw an exception when the user isn't found, or use a Maybe/Optional pattern. The choice depends on context, but the type declaration should accurately reflect what callers can expect.

Security Fundamentals That Matter

SQL injection remains the most common critical vulnerability in PHP applications despite decades of awareness and readily available solutions. When reviewing database queries, we need to verify that every query uses prepared statements or query builders that safely handle parameters. The pattern we're looking for is parameter binding, where user input never concatenates directly into SQL strings. An innocent-looking query like $db->query("SELECT * FROM users WHERE id = $userId") contains a vulnerability because $userId comes from user input and concatenates directly into the query. An attacker can provide "1 OR 1=1" as the user ID and access all users.

The fix seems obvious—use prepared statements—but PHP's database APIs offer multiple ways to use prepared statements incorrectly. With PDO, we want to see placeholders in the query and values bound separately: $stmt = $db->prepare("SELECT * FROM users WHERE id = ?"); $stmt->execute([$userId]). This prevents SQL injection regardless of what $userId contains because the database treats it as data, not executable SQL. But if we see $db->prepare("SELECT * FROM users WHERE id = $userId"), the prepared statement accomplishes nothing—the user input still concatenates into the query string before preparation.

Framework query builders add another layer to verify. Laravel's Eloquent, Symfony's Doctrine, and similar ORMs automatically use prepared statements for parameter binding, making them safer by default. When reviewing framework code, we look for raw queries that bypass the ORM's protections. Methods like DB::raw() or $em->getConnection()->executeQuery() allow raw SQL and require the same scrutiny as PDO queries. If user input appears anywhere in raw SQL, that's a vulnerability.

Cross-site scripting vulnerabilities arise when user input displays in HTML without proper escaping. PHP templates often mix logic and output, creating numerous opportunities for XSS. The key distinction is whether output undergoes context-appropriate escaping before rendering. In HTML context, we need HTML entity encoding: <?= htmlspecialchars($userInput, ENT_QUOTES, 'UTF-8') ?>. But developers often forget this, writing <?= $userInput ?> instead, which renders user input directly into HTML. An attacker can inject <script>alert('XSS')</script> and compromise other users' sessions.

Template engines like Twig and Blade auto-escape by default, providing better protection than raw PHP templates. When reviewing framework code, we verify that auto-escaping is enabled globally and look for places where developers bypass it. Twig's raw filter and Blade's {!! $variable !!} syntax disable escaping, which is sometimes necessary for trusted HTML but dangerous for user input. Any use of these features deserves careful examination to ensure the data is truly safe.

File upload handling presents a different class of security concerns. When users upload files, we need to verify the code validates file types, restricts file sizes, and stores files outside the web root or with permissions that prevent execution. Code that checks file extensions using the uploaded filename is insufficient—attackers can upload evil.php with a .jpg extension and execute it if the server doesn't verify the actual file type. Better validation checks MIME types and uses libraries like fileinfo to verify file contents match their claimed type.

Authentication and session management require scrutiny because mistakes expose entire applications. Password hashing should use password_hash() with bcrypt or Argon2, never MD5 or SHA1. The code password_hash($password, PASSWORD_BCRYPT) is correct, while md5($password) is critically flawed. Sessions should use secure, HTTP-only cookies, which we verify through configuration rather than code. The presence of session_regenerate_id() after login prevents session fixation attacks—its absence is a vulnerability. CSRF protection should appear on every state-changing form, typically through tokens that verify requests originated from the application rather than malicious sites.

Modern PHP Patterns and Architecture

Constructor property promotion, introduced in PHP 8.0, significantly reduces boilerplate in value objects and DTOs. Instead of declaring properties, assigning them in the constructor, and often adding getters, we can write public function __construct(public string $name, public int $age) and get all three automatically. When reviewing code that creates many simple objects, the presence of constructor property promotion suggests modern PHP knowledge, while verbose constructors with manual property assignment suggest older patterns. The choice matters less for single occurrences but significantly impacts maintenance when dozens of classes follow the same pattern.

Named arguments, also from PHP 8.0, improve code clarity when functions accept many parameters or when default parameters make positional arguments confusing. A call like createUser(name: $name, email: $email, role: 'admin') documents what each argument means better than createUser($name, $email, 'admin'). When reviewing function calls with multiple boolean or string parameters, named arguments often clarify intent. Their absence isn't necessarily wrong, but unclear parameter meanings suggest named arguments would help.

Match expressions provide a cleaner alternative to switch statements for simple value mapping. The expression $message = match($status) { 'pending' => 'Awaiting approval', 'approved' => 'Approved', 'rejected' => 'Rejected' } returns a value directly, handles unknown values by throwing an exception, and prevents fall-through bugs that plague switch statements. When we see switch statements that just assign values, match expressions usually improve the code. Switch statements remain appropriate for complex multi-statement cases, but simple value mapping benefits from match expressions.

Dependency injection patterns indicate architectural maturity. Code that instantiates dependencies directly using new creates tight coupling and prevents testing. A controller that contains $userRepository = new UserRepository($db) tightly couples to the specific repository implementation. Better design accepts dependencies through the constructor: public function __construct(private UserRepositoryInterface $userRepository). This allows different implementations, including test doubles, without changing the controller. When reviewing classes that use new to create dependencies, we should consider whether dependency injection would improve testability and flexibility.

PSR standards provide agreed-upon conventions that reduce cognitive load when reading code. PSR-12 defines code style, PSR-4 specifies autoloading, and various PSRs standardize interfaces for common concerns like HTTP messages and logging. Code that follows these standards integrates better with third-party packages and requires less time to understand. When reviewing code, significant deviations from PSR standards—unusual indentation, non-standard namespace structures, custom logging implementations—suggest either ignorance of conventions or deliberate choices that deserve explanation.

Namespace organization reflects architectural thinking. Well-organized code groups related classes in namespaces that mirror domain concepts: App\Domain\User\UserRepository, App\Domain\Order\OrderService. Poorly organized code dumps everything in flat namespaces or creates namespace hierarchies that don't match the actual relationships between classes. The presence of App\Services, App\Repositories, App\Controllers as top-level namespaces suggests organization by technical role rather than domain concept, which makes finding related code harder as applications grow.

Error Handling and Resilience

Exception handling in PHP serves two purposes: recovering from expected error conditions and failing safely when recovery isn't possible. When reviewing try-catch blocks, we need to distinguish between these purposes. A catch block that logs an exception and returns a default value attempts recovery: try { $user = $api->fetchUser($id); } catch (NotFoundException $e) { $user = null; }. This might be appropriate if the absence of a user is an expected condition that the code handles. But a catch block that swallows exceptions without handling them—catch (Exception $e) { }—hides problems instead of addressing them.

The error suppression operator @ represents one of PHP's most abused features. Placing @ before a function call suppresses errors that function might generate: $result = @file_get_contents($url). This prevents error messages but doesn't actually handle the error—if the file can't be read, $result becomes false, and subsequent code may fail in confusing ways. Better code handles errors explicitly: check return values, catch exceptions, or accept that errors should surface rather than hiding them. When we see @ in code review, it almost always indicates a problem that needs better handling.

Logging practices reveal how teams approach debugging and monitoring. Code that logs before throwing exceptions— $logger->error("User not found: $id"); throw new NotFoundException()—provides context that exception handlers might lack. Code that catches exceptions, logs them, and rethrows them creates duplicate log entries without adding value. Logging should happen either where the error occurs (before throwing) or where the error is handled (in catch blocks that don't rethrow), but not both. When we see extensive logging in happy-path code, that suggests debugging cruft that should be removed or converted to debug-level logging that doesn't run in production.

The distinction between logging errors and displaying them matters enormously in production. Development environments should display errors to aid debugging, but production environments should log errors and display generic messages to users. Code that contains echo $e->getMessage() or similar direct error display creates security vulnerabilities by exposing system details to attackers. When reviewing error handling, we verify that user-facing messages provide helpful context without revealing sensitive information, while logs contain detailed diagnostic data.

Fail-safe patterns ensure that errors degrade functionality gracefully rather than breaking entire features. A feature that fetches user preferences from a cache should fall back to database reads if the cache is unavailable, and fall back to default preferences if the database is unavailable. This layered approach keeps the application functional even when components fail. When reviewing code that depends on external services or infrastructure, we look for fallback strategies that prevent cascading failures.

Dependencies and Framework Considerations

Composer dependency management has become standard in modern PHP, but reviewing composer.json reveals important details about project maintenance. Wildcard version constraints like "vendor/package": "*" allow any version, creating reproducibility problems when dependencies update unexpectedly. Version constraints should be specific enough to prevent breaking changes while allowing compatible updates: "^8.0" allows versions from 8.0 up to but not including 9.0. When we see wildcard constraints or very loose version ranges, that signals potential stability issues.

The presence of development dependencies separate from production dependencies indicates proper dependency management. Tools like PHPUnit, PHP CS Fixer, and PHPStan belong in require-dev because production environments don't need them. When we see testing frameworks in the main require section, that bloats production deployments unnecessarily. Conversely, production dependencies that appear only in require-dev will break production deployments.

Framework usage patterns significantly impact maintainability. Code that fights against framework conventions—bypassing routing systems, working around the ORM, implementing custom session handling—creates maintenance burden without corresponding benefit. Laravel applications should use Eloquent or Query Builder for database access, follow controller conventions, and use Blade templates. Symfony applications should follow Symfony's structure. When we see framework applications that contain raw SQL queries, manual session manipulation, or custom authentication systems, we should question whether those deviations solve real problems or reflect lack of framework knowledge.

Framework-agnostic code has its place in domain logic that shouldn't depend on delivery mechanisms. A service that processes payments should work independently of whether it's called from a Laravel controller, a Symfony command, or a standalone script. When reviewing business logic, tight coupling to framework features suggests poor separation of concerns. References to Request, Response, or other framework objects deep in business logic indicate the code mixes concerns that should be separate.

Autoloading configuration affects performance and correctness. PSR-4 autoloading maps namespaces to directory structures, enabling efficient class loading. When we see composer.json with classmap or files autoloading for application code rather than just third-party packages, that suggests non-standard organization. Classmap autoloading works but requires running composer dump-autoload after adding new classes, while PSR-4 autoloading works immediately if files follow namespace conventions. Files autoloading, which loads specific files on every request, can significantly impact performance if overused.

The relationship between namespaces and directory structure should be obvious and consistent. A class App\Domain\User\UserRepository should exist at src/Domain/User/UserRepository.php or similar. When these don't match, autoloading fails or requires manual intervention. In code review, we verify that new classes follow established namespace conventions and that their physical location matches their namespace.

Bringing It Together

Effective PHP code review combines attention to modern language features, security awareness, architectural understanding, and practical judgment about what matters. Not every issue carries equal weight—a SQL injection vulnerability demands immediate attention, while a missing type declaration on a private method might not warrant blocking the pull request. The key is developing judgment about which patterns indicate serious problems and which represent stylistic choices or minor improvements that can wait.

As PHP continues evolving, our review criteria must evolve too. Features that were optional in PHP 7.4 become standard practice in PHP 8.2. Security threats change as attackers discover new techniques. Framework best practices shift as communities learn better patterns. Staying current requires continuous learning, but the fundamental principles remain constant: verify that code is secure, maintainable, and appropriate for the problem it solves.

The goal of code review isn't finding every possible improvement—it's preventing significant problems while helping the team maintain consistent, high-quality code. Better review practices catch security vulnerabilities before they reach production, identify architectural problems before they multiply throughout the codebase, and create learning opportunities that improve everyone's skills. When code review becomes systematic rather than arbitrary, teams ship better software with fewer critical issues escaping to production.