Search code examples
pythonpython-typing

Is It Bad Practice to Alter Code in Order to Satisfy The Linter

I use neovim and use the pyright linter through mason-lspconfig in order to statically type check my code. I write many type hints; here's an example:

def example(user_input: str) -> None:
    tokens: List[Token] = []
    position = 0
    length_of_input: int = len(user_input)
    
    while position < length_of_input:
        match: Union[re.Match[str], None] = None

        sliced_input = user_input[position:]

        # There's more code but you probably get the point

I have a series of questions regarding the best practices with type hinting:

  1. If a function returns None, should you still specify it like I do with the example function?
  2. I see many other Python programmers use AnyStr opposed to str. I'm not asking why, I just want to know if it matters, and is it a bad practice to use str instead of AnyStr.

Here's where I get to the part about satisfying the linter. Here's what my code initially looked like:

# There's a lot of context that goes with this but just assume factor_node_result.tokens is a List
tokens = factor_node_result.tokens.copy()

The linter tells me that

None has no method copy()

I changed it to this so that it stops complaining; however, both versions of my code work. The linter just stops complaining when I use this version:

if factor_node_result.tokens is not None:
    tokens = factor_node_result.tokens.copy()
  1. Is there anyway I can use type hinting magic or configure my linter to somehow ignore this kind of check
  2. Is it a bad practice to change code in order to satisfy the linter?
Solution

  • Thing is, if you have a good linter and you write decent code, whatever the linter complains about is a potential issue with your code.

    "But it works without the type" does not change that. For one, Python always works without the type hints, that's not why it's there. Secondly, unless you know absolutely sure that it will work for any possible value, you can't really say it works. And even then, the fact that it works now, doesn't mean it will still work in the future, when you call a function from somewhere else, or perhaps run the script on a new version of Python, with different dependencies, etc.

    As for checking for None, yes you should in some cases, if the function can reasonably return None (or exit without explicitly returning anything, which is the same). For example:

    from typing import Optional


def question(answer: bool) -> Optional[int]:
    if answer:
        return 42


wisdom = question(False)

if wisdom is None:
    print("You're not worthy!")
else:
    print(f"Your answer is {wisdom}")

    However, a function that always returns None (i.e. that solely relies on side effects, or is just a method operating on an object) shouldn't need the hint. After all, you wouldn't be calling methods on None.

    All four questions:

    • If a function returns None, should you still specify it like I do with the example function?
      Only if that makes it more clear to read, or if your tools can't figure out the return type without it. (and obviously not unless it's true)
    • [does using AnyStr matter], and is it a bad practice to use str instead of AnyStr.
      Yes it matters, since AnyStr includes both bytes and str. No, it's not bad practice to use str, if your code doesn't support bytes, you should use str. (keep in mind that it has to be either str or bytes for all instances of use in a single signature)
    • Is there any way I can use type hinting magic or configure my linter to somehow ignore this kind of check?
      Most linters and the like allow you to turn off certain checks, but you typically want to scope that type of change as narrowly as possible (some hinters and linters allow you to use annotations to only turn off the check for a single command, block, or other element). It can be a good idea when the linter is right but you can't help the problem (like a minor bug in a library you're using for example, or a problem with the linter itself). But in most cases, the warning will be there for a reason.
    • Is it a bad practice to change code in order to satisfy the linter?
      Not if you're using good tools. The linter will be warning because your code could cause trouble, even if it doesn't in practice right now. Changing the code in a way that satisfies the linter should typically help prevent problems down the line.

    Finally, here's an example where a linter will typically fail, and where specifying -> None helps:

    def fn(x):
    def always_true():
        return True
    if always_true():
        return
    return x


def stripped_lower(x: str):
    return x.lower().strip()


stripped_lower(fn('test'))

    This code will look fine to many linter and analysis tools. However, to you and me it should be clear that the function fn() can only ever return None, due to what always_true() returns. However, to a linter, True is just a runtime bool value, and it won't use that in its analysis, since a bool might be False as well. So, from its point of view, fn('test') may well return a string. stripped_lower() is clear about expecting a string, but will fail with an exception if passed None - which it always will.

    If you change the signature to def fn(x) -> None:, your tools will pick up on the problem and correctly show a type warning.

    Note that I said "a function that always returns None shouldn't need the hint" - that's true here as well. Although providing the hint helps here, this function is poorly written and rewriting it so that it doesn't need the hint would be better than adding the type hint. However, sometimes you don't have that luxury (perhaps you don't control the problematic parts of the code) and then adding the hint may be a better option.