Include mypy checks in noxfile and port initial pyi files

[savarin]

Addresses #1897.

The PR introduces mypy checks to the noxfile so that pre-specified files are type checked.

In addition, the .pyi stubs for packages/ssl_match_hostname are ported from typeshed. This is due to the fact that these files lie on the outermost part of the dependency tree. Additional .pyi stubs will be ported in due course.

[savarin]

The default mypy behavior is to follow imports, this flag disables it as per https://mypy.readthedocs.io/en/latest/running_mypy.html#following-imports.

[pquentin]

Great start, thanks!

When this get in, I'd like a pull request to add this to our Github continuous integration checks.

Another possible side quest is to try to make mypy src/urllib3 pass (that is, without strict mode). It'll be needed eventually and it reports way less errors than strict mode.

[pquentin]

I realized you just copied what Eland did but why not just use session.run("mypy", "--strict", "--follow-imports=skip", typed_file)? cc @sethmlarson

It seems to work: 1926f5e

Doing that actually finds an issue: src/urllib3/packages/ssl_match_hostname/_implementation.pyi:3: error: Function is missing a type annotation

This fixes it: 33c4c13

[sethmlarson]

TIL about --follow-imports=skip, I'll modify Eland to use that option instead.

[sethmlarson]

Remembered the reason why not to use it! If you use --follow-imports=skip then when you're using something from outside the file then mypy complains that that thing isn't typed in some cases. I'm thinking about switching to something like this on Eland though:

    errors = []
    popen = subprocess.Popen(
        "mypy --strict eland/",
        env=session.env,
        shell=True,
        stdout=subprocess.PIPE,
        stderr=subprocess.STDOUT,
    )

    mypy_output = ""
    while popen.poll() is None:
        mypy_output += popen.stdout.read(8192).decode()
    mypy_output += popen.stdout.read().decode()

    for line in mypy_output.split("\n"):
        filepath = line.partition(":")[0]
        if filepath in TYPED_FILES:
            errors.append(line)
    if errors:
        session.error("\n" + "\n".join(sorted(set(errors))))

Both faster and simpler :)

[savarin]

@pquentin Thank you for 33c4c13, I was pondering yesterday how do get the types here. May I ask how you got the types? I admit with my previous codebase I was doing print(type(...)).

@sethmlarson Great I've incorporated that. Just a quick flag that the filepath that are flagged are in fact the stub files, so I've made changes accordingly and renamed TYPED_FILES into STUB_FILES.

[pquentin]

@pquentin Thank you for 33c4c13, I was pondering yesterday how do get the types here. May I ask how you got the types? I admit with my previous codebase I was doing print(type(...)).

match_hostname is a function that was added to the Python standard library with Python 3.5 and Python 2.7.9. Since urllib3 supported earlier versions of Python 3 at the time and still supports Python <2.7.9 to this day, we just copied its code to urllib3. Knowing that, I simply copied the type hints from https://github.com/python/typeshed/blob/master/stdlib/2and3/ssl.pyi. I included that link in the code, but I should probably have been more explicit about it.

Does that make sense?

However that fix isn't needed since I've only seen the error when mypy is run in the context of nox, and using subprocess to run mypy fixes the issue.

@sethmlarson By the way, using subprocess.run(text=True) is probably even simpler as "wait for completion and provide all output to a string" will be already provided.

[pquentin]

However that fix isn't needed since I've only seen the error when mypy is run in the context of nox, and using subprocess to run mypy fixes the issue.

Actually, if I remove my changes from 33c4c13, I do see the error 🤷

[savarin]

Great start, thanks!

When this get in, I'd like a pull request to add this to our Github continuous integration checks.

Another possible side quest is to try to make mypy src/urllib3 pass (that is, without strict mode). It'll be needed eventually and it reports way less errors than strict mode.

@pquentin Appreciate the kind words! I'm afraid it's the first time I'm using Github CI, would be great if you could share high level what these changes look like. I'm also a little puzzled why the tests are failing, will experiment with a toy PR in a different branch.

[pquentin]

I'm afraid it's the first time I'm using Github CI, would be great if you could share high level what these changes look like.

Actually I just realized that you're just modifying nox -s lint, and we're already calling that in our Travis CI configuration.

I'm also a little puzzled why the tests are failing, will experiment with a toy PR in a different branch.

Not your fault, our tests are currently quite flaky...

[pquentin]

This looks great! Will probably need @sethmlarson to force merge it, as we know your work can't affect the failing (flaky) tests.

[pquentin]

Closing/reopening to rerun the tests.

[sethmlarson]

LGTM, thanks!

[pquentin]

@savarin Congratulations!

By the way I forgot, can you please open a new pull request that updates docs/contributing.rst to mention that we need to pip install mypy in addition of nox (since we're using mypy outside of the nox virtual environment)

[savarin]

@savarin Congratulations!

By the way I forgot, can you please open a new pull request that updates docs/contributing.rst to mention that we need to pip install mypy in addition of nox (since we're using mypy outside of the nox virtual environment)

Thank you! I've been saving this gif for this moment. New PR on the way.

Actually I just realized that you're just modifying nox -s lint, and we're already calling that in our Travis CI configuration.

Got it. Would it make sense to create nox -s type (or typing) just to be clear? Also wanted to ask why black is run twice in blacken and lint.

match_hostname is a function that was added to the Python standard library with Python 3.5 and Python 2.7.9. Since urllib3 supported earlier versions of Python 3 at the time and still supports Python <2.7.9 to this day, we just copied its code to urllib3. Knowing that, I simply copied the type hints from https://github.com/python/typeshed/blob/master/stdlib/2and3/ssl.pyi. I included that link in the code, but I should probably have been more explicit about it.

Does that make sense?

Makes sense. I was curious if there was a command/trick to get the types. Cool I'll take a closer look at typeshed and maybe our tests for the upcoming stubs.

[sethmlarson]

@savarin Let's keep Mypy within nox -s lint, makes sense to have it there :) Black is run once in blacken to reformat the code and a second time in lint with the --check flag which doesn't modify the code inplace but instead checks that all code doesn't need to be reformatted anymore.