Add option to allow heredoc after function open bracket

[igorsantos07]

Description

This may be a naive opinion, but I feel like such code shouldn't fail PSR2.Methods.FunctionCallSignature.ContentAfterOpenBracket:

throw new \Exception(<<<ERR
    Here be my long error message,
    which includes a description of what happened.
    There's also some example code of what to do instead.
    ERR,
    ErrorCodes::INVALID_WHATEVER
);

I can't see other clean way to write a Heredoc/Nowdoc onto a multiline function without spending a whole extra line just with the opening marker.

Suggested changelog entry

Add allowHereDocAfterOpenBracket option to PSR2.Methods.FunctionCallSignature

Related issues/external references

I didn't open one as the change was quite small and self-descriptive 🤷‍♂️

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
  • This change is only breaking for integrators, not for external standards or end-users.
• Documentation improvement

PR checklist

• I have checked there is no other PR open for the same change.
• I have read the Contribution Guidelines.
• I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
• I have added tests to cover my changes.

  I didn't find a way to write a test for a specific property?

• I have verified that the code complies with the projects coding standards.
• [Required for new sniffs] I have added XML documentation for the sniff.
• I have opened a sister-PR in the documentation repository to update the Wiki.

  It doesn't look there's any docs to update?

[jrfnl]

@igorsantos07 Thanks for your willingness to contribute to PHP_CodeSniffer.

Unfortunately, I'm inclined not to accept this PR.

You say you are adjusting a PSR-2 sniff, while you are in fact changing a PEAR sniff - and only by extension the PSR-2 sniff.

PSR-2 sniffs should comply with the PSR-2 standard which explicitly states:

Argument lists MAY be split across multiple lines, where each subsequent line is indented once. When doing so, the first item in the list MUST be on the next line, and there MUST be only one argument per line.

Additionally, if the second argument was on the same line as the heredoc/nowdoc closer (as allowed since PHP 7.3), PSR2 does not consider it a multi-line function call,
In other words, your code would be accepted without issues by the sniff if written like this:

throw new \Exception(<<<ERR
    Here be my long error message,
    which includes a description of what happened.
    There's also some example code of what to do instead.
    ERR, ErrorCodes::INVALID_WHATEVER
);

(hey, saves you another line...)

If you don't agree with the PSR-2 rule, either use a different sniff for function call formatting, or start a discussion in the FIG about changing the function call formatting rules in PER.

At a code technical level:

• ❌ Your PR description talks about PSR-2, while the actual sniff adjustment is made in the PEAR sniff, which means that the change has a much wider impact.
• ❌ The change is not accompanied by tests covering the impact of it (wherever it has one).
• As a general rule of thumb, adding new public properties options is discouraged in favour of different error codes.
  Now I can see why you went with a new option for this change, but I can't justify a new option being added if this addresses a problem perceived by one person only.
• ❌ The PR is not accompanied by a PR to the doc repo to document the new property.

I'm sorry for the negative reception, but this is exactly why I encourage people to first open an issue.

[igorsantos07]

Sorry? You're sorry for such a thorough answer to a PR written in less than an hour? Which you also answered in less than an hour?

This is simply awesome, whatever final outcome is it.
Writing the PR was fun anyway, and way less daunting than I expected it to be. Not to mention... Definitely not used to gentle OS maintainers 👏🏻

I went ahead with the PR as I'm used to issues dragging for weeks before any decision - with the PR, worst case scenario, we can keep using the fork until we settle on a code style for this scenario :)

I won't address the other points as the FIG suggestion does kill it all, at least for now.

Thanks for rocking the OS land!

[jrfnl]

@igorsantos07 Thank you for taking my feedback in the spirit it was intended. I always feel harsh when rejecting PRs outright, so it was nice to read your response on how it actually comes across.

Writing the PR was fun anyway, and way less daunting than I expected it to be.

Glad to hear it and I look forward to future interactions/PRs.