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.

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

Rewrite PSR12.Files.DeclareStatement sniff #578

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
449 changes: 294 additions & 155 deletions src/Standards/PSR12/Sniffs/Files/DeclareStatementSniff.php

Large diffs are not rendered by default.

20 changes: 18 additions & 2 deletions src/Standards/PSR12/Tests/Files/DeclareStatementUnitTest.1.inc
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ declare(/* test */ ticks /* test */ =1);
declare( ticks
=1 );

declare(ticks=1) { }
declare(ticks=1) { $test = true; }

declare(ticks=1)
{ }
{ $test = true; }

declare(ticks=1)
{
Expand Down Expand Up @@ -48,3 +48,19 @@ declare(ticks=1) {$test = true;

declare(ticks=1) { $test = true;
}

// Ensure that we handle non-number values properly.
declare(encoding='ISO-8859-1');
declare ( encoding = 'ISO-8859-1' ) ;

// Multi-directive statements should be handled correctly too.
declare(strict_types=1,ticks=1);
// Note that PSR12 makes no mention of the formatting of the comma, so all of these should be valid.
declare(strict_types=1, ticks=1);
declare(strict_types=1, ticks=1);
declare(strict_types=1,
ticks=1);
declare(strict_types=1,
ticks=1);
declare(strict_types=1 , ticks=1);
declare(strict_types=1 ,ticks=1);
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ declare(/* test */ ticks /* test */ =1);
declare(ticks=1);

declare(ticks=1) {
$test = true;
}

declare(ticks=1) {
$test = true;
}

declare(ticks=1) {
Expand All @@ -26,7 +28,8 @@ declare(ticks=1) {
}//end comment

declare(ticks=1) {
}$x =1;
}
$x =1;

declare(ticks=1) {
$test = true;
Expand All @@ -36,19 +39,36 @@ declare(ticks // phpcs:ignore Standard.Category.SniffName -- fixing is undesirab
=1);

declare(ticks=1) {
}$x =1; // phpcs:ignore Standard.Category.SniffName -- fixing is undesirable
}
$x =1; // phpcs:ignore Standard.Category.SniffName -- fixing is undesirable

declare(ticks=1) { // test
}

declare(ticks=1) {
$test = true;
$test = true;
}

declare(ticks=1) { /* test */ /* test */
$test = true;
$test = true;
}

declare(ticks=1) {
$test = true;
$test = true;
}

// Ensure that we handle non-number values properly.
declare(encoding='ISO-8859-1');
declare(encoding='ISO-8859-1');

// Multi-directive statements should be handled correctly too.
declare(strict_types=1,ticks=1);
// Note that PSR12 makes no mention of the formatting of the comma, so all of these should be valid.
declare(strict_types=1, ticks=1);
declare(strict_types=1, ticks=1);
declare(strict_types=1,
ticks=1);
declare(strict_types=1,
ticks=1);
declare(strict_types=1 , ticks=1);
declare(strict_types=1 ,ticks=1);
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php
// Parse error. Missing directive value.
declare(ticks=)
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php
// Parse error. Missing semicolon (or PHP close tag).
declare(ticks=1)
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php
// Parse error. Missing closing curly bracket
declare(ticks=1) {
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?php
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a copy of src/Standards/Generic/Tests/PHP/RequireStrictTypesUnitTest.5.inc to ensure that the sniff does not act on line 6.


// This is a parse error, but the sniff should still handle this correctly.
declare($something = $value);

const STRICT_TYPES = 1;
64 changes: 64 additions & 0 deletions src/Standards/PSR12/Tests/Files/DeclareStatementUnitTest.4.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php
declare(ticks=1);
declare (ticks=1);
declare( ticks=1);
declare(ticks =1);
declare(ticks= 1);
declare(ticks=1 );
declare(ticks=1) ;
declare (ticks=1);
declare( ticks=1);
declare(ticks =1);
declare(ticks= 1);
declare(ticks=1 );
declare(ticks=1) ;
declare (ticks=1);
declare( ticks=1);
declare(ticks =1);
declare(ticks= 1);
declare(ticks=1 );
declare(ticks=1) ;
declare
(ticks=1);
declare(
ticks=1);
declare(ticks
=1);
declare(ticks=
1);
declare(ticks=1
);
declare(ticks=1)
;
declare
(ticks=1);
declare(
ticks=1);
declare(ticks
=1);
declare(ticks=
1);
declare(ticks=1
);
declare(ticks=1)
;
declare/* test */(ticks=1);
declare(/* test */ticks=1);
declare(ticks/* test */=1);
declare(ticks=/* test */1);
declare(ticks=1/* test */);
declare(ticks=1)/* test */;
declare(ticks=1);/* test */
declare// test
(ticks=1);
declare(// test
ticks=1);
declare(ticks// test
=1);
declare(ticks=// test
1);
declare(ticks=1// test
);
declare(ticks=1)// test
;
declare(ticks=1);// test
23 changes: 23 additions & 0 deletions src/Standards/PSR12/Tests/Files/DeclareStatementUnitTest.5.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php declare(strict_types=1); ?>
The line above is correct. The line below should complain that it's not the first line.
<?php declare(strict_types=1); ?>

This line should complain. It's included because the optional semicolon is missing here.
<?php declare(strict_types=1) ?>

This should complain that all three parts are not on the first line of the file.
<?php
declare(strict_types=1);
?>

This is fine because it's not strict_types.
<?php declare(ticks=1); ?>

These can be automatically fixed because there is an incorect number of spaces
between the end of the declare statement and the closing PHP tag.
<?php declare(ticks=1)?>
<?php declare(ticks=1) ?>
<?php declare(ticks=1)/* test */?>

This is missing its closing PHP tag. It should be the last test in this file.
<?php declare(strict_types=1);
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php
// Parse error. Missing opening parenthesis.
declare
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php
// Parse error. Missing closing parenthesis.
declare(
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php
// Parse error. Missing directive.
declare()
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php
// Parse error. Missing equals sign.
declare(ticks)
66 changes: 62 additions & 4 deletions src/Standards/PSR12/Tests/Files/DeclareStatementUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,77 @@ public function getErrorList($testFile='')
10 => 1,
11 => 3,
12 => 2,
13 => 1,
14 => 2,
13 => 2,
14 => 1,
Comment on lines +48 to +49
Copy link
Member Author

Choose a reason for hiding this comment

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

The same things are being detected here. The difference is that we are highlighting the problem token, not the correct token. This is a consistency benefit from using the new private method on the sniff.

16 => 3,
18 => 1,
19 => 3,
22 => 1,
21 => 1,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the same as lines 13 & 14 above.

24 => 1,
26 => 3,
26 => 2,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the same as line 16 above.

28 => 3,
34 => 2,
38 => 1,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is an error that the previous version of this sniff didn't detect.

43 => 1,
46 => 1,
47 => 1,
49 => 1,
54 => 6,
];
case 'DeclareStatementUnitTest.4.inc':
return [
3 => 1,
4 => 1,
5 => 1,
6 => 1,
7 => 1,
8 => 1,
9 => 1,
10 => 1,
11 => 1,
12 => 1,
13 => 1,
14 => 1,
15 => 1,
16 => 1,
17 => 1,
18 => 1,
19 => 1,
20 => 1,
21 => 1,
23 => 1,
25 => 1,
27 => 1,
29 => 1,
31 => 1,
33 => 1,
35 => 1,
37 => 1,
39 => 1,
41 => 1,
43 => 1,
45 => 1,
46 => 1,
47 => 1,
48 => 1,
49 => 1,
50 => 1,
52 => 1,
54 => 1,
56 => 1,
58 => 1,
60 => 1,
62 => 1,
];
case 'DeclareStatementUnitTest.5.inc':
return [
3 => 1,
6 => 1,
10 => 2,
18 => 1,
19 => 1,
20 => 1,
23 => 1,
];
default:
return [];
Expand Down