Skip to content

semgrep-rules/javascript/react/correctness/hooks/set-state-no-op.yaml does not correctly account for variable shadowing in {...} destructuring inside a lambda #3709

@aaronshim

Description

@aaronshim

Describe the bug
The following (because of pattern-inside:) appears to suggest that this should only trigger if the $X in $Y($X) is still the same $X from const [$X, $Y] = useState(...);

    patterns:
      - pattern: $Y($X);
      - pattern-inside: |
          const [$X, $Y] = useState(...);

and we have the following example code in the rule page

const [actionsExpanded, setActionsExpanded] = useState<boolean>(false);
...
<Button
    small
    minimal
    onClick={() => {
      // ruleid:calling-set-state-on-current-state
      setActionsExpanded(actionsExpanded);
    }}
    intent={actionsExpanded ? Intent.PRIMARY : Intent.NONE}
  >

which triggers as expected. But this variant is not working as intended:

const [actionsExpanded, setActionsExpanded] = useState<boolean>(false);
...
<Button
    small
    minimal
    onClick={(actionsExpanded) => {
      // ruleid:calling-set-state-on-current-state SHOULD NOT trigger
      // because actionsExpanded is shadowed in the lambda.
      // Result: DOES NOT trigger (correct)
      setActionsExpanded(actionsExpanded);
    }}
    intent={actionsExpanded ? Intent.PRIMARY : Intent.NONE}
  >
...
<Button
    small
    minimal
    onClick={({actionsExpanded}) => {
      // ruleid:calling-set-state-on-current-state SHOULD NOT trigger
      // because actionsExpanded is shadowed in the lambda.
      // Result: DOES trigger (incorrect)
      setActionsExpanded(actionsExpanded);
    }}
    intent={actionsExpanded ? Intent.PRIMARY : Intent.NONE}
  >

Am I misunderstanding the capability of pattern-inside? Is there a different way of writing this rule that will respect {...} destructuring in a lambda?

To Reproduce

const [actionsExpanded, setActionsExpanded] = useState<boolean>(false);
...
<Button
    small
    minimal
    onClick={({actionsExpanded}) => {
      // ruleid:calling-set-state-on-current-state SHOULD NOT trigger
      // because actionsExpanded is shadowed in the lambda.
      // Result: DOES trigger (incorrect)
      setActionsExpanded(actionsExpanded);
    }}
    intent={actionsExpanded ? Intent.PRIMARY : Intent.NONE}
  >

Expected behavior

The rule should not trigger in the example above where $X is shadowed in a lambda like ({$X}) => {...}

Priority
How important is this to you?

  • P0: blocking me from making progress
  • P1: this will block me in the near future
  • P2: annoying but not blocking me

Additional Context
Thank you!

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions