cdgriffith / puremagic

Pure python implementation of identifying files based off their magic numbers
MIT License
158 stars 34 forks source link

Do not perform function call in argument defaults #74

Closed cclauss closed 2 months ago

cclauss commented 3 months ago

Any function call that's used in a default argument will only be performed once, at definition time. The returned value will then be reused by all calls to the function, which can lead to unexpected behavior.

% ruff check --select=B008

puremagic/main.py:68:41: B008 Do not perform function call `os.path.join` in argument defaults;
    instead, perform the call within the function, or read the default from a module-level singleton
    variable
Found 1 error.

% ruff rule B008

function-call-in-default-argument (B008)

Derived from the flake8-bugbear linter.

What it does

Checks for function calls in default function arguments.

Why is this bad?

Any function call that's used in a default argument will only be performed once, at definition time. The returned value will then be reused by all calls to the function, which can lead to unexpected behaviour.

Calls can be marked as an exception to this rule with the [lint.flake8-bugbear.extend-immutable-calls] configuration option.

Arguments with immutable type annotations will be ignored by this rule. Types outside of the standard library can be marked as immutable with the [lint.flake8-bugbear.extend-immutable-calls] configuration option as well.

Example

def create_list() -> list[int]:
    return [1, 2, 3]

def mutable_default(arg: list[int] = create_list()) -> list[int]:
    arg.append(4)
    return arg

Use instead:

def better(arg: list[int] | None = None) -> list[int]:
    if arg is None:
        arg = create_list()

    arg.append(4)
    return arg

If the use of a singleton is intentional, assign the result call to a module-level variable, and use that variable in the default argument:

ERROR = ValueError("Hosts weren't successfully added")

def add_host(error: Exception = ERROR) -> None:
    ...

Options

cdgriffith commented 3 months ago

I do only want that call to happen once at startup and stay as the default that way, this is intended behavior. May need to be re-written to just have that as a pre-defined global and use it in the function args that way