Return types

[bjones1]

I noticed that there's some ambiguity in the way that the CLI uses optional return values. In project_refactor.py:

def target(self, name: t.Optional[str] = None) -> t.Optional["Target"]:
        """
        Attempts to return a target matching `name`.
        If `name` isn't provided, returns the default (first) target.
        """

However, most of the tests don't check that target return a valid value:

        target = project.target(name)
        assert target.name == name

instead of the longer

        target = project.target(name)
        assert target is not None
        assert target.name == name

Should we instead offer two functions? I'd suggest:

• A has_target which returns True/False
• A target which raises an exception if the target isn't present.

[StevenClontz]

I don't mind changing to the more verbose test, although I was intentional when writing

target = project.target(name)
assert target.name == name

as the test fails via an exception when target is None.

I kind of like writing p.target('foo') is not None rather than p.has_target('foo'). But it's not a strong feeling if others feel stronger.

[StevenClontz]

Oh, I turned on mypy to investigate #469 and realized the obvious reason why my test isn't great: it correctly triggers type complaints. So I'd add an assert target is not None, unless we do implement has_target.

[bjones1]

That's one of my motivations for this discussion, certainly. In production code (as opposed to test code), I think there are also two cases: p.target(foo) which I assume will always succeed, and if p.target(foo) is None: where I can inform the user of a missing binary. Hence the idea on has_target to express these differences without requiring a lot of t = p.target(foo); assert t is not None statements littering the code.

[bjones1]

On a side note, I realized that the Python VSCode extension already supports mypy, so there's no need to install an extra extension.