[pylint] Re-implement unreachable (PLW0101)#10891
Conversation
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| PLW0101 | 21 | 21 | 0 | 0 | 0 |
Formatter (stable)
✅ ecosystem check detected no format changes.
Formatter (preview)
✅ ecosystem check detected no format changes.
|
Nice, I'm a fan of this. |
|
No promises, just started taking a look at it. |
|
Can you give me some background on why this was never activated? Was there just too many false positives? |
c993823 to
5897adb
Compare
CodSpeed Performance ReportMerging #10891 will not alter performanceComparing Summary
|
4d724c3 to
87a3e96
Compare
|
This is probably not completely ready for merging, but considering the size it's probably worth getting some feedback now. All the ecosystem checks seem like true positives, there might still be some false negatives, but those will be harder to find. |
|
@charliermarsh when you get the time please have a look at this |
|
Sorry for the emberassing long wait. I make this a priority for next week. The best way to review this change is probably to compare all changes starting after the "Revert" commit. If you have any other review recommendations, let me know :) |
|
@MichaReiser no worries. Ya that's a good strategy, but also take a look at my PR summary at the top, I tried to give an outline of the changes I made. |
87a3e96 to
11b53e6
Compare
|
I rebased the commit onto main |
ruff] Re-implement unreachablepylint] Re-implement unreachable (PLW0101)
|
@augustelalande Upon further reflection, I've decided to merge this in as-is. The various remaining tweaks do not really affect the core functionality of this rule, and could be tackled in follow-up PRs. This is an outstanding contribution to Ruff, and your hard work deserves to be part of the codebase! Thank you once again for your patience and persistence! |
|
Thanks @dylwil3 |
* main: [`ruff`] Avoid reporting when `ndigits` is possibly negative (`RUF057`) (#15234) Attribute panics to the mdtests that cause them (#15241) Show errors for attempted fixes only when passed `--verbose` (#15237) [`RUF`] Add rule to detect empty literal in deque call (`RUF025`) (#15104) TD003: remove issue code length restriction (#15175) Preserve multiline implicit concatenated strings in docstring positions (#15126) [`pyflakes`] Ignore errors in `@no_type_check` string annotations (`F722`, `F821`) (#15215) style(AIR302): rename removed_airflow_plugin_extension as check_airflow_plugin_extension (#15233) [`pylint`] Re-implement `unreachable` (`PLW0101`) (#10891) refactor(AIR303): move duplicate qualified_name.to_string() to Diagnostic argument (#15220) Misc. clean up to rounding rules (#15231) Avoid syntax error when removing int over multiple lines (#15230) Migrate renovate config (#15228) Remove `Type::tuple` in favor of `TupleType::from_elements` (#15218)
Summary
This PR re-introduces the control-flow graph implementation which was first introduced in #5384, and then removed in #9463 due to not being feature complete. Mainly, it lacked the ability to process
try-exceptblocks, along with some more minor bugs.Closes #8958 and #8959 and #14881.
Overview of Changes
I will now highlight the major changes implemented in this PR, in order of implementation.
continueorbreakstatements within the loop body and redirect them appropriately.tryprocessing with the following logic (resolves Completetry-excepthandling in control-flow graph #8959):ExceptionRaisedforking if an exception was (or will be) raised in the try block. This is not possible to know (except for trivial cases) so we assume both paths can be taken unconditionally.trypath the cfg goestry->else->finallyunconditionally.exceptpath the cfg will meet several conditionalExceptionCaughtwhich fork depending on the nature of the exception caught. Again there's no way to know which exceptions may be raised so both paths are assumed to be taken unconditionally.raisesorreturnswithin the blocks appropriately.flowchart TD start(("Start")) return(("End")) block0[["`*(empty)*`"]] block1["print(#quot;finally#quot;)\n"] block2["print(#quot;else#quot;)\n"] block3["print(#quot;try#quot;)\n"] block4[["Exception raised"]] block5["print(#quot;OtherException#quot;)\n"] block6["try: print(#quot;try#quot;) except Exception: print(#quot;Exception#quot;) except OtherException as e: print(#quot;OtherException#quot;) else: print(#quot;else#quot;) finally: print(#quot;finally#quot;)\n"] block7["print(#quot;Exception#quot;)\n"] block8["try: print(#quot;try#quot;) except Exception: print(#quot;Exception#quot;) except OtherException as e: print(#quot;OtherException#quot;) else: print(#quot;else#quot;) finally: print(#quot;finally#quot;)\n"] block9["try: print(#quot;try#quot;) except Exception: print(#quot;Exception#quot;) except OtherException as e: print(#quot;OtherException#quot;) else: print(#quot;else#quot;) finally: print(#quot;finally#quot;)\n"] start --> block9 block9 -- "Exception raised" --> block8 block9 -- "else" --> block3 block8 -- "Exception" --> block7 block8 -- "else" --> block6 block7 --> block1 block6 -- "OtherException" --> block5 block6 -- "else" --> block4 block5 --> block1 block4 --> return block3 --> block2 block2 --> block1 block1 --> block0 block0 --> returnwithprocessing with the following logic:withstatements have no conditional execution (apart from the hidden logic handling the enter and exit), so the block is assumed to execute unconditionally.withblocks even if the blocks containraiseorreturnstatements. This is handled in a post-processing step.Test Plan
Additional test fixtures and control-flow fixtures were added.