mirror of
				https://github.com/astral-sh/ruff.git
				synced 2025-10-31 12:05:57 +00:00 
			
		
		
		
	Mark RET501 fix unsafe if comments are inside (#18780)
				
					
				
			Co-authored-by: Charlie Marsh <crmarsh416@gmail.com>
This commit is contained in:
		
							parent
							
								
									e352a50b74
								
							
						
					
					
						commit
						dcf0a8d4d7
					
				
					 3 changed files with 45 additions and 4 deletions
				
			
		|  | @ -49,3 +49,13 @@ class Baz: | |||
|     def prop4(self) -> None: | ||||
|         print("I've run out of things to say") | ||||
|         return None | ||||
| 
 | ||||
| 
 | ||||
| # https://github.com/astral-sh/ruff/issues/18774 | ||||
| class _: | ||||
|     def foo(bar): | ||||
|         if not bar: | ||||
|             return | ||||
|         return ( | ||||
|             None # comment | ||||
|         ) | ||||
|  |  | |||
|  | @ -1,5 +1,6 @@ | |||
| use anyhow::Result; | ||||
| 
 | ||||
| use ruff_diagnostics::Applicability; | ||||
| use ruff_macros::{ViolationMetadata, derive_message_formats}; | ||||
| use ruff_python_ast::helpers::{is_const_false, is_const_true}; | ||||
| use ruff_python_ast::stmt_if::elif_else_range; | ||||
|  | @ -50,6 +51,11 @@ use super::super::visitor::{ReturnVisitor, Stack}; | |||
| ///         return
 | ||||
| ///     return
 | ||||
| /// ```
 | ||||
| ///
 | ||||
| /// ## Fix Safety
 | ||||
| /// This rule's fix is marked as unsafe for cases in which comments would be
 | ||||
| /// dropped from the `return` statement.
 | ||||
| ///
 | ||||
| #[derive(ViolationMetadata)] | ||||
| pub(crate) struct UnnecessaryReturnNone; | ||||
| 
 | ||||
|  | @ -381,10 +387,15 @@ fn unnecessary_return_none(checker: &Checker, decorator_list: &[Decorator], stac | |||
|         } | ||||
| 
 | ||||
|         let mut diagnostic = checker.report_diagnostic(UnnecessaryReturnNone, stmt.range()); | ||||
|         diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( | ||||
|             "return".to_string(), | ||||
|             stmt.range(), | ||||
|         ))); | ||||
|         let edit = Edit::range_replacement("return".to_string(), stmt.range()); | ||||
|         diagnostic.set_fix(Fix::applicable_edit( | ||||
|             edit, | ||||
|             if checker.comment_ranges().intersects(stmt.range()) { | ||||
|                 Applicability::Unsafe | ||||
|             } else { | ||||
|                 Applicability::Safe | ||||
|             }, | ||||
|         )); | ||||
|     } | ||||
| } | ||||
| 
 | ||||
|  |  | |||
|  | @ -40,3 +40,23 @@ RET501.py:14:9: RET501 [*] Do not explicitly `return None` in function if it is | |||
| 15 15 |  | ||||
| 16 16 |     @property | ||||
| 17 17 |     def prop(self) -> None: | ||||
| 
 | ||||
| RET501.py:59:9: RET501 [*] Do not explicitly `return None` in function if it is the only possible return value | ||||
|    | | ||||
| 57 |           if not bar: | ||||
| 58 |               return | ||||
| 59 | /         return ( | ||||
| 60 | |             None # comment | ||||
| 61 | |         ) | ||||
|    | |_________^ RET501 | ||||
|    | | ||||
|    = help: Remove explicit `return None` | ||||
| 
 | ||||
| ℹ Unsafe fix | ||||
| 56 56 |     def foo(bar): | ||||
| 57 57 |         if not bar: | ||||
| 58 58 |             return | ||||
| 59    |-        return ( | ||||
| 60    |-            None # comment | ||||
| 61    |-        ) | ||||
|    59 |+        return | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Nikolas Hearp
						Nikolas Hearp