Coding style for PHP Enumerations

I would prefer UPPER_CASE for constants, to be consistent with constants everywhere else
For the same reason I’d call it “UpperCamel naming” instead of “PascalCase capitalization”.
But otherwise I’d just vote for including this in our standards as-is.

+1 to UpperCamel casing for enum names.

My 0.02: +1 for #2, PascalCase for both enums and constants. STOP_SHOUTING_CONST_IDENTIFIERS...

Another +1 for UpperCamelCase for Enum cases. I see the similarity to Constants, but these are also very much like how we have used associative arrays as ad hoc enums for value maps.

Another +1 for UpperCamelCase for Enum cases. PHP docs also use it: https://www.php.net/manual/en/language.enumerations.basics.php

I don't have a preference either way, as long as we have a rule and its consistent

Now we're starting to see patches for core, I think we should look to address this. Bumping priority as a result.

Adding related issue that adds enums for core.

Also not fussed either way but I found https://stitcher.io/blog/php-enum-style-guide which has some thoughts. TL;DR the author is in favour of UPPER_CASE enums.

I'm in agreement with the article in #9 - upper case feels and looks more natural to me.

@mstrelan just pointed out that Symfony also use UpperCamelCase https://symfony.com/doc/current/reference/forms/types/enum.html

Symfony seems to be using UpperCamel, see https://symfony.com/doc/current/reference/forms/types/enum.html for example.

Distributions and recipes are using upper case https://git.drupalcode.org/project/distributions_recipes/-/raw/patch/rec...

From the point of using enums, the uppercase looks more consistent with constants. PascalCase looks more consistent with class names.

I'm for the uppercase.

This could use the examples and a comparison in the issue summary.

I believe Enumeration cases should be Pascal case for the following reasons:

Enumerations are not constants, they are classes

The idea behind making constants upper case, as I understand it, is making them easily distinguishable from variables. A enumeration::case is not a constant, it is it's own unique thing.

For example, a backed enumeration has this feature: Enum::Case->value

Also:

Enumerations can contain constants

Borrowing from the example on the PHP documentation page, https://www.php.net/manual/en/language.enumerations.constants.php, but changing the constant to upper case:

enum Size {
  case Small;
  case Medium;
  case Large;

  public const HUGE = Size::Large;
}

Size::HUGE is easily identified as a constant, vs Size::Large as a case.

If the cases are also upper case, the comparison would be Size::HUGE vs Size::LARGE.

Mixed case is more readable

A reference: https://en.wikipedia.org/wiki/All_caps

These standards are about making the code more readable, Pascal case does that.

Linking to a few enum issues. We might want to decide on the coding style first?

• #3426506: Create enums for File exists options and deprecate consts
• #3424769: Create enum for FilterInterface:TYPE_* constants
• #3410938: Create enums for RequirementSeverity and deprecate drupal_requirements_severity() constants

I used to be for UPPER_CASE because in a lot of places we're going to replace constants with enums. However, I think #17 lays out an excellent case for why enums are not constants and why, from a differentiation and readability point of view, PascalCase and following Symfony (see #11 & #12) and the PER coding style (see #2) is the right answer.

As an added bonus that means PHPCS won't shout at you if you move between Symfony or Drupal because you forget which uses which when contributing. It also prevents us from having things like the following in our codebases:

if ($value1 === SomeSymfonyEnum::Value && $value2 === SomeDrupalEnum::OTHER_VALUE) {}

I hope no one wants the above in their codebase ^^'

+1 for PascalCase (or UpperCamelCase as some call it)

+1 for PascalCase too

Updated the issue summary with the proposed resolution which aligns Drupal with the rest of the PHP ecosystem (PascalCase).

I think it's pretty clear Pascal is the way to go based on the new IS/comments in 17, as per slack I'm going to RTBC this as we're starting to get lots of Enum issues for core.

Jumped the gun, we need the proposed docs changes I believe

Fixed links in summary.

Here's a first attempt at a proposed documentation change.

I've used similar language as the PER standard which says 'The term "class" refers to all classes, interfaces, traits, and enums.'. I think treating those all the same is a good idea because even the PHP manual says "Enumerations are a restricting layer on top of classes and class constants, intended to provide a way to define a closed set of possible values for a type.".

I think the proposed docs read well, nice work

Just a heads up: We already have CacheTagOperation in core which is camelCase, not PascalCase, though.

It's only related to tests so can we still change that?

Yeah we can change that in tests, I think it's only in 10.3.x/11.x too? If so it technically won't be an API change as long as we do it before beta/rc.

Yeah, we only added that to 10.3.x/11.x as part of the many improvements to StandardPerformanceTest and the like.

Working to use the updated issue summary

I can't think of another occurrence of PascalCase in our coding standards. So, we should use UpperCamelCase.

Where on the page should the new section be added? What section is this suggesting it be before or after?

I think since Enum's are very class-like, this makes the most sense directly after Classes: https://www.drupal.org/docs/develop/standards/php/php-coding-standards#s...

Added myself as a supporter.

Added as a supporter

Added myself as a supporter

Removed the returnEarly function from the documentation proposal since it distracted from what the docs are focusing on which is the naming convention of the cases.

Even though we have 3 already, add myself as a supporter too.

The issue summary tasks need to be updated.

The CR needs some work to. The title is well, wrong. We are not adopting the PER Coding Style 2.0 for enums. That should change to just state that we have a standard for enum. I also think that if someone skims the CR the last sentence will give them the wrong idea. I am also not convinced we need to inform readers when Enumerations were added to PHP or that our standard shares something in common with other standards. The latter is a concern in that this may set a pattern that future changes are to documented against other standards.

Can it be as simple as this?

Enumerations use UpperCamelCase for the name of the 'enum' and names of the enum values.

enum Exists {
  case ErrorIfExists;
  case ErrorIfNotExists;
  case ReturnEarlyIfExists;
  case ReturnEarlyIfNotExists;

  // Do stuff.
}

Can someone check if Coder has anything for enumerations?

I couldn't find anything for enums in Drupal Coder or existing rules about enum case in https://github.com/slevomat/coding-standard or https://github.com/squizlabs/PHP_CodeSniffer

Agreed with @quietone that we should use "UpperCamelCase" not "PascalCase" in our standards docs. Also took the liberty of removing two "should"'s in favor of a single "must" in the proposed standards text:

Enums follow the same conventions as classes with the addition that for their cases the enums must use UpperCamelCase.
...

Also, edited the CR per the concerns in #40:

https://www.drupal.org/node/3439920/revisions/view/13524321/13525482

Updating remaining tasks. The CS committee was in broad agreement. Also, since there are already a few enums in core, and we really need to standardize before 10.3.0 ships, we want to "fast-track" this change. Tagging for needing announcement (although maybe we're going to jump over that formality in this case). 😂

#3440087: Update CacheTagOperation enum to use UpperCamelCase is there with an MR if we want to move quickly on that before the standard is formally adopted.

I was with larowlan in #8, meaning I didn't care either way as long as it was clear and consistent. After reading up on #17 I can't but agree that PascalCase seems like the only sensible way forward.

Not seeing much value in adding even more names to the IS so I'll leave that as is.

Committed #3440087: Update CacheTagOperation enum to use UpperCamelCase so that (I think?) core is now compliant with the proposed standard in advance.

A tiny nit on the wording: we use UpperCamel as a name on the classes page rather than UpperCamelCase. Can we use the same in the proposed standard description for this issue?

Proposed currently
Enums follow the same conventions as classes with the addition that for their cases the enums must use UpperCamelCase.

After update
Enums follow the same conventions as classes with the addition that for their cases the enums must use UpperCamel.

I think including "Case" is helpful for understanding. The CR title seems more weird as "Use UpperCamel for enum and enum case values".

How about friendly amendment, we standardize on "UpperCamelCase" and update the one other reference to it as part of fixing this?

Looking more closely https://www.drupal.org/node/608152#naming uses:

1. UpperCamel
2. lowerCamel
3. CamelCase

Ugh to #3. That should either be "UpperCamel", or we do:

1. UpperCamelCase
2. lowerCamelCase
3. UpperCamelCase

I don't want to slow this issue down, we can clean all this up later. But I believe we can tidy as we go like this. It's only the wording of the docs, not the underlying standard they describe.

Cheers,
-Derek

Meanwhile, trimming a lot of noise out of the summary here.

+1 for #48 updating names! thank you Derek

Added the point from #48 to a comment on the issue for the next meeting.

I support the current proposal. I also opened #3443118: Enum cases may need to change to UpperCamelCase to inform the distributions and recipes initiative of this likely-upcoming standard.

I see cases for both casing of the cases 😉 which means that following existing external standards is the right call. IMO the only reason we might not do so immediately is if it's inconsistent with other core standards (I don't see a conflict) or disruptive to core.

Here are the places enum is used in core already (outside of the Doctrine fork):

core/lib/Drupal/Core/Database/Transaction/StackItemType.php
core/lib/Drupal/Core/Database/Transaction/ClientConnectionTransactionState.php
core/lib/Drupal/Core/Database/Event/StatementEvent.php
core/lib/Drupal/Core/Form/ToConfig.php
core/lib/Drupal/Core/File/FileExists.php
core/lib/Drupal/Core/Config/Action/Exists.php
core/lib/Drupal/Core/DefaultContent/Existing.php
core/lib/Drupal/Core/Theme/ExtensionType.php
core/modules/system/tests/modules/performance_test/src/Cache/CacheTagOperation.php

I checked and all of them use UpperCamel (following #3443118: Enum cases may need to change to UpperCamelCase which fixed the config enum).

So +1 from me also.

Update steps per #3456119: Coding Standards Meeting Tuesday 2024-06-18 2100 UTC

I updated the doc page, step 8.1. It would be best if someone checked that before the CR is published.

@quietone I checked the documentation and made a minor change to add indentation to the enum example. Otherwise looks good to me!

@acbramley, thanks for checking!

I changed to the file Exists.php to include some errors, wrong case and extra spaces, and ran commit-code-check. None of the problems were identified.

ENUM exists {
   case ErrorIfExists;
  case  ErrorIfNotExists;

So, this does need an issue in coder.

In the recent meeting mstrlen mentioned the following sniffs

   rule ref="SlevomatCodingStandard.Classes.BackedEnumTypeSpacing"/
   rule ref="SlevomatCodingStandard.Classes.EnumCaseSpacing"/

With those enabled the following passes. There are two spaces after 'case' in the first line.

  case  ErrorIfExists;
  case ErrorIfNotExists;
  case ReturnEarlyIfExists;
  case ReturnEarlyIfNotExists;

And this fails. A space is needed before the comment line.

  /* Appends a number until name is unique. */
  case Rename;
  /* Replace the existing file. */
  case Replace;
  /* Do nothing and return FALSE. */
  case Error;

To need an extra line in one case and not the other is unexpected. So, I think there is more work needed on the style of enums.

I am thinking we should define the properties for SlevomatCodingStandard.Classes.EnumCaseSpacing. They are

minLinesCountBeforeWithComment: minimum number of lines before enum case with a documentation comment or attribute
maxLinesCountBeforeWithComment: maximum number of lines before enum case with a documentation comment or attribute
minLinesCountBeforeWithoutComment: minimum number of lines before enum case without a documentation comment or attribute
maxLinesCountBeforeWithoutComment: maximum number of lines before enum case without a documentation comment or attribute

The example does not include a ": type-string" either. The other sniff deals with that.

Enforcing what we have agreed to can be done with SlevomatCodingStandard.Classes.BackedEnumTypeSpacing. And a core issue is needed to enable it.

Another issue is needed to decide about comments for each case and the use of SlevomatCodingStandard.Classes.EnumCaseSpacing.

Created #3490054: Comment style for Enum and for core, #3490056: Enable SlevomatCodingStandard.Classes.BackedEnumTypeSpacing.

And a reminder that this was announced in https://www.drupal.org/about/core/blog/coding-standards-proposals-for-fi....

So, I think this is now complete.

Thanks everyone!

Thanks for cleaning this up and moving it along!

I just RTBC'ed #3490056: Enable SlevomatCodingStandard.Classes.BackedEnumTypeSpacing. I know we're still discussing #3490054: Comment style for Enum. However, neither of the proposed sniffs seems to check for the main thing we debated here: using UpperCamelCase for the case names. I'm not sure the best way to enforce that. Seems we need another followup, somewhere, but I don't know where. 😅

Was the "needs work" intentional or a mistake? Is it because you think we need another follow-up?

Indeed, intentional since we need to decide where to follow up to enforce this part of the standard.

Okay so would it make sense to mark this one as fixed again as it was about deciding a style? And then open a follow-up about enforcing it? Or is there a chance we need to enforce it in e.g. gitlab_templates and we can't open up a follow-up yet before we know where to open it?

I'm following the protocols that we use for the coding standards committee (of which I am a member). Even though this is indeed a decided policy, we're not supposed to mark this 'fixed' until all the follow-ups exist.

I don't have time for a deep-dive on this, so for now, doing the quick/easy thing of opening an issue in coder:
#3492126: Enforce UpperCamelCase case values in PHP enum definitions
We can decide there if that's really the best way to enforce this, and if not, move the issue to the core queue and rescope it to enable some existing sniff in the wild or whatever.

Cheers,
-Derek

All right, makes total sense. Thanks!

The enum case standard was implemented in Coder in #3492126: Enforce UpperCamelCase case values in PHP enum definitions.

The enum name itself Coder checking is planned in #3497433: Forbid upper case acronyms in class, enum, interface, trait names. This will also cover traits and will throw errors on all upper case acronyms (2 bad names in core, class SSH and class FTP)

fixing issue link

Amending attribution.