Problem

The guard if suffix and not suffix[0] == "." or suffix == "." or "/" in suffix: parses as (suffix and not suffix[0] == ".") or (suffix == ".") or ("/" in suffix). The leading suffix and ... short-circuit means an empty string "" falls through every clause and is accepted as a valid "suffix", silently triggering name[:-len("")] + "" later (line 1419), i.e. a no-op. If empty-suffix-means-strip is intentional that should be a documented branch; if it is not, it is a quiet correctness bug. Separately, not suffix[0] == "." (relying on not binding tighter than ==) is the kind of expression reviewers misread — suffix[0] != "." is unambiguous.

Why This Matters

The function's docstring promises "suffix (file extension of name) replaced" and the error message is "Invalid suffix"; silently accepting "" is a behavioural surprise that callers will write tests around without realising. Rewriting the condition once makes the intent explicit and lets the maintainer decide deliberately whether empty is permitted.

Suggested Fix

Decide on the empty-string semantics (either explicitly allow with a comment, or reject), and rewrite for clarity:

if not suffix or suffix[0] != "." or suffix == "." or "/" in suffix:
    raise ValueError(f"Invalid suffix {suffix!r}")

If empty-means-strip is a feature, branch on it before the validator:

if suffix == "":
    # explicit no-op / strip case
    ...
elif suffix[0] != "." or suffix == "." or "/" in suffix:
    raise ValueError(f"Invalid suffix {suffix!r}")

Either way, a test should be added documenting the chosen behaviour.

Details

🤖 Created by Kōan from audit session