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

Mark concatenation inside another expression as variable read #186

Merged
merged 9 commits into from
Sep 15, 2020

Conversation

sirbrillig
Copy link
Owner

@sirbrillig sirbrillig commented Jul 5, 2020

Concatenation (or other variable assignment) counts as a "write" to a variable, but not as a "read". That way code like this produces an unused variable warning:

$foo = 'hi';
$foo .= ' friend';

However, in PHP if the assignment is inside another expression, then the assignment should count as a "read" as well, as in the following code, which should produce no warning:

$foo = 'hi';
sayHi($foo .= ' friend');

This PR makes the sniff aware of two such cases: when the assignment is on the right-hand-side (RHS) of another assignment, or when the assignment is inside a function call. There may be other expressions that an assignment could be part of, but this should take care of the majority of them.

Fixes #183

The list of pass-by-reference functions is only currently used to
determine if a function's argument might be counted as a define. All
arguments passed to the function are counted as a read. Functions like
`array_pop()` will throw a PHP warning if used with an undefined
variable, so this removes them. There are probably many others in this
list that can be removed, but this is a start.
@sirbrillig sirbrillig marked this pull request as ready for review September 15, 2020 21:05
@sirbrillig sirbrillig changed the title Fix concat and assign Mark concatenation inside another expression as variable use Sep 15, 2020
@sirbrillig sirbrillig changed the title Mark concatenation inside another expression as variable use Mark concatenation inside another expression as variable read Sep 15, 2020
@sirbrillig sirbrillig merged commit 70b296b into master Sep 15, 2020
@sirbrillig sirbrillig deleted the fix-concat-and-assign branch September 15, 2020 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mixing concatenating assignment operator and assigment operation results in false positive.
1 participant