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

[pylint] invalid-characters-* #3552

Merged
merged 6 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Binary file not shown.
10 changes: 1 addition & 9 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2984,15 +2984,7 @@ where
flake8_pie::rules::no_unnecessary_spread(self, keys, values);
}
}
ExprKind::Yield { .. } => {
if self.settings.rules.enabled(&Rule::YieldOutsideFunction) {
pyflakes::rules::yield_outside_function(self, expr);
}
if self.settings.rules.enabled(&Rule::YieldInInit) {
pylint::rules::yield_in_init(self, expr);
}
}
ExprKind::YieldFrom { .. } => {
ExprKind::Yield { .. } | ExprKind::YieldFrom { .. } => {
if self.settings.rules.enabled(&Rule::YieldOutsideFunction) {
pyflakes::rules::yield_outside_function(self, expr);
}
Expand Down
68 changes: 42 additions & 26 deletions crates/ruff/src/checkers/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::registry::{AsRule, Rule};
use crate::rules::ruff::rules::Context;
use crate::rules::{
eradicate, flake8_commas, flake8_implicit_str_concat, flake8_pyi, flake8_quotes, pycodestyle,
pyupgrade, ruff,
pylint, pyupgrade, ruff,
};
use crate::settings::{flags, Settings};
use ruff_diagnostics::Diagnostic;
Expand All @@ -23,39 +23,43 @@ pub fn check_tokens(
) -> Vec<Diagnostic> {
let mut diagnostics: Vec<Diagnostic> = vec![];

let enforce_ambiguous_unicode_character = settings
.rules
.enabled(&Rule::AmbiguousUnicodeCharacterString)
|| settings
.rules
.enabled(&Rule::AmbiguousUnicodeCharacterDocstring)
|| settings
.rules
.enabled(&Rule::AmbiguousUnicodeCharacterComment);
let enforce_quotes = settings.rules.enabled(&Rule::BadQuotesInlineString)
|| settings.rules.enabled(&Rule::BadQuotesMultilineString)
|| settings.rules.enabled(&Rule::BadQuotesDocstring)
|| settings.rules.enabled(&Rule::AvoidableEscapedQuote);
let enforce_ambiguous_unicode_character = settings.rules.any_enabled(&[
&Rule::AmbiguousUnicodeCharacterString,
&Rule::AmbiguousUnicodeCharacterDocstring,
&Rule::AmbiguousUnicodeCharacterComment,
]);
let enforce_invalid_string_character = settings.rules.any_enabled(&[
&Rule::InvalidCharacterBackspace,
&Rule::InvalidCharacterSub,
&Rule::InvalidCharacterEsc,
&Rule::InvalidCharacterNul,
&Rule::InvalidCharacterZeroWidthSpace,
]);
let enforce_quotes = settings.rules.any_enabled(&[
&Rule::BadQuotesInlineString,
&Rule::BadQuotesMultilineString,
&Rule::BadQuotesDocstring,
&Rule::AvoidableEscapedQuote,
]);
let enforce_commented_out_code = settings.rules.enabled(&Rule::CommentedOutCode);
let enforce_compound_statements = settings
.rules
.enabled(&Rule::MultipleStatementsOnOneLineColon)
|| settings
.rules
.enabled(&Rule::MultipleStatementsOnOneLineSemicolon)
|| settings.rules.enabled(&Rule::UselessSemicolon);
let enforce_compound_statements = settings.rules.any_enabled(&[
&Rule::MultipleStatementsOnOneLineColon,
&Rule::MultipleStatementsOnOneLineSemicolon,
&Rule::UselessSemicolon,
]);
let enforce_invalid_escape_sequence = settings.rules.enabled(&Rule::InvalidEscapeSequence);
let enforce_implicit_string_concatenation = settings
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Use any_enabled?

.rules
.enabled(&Rule::SingleLineImplicitStringConcatenation)
|| settings
.rules
.enabled(&Rule::MultiLineImplicitStringConcatenation);
let enforce_trailing_comma = settings.rules.enabled(&Rule::TrailingCommaMissing)
|| settings
.rules
.enabled(&Rule::TrailingCommaOnBareTupleProhibited)
|| settings.rules.enabled(&Rule::TrailingCommaProhibited);

let enforce_trailing_comma = settings.rules.any_enabled(&[
&Rule::TrailingCommaMissing,
&Rule::TrailingCommaOnBareTupleProhibited,
&Rule::TrailingCommaProhibited,
]);
let enforce_extraneous_parenthesis = settings.rules.enabled(&Rule::ExtraneousParentheses);
let enforce_type_comment_in_stub = settings.rules.enabled(&Rule::TypeCommentInStub);

Expand Down Expand Up @@ -116,6 +120,18 @@ pub fn check_tokens(
}
}
}
// PLE2510, PLE2512, PLE2513
if enforce_invalid_string_character {
for (start, tok, end) in tokens.iter().flatten() {
if matches!(tok, Tok::String { .. }) {
diagnostics.extend(
pylint::rules::invalid_string_characters(locator, *start, *end, autofix.into())
.into_iter()
.filter(|diagnostic| settings.rules.enabled(diagnostic.kind.rule())),
);
}
}
}

// E701, E702, E703
if enforce_compound_statements {
Expand Down
7 changes: 6 additions & 1 deletion crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,14 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(Pylint, "E1205") => Rule::LoggingTooManyArgs,
(Pylint, "E1206") => Rule::LoggingTooFewArgs,
(Pylint, "E1307") => Rule::BadStringFormatType,
(Pylint, "E2502") => Rule::BidirectionalUnicode,
(Pylint, "E2510") => Rule::InvalidCharacterBackspace,
(Pylint, "E2512") => Rule::InvalidCharacterSub,
(Pylint, "E2513") => Rule::InvalidCharacterEsc,
(Pylint, "E2514") => Rule::InvalidCharacterNul,
(Pylint, "E2515") => Rule::InvalidCharacterZeroWidthSpace,
(Pylint, "E1310") => Rule::BadStrStripCall,
(Pylint, "E1507") => Rule::InvalidEnvvarValue,
(Pylint, "E2502") => Rule::BidirectionalUnicode,
(Pylint, "R0133") => Rule::ComparisonOfConstant,
(Pylint, "R0206") => Rule::PropertyWithParameters,
(Pylint, "R0402") => Rule::ConsiderUsingFromImport,
Expand Down
10 changes: 10 additions & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,11 @@ ruff_macros::register_rules!(
rules::pylint::rules::InvalidEnvvarValue,
rules::pylint::rules::BadStringFormatType,
rules::pylint::rules::BidirectionalUnicode,
rules::pylint::rules::InvalidCharacterBackspace,
rules::pylint::rules::InvalidCharacterSub,
rules::pylint::rules::InvalidCharacterEsc,
rules::pylint::rules::InvalidCharacterNul,
rules::pylint::rules::InvalidCharacterZeroWidthSpace,
rules::pylint::rules::BadStrStripCall,
rules::pylint::rules::CollapsibleElseIf,
rules::pylint::rules::UselessImportAlias,
Expand Down Expand Up @@ -848,6 +853,11 @@ impl Rule {
| Rule::BadQuotesMultilineString
| Rule::CommentedOutCode
| Rule::MultiLineImplicitStringConcatenation
| Rule::InvalidCharacterBackspace
| Rule::InvalidCharacterSub
| Rule::InvalidCharacterEsc
| Rule::InvalidCharacterNul
| Rule::InvalidCharacterZeroWidthSpace
| Rule::ExtraneousParentheses
| Rule::InvalidEscapeSequence
| Rule::SingleLineImplicitStringConcatenation
Expand Down
5 changes: 5 additions & 0 deletions crates/ruff/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ mod tests {
#[test_case(Rule::ConsiderUsingSysExit, Path::new("consider_using_sys_exit_4.py"); "PLR1722_4")]
#[test_case(Rule::ConsiderUsingSysExit, Path::new("consider_using_sys_exit_5.py"); "PLR1722_5")]
#[test_case(Rule::ConsiderUsingSysExit, Path::new("consider_using_sys_exit_6.py"); "PLR1722_6")]
#[test_case(Rule::InvalidCharacterBackspace, Path::new("invalid_characters.py"); "PLE2510")]
#[test_case(Rule::InvalidCharacterSub, Path::new("invalid_characters.py"); "PLE2512")]
#[test_case(Rule::InvalidCharacterEsc, Path::new("invalid_characters.py"); "PLE2513")]
#[test_case(Rule::InvalidCharacterNul, Path::new("invalid_characters.py"); "PLE2514")]
#[test_case(Rule::InvalidCharacterZeroWidthSpace, Path::new("invalid_characters.py"); "PLE2515")]
#[test_case(Rule::MagicValueComparison, Path::new("magic_value_comparison.py"); "PLR2004")]
#[test_case(Rule::UselessElseOnLoop, Path::new("useless_else_on_loop.py"); "PLW0120")]
#[test_case(Rule::GlobalVariableNotAssigned, Path::new("global_variable_not_assigned.py"); "PLW0602")]
Expand Down
205 changes: 205 additions & 0 deletions crates/ruff/src/rules/pylint/rules/invalid_string_characters.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
use rustpython_parser::ast::Location;

use ruff_macros::{derive_message_formats, violation};

use ruff_diagnostics::AlwaysAutofixableViolation;
use ruff_diagnostics::Fix;
use ruff_diagnostics::{Diagnostic, DiagnosticKind};
use ruff_python_ast::source_code::Locator;
use ruff_python_ast::types::Range;

/// ## What it does
/// Checks that strings don't have the control character BS
///
/// ## Why is this bad?
/// Control characters can display differently in different text editors and terminals. By using the \b sequence the string's
/// value will be the same but it will be visible in all text editors.
///
/// ## Example
/// ```python
/// x = ''
/// ```
///
/// Use instead:
/// ```python
/// x = '\b'
/// ```
#[violation]
pub struct InvalidCharacterBackspace;

impl AlwaysAutofixableViolation for InvalidCharacterBackspace {
#[derive_message_formats]
fn message(&self) -> String {
format!("Invalid unescaped character backspace, use \"\\b\" instead")
}

fn autofix_title(&self) -> String {
"Replace with escape sequence".to_string()
}
}

/// ## What it does
/// Checks that strings don't have the raw control character SUB
///
/// ## Why is this bad?
/// Control characters can display differently in different text editors and terminals. By using the \x1B sequence the string's
/// value will be the same but it will be visible in all text editors.
///
/// ## Example
/// ```python
/// x = ''
/// ```
///
/// Use instead:
/// ```python
/// x = '\x1A'
/// ```

r3m0t marked this conversation as resolved.
Show resolved Hide resolved
#[violation]
pub struct InvalidCharacterSub;

impl AlwaysAutofixableViolation for InvalidCharacterSub {
#[derive_message_formats]
fn message(&self) -> String {
format!("Invalid unescaped character SUB, use \"\\x1A\" instead")
}

fn autofix_title(&self) -> String {
"Replace with escape sequence".to_string()
}
}

/// ## What it does
/// Checks that strings don't have the raw control character ESC
///
/// ## Why is this bad?
/// Control characters can display differently in different text editors and terminals. By using the \x1B sequence the string's
/// value will be the same but it will be visible in all text editors.
///
/// ## Example
/// ```python
/// x = ''
/// ```
///
/// Use instead:
/// ```python
/// x = '\x1B'
/// ```

r3m0t marked this conversation as resolved.
Show resolved Hide resolved
#[violation]
pub struct InvalidCharacterEsc;

impl AlwaysAutofixableViolation for InvalidCharacterEsc {
#[derive_message_formats]
fn message(&self) -> String {
format!("Invalid unescaped character ESC, use \"\\x1B\" instead")
}

fn autofix_title(&self) -> String {
"Replace with escape sequence".to_string()
}
}

/// ## What it does
/// Checks that strings don't have the raw control character NUL (0 byte)
///
/// ## Why is this bad?
/// Control characters can display differently in different text editors and terminals. By using the \x1B sequence the string's
/// value will be the same but it will be visible in all text editors.
///
/// ## Example
/// ```python
/// x = ''
/// ```
///
/// Use instead:
/// ```python
/// x = '\0'
/// ```

r3m0t marked this conversation as resolved.
Show resolved Hide resolved
#[violation]
pub struct InvalidCharacterNul;

impl AlwaysAutofixableViolation for InvalidCharacterNul {
#[derive_message_formats]
fn message(&self) -> String {
format!("Invalid unescaped character NUL, use \"\\0\" instead")
}

fn autofix_title(&self) -> String {
"Replace with escape sequence".to_string()
}
}

/// ## What it does
/// Checks that strings don't have the zero width space character
///
/// ## Why is this bad?
/// This character can be invisible in some text editors and terminals. By using the \x1B sequence the string's
/// value will be the same but it will be visible in all text editors.
///
/// ## Example
/// ```python
/// x = 'Dear Sir/Madam'
/// ```
///
/// Use instead:
/// ```python
/// x = 'Dear Sir\u200B/\u200BMadam' # zero width space
/// ```

r3m0t marked this conversation as resolved.
Show resolved Hide resolved
#[violation]
pub struct InvalidCharacterZeroWidthSpace;

impl AlwaysAutofixableViolation for InvalidCharacterZeroWidthSpace {
#[derive_message_formats]
fn message(&self) -> String {
format!("Invalid unescaped character zero-width-space, use \"\\u200B\" instead")
}

fn autofix_title(&self) -> String {
"Replace with escape sequence".to_string()
}
}

/// PLE2510, PLE2512, PLE2513, PLE2514, PLE2515
pub fn invalid_string_characters(
locator: &Locator,
start: Location,
end: Location,
autofix: bool,
) -> Vec<Diagnostic> {
let mut diagnostics = Vec::new();
let text = locator.slice(Range::new(start, end));

for (row_offset, line) in text.lines().enumerate() {
r3m0t marked this conversation as resolved.
Show resolved Hide resolved
for (col_offset, m) in line.match_indices(&['\x08', '\x1A', '\x1B', '\0', '\u{200b}']) {
let col = if row_offset == 0 {
start.column() + col_offset
} else {
col_offset
};
let (replacement, rule): (&str, DiagnosticKind) = match m.chars().next().unwrap() {
'\x08' => ("\\b", InvalidCharacterBackspace.into()),
'\x1A' => ("\\x1A", InvalidCharacterSub.into()),
'\x1B' => ("\\x1B", InvalidCharacterEsc.into()),
'\0' => ("\\0", InvalidCharacterNul.into()),
'\u{200b}' => ("\\u200b", InvalidCharacterZeroWidthSpace.into()),
_ => unreachable!(),
};
let location = Location::new(start.row() + row_offset, col);
let end_location = Location::new(location.row(), location.column() + 1);
let mut diagnostic = Diagnostic::new(rule, Range::new(location, end_location));
if autofix {
diagnostic.amend(Fix::replacement(
replacement.to_string(),
location,
end_location,
));
}
diagnostics.push(diagnostic);
}
}

diagnostics
}
5 changes: 5 additions & 0 deletions crates/ruff/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ pub use invalid_all_format::{invalid_all_format, InvalidAllFormat};
pub use invalid_all_object::{invalid_all_object, InvalidAllObject};
pub use invalid_envvar_default::{invalid_envvar_default, InvalidEnvvarDefault};
pub use invalid_envvar_value::{invalid_envvar_value, InvalidEnvvarValue};
pub use invalid_string_characters::{
invalid_string_characters, InvalidCharacterBackspace, InvalidCharacterEsc, InvalidCharacterNul,
InvalidCharacterSub, InvalidCharacterZeroWidthSpace,
};
pub use logging::{logging_call, LoggingTooFewArgs, LoggingTooManyArgs};
pub use magic_value_comparison::{magic_value_comparison, MagicValueComparison};
pub use merge_isinstance::{merge_isinstance, ConsiderMergingIsinstance};
Expand Down Expand Up @@ -48,6 +52,7 @@ mod invalid_all_format;
mod invalid_all_object;
mod invalid_envvar_default;
mod invalid_envvar_value;
mod invalid_string_characters;
mod logging;
mod magic_value_comparison;
mod merge_isinstance;
Expand Down