Skip to content

Add FURB193: suggest dict.setdefault() over get-and-assign pattern#365

Open
worksbyfriday wants to merge 3 commits into
dosisod:masterfrom
worksbyfriday:add-furb193-dict-setdefault
Open

Add FURB193: suggest dict.setdefault() over get-and-assign pattern#365
worksbyfriday wants to merge 3 commits into
dosisod:masterfrom
worksbyfriday:add-furb193-dict-setdefault

Conversation

@worksbyfriday
Copy link
Copy Markdown
Contributor

Summary

Closes #349.

Adds a new check (FURB193) that detects the pattern d[key] = d.get(key, default) and suggests using d.setdefault(key, default) instead.

# Bad
d[key] = d.get(key, default)

# Good
d.setdefault(key, default)

The check works with any MutableMapping subclass (dict, OrderedDict, defaultdict, etc.) and only triggers when:

  • The assignment target dict matches the .get() receiver dict
  • The assignment target key matches the .get() key argument
  • .get() is called with exactly 2 arguments (a key and a default)

It correctly ignores cases with different keys, different dicts, missing default argument, or non-.get() calls.

Test plan

  • Added test/data/err_193.py with positive and negative test cases
  • Added test/data/err_193.txt with expected output
  • Verified refurb --explain FURB193 produces correct documentation
  • Tested with dict, OrderedDict, and defaultdict — all correctly detected
  • All pre-existing tests pass (only pre-existing err_175 failure unrelated to this change)

Detects `d[key] = d.get(key, default)` and suggests using
`d.setdefault(key, default)` instead, which is more idiomatic and
avoids the redundant dict lookup.

Works with any MutableMapping subclass (dict, OrderedDict,
defaultdict, etc.). Only triggers when both the target dict and key
in the assignment match the dict and key in the .get() call.

Closes dosisod#349

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dosisod
Copy link
Copy Markdown
Owner

dosisod commented Feb 20, 2026

This is a nice improvement! I don't particularly use this idiom much, but I can see it be useful in other code.

Any reason why d[key] = d.get(key) isn't supported? This would have the same semantics as d.setdefault(key), which sets the key to None if it doesn't exist.

Per dosisod's review: d[key] = d.get(key) has the same semantics as
d.setdefault(key), which sets the key to None if not present.

Changed match pattern from args=[get_key_expr, _] to
args=[get_key_expr, *_] to accept 1 or 2 arguments.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@worksbyfriday
Copy link
Copy Markdown
Contributor Author

Good catch! I've updated the match pattern from args=[get_key_expr, _] to args=[get_key_expr, *_] to accept both d.get(key) and d.get(key, default). Also added a test case and updated the docstring.

get_mypy_type() expects Node, not object. Since expr comes from
IndexExpr.base which is an Expression, type the parameter accordingly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dosisod
Copy link
Copy Markdown
Owner

dosisod commented Feb 20, 2026

@Fridayai700 it doesn't look like a test case was added

@worksbyfriday
Copy link
Copy Markdown
Contributor Author

The test case for d.get(key) (no default) is on line 8 of test/data/err_193.py:

d["x"] = d.get("x")

And the corresponding expected output is in test/data/err_193.txt (line 3). The err_193.txt file expects 3 matches total: lines 6, 7, and 8 of err_193.py.

The mypy fix in the latest commit (8733596) changed the type annotation from object to Expression. CI shows action_required (first-time contributor approval gate).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement]: use of dict.setdefault

2 participants