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.

Sign up for GitHub

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

Jump to bottom

Refine rules #222

Open
wants to merge 3 commits into
base: 8.2.x
Choose a base branch
from
Open

Refine rules #222

wants to merge 3 commits into from

Conversation

blue32a
Copy link

@blue32a blue32a commented Oct 3, 2020

Squiz.WhiteSpace.CastSpacing

This is included in PSR-12.

vendor/bin/phpcs --standard=PSR12 -e
Squiz (15 sniffs)
-----------------
  Squiz.Classes.ValidClassName
  Squiz.ControlStructures.ControlSignature
  Squiz.ControlStructures.ForEachLoopDeclaration
  Squiz.ControlStructures.ForLoopDeclaration
  Squiz.ControlStructures.LowercaseDeclaration
  Squiz.Functions.FunctionDeclaration
  Squiz.Functions.FunctionDeclarationArgumentSpacing
  Squiz.Functions.LowercaseFunctionKeywords
  Squiz.Functions.MultiLineFunctionDeclaration
  Squiz.Scope.MethodScope
  Squiz.WhiteSpace.CastSpacing
  Squiz.WhiteSpace.ControlStructureSpacing
  Squiz.WhiteSpace.ScopeClosingBrace
  Squiz.WhiteSpace.ScopeKeywordSpacing
  Squiz.WhiteSpace.SuperfluousWhitespace

PSR2.ControlStructures.ControlStructureSpacing

This is not included in PSR-12.

vendor/bin/phpcs --standard=PSR12 -e
PSR2 (9 sniffs)
---------------
  PSR2.Classes.ClassDeclaration
  PSR2.Classes.PropertyDeclaration
  PSR2.ControlStructures.ElseIfDeclaration
  PSR2.ControlStructures.SwitchDeclaration
  PSR2.Files.ClosingTag
  PSR2.Files.EndFileNewline
  PSR2.Methods.FunctionCallSignature
  PSR2.Methods.FunctionClosingBrace
  PSR2.Methods.MethodDeclaration

@blue32a
Removed duplicate rule
f2d7f87 
This is included in PSR-12.
@blue32a
Remove unnecessary exclusion
18e4ab3 
This is not included in PSR-12.
@blue32a
Sorted the rules
b2beffe
@blue32a blue32a requested a review from a team as a code owner October 3, 2020 11:14
@greg0ire greg0ire changed the base branch from master to 8.2.x October 25, 2020 10:05
@greg0ire
Copy link
Member

greg0ire commented Apr 3, 2021

This is not included in PSR-12.

Then how does it make PSR2.ControlStructures.ControlStructureSpacing.SpacingAfterOpenBrace unnecessary?

@blue32a
Copy link
Author

blue32a commented Apr 10, 2021

Then how does it make PSR2.ControlStructures.ControlStructureSpacing.SpacingAfterOpenBrace unnecessary?

It is included in PSR2.ControlStructures.ControlStructureSpacing and is not installed.
And it is checked by PSR12.ControlStructures.ControlStructureSpacing.
Do you need it?

@greg0ire
Copy link
Member

@carusogabriel , you introduced that line, can you maybe chime in?

Ocramius
Ocramius suggested changes Feb 15, 2022
View reviewed changes
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM-ish, but requires new major.

lib/Doctrine/ruleset.xml
@@ -21,8 +21,6 @@
<exclude name="PSR12.Traits.UseDeclaration"/>
<!-- checked by SlevomatCodingStandard.TypeHints.ReturnTypeHintSpacing.WhitespaceAfterNullabilitySymbol -->
<exclude name="PSR12.Functions.NullableTypeDeclaration.WhitespaceFound"/>
<!-- checked by PSR12.ControlStructures.ControlStructureSpacing -->
<exclude name="PSR2.ControlStructures.ControlStructureSpacing.SpacingAfterOpenBrace"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO to be kept

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ocramius Ocramius Ocramius requested changes

At least 3 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@blue32a @greg0ire @Ocramius