Use pyright 1.1.230 in CI, temporarily pin pyright-action to 1.0.4

[AlexWaygood]

Unblocks #7493

[AlexWaygood]

Can't say I understand the new errors @erictraut — any idea what's going on? :)

[AlexWaygood]

Looks like pyright is failing on master as well, actually https://github.com/python/typeshed/runs/5564499056?check_suite_focus=true

[JelleZijlstra]

hmmm

 {
            "file": "",
            "severity": "error",
            "message": "Package name \"\" is invalid"
        }

Maybe we messed up some configuration file?

[JelleZijlstra]

It last passed in https://github.com/python/typeshed/runs/5549915994?check_suite_focus=true and failed in https://github.com/python/typeshed/runs/5564499730?check_suite_focus=true. That's #7494, which looks harmless, and reverting it doesn't fix the problem. I noticed though that the action now prints pyright 1.1.218, node v16.13.0 while before it only printed pyright 1.1.218.

https://github.com/jakebailey/pyright-action/commits/main had a bunch of changes over the last day. @jakebailey could you take a look? Not sure what the problem is but somehow your changes broke typeshed CI.

[JelleZijlstra]

Maybe jakebailey/pyright-action@26b3503#r68830046

[AlexWaygood]

Pyright is now failing in the "correct" way after pinning pyright-action to v1.0.4, so we can say for certain that it was a change in pyright-action v1.0.5 or later that broke the CI!

[AlexWaygood]

The new errors pyright 1.1.230 is reporting are all to do with multiple-inheritance issues in the SQLAlchemy stubs (cc. @srittau). For now I'm just going to type: ignore them.

[srittau]

These issues could also be the reason that stubtest crashes.

[srittau]

Quickly checking the SQLalchemy stubs: The stubs are correct when compared to the implementation. Many classes define a Comparator inner class. pyright doesn't like it when two super classes define an inner class with the same name.

[AlexWaygood]

Well that last push didn't seem to trigger any CI at all...

[JelleZijlstra]

try turning it off and on again

[AlexWaygood]

Hooray!

[erictraut]

pyright doesn't like it when two super classes define an inner class with the same name.

This is intentional. This isn't safe in general, for the same reason that it's not safe for two superclasses to define different methods of the same name. If you think that SQLAlchemy is correct, you could disable the reportINcompatibleVariableOverride check in pyrightconfig.json.

[AlexWaygood]

pyright doesn't like it when two super classes define an inner class with the same name.

This is intentional. This isn't safe in general, for the same reason that it's not safe for two superclasses to define different methods of the same name. If you think that SQLAlchemy is correct, you could disable the reportINcompatibleVariableOverride check in pyrightconfig.json.

No, I think this is a good check for pyright to have -- but in this situation, I think typeshed should follow the upstream implementation as closely as possible, so it's better to add type: ignores rather than lie in the stubs. It's a problem with SQLAlchemy rather than typeshed.

Perhaps we should disable this check in pyrightconfig.json, though, since this isn't something typeshed ever really has any control over.

[srittau]

@AlexWaygood writes what I was about to write. @erictraut Would disabling it for the stubs and removing the # type: ignores break projects using the stubs that haven't disabled the option?

[erictraut]

Errors are generally not reported for type stubs that are consumed by other projects. They will see errors if they happen to open that type stub file in their editor. So a # type: ignore might be preferable there.

Another option is to use the new # pyright: ignore or # pyright: ignore[reportIncompatibleVariableOverride] comment which is specific to pyright and doesn't affect other type checkers. This was added just recently. Here's the documentation.

[srittau]

# pyright: ignore[...] will be very useful for the same reason we prefer # type: ignore[...] for mypy-specific ignores. But in the case of reportIncompatibleVariableOverride I think we should just disable it in typeshed for the reason Alex pointed out.