Run phpbcf for added MediaWiki rules #17419
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There's no point in scanning the copies of WordPress or WordPress-develop checked out for Docker, or the built files in _inc that are already listed in .gitignore. Also one file being .gitignored is no longer used, so let's remove it from there.
This will make it a bit cleaner in the future to manage tests and dependencies for the coding standard. And, if some other repo wants to use our coding standards, they can more easily do so. The new coding standard package imports the PHPCompatibility and WordPress standards, so those are removed from `.phpcs.xml.dist`. The phpcs configuration specific to Jetpack remains in `.phpcs.xml.dist`.
MediaWiki has a large suite of custom phpcs sniffs, several of which would be useful for Jetpack. * MediaWiki.AlternativeSyntax.LeadingZeroInFloat: Prefer `0.5` over `.5` for better readability. * MediaWiki.Classes.UnsortedUseStatements: Require `use` statements be sorted. * MediaWiki.Classes.UnusedUseStatement: Attempts to flag `use` statements where the used class isn’t actually used. * MediaWiki.ExtraCharacters.ParenthesesAroundKeyword: Flags things like `clone( $x )`, `continue( 2 )`, and `break( 2 )`, in addition to things like `require_once( '...' )` that are already getting flagged by PEAR.Files.IncludingFile.BracketsNotRequired. * MediaWiki.Usage.DirUsage: Prefer `__DIR__` to `dirname( __FILE__ )`. * MediaWiki.Usage.DoubleNotOperator: `! ! $x` is "clever code", `(bool) $x` would be clearer. * MediaWiki.Usage.InArrayUsage: Flags constructs like `in_array( $key, array_keys( $array ) )` and `in_array( $key, array_flip( $array ) )` that could be done more clearly and efficiently with `isset( $array[ $key ] )` or `array_key_exists( $key, $array )`. * MediaWiki.Usage.MagicConstantClosure: Flag use of `__METHOD__` and `__FUNCTION__` inside closures, as they don’t produce useful output. * MediaWiki.Usage.NestedFunctionsSniff: Defining a function (not a closure) inside a function is "clever code", if not an outright bug. * MediaWiki.Usage.PHPUnitAssertEquals: Prefer other PHPUnit assertions over `assertEquals()` in some cases for better type safety. * MediaWiki.Usage.PlusStringConcat: Flag attempts to concatenate strings with `+` rather than `.`. Useful for programmers coming from JS. * MediaWiki.Usage.ReferenceThis: `&$this` raises warnings since PHP 7.1, warn against it. Surprisingly not caught by the "PHPCompatibility" sniffs we’re importing. * MediaWiki.WhiteSpace.MultipleEmptyLines: Forbids multiple empty lines everywhere, versus Squiz.WhiteSpace.SuperfluousWhitespace.EmptyLines which only does so inside functions. * MediaWiki.WhiteSpace.SpaceAfterClosure: The WordPress sniffs seem to have good coverage of whitespace stuff, but miss that `function()` should be `function ()`.
And cleaned up some whitespace issues the MW sniffs leave behind without whitespace-fixing sniffs coming along behind them. Not verified because touching the files makes it whine about all the other existing errors in these files. 😀
anomiex
requested review from
georgestephanis,
kraftbj and
a team
as code owners
github-actions
bot
added
the
[Status] Needs Package Release
No I didn't. Github apparently did that automatically in my name. |
anomiex
added
[Status] Needs Review
|
GitHub auto-requests "code owners" as indicated in this file: https://www.github.com/Automattic/jetpack/tree/master/.github%2FCODEOWNERS Since we don't require codeowner approval, it's basically a way to alert individuals and teams that changes are made to code they have opted into have a vested interest in. |
anomiex
force-pushed
the
add/phpcs-rules-from-MediaWiki
branch
5 times, most recently
from
59f51a6
to
8985c17
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes proposed in this Pull Request:
This is the result of running
then cleaning up some whitespace issues the MW sniffs leave behind that would normally be fixed anyway by whitespace-fixing sniffs.
I'm not sure we actually want to merge this particular big cleanup patch, but it could be useful to see what the new sniffs touch. And to see whether the PHPUnit changes from
assertEqualsto stricter methods break anything.Jetpack product discussion
Related to #17406.
Does this pull request change what data or activity we track or use?
No.
Testing instructions:
Nothing should have changed.
Proposed changelog entry for your changes: