pylint

pylint copied to clipboard

Since we're on Python 3, we can use annotations for pylint.

@PCManticore since the person opted to work on this issue has backed off, I would like to work on this issue.

Hey @sushobhit27 I'm not sure who opted to work on this issue, but feel free to take it! The rough idea of this change would be:

• first add mypy in tox but not on Travis
• try to annotate as much pylint code as possible with type annotations
• if there are any bugs with the types that we pass from one place to another, fix them
• Once we can safely run mypy on pylint without any errors, integrate mypy on Travis

Feel free to send multiple smaller PRs for this, since it might require quite a few changes. I suggest starting just with the public APIs for now, e.g methods and functions that are public and that we also use internally. Probably we can skip the visit methods for now, because we already know the type, but if it helps, we can add annotations for them if we can find more bugs in the visit methods.

@PCManticore Agree that multiple smaller PRs will be required for this and starting just with the public APIs for now will be good starting point. Will analyze more and start working on this.

Before we add too many annotations to pylint, will we get more value out of adding annotations to astroid first? From what I understand of transitioning to a typed codebase, typing modules that are lower in the dependency tree first is a good idea because it makes it easier to correct mistakes later and you have more types available to you in the modules that have the dependency when it comes to typing those.

Sounds like a good plan to me, @AWhetter !

#2338 was merged but I'm not sure if we should close this issue now or after we have annotations for all the pylint code. Probably the latter as it's easier to track progress with an opened issue.

Sounds good to me. Perhaps we close this once we start getting useful information from mypy, not necessarily when pylint is 100% annotated?

@AWhetter Definitely. Since that is not measurable enough, I'd say we can close this once we have annotations where it matters. It's important that we now have the setup for running mypy against pylint, the annotations can be its own goal.

@PCManticore @AWhetter I will suggest to do in stages, first annotating functions used across pylint code and then covering checker functions.

I agree with @sushobhit27:

• first annotating functions used across pylint code :

  • That would be pylint.reporters, pylint.messages, pylint.lint, pylint.testutil, pylint.checker.base_checker and maybe pylint.config

• and then covering checker functions:

  • That would be everything in pylint.checkers, and pylint.extensions

List up to date as of 21/04/2022.

Done:

• [x] pylint/*.py
• [x] pylint/config/*.py
• [x] pylint/lint/*.py
• [x] pylint/message/*.py
• [x] pylint/reporters/*.py
• [x] pylint/testutils/*.py
• [x] pylint/utils/*.py

--

• [x] pylint/pyreverse/plantuml_printer.py
• [x] pylint/pyreverse/printer_factory.py
• [ ] pylint/pyreverse/inspector.py
• [x] pylint/pyreverse/__init__.py
• [ ] pylint/pyreverse/utils.py
• [ ] pylint/pyreverse/diadefslib.py
• [ ] pylint/pyreverse/vcg_printer.py
• [ ] pylint/pyreverse/writer.py
• [ ] pylint/pyreverse/diagrams.py
• [ ] pylint/pyreverse/main.py
• [x] pylint/pyreverse/printer.py
• [x] pylint/pyreverse/dot_printer.py

--

• [ ] pylint/checkers/logging.py
• [ ] pylint/checkers/spelling.py
• [ ] pylint/checkers/misc.py
• [ ] pylint/checkers/typecheck.py
• [ ] pylint/checkers/variables.py
• [ ] pylint/checkers/deprecated.py
• [ ] pylint/checkers/base_checker.py
• [ ] pylint/checkers/__init__.py
• [ ] pylint/checkers/format.py
• [ ] pylint/checkers/mapreduce_checker.py
• [ ] pylint/checkers/imports.py
• [ ] pylint/checkers/utils.py
• [ ] pylint/checkers/raw_metrics.py
• [ ] pylint/checkers/newstyle.py
• [ ] pylint/checkers/exceptions.py
• [ ] pylint/checkers/classes.py
• [ ] pylint/checkers/stdlib.py
• [ ] pylint/checkers/async.py
• [ ] pylint/checkers/refactoring/recommendation_checker.py
• [ ] pylint/checkers/refactoring/__init__.py
• [ ] pylint/checkers/refactoring/refactoring_checker.py
• [ ] pylint/checkers/refactoring/not_checker.py
• [ ] pylint/checkers/refactoring/len_checker.py
• [ ] pylint/checkers/similar.py
• [ ] pylint/checkers/design_analysis.py
• [ ] pylint/checkers/base.py
• [ ] pylint/checkers/strings.py

--

• [ ] pylint/extensions/empty_comment.py
• [ ] pylint/extensions/code_style.py
• [ ] pylint/extensions/check_docs.py
• [ ] pylint/extensions/broad_try_clause.py
• [ ] pylint/extensions/docparams.py
• [ ] pylint/extensions/__init__.py
• [ ] pylint/extensions/mccabe.py
• [ ] pylint/extensions/emptystring.py
• [ ] pylint/extensions/comparetozero.py
• [ ] pylint/extensions/confusing_elif.py
• [ ] pylint/extensions/_check_docs_utils.py
• [ ] pylint/extensions/consider_ternary_expression.py
• [ ] pylint/extensions/while_used.py
• [ ] pylint/extensions/typing.py
• [ ] pylint/extensions/redefined_variable_type.py
• [ ] pylint/extensions/docstyle.py
• [ ] pylint/extensions/check_elif.py
• [ ] pylint/extensions/set_membership.py
• [ ] pylint/extensions/overlapping_exceptions.py
• [x] pylint/extensions/bad_builtin.py

Just to give an update here, the typing of most base files is now done. What's left is the typing of all checkers, extensions and pyreverse. Most checkers should be relatively straightforward, but it will still take considerable time and effort!

With https://github.com/PyCQA/pylint/pull/7448 we have now turned on mypy strict mode on our codebase 🎉

The only thing left to do is https://github.com/PyCQA/pylint/pull/7115, which can close this issue. Sadly it is blocked by an astroid issue, but we'll get to that at some point.

Over the coming days/weeks I'll try and see if there is anything in our code that needs to be updated, but for now the typing effort is almost done!

pylint/extensions/check_docs.py should be renamed, because there is no "pylint/extensions/check_docs.py " in the pylint/extensions file.

I read an article that said that adding types was important for improving performance in python code. It is number 3 in the article: https://aglowiditsolutions.com/blog/python-optimization/

I fear that you misinterpreted this paragraph. Statically typed languages don't have the computational overhead dynamically typed languages like Python do. Adding type annotations to Python code however does not make this overhead vanish: these type annotations are not evaluated at runtime. You can try that for yourself by running this example:

a: str = "Hello"
b: str = "World"

a = 1
b = 2

print(a + b)

You will see that although we annotated the types for a and b as str, nothing is stopping us from assigning integers to them.

Type annotations have a lot of benefits, but (at least for pure Python, there may be other tools like numba that might help here) increased runtime performance is not one of them.

If everything were typed correctly it would be possible to compile with Mypyc to get a substantial speed improvement. Unfortunately Astroid is thoroughly type-incorrect, and getting it right would take some work. That effort is complicated by the fact that the existing type-incorrect behavior is considered part of the guaranteed public API. So there are both technical and political obstacles to getting things right.

the existing type-incorrect behavior is considered part of the guaranteed public API

Huh ? If something is wrongly typed in astroid we should fix the typing, I don't think there's anyone saying the contrary here. But if we want to change the way it's used for example by replacing an Optional[x] type typing to simply x then we need to make some kind of breaking changes and there's going to be a lot of code to modify to actually be able to do that in astroid as well as in pylint.

Here's an example:

    if isinstance(stmt, nodes.For):
        try:
            inferred_iterable = next(stmt.iter.infer(context=context))
        except (InferenceError, StopIteration):
            yield util.Uninferable
            return

stmt is a For node. stmt.iter.infer is accessed. But the declared type of For.iter is NodeNG | None. That's a type error. Astroid is full of cases like this.