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

Clean up disabled WPCS sniffs #4058

Closed
danielbachhuber opened this Issue May 9, 2017 · 16 comments

Comments

5 participants
@danielbachhuber
Member

danielbachhuber commented May 9, 2017

Many sniffs were disabled in a57b9fb (#4057).

We should eventually restore them, and then clean up affected files. When we restore them, it would be helpful to only restore one sniff per pull request to make the file churn easily understandable.

  • Generic.ControlStructures.InlineControlStructure.NotAllowed
  • Generic.Formatting.DisallowMultipleStatements.SameLine
  • Generic.Formatting.SpaceAfterCast.NoSpace
  • Generic.Functions.OpeningFunctionBraceKernighanRitchie.ContentAfterBrace
  • Generic.Functions.OpeningFunctionBraceKernighanRitchie.SpaceBeforeBrace
  • Generic.PHP.LowerCaseKeyword.Found
  • Generic.PHP.LowerCaseConstant.Found
  • Generic.PHP.NoSilencedErrors.Discouraged
  • Generic.WhiteSpace.DisallowSpaceIndent.SpacesUsed
  • Generic.WhiteSpace.ScopeIndent.Incorrect
  • Generic.WhiteSpace.ScopeIndent.IncorrectExact
  • PEAR.Functions.FunctionCallSignature.Indent
  • PEAR.Functions.FunctionCallSignature.EmptyLine
  • PEAR.Functions.FunctionCallSignature.SpaceAfterOpenBracket
  • PEAR.Functions.FunctionCallSignature.SpaceBeforeCloseBracket
  • PEAR.Functions.FunctionCallSignature.SpaceBeforeOpenBracket
  • PSR2.ControlStructures.ElseIfDeclaration.NotAllowed
  • Squiz.Strings.ConcatenationSpacing.PaddingFound
  • Squiz.ControlStructures.ControlSignature.SpaceAfterKeyword
  • Squiz.ControlStructures.ControlSignature.SpaceAfterCloseParenthesis
  • Squiz.Commenting.LongConditionClosingComment.Missing
  • Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingAfterOpen
  • Squiz.PHP.DisallowMultipleAssignments.Found
  • Squiz.Strings.DoubleQuoteUsage.NotRequired
  • Squiz.WhiteSpace.SuperfluousWhitespace.EndLine
  • WordPress.Arrays.ArrayDeclaration.NotLowerCase
  • WordPress.Arrays.ArrayDeclaration.NoComma
  • WordPress.Arrays.ArrayDeclaration.NoCommaAfterLast
  • WordPress.Arrays.ArrayDeclaration.SpaceAfterKeyword
  • WordPress.Arrays.ArrayDeclarationSpacing.AssociativeKeyFound
  • WordPress.Arrays.ArrayDeclarationSpacing.NoSpaceAfterArrayOpener
  • WordPress.Arrays.ArrayDeclarationSpacing.NoSpaceBeforeArrayCloser
  • WordPress.Arrays.ArrayKeySpacingRestrictions.SpacesAroundArrayKeys
  • WordPress.Arrays.ArrayKeySpacingRestrictions.NoSpacesAroundArrayKeys
  • WordPress.DB.RestrictedFunctions.mysql_mysql_host_to_cli_args
  • WordPress.Files.FileName.InvalidClassFileName
  • WordPress.Files.FileName.NotHyphenatedLowercase
  • WordPress.NamingConventions.ValidFunctionName.FunctionNameInvalid
  • WordPress.NamingConventions.ValidFunctionName.MethodNameInvalid
  • WordPress.NamingConventions.ValidVariableName.NotSnakeCaseMemberVar
  • WordPress.NamingConventions.ValidVariableName.MemberNotSnakeCase
  • WordPress.NamingConventions.ValidVariableName.NotSnakeCase
  • WordPress.PHP.YodaConditions.NotYoda
  • WordPress.WhiteSpace.CastStructureSpacing.NoSpaceBeforeOpenParenthesis
  • WordPress.WhiteSpace.CastStructureSpacing.NoSpaceAfterCloseParenthesis
  • WordPress.WhiteSpace.ControlStructureSpacing.BlankLineAfterEnd
  • WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceAfterStructureOpen
  • WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceAfterOpenParenthesis
  • WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceBeforeOpenParenthesis
  • WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceAfterCloseParenthesis
  • WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceBeforeCloseParenthesis
  • WordPress.WhiteSpace.ControlStructureSpacing.ExtraSpaceAfterOpenParenthesis
  • WordPress.WhiteSpace.OperatorSpacing.NoSpaceAfter
  • WordPress.WhiteSpace.OperatorSpacing.NoSpaceBefore

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 15, 2017

CS for php/WP_CLI/Iterators/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 15, 2017

CS for php/WP_CLI/Loggers/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 15, 2017

CS for php/WP_CLI/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 15, 2017

CS for php/commands/*
Autofixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 15, 2017

CS for php/WP_CLI/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 15, 2017

CS for php/WP_CLI/Bootstrap/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 15, 2017

CS for php/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 15, 2017

CS for php/WP_CLI/Dispatcher/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 15, 2017

CS for php/WP_CLI/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 15, 2017

CS for php/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 15, 2017

CS for php/WP_CLI/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.
@GaryJones

This comment has been minimized.

Show comment
Hide comment
@GaryJones

GaryJones Sep 15, 2017

Contributor

Noting that PRs for the commit references above are not yet created, until a blocker has been fixed.

Contributor

GaryJones commented Sep 15, 2017

Noting that PRs for the commit references above are not yet created, until a blocker has been fixed.

@Potherca

This comment has been minimized.

Show comment
Hide comment
@Potherca

Potherca Sep 18, 2017

FYI: The related merge-request(s) in dealerdirect/phpcodesniffer-composer-installer have just been released as v0.4.3.

Potherca commented Sep 18, 2017

FYI: The related merge-request(s) in dealerdirect/phpcodesniffer-composer-installer have just been released as v0.4.3.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 18, 2017

CS for php/commands/*
Autofixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 18, 2017

CS for php/WP_CLI/Bootstrap/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 18, 2017

CS for php/WP_CLI/Dispatcher/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 18, 2017

CS for php/WP_CLI/Iterators/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 18, 2017

CS for php/WP_CLI/Loggers/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 18, 2017

CS for php/WP_CLI/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 18, 2017

CS for php/WP_CLI/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 18, 2017

CS for php/WP_CLI/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 18, 2017

CS for php/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.

GaryJones added a commit to GaryJones/wp-cli that referenced this issue Sep 18, 2017

CS for php/*
Auto-fixing of sniffs no longer excluded.

See wp-cli#4058.
@GaryJones

This comment has been minimized.

Show comment
Hide comment
@GaryJones

GaryJones Sep 18, 2017

Contributor

@danielbachhuber said that pull requests that contained small groups of files that addressed multiple standards was acceptable, so that's what I've done, referenced in the 11 PRs above.

Contributor

GaryJones commented Sep 18, 2017

@danielbachhuber said that pull requests that contained small groups of files that addressed multiple standards was acceptable, so that's what I've done, referenced in the 11 PRs above.

@gitlost

This comment has been minimized.

Show comment
Hide comment
@gitlost

gitlost Sep 18, 2017

Contributor

Unfortunately as you can see it's not possible to do it that way as the tests will fail, so I think the originally suggested approach of only enabling one or two sniffs globally at a time is the way to go.

Contributor

gitlost commented Sep 18, 2017

Unfortunately as you can see it's not possible to do it that way as the tests will fail, so I think the originally suggested approach of only enabling one or two sniffs globally at a time is the way to go.

@GaryJones

This comment has been minimized.

Show comment
Hide comment
@GaryJones

GaryJones Sep 19, 2017

Contributor

Hmmm. That will make visual reviewing trickier since the sniffs work together to produce the end result.

There's no easy way that I know of (ping @jrfnl) to limit exclusions to just one group of files.

The other option is just to do all sniffs for all files in one go, which is what I wanted originally. Yes, it's a lot to review in one go, but the tests will pass, and it's the technical debt repayment for letting it get this far. The tests passing should show that no functionality has been changed and that it's safe to merge.

Contributor

GaryJones commented Sep 19, 2017

Hmmm. That will make visual reviewing trickier since the sniffs work together to produce the end result.

There's no easy way that I know of (ping @jrfnl) to limit exclusions to just one group of files.

The other option is just to do all sniffs for all files in one go, which is what I wanted originally. Yes, it's a lot to review in one go, but the tests will pass, and it's the technical debt repayment for letting it get this far. The tests passing should show that no functionality has been changed and that it's safe to merge.

@gitlost

This comment has been minimized.

Show comment
Hide comment
@gitlost

gitlost Sep 19, 2017

Contributor

the sniffs work together to produce the end result.

From my experimenting, many are independent.

The tests passing should show that no functionality has been changed

Alas the tests are far from comprehensive.

Anyway I got phpcbf (2.9.1) working locally and noticed that (without changing the current phpcs.xml.dist, which passes phpcs) running phpcbf applied PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket and PEAR.Functions.FunctionCallSignature.CloseBracketLine which messed up the source. After adding these to the exclusion list phpcbf ran without changing anything.

Then commenting out the first excluded rule (one of the scariest but also probably the most useful) Generic.ControlStructures.InlineControlStructure.NotAllowed produced what looked like a good result with only one indentation error that would need to be fixed manually.

So I'd recommend going with this:

@@ -20,7 +20,7 @@
     <exclude-pattern>*/vendor/*</exclude-pattern>
 
     <rule ref="WordPress-Core">
-        <exclude name="Generic.ControlStructures.InlineControlStructure.NotAllowed" />
+        <!--exclude name="Generic.ControlStructures.InlineControlStructure.NotAllowed" /-->
         <exclude name="Generic.Formatting.DisallowMultipleStatements.SameLine" />
         <exclude name="Generic.Formatting.SpaceAfterCast.NoSpace" />
         <exclude name="Generic.Functions.OpeningFunctionBraceKernighanRitchie.ContentAfterBrace" />
@@ -31,6 +31,8 @@
         <exclude name="Generic.WhiteSpace.DisallowSpaceIndent.SpacesUsed" />
         <exclude name="Generic.WhiteSpace.ScopeIndent.Incorrect" />
         <exclude name="Generic.WhiteSpace.ScopeIndent.IncorrectExact" />
+        <exclude name="PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket" /> <!--Unreported by phpcs but applied by phpcbf -->
+        <exclude name="PEAR.Functions.FunctionCallSignature.CloseBracketLine" /> <!--Unreported by phpcs but applied by phpcbf -->
         <exclude name="PEAR.Functions.FunctionCallSignature.Indent" />
         <exclude name="PEAR.Functions.FunctionCallSignature.EmptyLine" />
         <exclude name="PEAR.Functions.FunctionCallSignature.SpaceAfterOpenBracket" />

as the basis for the first PR, to be reviewed by everyone before merging.

Contributor

gitlost commented Sep 19, 2017

the sniffs work together to produce the end result.

From my experimenting, many are independent.

The tests passing should show that no functionality has been changed

Alas the tests are far from comprehensive.

Anyway I got phpcbf (2.9.1) working locally and noticed that (without changing the current phpcs.xml.dist, which passes phpcs) running phpcbf applied PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket and PEAR.Functions.FunctionCallSignature.CloseBracketLine which messed up the source. After adding these to the exclusion list phpcbf ran without changing anything.

Then commenting out the first excluded rule (one of the scariest but also probably the most useful) Generic.ControlStructures.InlineControlStructure.NotAllowed produced what looked like a good result with only one indentation error that would need to be fixed manually.

So I'd recommend going with this:

@@ -20,7 +20,7 @@
     <exclude-pattern>*/vendor/*</exclude-pattern>
 
     <rule ref="WordPress-Core">
-        <exclude name="Generic.ControlStructures.InlineControlStructure.NotAllowed" />
+        <!--exclude name="Generic.ControlStructures.InlineControlStructure.NotAllowed" /-->
         <exclude name="Generic.Formatting.DisallowMultipleStatements.SameLine" />
         <exclude name="Generic.Formatting.SpaceAfterCast.NoSpace" />
         <exclude name="Generic.Functions.OpeningFunctionBraceKernighanRitchie.ContentAfterBrace" />
@@ -31,6 +31,8 @@
         <exclude name="Generic.WhiteSpace.DisallowSpaceIndent.SpacesUsed" />
         <exclude name="Generic.WhiteSpace.ScopeIndent.Incorrect" />
         <exclude name="Generic.WhiteSpace.ScopeIndent.IncorrectExact" />
+        <exclude name="PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket" /> <!--Unreported by phpcs but applied by phpcbf -->
+        <exclude name="PEAR.Functions.FunctionCallSignature.CloseBracketLine" /> <!--Unreported by phpcs but applied by phpcbf -->
         <exclude name="PEAR.Functions.FunctionCallSignature.Indent" />
         <exclude name="PEAR.Functions.FunctionCallSignature.EmptyLine" />
         <exclude name="PEAR.Functions.FunctionCallSignature.SpaceAfterOpenBracket" />

as the basis for the first PR, to be reviewed by everyone before merging.

@jrfnl

This comment has been minimized.

Show comment
Hide comment
@jrfnl

jrfnl Sep 19, 2017

There's no easy way that I know of (ping @jrfnl) to limit exclusions to just one group of files.

PHPCS 3.x contains a gitmodified filter which may be useful here to limit the report to only those files which were modified. I haven't played with it yet, but I suspect this filter may only work in a pre-commit state, so not sure whether it would actually be useful here.

Looking at the discussion here, I would like to offer an alternative approach to the PR(s) for consideration:
Have only one PR, but have a series of commits in that PR each addressing a different thing. The final commit should then be a passing build, but having the individual commits in the PR will make it easier to review the PR.
The commits could either be the way @GaryJones has set things up already:

  • removing all the exclusions
  • addressing parts of the codebase in separate commits folder by folder

An alternative would be to:

  • remove one exclusion, run phpcbf over all code, review & commit
  • remove the next exclusion... etc

The first approach make reviewing commits easier as you see the final result for each file.
The second approach makes reviewing commits easier in that the reviewer only has to focus on one particular type of change for each commit.

Anyway I got phpcbf (2.9.1) working locally and noticed that (without changing the current phpcs.xml.dist, which passes phpcs) running phpcbf applied PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket and PEAR.Functions.FunctionCallSignature.CloseBracketLine which messed up the source. After adding these to the exclusion list phpcbf ran without changing anything.

There is a reason for those two being enabled for the fixer while being disabled for the sniffer, see WordPress-Coding-Standards/WordPress-Coding-Standards#968 and WordPress-Coding-Standards/WordPress-Coding-Standards#1020

As a side-note, I would say that the PEAR sniff used for this may need to be replaced by a more WP specific sniff, but that's going to be quite a lot of work, so might still take a while before that change is made in WPCS.

Side-note for @gitlost: I would strongly suggest using PHPCS master or at least 3.0.2 for big codebase fixing projects at the moment, as the WP Core CS project has led to improvement upstream which will be in the 3.x branch, but mostly won't be in the 2.x branch of PHPCS.

Anyway, hope this input helps.

jrfnl commented Sep 19, 2017

There's no easy way that I know of (ping @jrfnl) to limit exclusions to just one group of files.

PHPCS 3.x contains a gitmodified filter which may be useful here to limit the report to only those files which were modified. I haven't played with it yet, but I suspect this filter may only work in a pre-commit state, so not sure whether it would actually be useful here.

Looking at the discussion here, I would like to offer an alternative approach to the PR(s) for consideration:
Have only one PR, but have a series of commits in that PR each addressing a different thing. The final commit should then be a passing build, but having the individual commits in the PR will make it easier to review the PR.
The commits could either be the way @GaryJones has set things up already:

  • removing all the exclusions
  • addressing parts of the codebase in separate commits folder by folder

An alternative would be to:

  • remove one exclusion, run phpcbf over all code, review & commit
  • remove the next exclusion... etc

The first approach make reviewing commits easier as you see the final result for each file.
The second approach makes reviewing commits easier in that the reviewer only has to focus on one particular type of change for each commit.

Anyway I got phpcbf (2.9.1) working locally and noticed that (without changing the current phpcs.xml.dist, which passes phpcs) running phpcbf applied PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket and PEAR.Functions.FunctionCallSignature.CloseBracketLine which messed up the source. After adding these to the exclusion list phpcbf ran without changing anything.

There is a reason for those two being enabled for the fixer while being disabled for the sniffer, see WordPress-Coding-Standards/WordPress-Coding-Standards#968 and WordPress-Coding-Standards/WordPress-Coding-Standards#1020

As a side-note, I would say that the PEAR sniff used for this may need to be replaced by a more WP specific sniff, but that's going to be quite a lot of work, so might still take a while before that change is made in WPCS.

Side-note for @gitlost: I would strongly suggest using PHPCS master or at least 3.0.2 for big codebase fixing projects at the moment, as the WP Core CS project has led to improvement upstream which will be in the 3.x branch, but mostly won't be in the 2.x branch of PHPCS.

Anyway, hope this input helps.

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Sep 19, 2017

Member

Just to weigh in here.

To land the pull requests that have already been submitted, I've been reverting the "PHPCS: Remove exclusions that can be fixed" changeset so Travis passes.

Once all of the pull requests are merged, I'll create a new pull request restoring "PHPCS: Remove exclusions that can be fixed".

Member

danielbachhuber commented Sep 19, 2017

Just to weigh in here.

To land the pull requests that have already been submitted, I've been reverting the "PHPCS: Remove exclusions that can be fixed" changeset so Travis passes.

Once all of the pull requests are merged, I'll create a new pull request restoring "PHPCS: Remove exclusions that can be fixed".

@danielbachhuber danielbachhuber self-assigned this Sep 19, 2017

@gitlost

This comment has been minimized.

Show comment
Hide comment
@gitlost

gitlost Sep 19, 2017

Contributor

Have only one PR, but have a series of commits in that PR each addressing a different thing

Apart from a time lag issue with this, per-commit reviewing would be difficult to follow I think, and just doesn't sit well with github.

There is a reason for those two being enabled for the fixer while being disabled for the sniffer

I'm sorry but I consider this a bug. phpcbf should never apply anything not reported by phpcs.

I would strongly suggest using PHPCS master or at least 3.0.2

Ta, will do.

Contributor

gitlost commented Sep 19, 2017

Have only one PR, but have a series of commits in that PR each addressing a different thing

Apart from a time lag issue with this, per-commit reviewing would be difficult to follow I think, and just doesn't sit well with github.

There is a reason for those two being enabled for the fixer while being disabled for the sniffer

I'm sorry but I consider this a bug. phpcbf should never apply anything not reported by phpcs.

I would strongly suggest using PHPCS master or at least 3.0.2

Ta, will do.

@GaryJones

This comment has been minimized.

Show comment
Hide comment
@GaryJones

GaryJones Sep 19, 2017

Contributor

To land the pull requests that have already been submitted, I've been reverting the "PHPCS: Remove exclusions that can be fixed" changeset so Travis passes.

Once all of the pull requests are merged, I'll create a new pull request restoring "PHPCS: Remove exclusions that can be fixed".

That's a great approach - nice one.

Contributor

GaryJones commented Sep 19, 2017

To land the pull requests that have already been submitted, I've been reverting the "PHPCS: Remove exclusions that can be fixed" changeset so Travis passes.

Once all of the pull requests are merged, I'll create a new pull request restoring "PHPCS: Remove exclusions that can be fixed".

That's a great approach - nice one.

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Oct 5, 2017

Member

We can leave the remaining sniffs ignored:

<rule ref="WordPress-Core">
	<exclude name="WordPress.Files.FileName.InvalidClassFileName" />
	<exclude name="WordPress.Files.FileName.NotHyphenatedLowercase" />
	<exclude name="WordPress.NamingConventions.ValidFunctionName.FunctionNameInvalid" />
	<exclude name="WordPress.NamingConventions.ValidFunctionName.MethodNameInvalid" />
	<exclude name="WordPress.NamingConventions.ValidVariableName.NotSnakeCaseMemberVar" />
	<exclude name="WordPress.NamingConventions.ValidVariableName.MemberNotSnakeCase" />
	<exclude name="WordPress.NamingConventions.ValidVariableName.NotSnakeCase" />
</rule>
Member

danielbachhuber commented Oct 5, 2017

We can leave the remaining sniffs ignored:

<rule ref="WordPress-Core">
	<exclude name="WordPress.Files.FileName.InvalidClassFileName" />
	<exclude name="WordPress.Files.FileName.NotHyphenatedLowercase" />
	<exclude name="WordPress.NamingConventions.ValidFunctionName.FunctionNameInvalid" />
	<exclude name="WordPress.NamingConventions.ValidFunctionName.MethodNameInvalid" />
	<exclude name="WordPress.NamingConventions.ValidVariableName.NotSnakeCaseMemberVar" />
	<exclude name="WordPress.NamingConventions.ValidVariableName.MemberNotSnakeCase" />
	<exclude name="WordPress.NamingConventions.ValidVariableName.NotSnakeCase" />
</rule>
@jrfnl

This comment has been minimized.

Show comment
Hide comment
@jrfnl

jrfnl Oct 5, 2017

@danielbachhuber If you like, you can simplify that block to exclude the complete sniffs as you're now excluding (nearly all) the individual error messages.

The difference is that if WPCS would add new errors to one of these existing sniff, you would with the above block still get the new errors, while with the below block, you wouldn't see them. Up to you what you prefer.

<rule ref="WordPress-Core">
	<exclude name="WordPress.Files.FileName" />
	<exclude name="WordPress.NamingConventions.ValidFunctionName" />
	<exclude name="WordPress.NamingConventions.ValidVariableName" />
</rule>

jrfnl commented Oct 5, 2017

@danielbachhuber If you like, you can simplify that block to exclude the complete sniffs as you're now excluding (nearly all) the individual error messages.

The difference is that if WPCS would add new errors to one of these existing sniff, you would with the above block still get the new errors, while with the below block, you wouldn't see them. Up to you what you prefer.

<rule ref="WordPress-Core">
	<exclude name="WordPress.Files.FileName" />
	<exclude name="WordPress.NamingConventions.ValidFunctionName" />
	<exclude name="WordPress.NamingConventions.ValidVariableName" />
</rule>
@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Oct 6, 2017

Member

@jrfnl Ah. Want to submit a pull request to that effect?

Member

danielbachhuber commented Oct 6, 2017

@jrfnl Ah. Want to submit a pull request to that effect?

@jrfnl

This comment has been minimized.

Show comment
Hide comment
@jrfnl

jrfnl Oct 6, 2017

@danielbachhuber I'm working on other things atm, so will happily leave it for someone else to pick up.

jrfnl commented Oct 6, 2017

@danielbachhuber I'm working on other things atm, so will happily leave it for someone else to pick up.

@GaryJones

This comment has been minimized.

Show comment
Hide comment
@GaryJones

GaryJones Oct 6, 2017

Contributor

A couple of the excludes are handled in open PRs.

Contributor

GaryJones commented Oct 6, 2017

A couple of the excludes are handled in open PRs.

@jrfnl

This comment has been minimized.

Show comment
Hide comment
@jrfnl

jrfnl Oct 7, 2017

For whomever will be working on sorting out the ruleset - you may find the WPCS custom properties wiki page useful as well.

jrfnl commented Oct 7, 2017

For whomever will be working on sorting out the ruleset - you may find the WPCS custom properties wiki page useful as well.

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