[xml doc] NewGroupUseDeclarations and NewGroupUseConstFunction

[Flyingmana]

covers PHPCompatibility.UseDeclarations.NewGroupUseDeclarations and PHPCompatibility.UseDeclarations.NewUseConstFunction.

I can also split it up if needed.

For NewGroupUseDeclarations I was not sure how to properly split the 2 cases up, so I stayed with the cross compatible variant for both.

[jrfnl]

Hi @Flyingmana, thank you for your willingness to contribute!

I'm sorry it took a while before I got to reviewing the PR, but I was on a break for most of January/February and am still playing catch-up now.

To start with your questions:

I can also split it up if needed.

I'm fine with leaving this PR as-is, but would prefer one PR per doc moving forward as that way when one doc still needs fixes, it won't block other docs from being merged in.

For NewGroupUseDeclarations I was not sure how to properly split the 2 cases up, so I stayed with the cross compatible variant for both.

You can use multiple <standard> blocks in the XML docs, each with their own set of <code_comparion>s.

As that particular sniff is looking for two distinct changes, I believe that would be the way to go for that sniff.

For the second part (the trailing comma's), I'd expect the "valid" code sample to show a group use statement without a trailing comma.
For the "valid" code sample description, I'd suggest something like "PHP >= 7.0: Group Use Declarations without trailing commas."

PR review

I've reviewed the docs in detail. Please find my feedback below.

I hope the review helps and is clear enough. Please let me know if you have any questions.

To start, the directory name used in the Docs folder does not match the directory name used in the Sniffs folder, which means the docs are not accessible from the command line (at all).
Please change the subdirectory name in the Docs folder to UseDeclarations to make the docs usable.

Docs review checklist for PHPCompatibility.UseDeclarations.NewGroupUseDeclarations

• ✔️ Only space indentation is used.
• ⚠️ Indentation is consistent - with the exception of the indentation for the documentation attributes and closer.
• ✔️ The title matches the sniff name (or is close enough).
• ❌ Separate error codes have a separate <standard> block with their own code samples.
  (see my answer to your question about this above)
• ⚠️ The "standard" description explains sufficiently what was changed in PHP.
  Let me know if you'd like a suggestion on how to improve this.
• ✔️ The "standard" description uses proper punctuation.
• ✔️ Code sample descriptions use correct prefixes.
• ✔️ < and > are encoded in the code sample descriptions.
• ⚠️ The code sample descriptions are descriptive enough.
  See my suggestion above for the second "valid" example.
• ✔️ Code sample descriptions use proper punctuation.
• ⚠️ The code samples demonstrate the issue sufficiently.
  See my suggestion above for the second "valid" example.
• ❌ The code samples are valid PHP code.
• ❌ <em> tags are used to highlight in the code samples what the sniff is looking.
• ❌ The line length of the code samples stays within the character limit (48 chars).
• ❌ The readability of the code samples is good.
  I'd like to suggest formatting the group use statements as multi-line.
  This is the more common way of formatting those anyway and it improves the readability and prevents the line length issue.
  I'd also like to suggest using blank lines in the code samples to allow the left and right examples to match up, i.e. have the code sample for use function on the left start on the same line as the group use statement for use function on the right etc.

Suggestions:

• I'd suggest not using the namespace keyword as part of the namespaces used in the code samples as that is not allowed prior to PHP 8.0.
  Maybe change some\namespace to My\Project ?
  Also note the change in capitalization to more closely match the conventions for namespace names.
• I'd suggest not using the fn_* function names as it may be confusing to people what with fn having become a reserved keyword for arrow functions in PHP 7.4.
• Maybe use uppercase names for the constants ? (as that is more common/recognizable)
• Maybe use class/function/constant names which look like this could be real code ?

Docs review checklist for PHPCompatibility.UseDeclarations.NewUseConstFunction

• ✔️ Only space indentation is used.
• ⚠️ Indentation is consistent - with the exception of the indentation for the documentation attributes and closer.
• ⚠️ The title matches the sniff name (or is close enough).
  See inline comment.
• ✔️ Separate error codes have a separate <standard> block with their own code samples.
• ⚠️ The "standard" description explains sufficiently what was changed in PHP.
  See inline comment.
• ✔️ The "standard" description uses proper punctuation.
• ✔️ Code sample descriptions use correct prefixes.
• ✔️ < and > are encoded in the code sample descriptions.
• ⚠️ The code sample descriptions are descriptive enough.
  See inline comments.
• ⚠️ Code sample descriptions use proper punctuation.
  See inline comments.
• ⚠️ The code samples demonstrate the issue sufficiently.
  See suggestions below.
• ❌ The code samples are valid PHP code.
• ❌ <em> tags are used to highlight in the code samples what the sniff is looking.
• ✔️ The line length of the code samples stays within the character limit (48 chars).
• ⚠️ The readability of the code samples is good.
  I'd like to suggest using blank lines in the code samples to allow the left and right examples to match up, i.e. have the echo statements on the left start on the same line as the echo statements on the right etc.

Suggestions:

• Same remarks as I made for the other doc about the namespace used and the function/constant names.
• Maybe add a example to each with a non-namespaced native PHP funcion/constant being imported ?
• Maybe add a namespace declaration to the code samples ?

[jrfnl]

@Flyingmana Just checking - was there anything in the review which wasn't clear ? Will you be able to continue with this PR ?

[Flyingmana]

Hi, it will take a few weeks before I can continue with it. Just moved and have everything still in boxes and no working internet yet.

I think everything was clear, but will read closer over it when my Internet is working again.

[jrfnl]

@Flyingmana Understood. Congrats on the new home and good luck with the unpacking. (which reminds me... I still have some unpacked boxes standing around... from my last move a dozen years ago.... 😱 )

[jrfnl]

@Flyingmana How's the new home ? All settled in ? Presuming you got working internet by now, I'd love to see this PR updated so we can get it merged ;-)

[Flyingmana]

new home is great, and internet also working, just my life constantly keeping me busy 😅
will take a few more weeks as how life is, Iam getting married tomorrow 😊

(if someone else takes it over, its also ok for me)
But else I promise to continue on this PR "soon" 😅

[jrfnl]

new home is great, and internet also working, just my life constantly keeping me busy 😅 will take a few more weeks as how life is, Iam getting married tomorrow 😊

Blimey! Congratulations! Wishing you a great day tomorrow!

[Flyingmana]

I think I resolved most of the issues now, possible I missed something.

for NewUseConstFunction I added a placeholder in the example to have the actual changed lines on the same line, as without the cli output did truncate the empty lines in the beginning

----------------------------------------- CODE COMPARISON ------------------------------------------
| Cross-version compatible: Using constants and  | PHP >= 5.6: Importing constants and functions   |
| functions without importing them into the      | into a namespace using `use function` and `use  |
| namespace.                                     | const` declarations.                            |
----------------------------------------------------------------------------------------------------
| // placeholder for line numbers                | use const My\Project\ConstA;                    |
|                                                | use function My\Project\fn_a;                   |
|                                                |                                                 |
| echo My\Project\ConstA;                        | echo ConstA;                                    |
| echo My\Project\fn_a();                        | echo fn_a();                                    |
----------------------------------------------------------------------------------------------------