Deprecate MapReduceMixin and implements its methods in BaseChecker

[DanielNoord]

• Write a good description on what the PR does.
• If you used multiple emails or multiple names when contributing, add your mails
  and preferred name in script/.contributors_aliases.json

Type of Changes

	Type
✓	✨ New feature
✓	🔨 Refactoring

Description

Kinda high priority since main is broken due to two typing PRs apparently depending on each other but passing on their own.

main complains about checker.get_map_data() and checker.reduce_map_data in parallel.py. Instead of adding ignores there I think mypy is actually right and warns of a design flaw. There is no need to separate these MixIns (ideally we would deprecate the MixIn as there is little overhead of making BaseChecker a MapReduceMixin and just having the methods do nothing.
That's why I propose this solution.

[coveralls]

Pull Request Test Coverage Report for Build 2190395707

• 17 of 18 (94.44%) changed or added relevant lines in 4 files are covered.
• 11 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.003%) to 95.148%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pylint/checkers/base_checker.py	3	4	75.0%

Files with Coverage Reduction	New Missed Lines	%
pylint/checkers/stdlib.py	11	94.53%

Totals
Change from base Build 2189917137:	-0.003%
Covered Lines:	15785
Relevant Lines:	16590

---

💛 - Coveralls

[Pierre-Sassoulas]

Won't this imply that BaseChecker is able to perform map reduce when it's not ?

[DanielNoord]

Won't this imply that BaseChecker is able to perform map reduce when it's not ?

Yes and no I guess. It is able to perform it in so far as that it has a reduce_map_data method. However, that simply does nothing. The way the MixIn works is that get_map_data should return something (Any) which then gets appended to a list specific to that checker. When reducing all data the respective reduce_map_data method is then called to reduce the specific from of data that get_map_data returns.
So, you will never be able to get this to work with only implementing get_map_data and I think you will always catch this when writing a test for this.

---

The issue with the current implementation is that there is a dictionary mapping checker to data. Only checkers that implement get_map_data are added to this dict. However, when we interact with this dict later on mypy has no way of knowing that implementing get_map_data is a requirement for having an entry in this dictionary and just considers all entries to be BaseChecker, and then complains.

[Pierre-Sassoulas]

LGTM the final result is pretty nice and reasonable 👍

[jacobtylerwalls]

Just some cleanups.

[DanielNoord]

Thanks for the thorough review guys! Agree with @Pierre-Sassoulas!

[jacobtylerwalls]

Terrific, thank you!

[jacobtylerwalls]

(Maybe a better PR title, though... now that BaseChecker isn't a MapReduceMixin.)

[doublethefish]

I completely missed this change, but it's great to see. When I added the Mixin object the intent was to have as little impact over the core code as possible, it's always been clear that the right intemediary place for it was in the base-checker - longer term the map-reduce functionality should completely go in favour of a fully parallelised astroid or moved into astroid
or something.

I think mypy is actually right and warns of a design flaw

Mypy is definitely correct that there's an issue, but it's not a warning about a design flaw. Mypy is just showing that there's a lack of type checking and/or explicit error handling around whether the mix-in has been applied or not. After all python isn't C++ and is a dynamic language which allows for that sort of thing more easily :) IIRC the design implicitly ensures that the mix-in is applied.