Trait formatting

[dingo-d]

This PR depends on #100. Once that is merged, this PR should be rebased to the master branch.

The PR adds rules about the trait formatting rules and is the continuation of the additions of 'modern' PHP code in the WordPress PHP Coding Standards handbook based on the make post by Juliette Reinders Folmer.

[jrfnl]

@dingo-d Thanks again! Reviewed.

---

I wonder if this section is placed correctly.

PSR12 has these kind of rules in the "Classes, Properties, and Methods" chapter below the "Declare Statements, Namespace, and Import Statements" chapter.

In the WPCS handbook page, that would translate to moving this section to the "Object-Oriented Programming" chapter, which sounds about right.

What do you think ?

---

When comparing the section to the Make post, I'm missing some things.

• Under "Naming Conventions", line 91 "Class names should use capitalized words separated by underscores. Any acronyms should be all upper case." should probably be updated to include traits, interfaces and enums.
  Might be a nice idea to also add an example of one of these constructs to the code sample just below it (or replace one of the existing code samples).
• Under "Naming Conventions", line 110 "Class file names should be based on the class name with class- prepended and the underscores in the class name replaced with hyphens" should probably also be updated to include traits, interfaces and enums. Actually, you were right to not include this one as there was some unresolved discussion about that part of the proposal.
• "For everything else, the same formatting rules as already in place for class declarations apply." - to be honest aside from line 91 and the code sample below it, I can't really find any rules in the handbook about class formatting... oops. Let's revisit that later...

---

Also note: there is still a merge conflict and a duplicate section in the current commits (both for the fully qualified names section).

[dingo-d]

In the WPCS handbook page, that would translate to moving this section to the "Object-Oriented Programming" chapter, which sounds about right.

I thought about this, but because we've had Trait and Interface naming as one chapter (which is TBD) and Trait and interface formatting (which was in fact just Trait formatting), I've changed the name to just Trait formatting and move it to the Formatting section.

If we were to rename the chapter to the suggested Trait Use Statements then it would fit in the OOP section.

Do you want to go in that direction?

Merge conflicts

I guess this is the side effect of rebasing I did 🙈 I'll apply your commits and rebase to the master branch once more and fix the merge conflicts.

[dingo-d]

@jrfnl I've moved the traits use section to the OOP section, removed duplicate sections, and added the suggestions from the PR review.

Hope it looks better now.

[GaryJones]

at least one blank line after ... the blank line after may be omitted

This seems a rather weak and inconsistent standard, and dependent on exceptions. Going from potentially an infinite amount of blank lines after, to potentially zero would seem to make it harder to enforce than necessary. Can we change the "at least one blank line after" to [exactly] "one blank line after"? That way, it's either one line after, or zero lines after if that's the only thing in the class.

It is also ambiguous since the "one blank line before the first use statement, and at least one blank line after" could be read as "one blank line before the first use statement, and at least one blank line after [the first use statement]", which makes the Correct example below wrong.

Suggested change

	Trait `use` statements should be at the top of a class and should have exactly one blank line before the first `use` statement, and at least one blank line after. The only exception is when the class only contains the trait `use` statement, in which case the blank line after may be omitted.
	Trait `use` statements should be at the top of a class and should have exactly one blank line before the first `use` statement, and at least one blank line after the last statement or group. The only exception is when the class only contains the trait `use` statement, in which case the blank line after may be omitted.

The inclusion of "after the last statement or group" removes the ambiguity here.

[jrfnl]

Going from potentially an infinite amount of blank lines after, to potentially zero would seem to make it harder to enforce than necessary. Can we change the "at least one blank line after" to [exactly] "one blank line after"? That way, it's either one line after, or zero lines after if that's the only thing in the class.

This was a very deliberate choice of wording, both for the class only containing trait use as well as the class with more content. We use the same wording in multiple other places too.

In this case, whatever follows the trait use statement determines how many blank lines between the use and the next thing are allowed. The trait use "blank lines after" only determines that there should be at least one blank line.

In more general terms: strict rules for blank lines before something, loose rules for blank lines after.

If we don't do it this way, it can very easily lead to fixer conflicts, which is something we want to avoid.

As for the "class only containing trait use may omit the blank line after" part, again, this is largely to prevent fixer conflicts as generally speaking, there should be no stray blank lines at the end of a class and sniffs checking for that normally don't care what is directly above the closing brace, as long as it isn't a blank line.

[jrfnl]

@GaryJones Does it make more sense when you read it with that context in mind ?

[jrfnl]

@dingo-d "The only exception is when the class only contains the trait use statement," => "The only exception is when the class only contains trait use statements,"

[jrfnl]

@dingo-d Looks like @GaryJones' suggestion to remove ambiguity hasn't been applied yet.

[dingo-d]

I've fixed the issues and we can merge the PR. We can always discuss the parts that @GaryJones had issues within a subsequent issue/PR (Juliette gave good reasoning behind choosing the wording in that paragraph).

[jrfnl]

👍🏻

Merging this now as @GaryJones is AFK for the next few weeks. If needs be, additional textual tweaks can still be made in a follow up commit.

Concern has been answered.