Skip to content

Implement GH-10024: support linting multiple files at once using php -l #10710

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

Closed
wants to merge 5 commits into from
Closed
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
7 changes: 6 additions & 1 deletion sapi/cgi/cgi_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -2372,6 +2372,7 @@ consult the installation file that came with this distribution, or visit \n\
}
}

do_repeat:
if (script_file) {
/* override path_translated if -f on command line */
if (SG(request_info).path_translated) efree(SG(request_info).path_translated);
Expand Down Expand Up @@ -2512,7 +2513,6 @@ consult the installation file that came with this distribution, or visit \n\
PG(during_request_startup) = 0;
if (php_lint_script(&file_handle) == SUCCESS) {
zend_printf("No syntax errors detected in %s\n", ZSTR_VAL(file_handle.filename));
exit_status = 0;
} else {
zend_printf("Errors parsing %s\n", ZSTR_VAL(file_handle.filename));
exit_status = -1;
Expand Down Expand Up @@ -2581,6 +2581,11 @@ consult the installation file that came with this distribution, or visit \n\
}
}
}
if (behavior == PHP_MODE_LINT && argc - 1 > php_optind) {
php_optind++;
script_file = NULL;
goto do_repeat;
}
break;
}

Expand Down
117 changes: 117 additions & 0 deletions sapi/cgi/tests/012.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
--TEST--
multiple files syntax check
--SKIPIF--
<?php include "skipif.inc"; ?>
--INI--
display_errors=stdout
--FILE--
<?php
include "include.inc";

function run_and_output($cmd) {
if (!defined("PHP_WINDOWS_VERSION_MAJOR")) {
$cmd .= " 2>/dev/null";
}
exec($cmd, $output, $exit_code);
print_r($output);
// Normalize Windows vs Linux exit codes. On Windows exit code -1 is actually -1 instead of 255.
if ($exit_code < 0) {
$exit_code += 256;
}
var_dump($exit_code);
}

$php = get_cgi_path();
reset_env_vars();

$filename_good = __DIR__."/012_good.test.php";
$filename_good_escaped = escapeshellarg($filename_good);
$filename_bad = __DIR__."/012_bad.test.php";
$filename_bad_escaped = escapeshellarg($filename_bad);

$code = '
<?php

echo "hi";
';

file_put_contents($filename_good, $code);

$code = '
<?php

class test
private $var;
}

?>
';

file_put_contents($filename_bad, $code);

run_and_output("$php -n -l $filename_good_escaped $filename_good_escaped");
run_and_output("$php -n -l $filename_good_escaped some.unknown $filename_good_escaped");
run_and_output("$php -n -l $filename_good_escaped $filename_bad_escaped $filename_good_escaped");
run_and_output("$php -n -l $filename_bad_escaped $filename_bad_escaped");
run_and_output("$php -n -l $filename_bad_escaped some.unknown $filename_bad_escaped");
run_and_output("$php -n -l $filename_bad_escaped $filename_bad_escaped some.unknown");

echo "Done\n";
?>
--CLEAN--
<?php
@unlink($filename_good);
@unlink($filename_bad);
?>
--EXPECTF--
Array
(
[0] => No syntax errors detected in %s012_good.test.php
[1] => No syntax errors detected in %s012_good.test.php
)
int(0)
Array
(
[0] => No syntax errors detected in %s012_good.test.php
[1] => No input file specified.
Copy link
Member

Choose a reason for hiding this comment

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

This error is a bit confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this comes from lines 2457-2460. Either we can change the error message for CGI in general, or special case the linting case. I think changing the message in general might be dangerous for BC though.

Copy link
Member

Choose a reason for hiding this comment

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

This differs in behavior with CLI, in that it aborts when a file is not found. Is this a limitation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah we'd need to change error handling in lines 2451-2488: if a file cannot be opened or accessed CGI aborts. But I'm not sure if we should special case it because it will make things more complicated and possibly slow down regular use of CGI. Let me know what you think.

Copy link
Member

Choose a reason for hiding this comment

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

I doubt linting is often used with php-cgi, so this should be ok.

)
int(255)
Array
(
[0] => No syntax errors detected in %s012_good.test.php
[1] => <br />
[2] => <b>Parse error</b>: syntax error, unexpected token &quot;private&quot;, expecting &quot;{&quot; in <b>%s012_bad.test.php</b> on line <b>5</b><br />
[3] => Errors parsing %s012_bad.test.php
[4] => No syntax errors detected in %s012_good.test.php
)
int(255)
Array
(
[0] => <br />
[1] => <b>Parse error</b>: syntax error, unexpected token &quot;private&quot;, expecting &quot;{&quot; in <b>%s012_bad.test.php</b> on line <b>5</b><br />
[2] => Errors parsing %s012_bad.test.php
[3] => <br />
[4] => <b>Parse error</b>: syntax error, unexpected token &quot;private&quot;, expecting &quot;{&quot; in <b>%s012_bad.test.php</b> on line <b>5</b><br />
[5] => Errors parsing %s012_bad.test.php
)
int(255)
Array
(
[0] => <br />
[1] => <b>Parse error</b>: syntax error, unexpected token &quot;private&quot;, expecting &quot;{&quot; in <b>%s012_bad.test.php</b> on line <b>5</b><br />
[2] => Errors parsing %s012_bad.test.php
[3] => No input file specified.
)
int(255)
Array
(
[0] => <br />
[1] => <b>Parse error</b>: syntax error, unexpected token &quot;private&quot;, expecting &quot;{&quot; in <b>%s012_bad.test.php</b> on line <b>5</b><br />
[2] => Errors parsing %s012_bad.test.php
[3] => <br />
[4] => <b>Parse error</b>: syntax error, unexpected token &quot;private&quot;, expecting &quot;{&quot; in <b>%s012_bad.test.php</b> on line <b>5</b><br />
[5] => Errors parsing %s012_bad.test.php
[6] => No input file specified.
)
int(255)
Done
11 changes: 10 additions & 1 deletion sapi/cli/php_cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,10 @@ static int do_cli(int argc, char **argv) /* {{{ */
break;
}
behavior=PHP_MODE_LINT;
/* We want to set the error exit status if at least one lint failed.
* If all were successful we set the exit status to 0.
* We already set EG(exit_status) here such that only failures set the exit status. */
EG(exit_status) = 0;
break;

case 'q': /* do not generate HTTP headers */
Expand Down Expand Up @@ -962,7 +966,6 @@ static int do_cli(int argc, char **argv) /* {{{ */
case PHP_MODE_LINT:
if (php_lint_script(&file_handle) == SUCCESS) {
zend_printf("No syntax errors detected in %s\n", php_self);
EG(exit_status) = 0;
} else {
zend_printf("Errors parsing %s\n", php_self);
EG(exit_status) = 255;
Expand Down Expand Up @@ -1128,9 +1131,15 @@ static int do_cli(int argc, char **argv) /* {{{ */
}
if (request_started) {
php_request_shutdown((void *) 0);
request_started = 0;
}
if (translated_path) {
free(translated_path);
translated_path = NULL;
}
if (behavior == PHP_MODE_LINT && argc > php_optind && strcmp(argv[php_optind],"--")) {
script_file = NULL;
goto do_repeat;
}
/* Don't repeat fork()ed processes. */
if (--num_repeats && pid == getpid()) {
Expand Down
110 changes: 110 additions & 0 deletions sapi/cli/tests/024.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
--TEST--
multiple files syntax check
--SKIPIF--
<?php include "skipif.inc"; ?>
--FILE--
<?php

function run_and_output($cmd) {
exec($cmd, $output, $exit_code);
print_r($output);
var_dump($exit_code);
}

$php = getenv('TEST_PHP_EXECUTABLE_ESCAPED');

$filename_good = __DIR__."/024_good.test.php";
$filename_good_escaped = escapeshellarg($filename_good);
$filename_bad = __DIR__."/024_bad.test.php";
$filename_bad_escaped = escapeshellarg($filename_bad);

$code = '
<?php

echo "hi";
';

file_put_contents($filename_good, $code);

$code = '
<?php

class test
private $var;
}

?>
';

file_put_contents($filename_bad, $code);

run_and_output("$php -n -l $filename_good_escaped $filename_good_escaped 2>&1");
run_and_output("$php -n -l $filename_good_escaped some.unknown $filename_good_escaped 2>&1");
run_and_output("$php -n -l $filename_good_escaped $filename_bad_escaped $filename_good_escaped 2>&1");
run_and_output("$php -n -l $filename_bad_escaped $filename_bad_escaped 2>&1");
run_and_output("$php -n -l $filename_bad_escaped some.unknown $filename_bad_escaped 2>&1");
run_and_output("$php -n -l $filename_bad_escaped $filename_bad_escaped some.unknown 2>&1");

echo "Done\n";
?>
--CLEAN--
<?php
@unlink($filename_good);
@unlink($filename_bad);
?>
--EXPECTF--
Array
(
[0] => No syntax errors detected in %s024_good.test.php
[1] => No syntax errors detected in %s024_good.test.php
)
int(0)
Array
(
[0] => No syntax errors detected in %s024_good.test.php
[1] => Could not open input file: some.unknown
[2] => No syntax errors detected in %s024_good.test.php
)
int(1)
Array
(
[0] => No syntax errors detected in %s024_good.test.php
[1] =>
[2] => Parse error: syntax error, unexpected token "private", expecting "{" in %s on line %d
[3] => Errors parsing %s024_bad.test.php
[4] => No syntax errors detected in %s024_good.test.php
)
int(255)
Array
(
[0] =>
[1] => Parse error: syntax error, unexpected token "private", expecting "{" in %s on line %d
[2] => Errors parsing %s024_bad.test.php
[3] =>
[4] => Parse error: syntax error, unexpected token "private", expecting "{" in %s on line %d
[5] => Errors parsing %s024_bad.test.php
)
int(255)
Array
(
[0] =>
[1] => Parse error: syntax error, unexpected token "private", expecting "{" in %s on line %d
[2] => Errors parsing %s024_bad.test.php
[3] => Could not open input file: some.unknown
[4] =>
[5] => Parse error: syntax error, unexpected token "private", expecting "{" in %s on line %d
[6] => Errors parsing %s024_bad.test.php
)
int(255)
Array
(
[0] =>
[1] => Parse error: syntax error, unexpected token "private", expecting "{" in %s on line %d
[2] => Errors parsing %s024_bad.test.php
[3] =>
[4] => Parse error: syntax error, unexpected token "private", expecting "{" in %s on line %d
[5] => Errors parsing %s024_bad.test.php
[6] => Could not open input file: some.unknown
)
int(1)
Done