Fix some undefined index notices in the `WordPress.XSS.EscapeOutput` sniff. #770

Merged
merged 1 commit into from Jan 9, 2017

Projects

None yet

3 participants

@jrfnl
Contributor
jrfnl commented Jan 5, 2017

These would be thrown when live coding/running the sniffs on code with parse errors.

Quick fix for #248 (comment)

/cc @grappler

@jrfnl jrfnl Fix some undefined index notices in the `WordPress.XSS.EscapeOutput` …
…sniff.

These would be thrown when live coding/running the sniffs on code with parse errors.
c07b4c4
@jrfnl jrfnl added this to the 0.11.0 milestone Jan 5, 2017
@@ -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;
@JDGrimes
JDGrimes Jan 5, 2017 Contributor

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?

@jrfnl
jrfnl Jan 5, 2017 Contributor

IMHO that's what the Generic.PHP.Syntax sniff is for (included in extra)

@JDGrimes
JDGrimes Jan 5, 2017 Contributor

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;
@JDGrimes
JDGrimes Jan 5, 2017 Contributor

Same as above.

@JDGrimes JDGrimes added a commit that referenced this pull request Jan 5, 2017
@JDGrimes JDGrimes Don't error for syntax errors 2c6c989
@grappler

Tested the PR and confirm it works. 😄

@JDGrimes JDGrimes added a commit that referenced this pull request Jan 5, 2017
@JDGrimes @JDGrimes JDGrimes + JDGrimes Don't error for syntax errors ce37414
@jrfnl jrfnl merged commit 1cc17af into develop Jan 9, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@jrfnl jrfnl deleted the feature/issue-248-undefined-notices branch Jan 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment