Skip to content

Don't unwrap a lambda whose callee contains a call - #572

Open
Eljees wants to merge 1 commit into
Instagram:mainfrom
Eljees:fix/509-no-redundant-lambda-unsafe-unwrap
Open

Don't unwrap a lambda whose callee contains a call#572
Eljees wants to merge 1 commit into
Instagram:mainfrom
Eljees:fix/509-no-redundant-lambda-unsafe-unwrap

Conversation

@Eljees

@Eljees Eljees commented Aug 28, 2026

Copy link
Copy Markdown

Fixes #509.

The problem

NoRedundantLambda rewrites lambda: f() into f. That is only equivalent
while f is re-evaluated on every call. When the callee itself contains a
call, the autofix moves that inner call out of the lambda body, so it runs
once at definition time and its result is frozen:

# before
scheduler.add(lambda: datetime.now().isoformat())

# after the autofix
scheduler.add(datetime.now().isoformat)

The rewritten version binds isoformat to one fixed datetime instance
captured when the line was evaluated, instead of reading the clock on each
call. The lambda was not redundant, and the fix is applied automatically.

The change

Skip the report when the callee contains a call of its own:

if m.findall(call.func, m.Call()):
    return

Three valid cases pin the behaviour, covering the shapes this can take:
an attribute of a call result, a method on a returned object, and a
subscript of a call result.

Tests

ran result
main 389 OK
new cases, without the guard 392 FAILED (failures=3)
new cases, with the guard 392 OK

Without the guard the three new cases fail on the rule's own output rather
than on a count:

FAIL: test_VALID_10 (fixit.tests.NoRedundantLambda.test_VALID_10)
AssertionError: 1 != 0 : Expected zero reports for this "valid" test case.
Instead, found: LintViolation(rule_name='NoRedundantLambda', ...
message='The lambda that is wrapping datetime.now.isoformat is redundant.
It can unwrapped safely and used purely.')

No test that passes on main fails with the change. flake8 on the touched
file is clean.

One thing this deliberately does not cover

The existing invalid case

lambda x, y, z: (t + u).math_call(x, y, z)

is unsafe for a related but different reason: t + u is evaluated once
after unwrapping. That callee contains no call, so this change leaves it
reported exactly as before. I did not want to widen the scope without
knowing whether you consider that one intended — happy to extend the guard
to any non-trivial callee expression if you'd prefer.

NoRedundantLambda rewrites lambda: f() into f, which is safe only while
the callee is re-evaluated on each call. When the callee itself contains
a call, unwrapping evaluates that inner call once, at definition time,
and freezes its result:

    lambda: datetime.now().isoformat()  ->  datetime.now().isoformat

The rewritten form binds one fixed datetime instead of reading the clock
on each call, so the autofix silently changes behaviour.

Skip the report when the callee contains a call of its own, and pin the
behaviour with three valid cases covering an attribute of a call, a
method on a returned object, and a subscript of a call result.

Fixes Instagram#509
@Eljees
Eljees requested a review from amyreese as a code owner August 28, 2026 17:06
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NoRedundantLambda's autofix is not always safe

1 participant