In #3048495-7: Fix Drupal.Semantics.FunctionTriggerError coding standard Lendude discovered a bug in the new sniff where text inside a call to sprintf() which is inside the @trigger_error() is not properly processed.
I've done a quick bit of investigation and it is not a problem with the regex, it is due to $argument['end'] not pointing to the text to test for. I'll work on a fix for this.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | coder semantics test output.png | 66.3 KB | jonathan1055 |
| #5 | 3049093-5.sprint_in_trigger_error.test_data.patch | 5.71 KB | jonathan1055 |
Comments
Comment #2
jonathan1055 commentedHere is a patch which add sprintf to the test data. The tests now fail, as shown in the screen grabs.
Comment #3
jonathan1055 commentedHere's a patch which also includes the fix to
coder/coder_sniffer/Drupal/Sniffs/Semantics/FunctionTriggerErrorSniff.phpThe attached screen-grab shows the output when run on the core file
ReverseProxyMiddleware.phpThe fix was quite simple. Instead of
which assumes there is only one item in the message, we now have
which finds the actual text.
This will need a pull request on github, but at least anyone can use this patch now if they want to work on fixing the core files.
Comment #4
klausiMakes sense, can you open a pull request against https://github.com/pfrenssen/coder so that we see the automated test runs?
It looks like you are replacing a bad example. Instead, we should add another bad example so that we don't lose coverage.
Comment #5
jonathan1055 commentedYes, I'll create a PR.
I have altered the UnitTest.inc file to make it easier to add new rows. All the failures are now in one block on every other line, with only one line of comment between. This means we I add a 'good' row it is easier to adjust the expected rows array.
Also attached is my local phpunit result.
Comment #6
jonathan1055 commentedThe pull request is https://github.com/pfrenssen/coder/pull/33
all tests pass
Comment #8
klausiThanks a lot Jonathan, commited!
Comment #9
jonathan1055 commentedThanks for merging and committing.
Just asking, does Coder (i.e you and @pfrenssen) have a policy on (a) when to release a new version and (b) when drupal.org should get that version. As the fixing of trigger_error for standards is currently being worked on in #3048495: Fix Drupal.Semantics.FunctionTriggerError coding standard will there be a 3.4 release soon so that those users can get the corrected sniff? I am aware that you just had to release 3.3 and that drupal.org is still on 3.2 - see #3049433: Coding standards not running on d.o. - upgrade to coder 8.3.4
Comment #10
klausiWe don't have an official release policy. My general rule is to do a release every 3 months if there are changes on the dev branch.
However, we can do releases more frequently if required by Drupal core. We should make sure that #3048495: Fix Drupal.Semantics.FunctionTriggerError coding standard runs the dev version of Coder and everything is working fine before making a release.
Comment #11
jonathan1055 commentedThanks @klausi for the info.
I expect you might know this already, but there is more discussion on #3024461-89: Adopt consistent deprecation format for core and contrib deprecation messages and the sniffs may need to change. So hold off from tagging a new coder release just yet, as we may need another quite soon.
Comment #12
jonathan1055 commentedFixed so unassigning myself.