-
-
Notifications
You must be signed in to change notification settings - Fork 494
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
Fix some undefined index notices in the WordPress.XSS.EscapeOutput
sniff.
#770
Conversation
…sniff. These would be thrown when live coding/running the sniffs on code with parse errors.
@@ -208,6 +208,11 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) { | |||
|
|||
if ( T_OPEN_PARENTHESIS === $this->tokens[ $i ]['code'] ) { | |||
|
|||
if ( ! isset( $this->tokens[ $i ]['parenthesis_closer'] ) ) { | |||
// Live coding or parse error. | |||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually when PHPCS's core sniffs detect a parse error, they don't just skip out, they add a warning. At least they do in some cases. Should we consider doing the same here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO that's what the Generic.PHP.Syntax
sniff is for (included in extra
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that makes sense. So I guess for consistency we should change the one place we give an error for syntax issues. But that can be a separate PR, I guess.
$i = $this->tokens[ $function_opener ]['parenthesis_closer']; | ||
} else { | ||
// Live coding or parse error. | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested the PR and confirm it works. 😄
These would be thrown when live coding/running the sniffs on code with parse errors.
Quick fix for #248 (comment)
/cc @grappler