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

Reply via email to