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

Incorrect line numbers output for Unused Variable errors #446

Closed
paulschreiber opened this issue Sep 5, 2019 · 6 comments
Closed

Incorrect line numbers output for Unused Variable errors #446

paulschreiber opened this issue Sep 5, 2019 · 6 comments
Milestone

Comments

@paulschreiber
Copy link
Contributor

Bug Description

Incorrect line numbers are output for Unused Variable errors.

Minimal Code Snippet

<?php
function foo( $html ) {
	list( $red, $green, $blue ) = something( $html );
	count( $red );
}

Error Code

$ vendor/bin/phpcs --severity=1  ~/test.php 
W 1 / 1 (100%)



FILE: Users/schreipa/test.php
---------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------
 1 | WARNING | Unused variable `$green`.
   |         | (WordPressVIPMinimum.Variables.VariableAnalysis.UnusedVariable)
 1 | WARNING | Unused variable `$blue`.
   |         | (WordPressVIPMinimum.Variables.VariableAnalysis.UnusedVariable)
---------------------------------------------------------------------------------------------------------

Time: 295ms; Memory: 10MB

Environment

Use php -v and composer show to get versions.

Question Answer
PHP version 7.3.3
PHP_CodeSniffer version 3.4.2
VIPCS version 2.0.0

Additional Context (optional)

Enable these rulesets:

	<rule ref="WordPress-Extra" />
	<rule ref="WordPress-VIP-Go" />
@GaryJones
Copy link
Contributor

Good catch, and thanks for reporting.

The version of VariableAnalysis we have is somewhat out of date - #236 is the ticket to resolve this, and I'd like to find time with @sirbrillig on the best way to get VIPCS using the up to date version.

I'll leave this ticket open for now, as a reminder to check that this particular edge case is addressed once we have the new version of VA, but it's honestly unlikely to get addressed directly in the meantime.

@sirbrillig
Copy link
Member

👋 @GaryJones I'm happy to go over this together some time! Maybe ping me on Slack and we can chat about it in more detail?

@paulschreiber
Copy link
Contributor Author

Ran in to this again. Any luck with your investigation?

@paulschreiber
Copy link
Contributor Author

Hit this again today with liveblog.

@sirbrillig
Copy link
Member

Looks like the discussion on this has moved to #449

(Please let me know if I can help, VIP friends!)

@GaryJones
Copy link
Contributor

FILE: /Users/gary/code/phpcs-test.php
-------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------
 3 | WARNING | Unused variable $green.
   |         | (VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable)
 3 | WARNING | Unused variable $blue.
   |         | (VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable)
-------------------------------------------------------------------------------------------------------

Time: 112ms; Memory: 6MB

Now that #449 is completed to use VariableAnalysis proper, I've confirmed that the provided example above doesn't give an error with the develop version of WordPress-VIP-Go, so this is now fixed for VIPCS 2.2.0.

Also, note #498 where we may silence the violations for unused variables completely.

@GaryJones GaryJones added this to the 2.2.0 milestone Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants