Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion conflicting with black 2024 style for inlined semantic / empty classes #9398

Closed
Pierre-Sassoulas opened this issue Jan 29, 2024 · 7 comments · Fixed by #9697 or #9702
Closed
Assignees
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Jan 29, 2024

Bug description

class DebugTrueDetected(Exception): ...

Command used

pylint a.py

Pylint output

C0321: More than one statement on a single line (multiple-statements)

Expected behavior

No conflict with black I guess ?

Pylint version

pylint 3.0.3
@Pierre-Sassoulas Pierre-Sassoulas added Discussion 🤔 Needs decision 🔒 Needs a decision before implemention or rejection labels Jan 29, 2024
@jacobtylerwalls
Copy link
Member

Figure we should exempt Ellipsis and Pass from the calculation.

@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Discussion 🤔 Needs decision 🔒 Needs a decision before implemention or rejection labels Jan 30, 2024
@Pierre-Sassoulas Pierre-Sassoulas self-assigned this Jan 30, 2024
@mbyrnepr2
Copy link
Member

black moves pass to the next line in this example, so might want to focus on ... only?

class Apple: pass

@jacobtylerwalls
Copy link
Member

Either way, but I don't think it hurts to exempt pass from the pylint check. I'm surprised black treats them differently.

@Pierre-Sassoulas Pierre-Sassoulas added False Positive 🦟 A message is emitted but nothing is wrong with the code and removed Enhancement ✨ Improvement to a component labels Jan 30, 2024
@Pierre-Sassoulas Pierre-Sassoulas added this to the 3.0.4 milestone Jan 30, 2024
@Pierre-Sassoulas
Copy link
Member Author

Agree with @jacobtylerwalls, let's make pylint consistent for those not using black.

@Pierre-Sassoulas
Copy link
Member Author

After looking into it, a good half of our examples and functional tests are just something with pass at the end. We might want to add an option to permit to still raise for pass or ... ?

@jacobtylerwalls
Copy link
Member

Are we sure that's not just a shortcut taken by test authors? Is there any original discussion about the check?

@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 3.0.4, 3.1.1 Feb 23, 2024
@Pierre-Sassoulas
Copy link
Member Author

So it seems it's a decision that was done a long time ago but not in Logilab so we have a little context, I've done a little lot of digging and the functional test come from 0781e73 and the original file was renamed here ee2571c, then #3224 happened, and was moved previously here 33b8185, #2236 happened, moved in 26eb7f9 and in 170b305 . Then the history of the antique test/test_format.py point to f1209f5. Maybe there's more info on bitbucket (can't find the repo though) but it seems Jacob's hypothesis is likely.

@jacobtylerwalls jacobtylerwalls modified the milestones: 3.1.1, 3.2.1, 3.2.2 May 13, 2024
@cdce8p cdce8p modified the milestones: 3.2.2, 3.2.3 May 20, 2024
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Jun 5, 2024
github-actions bot pushed a commit that referenced this issue Jun 5, 2024
…9697)

* Add more test cases to cover pass / ...
* Define the confidence as HIGH
* Exclude the class with Ellipsis from the check

Closes #9398

(cherry picked from commit afd5edf)
Pierre-Sassoulas added a commit that referenced this issue Jun 5, 2024
…9697) (#9698)

* Add more test cases to cover pass / ...
* Define the confidence as HIGH
* Exclude the class with Ellipsis from the check

Closes #9398

(cherry picked from commit afd5edf)

Co-authored-by: Pierre Sassoulas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
4 participants