Skip to content

Commit

Permalink
[ruff] Implement incorrectly-parenthesized-tuple-in-subscript (`RUF…
Browse files Browse the repository at this point in the history
…031`) (#12480)

Implements the new fixable lint rule `RUF031` which checks for the use or omission of parentheses around tuples in subscripts, depending on the setting `lint.ruff.parenthesize-tuple-in-getitem`. By default, the use of parentheses is considered a violation.
  • Loading branch information
dylwil3 committed Aug 7, 2024
1 parent d380b37 commit 7997da4
Show file tree
Hide file tree
Showing 15 changed files with 552 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ linter.pylint.max_statements = 50
linter.pylint.max_public_methods = 20
linter.pylint.max_locals = 15
linter.pyupgrade.keep_runtime_typing = false
linter.ruff.parenthesize_tuple_in_subscript = false

# Formatter Settings
formatter.exclude = []
Expand Down
28 changes: 28 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF031.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
d = {(1,2):"a",(3,4):"b",(5,6,7):"c",(8,):"d"}
d[(1,2)]
d[(
1,
2
)]
d[
1,
2
]
d[(2,4)]
d[(5,6,7)]
d[(8,)]
d[tuple(1,2)]
d[tuple(8)]
d[1,2]
d[3,4]
d[5,6,7]
e = {((1,2),(3,4)):"a"}
e[((1,2),(3,4))]
e[(1,2),(3,4)]

token_features[
(window_position, feature_name)
] = self._extract_raw_features_from_token

d[1,]
d[(1,)]
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
d = {(1,2):"a",(3,4):"b",(5,6,7):"c",(8,):"d"}
d[(1,2)]
d[(
1,
2
)]
d[
1,
2
]
d[(2,4)]
d[(5,6,7)]
d[(8,)]
d[tuple(1,2)]
d[tuple(8)]
d[1,2]
d[3,4]
d[5,6,7]
e = {((1,2),(3,4)):"a"}
e[((1,2),(3,4))]
e[(1,2),(3,4)]

token_features[
(window_position, feature_name)
] = self._extract_raw_features_from_token
d[1,]
d[(1,)]
4 changes: 4 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
refurb::rules::fstring_number_format(checker, subscript);
}

if checker.enabled(Rule::IncorrectlyParenthesizedTupleInSubscript) {
ruff::rules::subscript_with_parenthesized_tuple(checker, subscript);
}

pandas_vet::rules::subscript(checker, value, expr);
}
Expr::Tuple(ast::ExprTuple {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "028") => (RuleGroup::Preview, rules::ruff::rules::InvalidFormatterSuppressionComment),
(Ruff, "029") => (RuleGroup::Preview, rules::ruff::rules::UnusedAsync),
(Ruff, "030") => (RuleGroup::Preview, rules::ruff::rules::AssertWithPrintMessage),
(Ruff, "031") => (RuleGroup::Preview, rules::ruff::rules::IncorrectlyParenthesizedTupleInSubscript),
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
(Ruff, "101") => (RuleGroup::Preview, rules::ruff::rules::RedirectedNOQA),

Expand Down
18 changes: 18 additions & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Ruff-specific rules.

pub(crate) mod rules;
pub mod settings;
pub(crate) mod typing;

#[cfg(test)]
Expand All @@ -19,6 +20,7 @@ mod tests {
use crate::settings::types::{
CompiledPerFileIgnoreList, PerFileIgnore, PreviewMode, PythonVersion,
};
use crate::settings::LinterSettings;
use crate::test::{test_path, test_resource_path};
use crate::{assert_messages, settings};

Expand Down Expand Up @@ -55,6 +57,7 @@ mod tests {
#[test_case(Rule::InvalidFormatterSuppressionComment, Path::new("RUF028.py"))]
#[test_case(Rule::UnusedAsync, Path::new("RUF029.py"))]
#[test_case(Rule::AssertWithPrintMessage, Path::new("RUF030.py"))]
#[test_case(Rule::IncorrectlyParenthesizedTupleInSubscript, Path::new("RUF031.py"))]
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
Expand All @@ -66,6 +69,21 @@ mod tests {
Ok(())
}

#[test]
fn prefer_parentheses_getitem_tuple() -> Result<()> {
let diagnostics = test_path(
Path::new("ruff/RUF031_prefer_parens.py"),
&LinterSettings {
ruff: super::settings::Settings {
parenthesize_tuple_in_subscript: true,
},
..LinterSettings::for_rule(Rule::IncorrectlyParenthesizedTupleInSubscript)
},
)?;
assert_messages!(diagnostics);
Ok(())
}

#[test_case(Path::new("RUF013_0.py"))]
#[test_case(Path::new("RUF013_1.py"))]
fn implicit_optional_py39(path: &Path) -> Result<()> {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::ExprSubscript;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for consistent style regarding whether tuples in subscripts
/// are parenthesized.
///
/// The exact nature of this violation depends on the setting
/// [`lint.ruff.parenthesize-tuple-in-subscript`]. By default, the use of
/// parentheses is considered a violation.
///
/// ## Why is this bad?
/// It is good to be consistent and, depending on the codebase, one or the other
/// convention may be preferred.
///
/// ## Example
///
/// ```python
/// directions = {(0, 1): "North", (-1, 0): "East", (0, -1): "South", (1, 0): "West"}
/// directions[(0, 1)]
/// ```
///
/// Use instead (with default setting):
///
/// ```python
/// directions = {(0, 1): "North", (-1, 0): "East", (0, -1): "South", (1, 0): "West"}
/// directions[0, 1]
/// ```

#[violation]
pub struct IncorrectlyParenthesizedTupleInSubscript {
prefer_parentheses: bool,
}

impl AlwaysFixableViolation for IncorrectlyParenthesizedTupleInSubscript {
#[derive_message_formats]
fn message(&self) -> String {
if self.prefer_parentheses {
format!("Use parentheses for tuples in subscripts.")
} else {
format!("Avoid parentheses for tuples in subscripts.")
}
}

fn fix_title(&self) -> String {
if self.prefer_parentheses {
"Parenthesize the tuple.".to_string()
} else {
"Remove the parentheses.".to_string()
}
}
}

/// RUF031
pub(crate) fn subscript_with_parenthesized_tuple(checker: &mut Checker, subscript: &ExprSubscript) {
let prefer_parentheses = checker.settings.ruff.parenthesize_tuple_in_subscript;
let Some(tuple_subscript) = subscript.slice.as_tuple_expr() else {
return;
};
if tuple_subscript.parenthesized == prefer_parentheses {
return;
}
let locator = checker.locator();
let source_range = subscript.slice.range();
let new_source = if prefer_parentheses {
format!("({})", locator.slice(source_range))
} else {
locator.slice(source_range)[1..source_range.len().to_usize() - 1].to_string()
};
let edit = Edit::range_replacement(new_source, source_range);
checker.diagnostics.push(
Diagnostic::new(
IncorrectlyParenthesizedTupleInSubscript { prefer_parentheses },
source_range,
)
.with_fix(Fix::safe_edit(edit)),
);
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub(crate) use default_factory_kwarg::*;
pub(crate) use explicit_f_string_type_conversion::*;
pub(crate) use function_call_in_dataclass_default::*;
pub(crate) use implicit_optional::*;
pub(crate) use incorrectly_parenthesized_tuple_in_subscript::*;
pub(crate) use invalid_formatter_suppression_comment::*;
pub(crate) use invalid_index_type::*;
pub(crate) use invalid_pyproject_toml::*;
Expand Down Expand Up @@ -41,6 +42,7 @@ mod explicit_f_string_type_conversion;
mod function_call_in_dataclass_default;
mod helpers;
mod implicit_optional;
mod incorrectly_parenthesized_tuple_in_subscript;
mod invalid_formatter_suppression_comment;
mod invalid_index_type;
mod invalid_pyproject_toml;
Expand Down
23 changes: 23 additions & 0 deletions crates/ruff_linter/src/rules/ruff/settings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//! Settings for the `ruff` plugin.

use crate::display_settings;
use ruff_macros::CacheKey;
use std::fmt;

#[derive(Debug, Clone, CacheKey, Default)]
pub struct Settings {
pub parenthesize_tuple_in_subscript: bool,
}

impl fmt::Display for Settings {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
display_settings! {
formatter = f,
namespace = "linter.ruff",
fields = [
self.parenthesize_tuple_in_subscript
]
}
Ok(())
}
}
Loading

0 comments on commit 7997da4

Please sign in to comment.