Add consider-ternary-expression extension

[nickdrozd]

Type of Changes

	Type
✓	✨ New feature

Description

Closes #4366

[nickdrozd]

Something is off with the functional test specification. Am I missing something obvious?

[Pierre-Sassoulas]

Hey, thank you for implementing this. Could you revert all the ternary changes that do not fit on one line using black ? I think bigger ternary make the code harder to understand.

[cdce8p]

Hey, thank you for implementing this. Could you revert all the ternary changes that do not fit on one line using black ? I think bigger ternary make the code harder to understand.

I agree with that. Ternary ifs should only really be used for short conditions, something like

var = 2 if some_var else 3

[Pierre-Sassoulas]

To the point that I'm wondering if the checker should not be changed to :

• if it's a ternary and it's longer than 88 characters, tell the user to remove the ternary
• It's it's not a ternary and it's shorter than 88 characters, tell the user to consider a ternary

What do you think @nickdrozd ?

[cdce8p]

To the point that I'm wondering if the checker should not be changed to :

• if it's a ternary and it's longer than 88 characters, tell the user to remove the ternary
• It's it's not a ternary and it's shorter than 88 characters, tell the user to consider a ternary

I'm really not sure we should enforce any of it. IMO it doesn't make the code inherently better to use either.
I.e. even for the existing pylint code I don't think we should do most of these changes.

[nickdrozd]

Okay, I took out the changes that don't result in one-liners.

Personally, I would change all of them. It's not about lines of text, it's about complexity -- one single assign statement is less complex than two. It's a shame that Python makes if-expressions so clunky to use. In Lisp and Rust if-expressions are used all over the place with no problems.

As for the check itself, I think it's okay if it suggests some more extreme changes, like if-expressions that would get broken across multiple lines. The check is an extension, disabled by default. So anyone who enables the extension can be safely assumed to be a relatively sophisticated Pylint user, and therefore also someone who can deal with any false positives or iffy cases.

[coveralls]

Pull Request Test Coverage Report for Build 1151047154

• 31 of 35 (88.57%) changed or added relevant lines in 10 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.03%) to 92.81%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pylint/checkers/imports.py	1	2	50.0%
pylint/extensions/consider_ternary_expression.py	21	24	87.5%

Totals
Change from base Build 1149750110:	0.03%
Covered Lines:	13450
Relevant Lines:	14492

---

💛 - Coveralls

[Pierre-Sassoulas]

Thank you for cleaning up the MR, I left some comment, mostly I think the expected name is "ternary operator" or "ternary conditional".

[Pierre-Sassoulas]

Suggested change

	name = "consider_if_expression"
	name = "consider_ternary_expression"

[Pierre-Sassoulas]

Suggested change

	"Consider rewriting as an if-expression",
	"Consider rewriting as a ternary if-expression",

[Pierre-Sassoulas]

Suggested change

	"consider-if-expression",
	"consider-ternary-expression",

[Pierre-Sassoulas]

Suggested change

	"rewritten with a single assingment and an if-expression",
	"rewritten with a single assignment as a ternary conditional",

[Pierre-Sassoulas]

Could you add the typing, please ? :)

[Pierre-Sassoulas]

As for the check itself, I think it's okay if it suggests some more extreme changes,

Yes I agree, I was trying to transform it into something complicated that I would personally use, but if someone want only ternary and have to activate this plugin, then it can be very opinionated without problems.

[Pierre-Sassoulas]

Great work, thank you !

[zrothberg]

https://nickdrozd.github.io/2021/09/02/new-pylint-checks.html

This dude just used your repo to write a terrible article.