Undefined array key "parenthesis_closer"

[LukeWCS]

Describe the bug

--------------------------------------------------------------------------------
 1 | ERROR | An error occurred during processing; checking has been aborted.
   |       | The error message was: Undefined array key "parenthesis_closer" in
   |       | /vendor/squizlabs/php_codesniffer/src/Standards/Generic/Sniffs/Functions/OpeningFunctionBraceBsdAllmanSniff.php
   |       | on line 75
   |       | The error originated in the
   |       | Generic.Functions.OpeningFunctionBraceBsdAllman sniff on line 75.
--------------------------------------------------------------------------------

Code sample

Line 101 in acp_controller.php (see attachment)

	public function display_options(string): array

codesniffer#1423.zip

Variant 2

		public function add_page_header_link

To reproduce

Steps to reproduce the behavior:

1. Create a file called test.php with the code sample above...
2. Run phpcs test.php ...
3. See error message displayed

PHPCS output here

Expected behavior

No program error. ^^

Versions (please complete the following information)

Operating System	Windows 10/Linux
PHP version	8.5
PHP_CodeSniffer version	4.0.1 (0525c73)
Standard	Generic
Install type	Composer

Additional context

Hi Juliette

While sporadically reviewing the reports from my test suite "phpBB Ext Check," I noticed this error in one of the reports. Since all files are deleted from the server after the check, I can't post the code causing the error here. However, if the information provided here isn't sufficient to locate the problem, I would contact the developer to request the ZIP file that contains the error.

Please confirm

• I have searched the issue list and am not opening a duplicate issue.
• I have read the Contribution Guidelines and this is not a support question.
• I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
• I have verified the issue still exists in the 4.x branch of PHP_CodeSniffer.

[jrfnl]

However, if the information provided here isn't sufficient to locate the problem, I would contact the developer to request the ZIP file that contains the error.

@LukeWCS Having the code available which triggers the error would really really help as otherwise it will be up to the (limited) fantasy of whomever tries to come up with a code sample to reproduce this and that code sample may still end up being completely different.
Of course, we could blindly add defensive coding, but that may still break things or "fix" things in the wrong place (in the sniff vs the tokenizer), so I'd rather see the code sample so we can determine what the correct fix should be.

I'll mark this issue as "awaiting feedback" until a code sample which can be used to reproduce the issue has been added.

[LukeWCS]

Just a quick update: all right, then I'll contact the developer and ask if he still has the ZIP file or if he remembers exactly what he changed in the code that caused this problem.

edit: Just FYI: Re: phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

[rodrigoprimo]

I was able to come up with a "live coding" sample that allows reproducing this issue:

<?php function foo {}

This problem started happening after da0d224. That commit refactored Tokenizer::createTokenMap() to set the parenthesis_opener/parenthesis_closer keys on parenthesis owner tokens (like T_FUNCTION) only if a ( is found after the token. Previously, these keys were always set (to null) on all parenthesis owners upfront.

In PHPCS 3.x, parenthesis_closer = null was always set on all parenthesis owner tokens upfront, so this warning was not triggered.

@jrfnl, let me know if you would like me to submit a PR to fix this issue. I think it should be pretty straightforward.

Additional notes

On a quick search, I noticed that Squiz.Functions.FunctionDuplicateArgument is also affected. I was not able to find other sniffs that are affected, but it is worth double checking.

I'm not sure if it is too late for that and if this change is intentional, but I wonder if the 4.0 upgrade guide should be updated to mention this change?

[jrfnl]

let me know if you would like me to submit a PR to fix this issue. I think it should be pretty straightforward.

Please hold off until the real code sample is known. As I stated above, it's not difficult to come up with a code sample to trigger this, but the real code sample may trigger the issue in a completely different way. Better know for sure what we're dealing with.

Additional notes

On a quick search, I noticed that Squiz.Functions.FunctionDuplicateArgument is also affected. I was not able to find other sniffs that are affected, but it is worth double checking.

👍🏻

I'm not sure if it is too late for that and if this change is intentional, but I wonder if the 4.0 upgrade guide should be updated to mention this change?

It's in the changelog for the 4.0.0beta1 release: https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/4.x/CHANGELOG-4.x.md#fixed-3

[rodrigoprimo]

Please hold off until the real code sample is known. As I stated above, it's not difficult to come up with a code sample to trigger this, but the real code sample may trigger the issue in a completely different way. Better know for sure what we're dealing with.

👍🏻

It's in the changelog for the 4.0.0beta1 release: https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/4.x/CHANGELOG-4.x.md#fixed-3

What I have in mind is a note in the 4.0 developer upgrade guide documenting that there are now scenarios where scope_* is set but parenthesis_* is not.

[LukeWCS]

The developer was indeed able to reconstruct the temporary state, and I was also able to immediately reproduce the problem in my phpBB Ext Check development environment. I've added the information to the initial post here and also uploaded an archive containing the complete Ext Check report and the problematic file of the phpBB extension in question. See "Code Sample".

You can find the error message in the report within the "phpBB PHP Strict Standard Extensions" module. The crucial clue as to where the problem might lie was provided by the "Extension Pre Validator" module.

[jrfnl]

It's in the changelog for the 4.0.0beta1 release: https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/4.x/CHANGELOG-4.x.md#fixed-3

What I have in mind is a note in the 4.0 developer upgrade guide documenting that there are now scenarios where scope_* is set but parenthesis_* is not.

IIRC that was always already possible, so that's not new.

[rodrigoprimo]

Thanks for updating the issue description with the code sample, @LukeWCS.

I'm able to reproduce the problem with this code sample if I add the opening and closing braces (without that scope_opener is not set for T_FUNCTION and the sniff bails before line 75):

<?php

public function display_options(string): array {}

As far as I can see, the cause of this problem is the same as what I described above (#1423 (comment)).

[LukeWCS]

@rodrigoprimo

Thanks for updating the issue description with the code sample, @LukeWCS.

No problem. To be honest, I had a bad feeling when I arrived here without a code sample. ^^ The thing is, my test suite actually has a mechanism that automatically saves the report and the uploaded ZIP file to a separate folder in case of an exception (any kind of technical malfunction), so I can review it later at my leisure. With Ext Check, all uploaded files are normally deleted immediately after the check, or at the latest after 5 minutes. Reports are retained for 48 hours so developers can save them if they wish.

The problem here, however, as you can see from the report, was that EC didn't detect the exception error, but instead reported a regular analysis error. I wish CS would return a clear RC code for such technical problems. In CS4, these are bits; perhaps Juliette could add another RC bit for such issues? Looks at Juliette with big cat eyes