EBernhardson has uploaded a new change for review. https://gerrit.wikimedia.org/r/291001
Change subject: Assert fixers do as intended ...................................................................... Assert fixers do as intended We didn't have any tests that fixers do what we expect them to do. Patch adds a test for any .php file that has a file of the same name with .fixed as a suffix to assert the output of the code matches the recorded fixed content. Again i have not manually reviewed these fixed files, they match the current state of the repository and should perhaps be manually reviewed. This was prompted by I3dca4c37 which tries to fix a bad fixer that was eating code. .fixed files generated with: vendor/bin/phpcbf --standard=MediaWiki --suffix=.fixed MediaWiki\Tests\files Change-Id: Ie7102faf842f56e6b93b04f02e90606f19fcaee1 --- M MediaWiki/Tests/MediaWikiStandardTest.php A MediaWiki/Tests/MediaWikiTestHelper.php A MediaWiki/Tests/files/AlternativeSyntax/alternative_syntax_fail.php.fixed A MediaWiki/Tests/files/ControlStructures/if_else_structure_fail.php.fixed A MediaWiki/Tests/files/ExtraCharacters/extra_characters_before_phpopen_tag_fail.php.fixed A MediaWiki/Tests/files/NamingConventions/case_global_name_fail.php.fixed A MediaWiki/Tests/files/NamingConventions/wg_global_name2_fail.php.fixed A MediaWiki/Tests/files/NamingConventions/wg_global_name_fail.php.fixed A MediaWiki/Tests/files/Usage/dir_usage_fail.php.fixed A MediaWiki/Tests/files/VariableAnalysis/unused_global_variables_fail.php.fixed A MediaWiki/Tests/files/WhiteSpace/multiple_empty_lines_fail.php.fixed A MediaWiki/Tests/files/WhiteSpace/short_array_fail.php.fixed A MediaWiki/Tests/files/WhiteSpace/space_after_delim_singleline_comment_fail.php.fixed A MediaWiki/Tests/files/WhiteSpace/space_before_parens_fail.php.fixed A MediaWiki/Tests/files/WhiteSpace/spacey_parenthesis_fail.php.fixed M composer.json 16 files changed, 204 insertions(+), 2 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/tools/codesniffer refs/changes/01/291001/1 diff --git a/MediaWiki/Tests/MediaWikiStandardTest.php b/MediaWiki/Tests/MediaWikiStandardTest.php index 44f57a3..9885698 100644 --- a/MediaWiki/Tests/MediaWikiStandardTest.php +++ b/MediaWiki/Tests/MediaWikiStandardTest.php @@ -33,7 +33,8 @@ public function setUp() { parent::setUp(); if ( empty( $this->helper ) ) { - $this->helper = new TestHelper(); + include_once __DIR__ . '/MediaWikiTestHelper.php'; + $this->helper = new MediaWikiTestHelper(); } } @@ -82,6 +83,28 @@ $this->assertEquals( $expect, $outputStr ); } + public static function testFixProvider() { + $tests = self::testProvider(); + foreach ( array_keys( $tests ) as $idx ) { + $fixed = $tests[$idx][0] . ".fixed"; + if ( file_exists( $fixed ) ) { + $tests[$idx][2] = $fixed; + } else { + unset( $tests[$idx] ); + } + } + return $tests; + } + + /** + * @dataProvider testFixProvider + */ + public function testFix( $file, $standard, $fixedFile ) { + $outputStr = $this->helper->runPhpCbf( $file, $standard ); + $expect = file_get_contents( $fixedFile ); + $this->assertEquals( $expect, $outputStr ); + } + /** * strip down the output to only the warnings * diff --git a/MediaWiki/Tests/MediaWikiTestHelper.php b/MediaWiki/Tests/MediaWikiTestHelper.php new file mode 100644 index 0000000..a303003 --- /dev/null +++ b/MediaWiki/Tests/MediaWikiTestHelper.php @@ -0,0 +1,62 @@ +<?php + +use org\bovigo\vfs\vfsStream; + +class MediaWikiTestHelper extends TestHelper { + public function __construct() { + parent::__construct(); + $this->vfsRoot = vfsStream::setup('root'); + } + + public function runPhpCbf( $file, $standard = '' ) { + if ( empty( $standard ) ) { + $standard = $this->rootDir . '/ruleset.xml'; + } + $defaults = $this->phpcs->getDefaults(); + + if ( + defined( 'PHP_CodeSniffer::VERSION' ) && + version_compare( PHP_CodeSniffer::VERSION, '1.5.0' ) != -1 + ) { + $standard = [ $standard ]; + } + $options = [ + 'encoding' => 'utf-8', + 'files' => [ $file ], + 'standard' => $standard, + 'reports' => [ 'diff' => vfsStream::url('root/phpcbf-fixed.diff') ] + ] + $defaults; + + ob_start(); + $this->phpcs->process( $options ); + ob_end_clean(); + + if ( !$this->vfsRoot->hasChild( 'phpcbf-fixed.diff' ) ) { + // no diff generated, return source file + return file_get_contents( $file ); + } + + // patch the source file and output to stdout + $cmd = "patch -p0 -u -o -"; + $descriptorSpec = [ + 0 => ['pipe', 'r'], + 1 => ['pipe', 'w'], + 2 => ['file', '/dev/null', 'w'], + ]; + $process = proc_open($cmd, $descriptorSpec, $pipes); + if ( !$process ) { + throw new RuntimeException( "Failed to run $cmd" ); + } + + fwrite( $pipes[0], $this->vfsRoot->getChild( 'phpcbf-fixed.diff' )->getContent() ); + fclose( $pipes[0] ); + + $output = stream_get_contents($pipes[1]); + fclose($pipes[1]); + + $retval = proc_close($process); + + // test retval? + return $output; + } +} diff --git a/MediaWiki/Tests/files/AlternativeSyntax/alternative_syntax_fail.php.fixed b/MediaWiki/Tests/files/AlternativeSyntax/alternative_syntax_fail.php.fixed new file mode 100644 index 0000000..94378f9 --- /dev/null +++ b/MediaWiki/Tests/files/AlternativeSyntax/alternative_syntax_fail.php.fixed @@ -0,0 +1,29 @@ +<?php + +declare ( ticks = 1 ): + echo "foo"; +enddeclare; + +for ( $i = 0; $i < 5; $i++ ): + echo $i; +endfor; + +foreach ( [ 1, 2, 3 ] as $i ): + echo $i; +endforeach; + +$x = 1; + +if ( $x < 2 ): + echo $x; +endif; + +switch ( $x ): + case 2: + echo $x; + break; +endswitch; + +while ( $x > 2 ): + echo $i; +endwhile; diff --git a/MediaWiki/Tests/files/ControlStructures/if_else_structure_fail.php.fixed b/MediaWiki/Tests/files/ControlStructures/if_else_structure_fail.php.fixed new file mode 100644 index 0000000..6c855ab --- /dev/null +++ b/MediaWiki/Tests/files/ControlStructures/if_else_structure_fail.php.fixed @@ -0,0 +1,11 @@ +<?php + +function wfFooFoo( $a ) { + if ( $a ) { + + } elseif ( $a ) { + + } else { + + } +} diff --git a/MediaWiki/Tests/files/ExtraCharacters/extra_characters_before_phpopen_tag_fail.php.fixed b/MediaWiki/Tests/files/ExtraCharacters/extra_characters_before_phpopen_tag_fail.php.fixed new file mode 100644 index 0000000..abc2e81 --- /dev/null +++ b/MediaWiki/Tests/files/ExtraCharacters/extra_characters_before_phpopen_tag_fail.php.fixed @@ -0,0 +1,7 @@ +someText<?php +// text before first php open +function wfFoo() { + +} +?> +T<?php // this php open tag will in any case be ignored diff --git a/MediaWiki/Tests/files/NamingConventions/case_global_name_fail.php.fixed b/MediaWiki/Tests/files/NamingConventions/case_global_name_fail.php.fixed new file mode 100644 index 0000000..501e089 --- /dev/null +++ b/MediaWiki/Tests/files/NamingConventions/case_global_name_fail.php.fixed @@ -0,0 +1,7 @@ +<?php + +function wfFooFoo() { + // The below should have capital after wg + global $wgsomething; + $wgsomething = 5; +} diff --git a/MediaWiki/Tests/files/NamingConventions/wg_global_name2_fail.php.fixed b/MediaWiki/Tests/files/NamingConventions/wg_global_name2_fail.php.fixed new file mode 100644 index 0000000..c05f938 --- /dev/null +++ b/MediaWiki/Tests/files/NamingConventions/wg_global_name2_fail.php.fixed @@ -0,0 +1,8 @@ +<?php + +function wfFooFoo() { + // The first global is fine, the second isn't + global $wgContLang, $LocalInterwikis; + $wgContLang = 'en'; + $LocalInterwikis = false; +} diff --git a/MediaWiki/Tests/files/NamingConventions/wg_global_name_fail.php.fixed b/MediaWiki/Tests/files/NamingConventions/wg_global_name_fail.php.fixed new file mode 100644 index 0000000..2f00882 --- /dev/null +++ b/MediaWiki/Tests/files/NamingConventions/wg_global_name_fail.php.fixed @@ -0,0 +1,7 @@ +<?php + +function wfFooFoo() { + // The below should start with wg... + global $someotherglobal; + $someotherglobal = 5; +} diff --git a/MediaWiki/Tests/files/Usage/dir_usage_fail.php.fixed b/MediaWiki/Tests/files/Usage/dir_usage_fail.php.fixed new file mode 100644 index 0000000..79aa52b --- /dev/null +++ b/MediaWiki/Tests/files/Usage/dir_usage_fail.php.fixed @@ -0,0 +1,4 @@ +<?php + +$tmp = __DIR__; +echo "Done"; diff --git a/MediaWiki/Tests/files/VariableAnalysis/unused_global_variables_fail.php.fixed b/MediaWiki/Tests/files/VariableAnalysis/unused_global_variables_fail.php.fixed new file mode 100644 index 0000000..4c47bdf --- /dev/null +++ b/MediaWiki/Tests/files/VariableAnalysis/unused_global_variables_fail.php.fixed @@ -0,0 +1,6 @@ +<?php + +function wfFooFoo() { + // The global variable is not used + global $wgSomething; +} diff --git a/MediaWiki/Tests/files/WhiteSpace/multiple_empty_lines_fail.php.fixed b/MediaWiki/Tests/files/WhiteSpace/multiple_empty_lines_fail.php.fixed new file mode 100644 index 0000000..528a829 --- /dev/null +++ b/MediaWiki/Tests/files/WhiteSpace/multiple_empty_lines_fail.php.fixed @@ -0,0 +1,10 @@ +<?php +$a = true; + +$b = false; + +function wfFoo() { + $a = 1; + + $b = 2; +} diff --git a/MediaWiki/Tests/files/WhiteSpace/short_array_fail.php.fixed b/MediaWiki/Tests/files/WhiteSpace/short_array_fail.php.fixed new file mode 100644 index 0000000..2537b73 --- /dev/null +++ b/MediaWiki/Tests/files/WhiteSpace/short_array_fail.php.fixed @@ -0,0 +1,6 @@ +<?php + +$a = []; +$b = [ 'faaaail' ]; +$c = []; +$d = [ 'faaaaail', ]; diff --git a/MediaWiki/Tests/files/WhiteSpace/space_after_delim_singleline_comment_fail.php.fixed b/MediaWiki/Tests/files/WhiteSpace/space_after_delim_singleline_comment_fail.php.fixed new file mode 100644 index 0000000..193ba31 --- /dev/null +++ b/MediaWiki/Tests/files/WhiteSpace/space_after_delim_singleline_comment_fail.php.fixed @@ -0,0 +1,8 @@ +<?php +// a comment with tabs instead of a space +// +# yet another comment with tabs instead of a space. +# +// A comment without a space +# Yup, no spaces. + diff --git a/MediaWiki/Tests/files/WhiteSpace/space_before_parens_fail.php.fixed b/MediaWiki/Tests/files/WhiteSpace/space_before_parens_fail.php.fixed new file mode 100644 index 0000000..8b8cfbd --- /dev/null +++ b/MediaWiki/Tests/files/WhiteSpace/space_before_parens_fail.php.fixed @@ -0,0 +1,5 @@ +<?php +function wfBar() { + wfFoo(); + $a = []; +} diff --git a/MediaWiki/Tests/files/WhiteSpace/spacey_parenthesis_fail.php.fixed b/MediaWiki/Tests/files/WhiteSpace/spacey_parenthesis_fail.php.fixed new file mode 100644 index 0000000..18aa440 --- /dev/null +++ b/MediaWiki/Tests/files/WhiteSpace/spacey_parenthesis_fail.php.fixed @@ -0,0 +1,8 @@ +<?php + +function wfFooBar( $a, $b ) { + $a->foo(); + $a->foo( $b ); + $a->foo( $b ); + $c = []; +} diff --git a/composer.json b/composer.json index 4377cc0..99d5ac1 100644 --- a/composer.json +++ b/composer.json @@ -10,7 +10,8 @@ }, "require-dev": { "jakub-onderka/php-parallel-lint": "0.9.*", - "phpunit/phpunit": "~4.1" + "phpunit/phpunit": "~4.1", + "mikey179/vfsStream": "~1.6" }, "scripts": { "test": [ -- To view, visit https://gerrit.wikimedia.org/r/291001 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie7102faf842f56e6b93b04f02e90606f19fcaee1 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/tools/codesniffer Gerrit-Branch: master Gerrit-Owner: EBernhardson <ebernhard...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits