Update deprecation notices
Description
This PR will move deprecation notices from error_log to trigger_error. It will cause all deprecation notices in the unit tests to be expected and tested against. The test suite output should now be cleaner and considered 'passing'.
There's some issues with utilizing error_log in a package.
- No Immediate Notification: error_log simply writes a message to the error log without integrating with the error handling system. This means that the user may never notice that a deprecated feature is being used.
- Lack of Flexibility: With error_log, there’s no opportunity for developers to intercept or act upon the deprecation notice during execution.
Whereas, trigger_error will allow the developer to catch this notice. trigger_error() emits an error that goes through PHP’s error handling system. This means that deprecation warnings can be captured by custom error handlers or displayed in development environments.
Given that deprecation warnings are meant to notify developers without altering the core behavior of the package, this change should seen as non-breaking from an API perspective and can be added to a minor release.
Overview
Support
PHP >= 7.3 supports trigger_error.
Implementation
Instead of calling error_log(), developers of this package should call trigger_error($msg, E_USER_DEPRECATED).
This is the only change required.
if (!is_null($actors)) {
$msg = "'actors' is deprecated. Please use 'actorNames' instead";
trigger_error($msg, E_USER_DEPRECATED);
$params["actors"] = $actors;
};
Developers of this package should continue to use the @deprecated docblock to help IDEs recognize the deprecation.
/*
* ...
* @var null|array $actors Actor names that Audit Log Events will be filtered by. @deprecated 3.3.0 Use $actorNames instead. This method will be removed in a future major version.
*/
Testing
There is a new TestHelper method called assertDeprecationTriggered that will accept two variables: The deprecation method you're expecting and the method call that is going to throw it. The test will fail if the $expected_warning is not thrown.
Example:
$auditLogExport = $this->assertDeprecationTriggered(
"'actors' is deprecated. Please use 'actorNames' instead",
fn() => $this->al->createExport($organizationId, $rangeStart, $rangeEnd, $actions, $actors, $targets, $actorNames, $actorIds)
);
The rest of the test will be evaluated as expected.
Future Considerations
PHP 8.4 introduced a Deprecated attribute. That would update our syntax.
#[\Deprecated(message: "use safe_replacement() instead", since: "1.5")]
The package currently supports >=7.3.0, so I did not implement this. If we want to future proof our deprecation notices, we could look at conditionally including the attribute. This felt too messy without much upside, so I am not attempting it.