New checker - Detect use of unnecessary ellipsis

[mbyrnepr2]

Closes #5460

Emit a warning when ... is used and could be omitted. Similar to the unnecessary-pass checker.

Type of Changes

	Type
	🐛 Bug fix
✓	✨ New feature
	🔨 Refactoring
	📜 Docs

Description

Closes #5460

[coveralls]

Pull Request Test Coverage Report for Build 1574520196

• 15 of 15 (100.0%) changed or added relevant lines in 1 file are covered.
• 41 unchanged lines in 3 files lost coverage.
• Overall coverage increased (+0.005%) to 93.661%

---

Files with Coverage Reduction	New Missed Lines	%
pylint/testutils/functional/test_file.py	1	96.36%
pylint/testutils/lint_module_test.py	12	85.63%
pylint/checkers/variables.py	28	95.63%

Totals
Change from base Build 1572908427:	0.005%
Covered Lines:	14198
Relevant Lines:	15159

---

💛 - Coveralls

[Pierre-Sassoulas]

LGTM !

[Pierre-Sassoulas]

Well except the fact it's in the string checker, it's not really related to strings, right ?

Did not check where the code was added

[DanielNoord]

Just had a quick look and want to interrupt before @Pierre-Sassoulas might merge this:

Expected output of the tests is incorrect as it is missing the end line and column. This is because node is not being passed to add_message. This should be done!

@Pierre-Sassoulas This is something for future reviews as well: node should be made mandatory (is on my todo list) but that requires some work. In the meantime we should require it for most new PRs.

[Pierre-Sassoulas]

node should be made mandatory (is on my todo list) but that requires some work.

You're right, my bad. Isn't that another breaking change for 3.0 ?

[DanielNoord]

You're right, my bad. Isn't that another breaking change for 3.0 ?

Yeah! But adding a DeprecationWarning now would make us trigger it ourself. I'd like to fix that before doing so.

Just to let you know, I currently plan to first fix unittests and then fix statement(). After this it's back to typing PyLinter in terms of maintenance.
I'll do some simpler crash/bug fixes in between. :)

[DanielNoord]

You're right, my bad. Isn't that another breaking change for 3.0 ?

Yeah! But adding a DeprecationWarning now would make us trigger it ourself. I'd like to fix that before doing so.

Just to let you know, I currently plan to first fix unittests and then fix statement(). After this it's back to typing PyLinter in terms of maintenance.
I'll do some simpler crash/bug fixes in between. :)

[Pierre-Sassoulas]

adding a DeprecationWarning now would make us trigger it ourself.

Sounds good !

[Pierre-Sassoulas]

Please create a new file pylint/checker/ellipsis_checker.py, as we want to burst base.py in #5489

[Pierre-Sassoulas]

Thanks for separating the checker 👍

[DanielNoord]

Note that I haven't done a full review but I think the 18 prefix is already being used. I'm on mobile so I can't link the file, but there is a script in the scripts folder that can point you to a new unused prefix.

[Pierre-Sassoulas]

I think the 18 prefix is already being used

We could add a pre-commit script to check that, what do you think @DanielNoord ? Maybe based on the existing script for unused prefix.

[DanielNoord]

There is an open issue about this to which I assigned myself. Haven't gotten around to it because the unittests move take so much time and I think they are more important.

[mbyrnepr2]

Thanks for the advice @Pierre-Sassoulas & @DanielNoord 👍

[DanielNoord]

Code itself looks good, I would only add some additional comments and explanations.

I'm also wondering if we should add a confidence level to this message.

[DanielNoord]

Thanks for these changes, much better!

I just thought of one additional test case I think we should add by default: overloads.
Could you add an example? The mypy documentation should help with coming up with a simple but effective example.

[DanielNoord]

👍