From db852a0b11a156e137c96934c43f0466e46d2140 Mon Sep 17 00:00:00 2001 From: Aarni Koskela Date: Fri, 3 Feb 2023 14:43:39 +0200 Subject: [PATCH] Move ruff violations (#2526) --- src/checkers/noqa.rs | 8 +- src/registry.rs | 12 +- .../ruff/rules/ambiguous_unicode_character.rs | 94 ++++++++- .../keyword_argument_before_star_argument.rs | 46 +++++ src/rules/ruff/rules/mod.rs | 49 ++--- ..._of_concatenating_to_collection_literal.rs | 19 +- src/rules/ruff/rules/unused_noqa.rs | 69 +++++++ src/violations.rs | 178 ------------------ 8 files changed, 246 insertions(+), 229 deletions(-) create mode 100644 src/rules/ruff/rules/keyword_argument_before_star_argument.rs create mode 100644 src/rules/ruff/rules/unused_noqa.rs diff --git a/src/checkers/noqa.rs b/src/checkers/noqa.rs index 385f7feb7c..0ae9830f29 100644 --- a/src/checkers/noqa.rs +++ b/src/checkers/noqa.rs @@ -5,12 +5,12 @@ use rustpython_parser::ast::Location; use crate::ast::types::Range; use crate::fix::Fix; +use crate::noqa; use crate::noqa::{is_file_exempt, Directive}; use crate::registry::{Diagnostic, DiagnosticKind, Rule}; use crate::rule_redirects::get_redirect_target; +use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA}; use crate::settings::{flags, Settings}; -use crate::violations::UnusedCodes; -use crate::{noqa, violations}; pub fn check_noqa( diagnostics: &mut Vec, @@ -106,7 +106,7 @@ pub fn check_noqa( let end = start + lines[row][start_byte..end_byte].chars().count(); let mut diagnostic = Diagnostic::new( - violations::UnusedNOQA { codes: None }, + UnusedNOQA { codes: None }, Range::new(Location::new(row + 1, start), Location::new(row + 1, end)), ); if matches!(autofix, flags::Autofix::Enabled) @@ -160,7 +160,7 @@ pub fn check_noqa( let end = start + lines[row][start_byte..end_byte].chars().count(); let mut diagnostic = Diagnostic::new( - violations::UnusedNOQA { + UnusedNOQA { codes: Some(UnusedCodes { disabled: disabled_codes .iter() diff --git a/src/registry.rs b/src/registry.rs index 2bc3cc1e89..2bcc54da53 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -488,12 +488,12 @@ ruff_macros::define_rule_mapping!( // flake8-self SLF001 => rules::flake8_self::rules::PrivateMemberAccess, // ruff - RUF001 => violations::AmbiguousUnicodeCharacterString, - RUF002 => violations::AmbiguousUnicodeCharacterDocstring, - RUF003 => violations::AmbiguousUnicodeCharacterComment, - RUF004 => violations::KeywordArgumentBeforeStarArgument, - RUF005 => violations::UnpackInsteadOfConcatenatingToCollectionLiteral, - RUF100 => violations::UnusedNOQA, + RUF001 => rules::ruff::rules::AmbiguousUnicodeCharacterString, + RUF002 => rules::ruff::rules::AmbiguousUnicodeCharacterDocstring, + RUF003 => rules::ruff::rules::AmbiguousUnicodeCharacterComment, + RUF004 => rules::ruff::rules::KeywordArgumentBeforeStarArgument, + RUF005 => rules::ruff::rules::UnpackInsteadOfConcatenatingToCollectionLiteral, + RUF100 => rules::ruff::rules::UnusedNOQA, ); #[derive(EnumIter, Debug, PartialEq, Eq, RuleNamespace)] diff --git a/src/rules/ruff/rules/ambiguous_unicode_character.rs b/src/rules/ruff/rules/ambiguous_unicode_character.rs index 57ea024b45..1c781c8179 100644 --- a/src/rules/ruff/rules/ambiguous_unicode_character.rs +++ b/src/rules/ruff/rules/ambiguous_unicode_character.rs @@ -1,4 +1,7 @@ +use crate::define_violation; +use crate::violation::AlwaysAutofixableViolation; use once_cell::sync::Lazy; +use ruff_macros::derive_message_formats; use rustc_hash::FxHashMap; use crate::ast::types::Range; @@ -8,7 +11,90 @@ use crate::registry::{Diagnostic, DiagnosticKind}; use crate::rules::ruff::rules::Context; use crate::settings::{flags, Settings}; use crate::source_code::Locator; -use crate::violations; + +define_violation!( + pub struct AmbiguousUnicodeCharacterString { + pub confusable: char, + pub representant: char, + } +); +impl AlwaysAutofixableViolation for AmbiguousUnicodeCharacterString { + #[derive_message_formats] + fn message(&self) -> String { + let AmbiguousUnicodeCharacterString { + confusable, + representant, + } = self; + format!( + "String contains ambiguous unicode character '{confusable}' (did you mean \ + '{representant}'?)" + ) + } + + fn autofix_title(&self) -> String { + let AmbiguousUnicodeCharacterString { + confusable, + representant, + } = self; + format!("Replace '{confusable}' with '{representant}'") + } +} + +define_violation!( + pub struct AmbiguousUnicodeCharacterDocstring { + pub confusable: char, + pub representant: char, + } +); +impl AlwaysAutofixableViolation for AmbiguousUnicodeCharacterDocstring { + #[derive_message_formats] + fn message(&self) -> String { + let AmbiguousUnicodeCharacterDocstring { + confusable, + representant, + } = self; + format!( + "Docstring contains ambiguous unicode character '{confusable}' (did you mean \ + '{representant}'?)" + ) + } + + fn autofix_title(&self) -> String { + let AmbiguousUnicodeCharacterDocstring { + confusable, + representant, + } = self; + format!("Replace '{confusable}' with '{representant}'") + } +} + +define_violation!( + pub struct AmbiguousUnicodeCharacterComment { + pub confusable: char, + pub representant: char, + } +); +impl AlwaysAutofixableViolation for AmbiguousUnicodeCharacterComment { + #[derive_message_formats] + fn message(&self) -> String { + let AmbiguousUnicodeCharacterComment { + confusable, + representant, + } = self; + format!( + "Comment contains ambiguous unicode character '{confusable}' (did you mean \ + '{representant}'?)" + ) + } + + fn autofix_title(&self) -> String { + let AmbiguousUnicodeCharacterComment { + confusable, + representant, + } = self; + format!("Replace '{confusable}' with '{representant}'") + } +} /// See: static CONFUSABLES: Lazy> = Lazy::new(|| { @@ -1626,17 +1712,17 @@ pub fn ambiguous_unicode_character( let end_location = Location::new(location.row(), location.column() + 1); let mut diagnostic = Diagnostic::new::( match context { - Context::String => violations::AmbiguousUnicodeCharacterString { + Context::String => AmbiguousUnicodeCharacterString { confusable: current_char, representant, } .into(), - Context::Docstring => violations::AmbiguousUnicodeCharacterDocstring { + Context::Docstring => AmbiguousUnicodeCharacterDocstring { confusable: current_char, representant, } .into(), - Context::Comment => violations::AmbiguousUnicodeCharacterComment { + Context::Comment => AmbiguousUnicodeCharacterComment { confusable: current_char, representant, } diff --git a/src/rules/ruff/rules/keyword_argument_before_star_argument.rs b/src/rules/ruff/rules/keyword_argument_before_star_argument.rs new file mode 100644 index 0000000000..9b1856c40e --- /dev/null +++ b/src/rules/ruff/rules/keyword_argument_before_star_argument.rs @@ -0,0 +1,46 @@ +use crate::ast::types::Range; +use crate::define_violation; +use crate::registry::Diagnostic; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Expr, ExprKind, Keyword, KeywordData}; + +define_violation!( + pub struct KeywordArgumentBeforeStarArgument { + pub name: String, + } +); +impl Violation for KeywordArgumentBeforeStarArgument { + #[derive_message_formats] + fn message(&self) -> String { + let KeywordArgumentBeforeStarArgument { name } = self; + format!("Keyword argument `{name}` must come after starred arguments") + } +} + +/// RUF004 +pub fn keyword_argument_before_star_argument( + args: &[Expr], + keywords: &[Keyword], +) -> Vec { + let mut diagnostics = vec![]; + if let Some(arg) = args + .iter() + .rfind(|arg| matches!(arg.node, ExprKind::Starred { .. })) + { + for keyword in keywords { + if keyword.location < arg.location { + let KeywordData { arg, .. } = &keyword.node; + if let Some(arg) = arg { + diagnostics.push(Diagnostic::new( + KeywordArgumentBeforeStarArgument { + name: arg.to_string(), + }, + Range::from_located(keyword), + )); + } + } + } + } + diagnostics +} diff --git a/src/rules/ruff/rules/mod.rs b/src/rules/ruff/rules/mod.rs index 4717d6638c..a038b320d3 100644 --- a/src/rules/ruff/rules/mod.rs +++ b/src/rules/ruff/rules/mod.rs @@ -1,14 +1,20 @@ -use rustpython_ast::{Expr, ExprKind, Keyword, KeywordData}; - -use crate::ast::types::Range; -use crate::registry::Diagnostic; -use crate::violations; - mod ambiguous_unicode_character; +mod keyword_argument_before_star_argument; mod unpack_instead_of_concatenating_to_collection_literal; +mod unused_noqa; -pub use ambiguous_unicode_character::ambiguous_unicode_character; -pub use unpack_instead_of_concatenating_to_collection_literal::unpack_instead_of_concatenating_to_collection_literal; +pub use ambiguous_unicode_character::{ + ambiguous_unicode_character, AmbiguousUnicodeCharacterComment, + AmbiguousUnicodeCharacterDocstring, AmbiguousUnicodeCharacterString, +}; +pub use keyword_argument_before_star_argument::{ + keyword_argument_before_star_argument, KeywordArgumentBeforeStarArgument, +}; +pub use unpack_instead_of_concatenating_to_collection_literal::{ + unpack_instead_of_concatenating_to_collection_literal, + UnpackInsteadOfConcatenatingToCollectionLiteral, +}; +pub use unused_noqa::{UnusedCodes, UnusedNOQA}; #[derive(Clone, Copy)] pub enum Context { @@ -16,30 +22,3 @@ pub enum Context { Docstring, Comment, } - -/// RUF004 -pub fn keyword_argument_before_star_argument( - args: &[Expr], - keywords: &[Keyword], -) -> Vec { - let mut diagnostics = vec![]; - if let Some(arg) = args - .iter() - .rfind(|arg| matches!(arg.node, ExprKind::Starred { .. })) - { - for keyword in keywords { - if keyword.location < arg.location { - let KeywordData { arg, .. } = &keyword.node; - if let Some(arg) = arg { - diagnostics.push(Diagnostic::new( - violations::KeywordArgumentBeforeStarArgument { - name: arg.to_string(), - }, - Range::from_located(keyword), - )); - } - } - } - } - diagnostics -} diff --git a/src/rules/ruff/rules/unpack_instead_of_concatenating_to_collection_literal.rs b/src/rules/ruff/rules/unpack_instead_of_concatenating_to_collection_literal.rs index 87a8dcc0a7..0e6d319e28 100644 --- a/src/rules/ruff/rules/unpack_instead_of_concatenating_to_collection_literal.rs +++ b/src/rules/ruff/rules/unpack_instead_of_concatenating_to_collection_literal.rs @@ -1,3 +1,6 @@ +use crate::define_violation; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; use rustpython_ast::{Expr, ExprContext, ExprKind, Operator}; use crate::ast::helpers::{create_expr, has_comments, unparse_expr}; @@ -5,7 +8,19 @@ use crate::ast::types::Range; use crate::checkers::ast::Checker; use crate::fix::Fix; use crate::registry::Diagnostic; -use crate::violations; + +define_violation!( + pub struct UnpackInsteadOfConcatenatingToCollectionLiteral { + pub expr: String, + } +); +impl Violation for UnpackInsteadOfConcatenatingToCollectionLiteral { + #[derive_message_formats] + fn message(&self) -> String { + let UnpackInsteadOfConcatenatingToCollectionLiteral { expr } = self; + format!("Consider `{expr}` instead of concatenation") + } +} fn make_splat_elts( splat_element: &Expr, @@ -81,7 +96,7 @@ pub fn unpack_instead_of_concatenating_to_collection_literal(checker: &mut Check }; let mut diagnostic = Diagnostic::new( - violations::UnpackInsteadOfConcatenatingToCollectionLiteral { + UnpackInsteadOfConcatenatingToCollectionLiteral { expr: new_expr_string.clone(), }, Range::from_located(expr), diff --git a/src/rules/ruff/rules/unused_noqa.rs b/src/rules/ruff/rules/unused_noqa.rs new file mode 100644 index 0000000000..feb0421f4f --- /dev/null +++ b/src/rules/ruff/rules/unused_noqa.rs @@ -0,0 +1,69 @@ +use crate::define_violation; +use crate::violation::AlwaysAutofixableViolation; +use itertools::Itertools; +use ruff_macros::derive_message_formats; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct UnusedCodes { + pub unknown: Vec, + pub disabled: Vec, + pub unmatched: Vec, +} + +define_violation!( + pub struct UnusedNOQA { + pub codes: Option, + } +); +impl AlwaysAutofixableViolation for UnusedNOQA { + #[derive_message_formats] + fn message(&self) -> String { + let UnusedNOQA { codes } = self; + match codes { + None => format!("Unused blanket `noqa` directive"), + Some(codes) => { + let mut codes_by_reason = vec![]; + if !codes.unmatched.is_empty() { + codes_by_reason.push(format!( + "unused: {}", + codes + .unmatched + .iter() + .map(|code| format!("`{code}`")) + .join(", ") + )); + } + if !codes.disabled.is_empty() { + codes_by_reason.push(format!( + "non-enabled: {}", + codes + .disabled + .iter() + .map(|code| format!("`{code}`")) + .join(", ") + )); + } + if !codes.unknown.is_empty() { + codes_by_reason.push(format!( + "unknown: {}", + codes + .unknown + .iter() + .map(|code| format!("`{code}`")) + .join(", ") + )); + } + if codes_by_reason.is_empty() { + format!("Unused `noqa` directive") + } else { + format!("Unused `noqa` directive ({})", codes_by_reason.join("; ")) + } + } + } + } + + fn autofix_title(&self) -> String { + "Remove unused `noqa` directive".to_string() + } +} diff --git a/src/violations.rs b/src/violations.rs index aa49956570..b4c85c3833 100644 --- a/src/violations.rs +++ b/src/violations.rs @@ -2,8 +2,6 @@ use itertools::Itertools; use ruff_macros::derive_message_formats; -use serde::{Deserialize, Serialize}; - use crate::define_violation; use crate::violation::{AlwaysAutofixableViolation, Violation}; @@ -526,179 +524,3 @@ impl Violation for DotFormatInException { format!("Exception must not use a `.format()` string directly, assign to variable first") } } - -// ruff - -define_violation!( - pub struct AmbiguousUnicodeCharacterString { - pub confusable: char, - pub representant: char, - } -); -impl AlwaysAutofixableViolation for AmbiguousUnicodeCharacterString { - #[derive_message_formats] - fn message(&self) -> String { - let AmbiguousUnicodeCharacterString { - confusable, - representant, - } = self; - format!( - "String contains ambiguous unicode character '{confusable}' (did you mean \ - '{representant}'?)" - ) - } - - fn autofix_title(&self) -> String { - let AmbiguousUnicodeCharacterString { - confusable, - representant, - } = self; - format!("Replace '{confusable}' with '{representant}'") - } -} - -define_violation!( - pub struct AmbiguousUnicodeCharacterDocstring { - pub confusable: char, - pub representant: char, - } -); -impl AlwaysAutofixableViolation for AmbiguousUnicodeCharacterDocstring { - #[derive_message_formats] - fn message(&self) -> String { - let AmbiguousUnicodeCharacterDocstring { - confusable, - representant, - } = self; - format!( - "Docstring contains ambiguous unicode character '{confusable}' (did you mean \ - '{representant}'?)" - ) - } - - fn autofix_title(&self) -> String { - let AmbiguousUnicodeCharacterDocstring { - confusable, - representant, - } = self; - format!("Replace '{confusable}' with '{representant}'") - } -} - -define_violation!( - pub struct AmbiguousUnicodeCharacterComment { - pub confusable: char, - pub representant: char, - } -); -impl AlwaysAutofixableViolation for AmbiguousUnicodeCharacterComment { - #[derive_message_formats] - fn message(&self) -> String { - let AmbiguousUnicodeCharacterComment { - confusable, - representant, - } = self; - format!( - "Comment contains ambiguous unicode character '{confusable}' (did you mean \ - '{representant}'?)" - ) - } - - fn autofix_title(&self) -> String { - let AmbiguousUnicodeCharacterComment { - confusable, - representant, - } = self; - format!("Replace '{confusable}' with '{representant}'") - } -} - -define_violation!( - pub struct KeywordArgumentBeforeStarArgument { - pub name: String, - } -); -impl Violation for KeywordArgumentBeforeStarArgument { - #[derive_message_formats] - fn message(&self) -> String { - let KeywordArgumentBeforeStarArgument { name } = self; - format!("Keyword argument `{name}` must come after starred arguments") - } -} - -define_violation!( - pub struct UnpackInsteadOfConcatenatingToCollectionLiteral { - pub expr: String, - } -); -impl Violation for UnpackInsteadOfConcatenatingToCollectionLiteral { - #[derive_message_formats] - fn message(&self) -> String { - let UnpackInsteadOfConcatenatingToCollectionLiteral { expr } = self; - format!("Consider `{expr}` instead of concatenation") - } -} - -#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] -pub struct UnusedCodes { - pub unknown: Vec, - pub disabled: Vec, - pub unmatched: Vec, -} - -define_violation!( - pub struct UnusedNOQA { - pub codes: Option, - } -); -impl AlwaysAutofixableViolation for UnusedNOQA { - #[derive_message_formats] - fn message(&self) -> String { - let UnusedNOQA { codes } = self; - match codes { - None => format!("Unused blanket `noqa` directive"), - Some(codes) => { - let mut codes_by_reason = vec![]; - if !codes.unmatched.is_empty() { - codes_by_reason.push(format!( - "unused: {}", - codes - .unmatched - .iter() - .map(|code| format!("`{code}`")) - .join(", ") - )); - } - if !codes.disabled.is_empty() { - codes_by_reason.push(format!( - "non-enabled: {}", - codes - .disabled - .iter() - .map(|code| format!("`{code}`")) - .join(", ") - )); - } - if !codes.unknown.is_empty() { - codes_by_reason.push(format!( - "unknown: {}", - codes - .unknown - .iter() - .map(|code| format!("`{code}`")) - .join(", ") - )); - } - if codes_by_reason.is_empty() { - format!("Unused `noqa` directive") - } else { - format!("Unused `noqa` directive ({})", codes_by_reason.join("; ")) - } - } - } - } - - fn autofix_title(&self) -> String { - "Remove unused `noqa` directive".to_string() - } -}