Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Enum cases should not be altered #20

Closed
Procionegobbo opened this issue Jun 23, 2022 · 7 comments
Closed

Enum cases should not be altered #20

Procionegobbo opened this issue Jun 23, 2022 · 7 comments
Assignees
Labels

Comments

@Procionegobbo
Copy link

  • Pint Version: 0.1.5
  • PHP Version: 8.1.1

Description:

Enum cases should not be reformatted even regardless of keyword name.

Enum class: app/Enum/SegmentCastMissingAs.php

namespace App\Enum;

enum SegmentCastMissingAs: string
{
    case NULL = 'null';
    case FALSE = 'false';
    case TRUE = 'true';
    case EMPTY_STRING = 'empty_string';
    case ZERO = 'zero';
    case IGNORE = 'ignore';
}

Pint change this style using Laravel preset:

-case NULL = 'null';
-case FALSE = 'false';
-case TRUE = 'true';
+case null = 'null';
+case false = 'false';
+case true = 'true';
case EMPTY_STRING = 'empty_string';
case ZERO = 'zero';
case IGNORE = 'ignore';

Steps To Reproduce:

  1. create class app/Enum/SegmentCastMissingAs.php
  2. Run /vendor/bin/pint --preset laravel app/Enum/SegmentCastMissingAs.php
@driesvints
Copy link
Member

I think these are reserved constants and should not be used as ENUM values: https://www.php.net/manual/en/reserved.constants.php ?

@Procionegobbo
Copy link
Author

Partially true: you cannot declare a constant with those names but nothing prevents you from using as an Enum case.

@claudiodekker
Copy link
Contributor

Probably caused by https://cs.symfony.com/doc/rules/casing/native_function_type_declaration_casing.html (#10)

Unfortunately you can't exclude this just for enum cases by default, as this is an enable-or-disable kind of rule.

Furthermore, as @driesvints mentioned you probably shouldn't be 'working around' reserved constants anyway. Just because the language doesn't cover it or block you from doing so, doesn't necessarily mean it's a good idea either.

In either case, if you're really eager to disable this, you can manually disable the rule in a pint.json config file:

{
    "rules": {
        "native_function_type_declaration_casing": false
    }
}

@driesvints
Copy link
Member

I feel @claudiodekker is right. This is going to be an edge case at best.

@claudiodekker
Copy link
Contributor

claudiodekker commented Jun 23, 2022

Alternatively, a custom rule could be considered to still allow it just for Enum cases, but I don't think I'm the right person to judge as to what such a fixer would look like, and whether from a theoretical point of view it's actually a good idea to support it anyway.

@Procionegobbo
Copy link
Author

Procionegobbo commented Jun 23, 2022

Just because the language doesn't cover it or block you from doing so, doesn't necessarily mean it's a good idea either.

I agree that this could be an edge case but the language does not simply not blocking this case but allowing it.
There is no such limitation on Enum cases (and class constants if I'm not wrong) naming and Feel that the changes made in the example class are not consistent with PHP syntax.
Maybe this is relevant sebastianbergmann/phpunit#4813

I don't feel comfortable disabling the rule because it's useful for numerous other case but I'll add to pint.json for now.

@jtomek
Copy link

jtomek commented Jul 6, 2022

Hello. I just ran into this too. I have an application that heavily relies on tables and related logic. My use case is to cast columns into Enums and group column-related helpers there, such as header(), default(), validators(), etc.

Solution: I changed case True and case False to case Yes and case No. :)

ablancobarreda added a commit to ablancobarreda/pint that referenced this issue Jan 5, 2023
Update BusinessDeliveryZonesController.php
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants