Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add WordpressArray sniff #299

Open
michnovka opened this issue Oct 19, 2022 · 4 comments
Open

Add WordpressArray sniff #299

michnovka opened this issue Oct 19, 2022 · 4 comments

Comments

@michnovka
Copy link
Contributor

I would like to start discussion on adding these rules to doctrine CS:

    <rule ref="WordPress.Arrays.ArrayDeclarationSpacing">
        <exclude name="WordPress.Arrays.ArrayDeclarationSpacing.NoSpaceAfterArrayOpener"/>
        <exclude name="WordPress.Arrays.ArrayDeclarationSpacing.NoSpaceBeforeArrayCloser"/>
        <exclude-pattern>tests/*</exclude-pattern>
    </rule>

I got this idea when checking https://github.com/doctrine/orm/pull/10126/files#diff-71c09c517db3322ed613e4cdf6cd41ea2323bf44c1677d3c249dd83da4fbeb25R29-R30

The main argument for this change is that together with SlevomatCodingStandard.Arrays.TrailingArrayComma.MissingTrailingComma which is already applied this makes for much cleaner diffs when making changes.

I also prefer:

#[DiscriminatorMap([
    'cc' => Component\ConcreteComponent::class,
    'cd' => Decorator\ConcreteDecorator::class,
])]
abstract class Component

over

#[DiscriminatorMap(['cc' => Component\ConcreteComponent::class,
        'cd' => Decorator\ConcreteDecorator::class])]
abstract class Component

WDYT?

@simPod
Copy link
Contributor

simPod commented Oct 19, 2022

I'd like something like that actually, kinda dislike formatting the later manually which I always do.

@malarzm
Copy link
Member

malarzm commented Oct 19, 2022

I had no clue the latter is allowed 🙃 So

#[DiscriminatorMap([
    'cc' => Component\ConcreteComponent::class,
    'cd' => Decorator\ConcreteDecorator::class,
])]
abstract class Component

is the way to go for me 👍

@derrabus
Copy link
Member

Does that mean we'll have to add an extra dependency for that rule?

@michnovka
Copy link
Contributor Author

michnovka commented Oct 27, 2022

Does that mean we'll have to add an extra dependency for that rule?

@derrabus yea, however I am against adding Wordpress CS after further consideration. Unlike Slevomat, this CS is not for generic use, but mainly it says what "wordpress code should look like". There were cases where some sniffs changed behavior. So e.g. the one I proposed (WordPress.Arrays.ArrayDeclarationSpacing) checks so many things to give code which looks like whatever the wordpress devs consider standard at any given moment.

So instead, I opened an issue at Slevomat CS - slevomat/coding-standard#1456 and I would like to port some of the sniffs that would be useful for our purpose.

I will likely get time to work on this in November, so I suggest we wait until this is finished and then we can use just Slevomat without additional dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants