Skip to content
This repository has been archived by the owner on Dec 1, 2024. It is now read-only.

add lambda support to Unused.*Linter #342

Merged
merged 1 commit into from
Feb 12, 2021
Merged

Conversation

jjergus
Copy link
Contributor

@jjergus jjergus commented Feb 6, 2021

Fixes #341

  • improves some other cases (unused params/variables inside lambdas are caught)
  • some false negatives still exist (using a shadowed param/variable inside a lambda also marks it as used in the enclosing function) -- probably fixable but I didn't want to do too much in one PR

@jjergus
Copy link
Contributor Author

jjergus commented Feb 6, 2021

@ryangreenberg I can't add you as a reviewer but you might also be a good person to review this.

Copy link
Contributor

@ryangreenberg ryangreenberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition! I think your use of traverse() in a few places makes it easier to read and less dependent on knowing the exact hierarchy of the AST.

@@ -25,15 +25,26 @@ final class UnusedParameterLinter extends AutoFixingASTLinter {
return null;
}

$name = $node->getName();
if (!$name is VariableToken) {
$name_node = $node->getName();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a future change, I think this is roughly where we want to check that $node is a child of $functionish, not any descendant to fix #341

@jjergus jjergus merged commit 52c21a8 into hhvm:master Feb 12, 2021
@jjergus jjergus deleted the unused-fix branch February 12, 2021 23:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UnusedParameter and UnusedVariable false positives in lambda default arguments
3 participants