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

List assignment next to array causes unused variable warning #317

Closed
djibarian opened this issue Mar 14, 2024 · 5 comments · Fixed by #318
Closed

List assignment next to array causes unused variable warning #317

djibarian opened this issue Mar 14, 2024 · 5 comments · Fixed by #318
Labels

Comments

@djibarian
Copy link

This bit is throwing a VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable false positive for $notificationId. It seems it has to do with array destructuring the result, because if changed to $bar it disappears.

static function foo(DBC $db, int $notificationId): void {
	[$bar] = $db->queryGetRowIndexed("
		SELECT blah...
	", [
		"n.notificationId" => $notificationId
	]);
}

And this one too, in $level, although in this case it looks related to that "self reference assignment":

static function dotToMultiArray(
	array $array
): array {
	$multi = [];
	foreach($array as $key => $value){
		$level = &$multi;
		foreach(explode(".", $key) as $node)
			$level = &$level[$node];
		$level = $value;
	}
	return $multi;
}
@sirbrillig sirbrillig added the bug label Mar 24, 2024
@sirbrillig
Copy link
Owner

sirbrillig commented Mar 24, 2024

Thanks for the report!

It seems it has to do with array destructuring the result, because if changed to $bar it disappears.

I think you're right because this passes:

static function foo(DBC $db, int $notificationId): void {
	list($bar) = $db->queryGetRowIndexed('SELECT blah... ', [ 'n.notificationId' => $notificationId ]);
	echo $bar;
}

Whereas this fails:

static function foo(DBC $db, int $notificationId): void {
	[$bar] = $db->queryGetRowIndexed('SELECT blah... ', [ 'n.notificationId' => $notificationId ]);
	echo $bar;
}

Even though both are destructuring. What's really weird is that the warning is Unused function parameter $notificationId. which has nothing to do with $bar.

Here's a more minimal case with the same bug:

function foo(int $baz) {
	[$bar] = doSomething([$baz]);
	return $bar;
}

Investigating...

@sirbrillig sirbrillig changed the title False positives with UnusedVariable List assignment next to array causes unused variable warning Mar 24, 2024
@sirbrillig
Copy link
Owner

sirbrillig commented Mar 24, 2024

#318 should fix the issue you found, although you'll still get an unused variable warning in your first example unless you do something with $bar like print it or return it since otherwise $bar is not used.

As for the second issue,

function dotToMultiArray( array $array ): array {
	$multi = [];
	foreach($array as $key => $value){
		$level = &$multi;                // $level is assigned
		foreach(explode(".", $key) as $node)
			$level = &$level[$node]; // $level is assigned again
		$level = $value;                 // $level is assigned a third time
	}
	return $multi;                           // We ignore $level and return $multi
}

In this case, $level is marked as unused because it is. It's only ever assigned a value but that value is never used for anything. Maybe I'm misunderstanding something?

I think that example is equivalent to:

function dotToMultiArray( array $array ): array {
	$multi = [];
	return $multi;
}

@djibarian
Copy link
Author

...although you'll still get an unused variable warning in your first example unless you do something with $bar like print it or return it since otherwise $bar is not used.

Yes, the function wasn't complete and $bar is used afterwards. Thanks for the quick fix!

Re the second case, $level is indeed used, look carefully. It is used as a pointer to traverse a multi-dimensional array (first and second) and set the value of one of its nodes (third). Probably the assignments by reference are confusing the linter.

@sirbrillig
Copy link
Owner

Interesting... Does that actually work to run $level = $value and assign to a sub element of $multi? I thought that assigning directly to the variable would overwrite the reference itself? Is it because the reference is pointing to an array element and not the array?

@sirbrillig
Copy link
Owner

Oh! It does. Wild. I made #319 to investigate that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants