smarty-php
Make SmartyCompilerException play nicer with error handler libraries
When a SmartyCompilerException is thrown, the Exception::$line is changed to the line of the template, but Exception::$file wasn't.
This resulted in error handler libraries (filp/whoops, nunomaduro/collision, spatie/ignition, sentry/sentry, etc.) displaying the wrong file in the error message, as they display the Exception::$file (through getFile()), but SmartyCompilerException::$template was the variable where the filename was written into. Not to mention that $line was the only property set with a setter method, everything else was public properties set directly, which was inconsistent.
This behaviour can be checked at https://original-smarty.displeases.me/ (along with the composer.json file).
In this merge request, I have written a new constructor for SmartyCompilerException, which can optionally change the file and the line if provided with these parameters.
This new constructor can be observed in action at https://fixing-smarty.displeases.me/ (along with the composer.json file).
According to the composer.json file, Smarty requires at least PHP 7.1, so the type declarations I've used should not be a problem: https://3v4l.org/o8Yrb
The unit tests provided by Smarty all passed, but me removing $source, $template, and $desc is a breaking change for any users who were manually handling (or throwing?) SmartyCompilerException themselves.
I like this fix very much, but as you mention, it would require a new major release. What if we keep the public properties template, source and desc and 'fix' the incorrect filename. I believe that would just be a bugfix. No one should have breaking issues (yes, someone always does).
Should I add them back a public properties, or as protected properties with magic getters and setters?
The former is the same it was before, so if anyone uses reflections on the exception, nothing is changed.
While the latter has the advantage, that we can (eventually?) trigger an E_USER_DEPRECATED on use (reading and writing separately), and usage remains the same too, but if someone examines the exception with a reflection, the result is different.
For now, I'd just add them back. If we choose to deprecate them in the future, option 2 can be implemented then.
Sorry about the delay
I have added back the public properties (and the no longer used setLine() function), and I fill them up again