diff --git a/crates/pyrefly_python/src/ignore.rs b/crates/pyrefly_python/src/ignore.rs index 45accc4e7..fe7d40347 100644 --- a/crates/pyrefly_python/src/ignore.rs +++ b/crates/pyrefly_python/src/ignore.rs @@ -283,15 +283,24 @@ impl Ignore { false } - /// Get all pyrefly ignores. + #[inline] + /// Get pyrefly ignores that are global, and do not have an error code. + pub fn get_pyrefly_all_ignores(&self) -> SmallSet { + self._get_pyrefly_ignores(true) + } + + #[inline] pub fn get_pyrefly_ignores(&self) -> SmallSet { + self._get_pyrefly_ignores(false) + } + fn _get_pyrefly_ignores(&self, only_all: bool) -> SmallSet { self.ignores .iter() .filter(|ignore| { ignore .1 .iter() - .any(|s| s.tool == Tool::Pyrefly && s.kind.is_empty()) + .any(|s| s.tool == Tool::Pyrefly && (!only_all || s.kind.is_empty())) }) .map(|(line, _)| *line) .collect() @@ -306,6 +315,8 @@ mod tests { #[test] fn test_parse_ignores() { + // TODO: The second component of `expect` (the error code) + // is not actually tested. fn f(x: &str, expect: &[(Tool, u32)]) { assert_eq!( &Ignore::parse_ignores(x) @@ -325,6 +336,7 @@ mod tests { "code # mypy: ignore\n# pyre-fixme\nmore code", &[(Tool::Mypy, 1), (Tool::Pyre, 3)], ); + f("# pyrefly: ignore[bad-]\n", &[(Tool::Pyrefly, 2)]); f( "# type: ignore\n# mypy: ignore\n# bad\n\ncode", &[(Tool::Any, 4), (Tool::Mypy, 4)], diff --git a/pyrefly/lib/state/ide.rs b/pyrefly/lib/state/ide.rs index 04744be42..ad62f463f 100644 --- a/pyrefly/lib/state/ide.rs +++ b/pyrefly/lib/state/ide.rs @@ -5,6 +5,8 @@ * LICENSE file in the root directory of this source tree. */ +use dupe::Dupe; +use pyrefly_python::module::Module; use pyrefly_python::module_name::ModuleName; use pyrefly_python::short_identifier::ShortIdentifier; use pyrefly_python::symbol_kind::SymbolKind; @@ -23,6 +25,7 @@ use crate::binding::binding::Key; use crate::binding::bindings::Bindings; use crate::binding::narrow::identifier_and_chain_for_expr; use crate::binding::narrow::identifier_and_chain_prefix_for_expr; +use crate::error::error::Error; use crate::export::exports::Export; use crate::state::handle::Handle; @@ -186,3 +189,66 @@ pub fn insert_import_edit( ); (position, insert_text) } + +pub fn create_ignore_code_action( + error: &Error, + module_info: &Module, +) -> Option<(String, Module, TextRange, String)> { + let ignore_lines_in_module = module_info.ignore().get_pyrefly_ignores(); + let start_line = error.display_range().start.line; + + if ignore_lines_in_module.contains(&start_line) { + create_add_to_existing_ignore_action(error, module_info) + } else { + create_new_ignore_action(error, module_info) + } +} + +fn create_add_to_existing_ignore_action( + error: &Error, + module_info: &Module, +) -> Option<(String, Module, TextRange, String)> { + let dec = error.display_range().start.line.decrement()?; + let suppression_comment = module_info + .lined_buffer() + .content_in_line_range(dec, dec) + .trim_end(); + let bracket_pos = suppression_comment.rfind(']')?; + let row_offset = module_info.lined_buffer().line_start(dec); + let text_range = TextRange::new( + row_offset + TextSize::from(bracket_pos as u32), + row_offset + TextSize::from((bracket_pos + 1) as u32), + ); + + Some(( + format!("Add {} to above ignore comment", error.error_kind()), + module_info.dupe(), + text_range, + format!(", {}]", error.error_kind().to_name()), + )) +} + +fn create_new_ignore_action( + error: &Error, + module_info: &Module, +) -> Option<(String, Module, TextRange, String)> { + let start = error.range().start(); + let display_pos = module_info.display_pos(start); + let start = module_info.lined_buffer().line_start(display_pos.line); + let line = module_info + .lined_buffer() + .content_in_line_range(display_pos.line, display_pos.line); + let offset = line.find(|c: char| !c.is_whitespace()).unwrap_or(0); + let leading_indentation = " ".repeat(offset as usize); + + Some(( + format!("Suppress {} with ignore comment", error.error_kind()), + module_info.dupe(), + TextRange::new(start, start), + format!( + "{}# pyrefly: ignore[{}]\n", + leading_indentation, + error.error_kind().to_name() + ), + )) +} diff --git a/pyrefly/lib/state/lsp.rs b/pyrefly/lib/state/lsp.rs index 8a47a7b44..a4418610f 100644 --- a/pyrefly/lib/state/lsp.rs +++ b/pyrefly/lib/state/lsp.rs @@ -69,6 +69,7 @@ use crate::export::exports::ExportLocation; use crate::graph::index::Idx; use crate::state::handle::Handle; use crate::state::ide::IntermediateDefinition; +use crate::state::ide::create_ignore_code_action; use crate::state::ide::insert_import_edit; use crate::state::ide::key_to_intermediate_definition; use crate::state::require::Require; @@ -1424,6 +1425,17 @@ impl<'a> Transaction<'a> { _ => {} } } + code_actions.extend( + self.get_errors(vec![handle]) + .collect_errors() + .shown + .iter() + // NOTE: For these suppressions the cursor needs to literally + // be on the error range, instead of just the line. This + // is typical with LSPs but worth noting. + .filter(|error| error.range().contains_range(range)) + .filter_map(|error| create_ignore_code_action(error, &module_info)), + ); code_actions.sort_by(|(title1, _, _, _), (title2, _, _, _)| title1.cmp(title2)); Some(code_actions) } diff --git a/pyrefly/lib/test/lsp/code_actions.rs b/pyrefly/lib/test/lsp/code_actions.rs index a35660581..8fdd824b0 100644 --- a/pyrefly/lib/test/lsp/code_actions.rs +++ b/pyrefly/lib/test/lsp/code_actions.rs @@ -25,12 +25,16 @@ fn apply_patch(info: &ModuleInfo, range: TextRange, patch: String) -> (String, S (before, after) } -fn get_test_report(state: &State, handle: &Handle, position: TextSize) -> String { +fn get_test_report_insert_import(state: &State, handle: &Handle, position: TextSize) -> String { let mut report = "Code Actions Results:\n".to_owned(); let transaction = state.transaction(); for (title, info, range, patch) in transaction .local_quickfix_code_actions(handle, TextRange::new(position, position)) - .unwrap_or_default() + .into_iter() + .flatten() + // NOTE: We do NOT patch buffer with ignore comment code action that is + // tested separately + .filter(|(title, _, _, _)| !title.contains("ignore")) { let (before, after) = apply_patch(&info, range, patch); report.push_str("# Title: "); @@ -45,6 +49,128 @@ fn get_test_report(state: &State, handle: &Handle, position: TextSize) -> String report } +fn get_test_report_ignore_code_action( + state: &State, + handle: &Handle, + position: TextSize, +) -> String { + let mut report = "Code Actions Results:\n".to_owned(); + let transaction = state.transaction(); + for (title, info, range, patch) in transaction + .local_quickfix_code_actions(handle, TextRange::new(position, position)) + .into_iter() + .flatten() + // NOTE: We ONLY patch buffer with ignore comment code action so + // we can test it separately + .filter(|(title, _, _, _)| title.contains("ignore")) + { + let (before, after) = apply_patch(&info, range, patch); + report.push_str("# Title: "); + report.push_str(&title); + report.push('\n'); + report.push_str("\n## Before:\n"); + report.push_str(&before); + report.push_str("\n## After:\n"); + report.push_str(&after); + report.push('\n'); + } + report +} + +#[test] +fn add_ignore_comment() { + let report = get_batched_lsp_operations_report_allow_error( + &[ + ("a", "my_export = 3\nanother_thing = 4"), + ("b", "from a import another_thing\nmy_export\n# ^"), + ( + "c", + r#" +def func(x: int) -> int: + pass +func(y) +# ^"#, + ), + ( + "d", + r#" +def func(x: int) -> int: +# ^ + pass +func(y)"#, + ), + ], + get_test_report_ignore_code_action, + ); + assert_eq!( + r#" +# a.py + +# b.py +2 | my_export + ^ +Code Actions Results: +# Title: Suppress UnknownName with ignore comment + +## Before: +from a import another_thing +my_export +# ^ +## After: +from a import another_thing +# pyrefly: ignore[unknown-name] +my_export +# ^ + + + +# c.py +4 | func(y) + ^ +Code Actions Results: +# Title: Suppress UnknownName with ignore comment + +## Before: + +def func(x: int) -> int: + pass +func(y) +# ^ +## After: + +def func(x: int) -> int: + pass +# pyrefly: ignore[unknown-name] +func(y) +# ^ + + + +# d.py +2 | def func(x: int) -> int: + ^ +Code Actions Results: +# Title: Suppress BadReturn with ignore comment + +## Before: + +def func(x: int) -> int: +# ^ + pass +func(y) +## After: + +# pyrefly: ignore[bad-return] +def func(x: int) -> int: +# ^ + pass +func(y) +"# + .trim(), + report.trim() + ); +} + #[test] fn basic_test() { let report = get_batched_lsp_operations_report_allow_error( @@ -54,7 +180,7 @@ fn basic_test() { ("c", "my_export\n# ^"), ("d", "my_export = 3\n"), ], - get_test_report, + get_test_report_insert_import, ); // We should suggest imports from both a and d, but not b. assert_eq!( @@ -102,7 +228,7 @@ fn insertion_test_comments() { ("a", "my_export = 3\n"), ("b", "# i am a comment\nmy_export\n# ^"), ], - get_test_report, + get_test_report_insert_import, ); // We will insert the import after a comment, which might not be the intended target of the // comment. This is not ideal, but we cannot do much better without sophisticated comment @@ -139,7 +265,7 @@ fn insertion_test_existing_imports() { ("a", "my_export = 3\n"), ("b", "from typing import List\nmy_export\n# ^"), ], - get_test_report, + get_test_report_insert_import, ); // Insert before all imports. This might not adhere to existing import sorting code style. assert_eq!( @@ -174,7 +300,7 @@ fn insertion_test_duplicate_imports() { ("a", "my_export = 3\nanother_thing = 4"), ("b", "from a import another_thing\nmy_export\n# ^"), ], - get_test_report, + get_test_report_insert_import, ); // The insertion won't attempt to merge imports from the same module. // It's not illegal, but it would be nice if we do merge.