[flake8-comprehensions] Detect overshadowed list/set/dict, ignore variadics and named expressions (C417)

[InSyncWithFoo]

Summary

Part of #15809 and #15876.

This change brings several bugfixes:

• The nested map() call in list(map(lambda x: x, [])) where list is overshadowed is now correctly reported.
• The call will no longer reported if:
  • Any arguments given to map() are variadic.
  • Any of the iterables contain a named expression.

Test Plan

cargo nextest run and cargo insta test.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[MichaReiser]

Thank you. It's sort of difficult to see the actual fixes in between the refactors you did. Would you mind commenting on the diff to highlight the fixes for each bullet point?

[InSyncWithFoo]

Would you mind commenting on the diff to highlight the fixes for each bullet point?

The diff is a tad too big to reason about, so here's something else to the same effect.

---

The nested map() call in list(map(lambda x: x, [])) where list is overshadowed is now correctly reported.

Before:

if parent
    .and_then(Expr::as_call_expr)
    .and_then(|call| call.func.as_name_expr())
    .is_some_and(|name| matches!(name.id.as_str(), "list" | "set" | "dict"))
{
    return;
}

After:

if parent_call_func.is_some_and(|func| is_list_set_or_dict(func, semantic)) {
    return;
}

// ...

fn is_list_set_or_dict(func: &Expr, semantic: &SemanticModel) -> bool {
    semantic
        .resolve_qualified_name(func)
        .is_some_and(|qualified_name| {
            matches!(
                qualified_name.segments(),
                ["" | "builtins", "list" | "set" | "dict"]
            )
        })
}

• The call will no longer reported if:
  • Any arguments given to map() are variadic.
  • Any of the iterables contain a named expression.

This check was present, but only for the ::Generator branch:

// For example, (x+1 for x in (c:=a)) is invalid syntax
// so we can't suggest it.
if any_over_expr(iterable, &|expr| expr.is_named_expr()) {
    return;
}

Now, it is placed outside of the branches, along with a new check for starred expressions:

for iterable in iterables {
    // For example, (x+1 for x in (c:=a)) is invalid syntax
    // so we can't suggest it.
    if any_over_expr(iterable, &|expr| expr.is_named_expr()) {
        return;
    }

    if iterable.is_starred_expr() {
        return;
    }
}

Similarly, other pieces that are applicable to all branches have also been moved into map_lambda_and_iterables().

[MichaReiser]

Perfect and thanks for the explanations. They were very helpful when reviewing the PR