Changing the default value for reportOptionalMemberAccess under basic type checking mode

[Rapptz]

While writing some code, I ran into an issue that I feel was easily preventable with what I would assume to be the default type checking mode of the type checker:

class Test:
    def is_finished(self) -> bool:
        ...

def edit(*, test: Optional[Test]):
    if test.is_finished():
        pass

It took me by surprise so I wanted to open an issue about it until I searched for the previous issues and documentation and found that this is by design (ref: #1164 and https://github.com/microsoft/pyright/blob/main/docs/configuration.md#diagnostic-rule-defaults). When I was discussing this with a group of Python developers we were all caught by surprise that in the basic type checking mode this was not flagged until someone pointed out that it is enabled in strict mode.

I checked other type checkers and the default configuration for this was less surprising to me:

• mypy reports an error without enabling strict
• pyre reports an error by default
• pytype reports an error by default
• PyCharm does not report an error at all (I'm not sure how to)

Since it's a commonly enabled check and it is a big noob trap, I think it'd be beneficial to change the default from none to error for the basic type checking mode.

[erictraut]

Thanks for the suggestion. This isn't the first time this has been suggested, and I think it's worth considering.

For basic type checking mode, we were pretty conservative when deciding which diagnostic rules to enable by default. The problem is that if we enable too many rules by default, users who are new to type checking are immediately turned off to it because they see a "sea of red" in their code, including many potential false positive errors. By including only the checks that are most likely not to generate false positives, we allow users to start slow and add more checks as they start to see value in type checking. At least that's the rationale behind the current behavior.

Now that so many people are using pyright (mostly in the form of pylance), a change like this could be pretty disruptive to existing users.

[Rapptz]

Do you have any examples of false positives that would be triggered by this case?

I understand and appreciate the rationale of the separation between strict and basic. I think for my use cases, strict is too noisy since there are a lot of errors that I'd have to disable since they're too opinionated on my codebase. However, I also think for the average user who wants to get into type checking, their first thought would be that the static analyzer would help them catch type related errors at "compile time" rather than at runtime. So when this slips by without a diagnostic it feels like either there's a bug in the type checker or that I'm doing something wrong.

I consider myself decently knowledgeable at type checking and some of their intricacies but this still took me by surprise and led to a bug that could have been easily preventable.

[erictraut]

The most common false positive involves a very common pattern where instance variables are initialized to None in a constructor but later initialized to a value with some other type. Programmers who use this pattern often rely on knowledge of call order to guarantee that any use of that instance variable occurs only after it has been initialized with a non-None value, but a static type checker cannot determine the call order. The solution is often to refactor the code or add assert statements in several places.

[nihaals]

Maybe adding another mode which is a middle ground between basic and strict would help? strict is far too strict for most projects, while basic doesn't include errors like this, which some users would find helpful. It sounds like this balance might be popular for users who want something as strict as possible while being usable for the majority of projects.

[erictraut]

After much internal discussion, we've decided to enable the six "reportOptional..." diagnostic checks by default in "basic" mode. We're going to make this change in the next release of pyright and see what feedback we receive.