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

false positive on Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed and compact #3745

Open
tebeso opened this issue Jan 12, 2023 · 5 comments

Comments

@tebeso
Copy link

tebeso commented Jan 12, 2023

Describe the bug
Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed is triggered when variables are used via compact in function

Code sample

public static function insertMessage(int $id, string $language, string $translation): void
{
   Yii::$app->db->createCommand()->insert(
      'message',
      compact('id', 'language', 'translation')
   )->execute();
}
@westonruter
Copy link
Contributor

Just seeing this as well. My code sample:

function get_transient_key( string $content, array $attributes, bool $is_feed, array $auto_detect_languages ): ?string {
	$hash_input = wp_json_encode(
		array_merge(
			compact( 'content', 'attributes', 'is_feed', 'auto_detect_languages' ),
			[
				'version' => PLUGIN_VERSION,
			]
		)
	);
	if ( ! is_string( $hash_input ) ) {
		return null;
	}
	return 'shcb-' . md5( $hash_input );
}

@jrfnl
Copy link
Contributor

jrfnl commented Oct 27, 2023

I concur this is a bug, but one which may not be that easy to fix.

To be honest, in my personal opinion, the use of compact() is a bit outdated and I'm actually surprised that PHP hasn't deprecated the function yet.

Out of curiousity: why not just declare the array ?

To take @westonruter's code sample, this could be rewritten to:

	$hash_input = wp_json_encode(
		array(
			'content' => $content,
			'attributes' => $attributes,
			'is_feed' => $is_feed,
			'auto_detect_languages' => $auto_detect_languages,
			'version' => PLUGIN_VERSION,
		)
	);

This is also a micro-optimization as it gets rid of two unnecessary function calls.

@westonruter
Copy link
Contributor

I guess because it's less redundant. JavaScript supports this same syntax as part of the language, for example if the above were written in JS instead:

const hash_input = JSON.stringify(
    {
        content, 
        attributes, 
        is_feed, 
        auto_detect_languages,
        version: PLUGIN_VERSION
    }
);

Granted this is PHP and not JS.

@jrfnl
Copy link
Contributor

jrfnl commented Oct 27, 2023

Granted this is PHP and not JS.

Exactly.

@tebeso
Copy link
Author

tebeso commented Oct 27, 2023

In my case, i was working with a legacy app, where I had to live with compact (only had to use it for this one project and didnt like it at all, but thats another thing)

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