mhammond / pywin32

Python for Windows (pywin32) Extensions
4.9k stars 783 forks source link

Make multiline ISC explicit #2266

Closed Avasam closed 1 month ago

Avasam commented 1 month ago

Follow-up to #2225 adodbapi's version of this PR: #2265

Make multiline Implicit String Concatenation explicit to avoid mixing up parameters and concatenation. Also fixed missing closing parenthesis on win32com.client.build.MapEntry's string representation since I was already modifying that line.

Found using ruff check --select=ISC002 --exclude=adodbapi.

mhammond commented 1 month ago

I don't really like this - the fact strings like this are handled is well documented as a feature of strings.

Avasam commented 1 month ago

It is well documented, but it's also easily a source of potential errors when writing (easy to forget a comma, resulting in an accidental ISC), and misinterpretation at a quick glance.

Although less so with type-checkers in place, since parameter count and tuple length are checked but only in some cases (which'll grow and improve with time and annotations).

That being said, that's just my opinion, , and this PR a suggestion. If you do want to keep multiline implicit string concatenation, then I'll comment the config as such.

mhammond commented 1 month ago

I don't think we need to be 100% consistent here - eg, I wouldn't ask someone who did concatenate a string split across 2 lines to change it, just like I don't think it's worth asking/insisting someone who split strings to change it to a concat. I don't want to get too dogmatic here, and now that we've covered all the low-hanging fruit and removed all the likely footguns, I fear we are starting to move into that territory.

Avasam commented 1 month ago

Closing in favor of #2273 !