Stricter mypy config

[semohr]

Description

Since we are working a lot on type hints at the moment, how about we review the available mypy configuration options and decide on a standard setup going forward?

This is meant as a discussion to align on which settings we want to adopt to improve our code quality, type safety, and overall developer experience.

---

Proposal

I propose enabling the following options:

allow_redefinition = true

• Why: This allows reassigning or shadowing a name with a different type within a function or scope. It’s helpful for code clarity, especially in cases like variable narrowing or reusing loop variables.
• Example:

  x: int = 1
  x: str = "foo"  # Without this flag, mypy would complain
  

check_untyped_defs = true

• Why: This tells mypy to check the bodies of functions even if they don’t have type annotations. It helps catch errors early, even in partially typed code.
• Impact: Encourages typing functions over time while still providing coverage for legacy code.

disallow_incomplete_defs = true

• Why: Disallows defining functions with partial type annotations (including arguments). This enforces consistency and helps ensure that function if they are typed, are typed fully.
• Example:

  def f(x : int, y):  # This would error without a type for y and return type
      ...
  

disallow_any_generics = true

• Why: Requires specifying type parameters for generic types like List, Dict, etc. Avoids untyped containers which can mask bugs or make type inference harder.
• Example:

  x: list = []  # Error
  x: list[int] = []  # OK
  

disallow_untyped_decorators = true

• Why: Ensures that decorators are properly typed. Untyped decorators can cause the function they decorate to become untyped too, making it harder for mypy to validate code.
• Impact: Should help with typing pipelines where we heavily use decorators.

---

Me can even go stricter (e.g. disallow_untyped_calls, warn_unused_ignores) or more lenient (e.g. leave check_untyped_defs off). I think the proposed setup strikes a good balance between safety and practicality.

Would love to hear thoughts or additions!

---

This is based on the https://github.com/beetbox/beets/pull/5701 PR.

[wisp3rwind]

I'll try to look at this in more detail at some point (right now, I'm not very familiar with most of mypy's options).

In general, in my opinion, we should eventually set most of these. Probably rather sooner than later. For example, when looking at your PR https://github.com/beetbox/beets/pull/5701, I realized that almost all callers of the plugin interface are not in fact typed right now. Thus, we get very little feedback from mypy at this point.

I'm uncertain about allow_redefinitions: I've definitely worked around a few of these already in beets. On the other hand, maybe in some cases that actually helped to improve clarity? I'll have to look around for those cases to get an idea. I feel like this might behave differently in Python than in, say, Rust, where shadowing would require an explicit let ... statement.

Last time I ran mypy with --check-untyped-defs, the output was still a bit overwhelming. Maybe let's wait for the current wave of typings PRs to be merged, and then review? I'd also argue to get these in one by one if they require changes: In my experience (and I'm also guilty of this), one big factor that leads to stalling PRs for beets has been when too many things were done in one go.

[semohr]

You're right that most plugins don’t fully leverage type hints yet. Before merging additional typing-related PRs like #5734 or #5716, I thought it might be worth configuring some stricter mypy options now. That way, we can avoid having to do another round of refactors later just to adapt to those settings.

I realize enabling more options—like --check-untyped-defs—can make a lot of existing issues visible all at once, which can definitely feel overwhelming. But we don’t have to fix everything immediately, especially since mypy isn't currently blocking PRs. In some cases, we could even ignore certain new errors temporarily.

Looking ahead, I think it's useful to start moving toward these stricter checks for new code, even if some legacy code doesn’t yet meet the same standard. It's okay if some parts lag behind a bit as long as we keep improving incrementally.

[wisp3rwind]

We need to take https://github.com/beetbox/beets/pull/5728 or https://github.com/beetbox/beets/pull/5745 into account whenever we do this.

[snejus]

allow_redefinition = true

* **Why**: This allows reassigning or shadowing a name with a different type within a function or scope. It’s helpful for code clarity, especially in cases like variable narrowing or reusing loop variables.

* **Example**:
  ```python
  x: int = 1
  x: str = "foo"  # Without this flag, mypy would complain
  ```

I think code is clearer by default, aka allow_redefinition = false. Simple example, with allow_redefinition = true this is a legitimate implementation:

def process_items(items: list[str]) -> None:
    # do something
    items = iter(items)
    # something
    process(items[0])

When an exception is raised, I will debug this by looking at the function implementation. Seeing that the exception is raised even though items is a list I will be confused and will have to track reassignments for the type change.

Instead, we should be forced to either

def process_items(items: list[str]) -> None:
    # do something
    process(iter(items)[0])

or

def process_items(items: list[str]) -> None:
    # do something
    items_iter = iter(items)
    process(items_iter[0])