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

[flake8-use-pathlib] Dotless suffix passed to Path.with_suffix() (PTH210) #14779

Merged
merged 9 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 99 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF049.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
from pathlib import (
Path,
PosixPath,
PurePath,
PurePosixPath,
PureWindowsPath,
WindowsPath,
)
import pathlib


path = Path()
posix_path: pathlib.PosixPath = PosixPath()
pure_path: PurePath = PurePath()
pure_posix_path = pathlib.PurePosixPath()
pure_windows_path: PureWindowsPath = pathlib.PureWindowsPath()
windows_path: pathlib.WindowsPath = pathlib.WindowsPath()


### Errors
path.with_suffix("py")
path.with_suffix(r"s")
path.with_suffix(u'' "json")
path.with_suffix(suffix="js")

posix_path.with_suffix("py")
posix_path.with_suffix(r"s")
posix_path.with_suffix(u'' "json")
posix_path.with_suffix(suffix="js")

pure_path.with_suffix("py")
pure_path.with_suffix(r"s")
pure_path.with_suffix(u'' "json")
pure_path.with_suffix(suffix="js")

pure_posix_path.with_suffix("py")
pure_posix_path.with_suffix(r"s")
pure_posix_path.with_suffix(u'' "json")
pure_posix_path.with_suffix(suffix="js")

pure_windows_path.with_suffix("py")
pure_windows_path.with_suffix(r"s")
pure_windows_path.with_suffix(u'' "json")
pure_windows_path.with_suffix(suffix="js")

windows_path.with_suffix("py")
windows_path.with_suffix(r"s")
windows_path.with_suffix(u'' "json")
windows_path.with_suffix(suffix="js")


### No errors
path.with_suffix()
path.with_suffix('')
path.with_suffix(".py")
path.with_suffix("foo", "bar")
path.with_suffix(suffix)
path.with_suffix(f"oo")
path.with_suffix(b"ar")

posix_path.with_suffix()
posix_path.with_suffix('')
posix_path.with_suffix(".py")
posix_path.with_suffix("foo", "bar")
posix_path.with_suffix(suffix)
posix_path.with_suffix(f"oo")
posix_path.with_suffix(b"ar")

pure_path.with_suffix()
pure_path.with_suffix('')
pure_path.with_suffix(".py")
pure_path.with_suffix("foo", "bar")
pure_path.with_suffix(suffix)
pure_path.with_suffix(f"oo")
pure_path.with_suffix(b"ar")

pure_posix_path.with_suffix()
pure_posix_path.with_suffix('')
pure_posix_path.with_suffix(".py")
pure_posix_path.with_suffix("foo", "bar")
pure_posix_path.with_suffix(suffix)
pure_posix_path.with_suffix(f"oo")
pure_posix_path.with_suffix(b"ar")

pure_windows_path.with_suffix()
pure_windows_path.with_suffix('')
pure_windows_path.with_suffix(".py")
pure_windows_path.with_suffix("foo", "bar")
pure_windows_path.with_suffix(suffix)
pure_windows_path.with_suffix(f"oo")
pure_windows_path.with_suffix(b"ar")

windows_path.with_suffix()
windows_path.with_suffix('')
windows_path.with_suffix(".py")
windows_path.with_suffix("foo", "bar")
windows_path.with_suffix(suffix)
windows_path.with_suffix(f"oo")
windows_path.with_suffix(b"ar")
110 changes: 110 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF049_1.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
from pathlib import (
Path,
PosixPath,
PurePath,
PurePosixPath,
PureWindowsPath,
WindowsPath,
)


def test_path(p: Path) -> None:
## Errors
p.with_suffix("py")
p.with_suffix(r"s")
p.with_suffix(u'' "json")
p.with_suffix(suffix="js")

## No errors
p.with_suffix()
p.with_suffix('')
p.with_suffix(".py")
p.with_suffix("foo", "bar")
p.with_suffix(suffix)
p.with_suffix(f"oo")
p.with_suffix(b"ar")


def test_posix_path(p: PosixPath) -> None:
## Errors
p.with_suffix("py")
p.with_suffix(r"s")
p.with_suffix(u'' "json")
p.with_suffix(suffix="js")

## No errors
p.with_suffix()
p.with_suffix('')
p.with_suffix(".py")
p.with_suffix("foo", "bar")
p.with_suffix(suffix)
p.with_suffix(f"oo")
p.with_suffix(b"ar")


def test_pure_path(p: PurePath) -> None:
## Errors
p.with_suffix("py")
p.with_suffix(r"s")
p.with_suffix(u'' "json")
p.with_suffix(suffix="js")

## No errors
p.with_suffix()
p.with_suffix('')
p.with_suffix(".py")
p.with_suffix("foo", "bar")
p.with_suffix(suffix)
p.with_suffix(f"oo")
p.with_suffix(b"ar")


def test_pure_posix_path(p: PurePosixPath) -> None:
## Errors
p.with_suffix("py")
p.with_suffix(r"s")
p.with_suffix(u'' "json")
p.with_suffix(suffix="js")

## No errors
p.with_suffix()
p.with_suffix('')
p.with_suffix(".py")
p.with_suffix("foo", "bar")
p.with_suffix(suffix)
p.with_suffix(f"oo")
p.with_suffix(b"ar")


def test_pure_windows_path(p: PureWindowsPath) -> None:
## Errors
p.with_suffix("py")
p.with_suffix(r"s")
p.with_suffix(u'' "json")
p.with_suffix(suffix="js")

## No errors
p.with_suffix()
p.with_suffix('')
p.with_suffix(".py")
p.with_suffix("foo", "bar")
p.with_suffix(suffix)
p.with_suffix(f"oo")
p.with_suffix(b"ar")


def test_windows_path(p: WindowsPath) -> None:
## Errors
p.with_suffix("py")
p.with_suffix(r"s")
p.with_suffix(u'' "json")
p.with_suffix(suffix="js")

## No errors
p.with_suffix()
p.with_suffix('')
p.with_suffix(".py")
p.with_suffix("foo", "bar")
p.with_suffix(suffix)
p.with_suffix(f"oo")
p.with_suffix(b"ar")
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1096,6 +1096,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::UnnecessaryCastToInt) {
ruff::rules::unnecessary_cast_to_int(checker, call);
}
if checker.enabled(Rule::DotlessWithSuffix) {
ruff::rules::dotless_with_suffix(checker, call);
}
}
Expr::Dict(dict) => {
if checker.any_enabled(&[
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 @@ -985,6 +985,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "041") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryNestedLiteral),
(Ruff, "046") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryCastToInt),
(Ruff, "048") => (RuleGroup::Preview, rules::ruff::rules::MapIntVersionParsing),
(Ruff, "049") => (RuleGroup::Preview, rules::ruff::rules::DotlessWithSuffix),
(Ruff, "052") => (RuleGroup::Preview, rules::ruff::rules::UsedDummyVariable),
(Ruff, "055") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryRegularExpression),
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,8 @@ mod tests {
#[test_case(Rule::UnnecessaryRegularExpression, Path::new("RUF055_0.py"))]
#[test_case(Rule::UnnecessaryRegularExpression, Path::new("RUF055_1.py"))]
#[test_case(Rule::UnnecessaryCastToInt, Path::new("RUF046.py"))]
#[test_case(Rule::DotlessWithSuffix, Path::new("RUF049.py"))]
#[test_case(Rule::DotlessWithSuffix, Path::new("RUF049_1.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
Expand Down
117 changes: 117 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/dotless_with_suffix.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast::{Arguments, Expr, ExprAttribute, ExprCall, ExprStringLiteral, StringFlags};
use ruff_python_semantic::analyze::typing;
use ruff_python_semantic::SemanticModel;

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

/// ## What it does
/// Checks for `Path.with_suffix()` calls where
/// the given suffix does not have a leading dot.
///
/// ## Why is this bad?
/// `Path.with_suffix()` will raise an error at runtime
/// if the given suffix is not prefixed with a dot.
///
/// ## Known problems
/// This rule is prone to false positives and negatives
/// due to type inference limitations.
///
/// ## Examples
///
/// ```python
/// path.with_suffix("py")
/// ```
///
/// Use instead:
///
/// ```python
/// path.with_suffix(".py")
/// ```
#[derive(ViolationMetadata)]
pub(crate) struct DotlessWithSuffix;

impl AlwaysFixableViolation for DotlessWithSuffix {
#[derive_message_formats]
fn message(&self) -> String {
"Dotless suffix passed to `.with_suffix()`".to_string()
}

fn fix_title(&self) -> String {
"Add a leading dot".to_string()
}
}

/// RUF049
pub(crate) fn dotless_with_suffix(checker: &mut Checker, call: &ExprCall) {
let (func, arguments) = (&call.func, &call.arguments);

if !is_path_with_suffix_call(checker.semantic(), func) {
return;
}

let Some(string) = single_string_literal_argument(arguments) else {
return;
};

if matches!(string.value.chars().next(), None | Some('.')) {
return;
}
InSyncWithFoo marked this conversation as resolved.
Show resolved Hide resolved

let diagnostic = Diagnostic::new(DotlessWithSuffix, call.range);
let Some(fix) = add_leading_dot_fix(string) else {
return;
};

checker.diagnostics.push(diagnostic.with_fix(fix));
InSyncWithFoo marked this conversation as resolved.
Show resolved Hide resolved
}

fn is_path_with_suffix_call(semantic: &SemanticModel, func: &Expr) -> bool {
let Expr::Attribute(ExprAttribute { value, attr, .. }) = func else {
return false;
};

if attr != "with_suffix" {
return false;
}

let Expr::Name(name) = value.as_ref() else {
return false;
};
let Some(binding) = semantic.only_binding(name).map(|id| semantic.binding(id)) else {
return false;
};

typing::is_pathlib_path(binding, semantic)
}

fn single_string_literal_argument(arguments: &Arguments) -> Option<&ExprStringLiteral> {
if arguments.len() > 1 {
return None;
}

match arguments.find_argument("suffix", 0)? {
Expr::StringLiteral(string) => Some(string),
_ => None,
}
InSyncWithFoo marked this conversation as resolved.
Show resolved Hide resolved
}

fn add_leading_dot_fix(string: &ExprStringLiteral) -> Option<Fix> {
let first_part = string.value.iter().next()?;

// |r"foo"
let before_prefix = first_part.range.start();

// r|"foo"
let prefix_length = first_part.flags.prefix().as_str().len();
let after_prefix = before_prefix.checked_add(u32::try_from(prefix_length).ok()?.into())?;

// r"|foo"
let quote_length = first_part.flags.quote_str().len();
let after_leading_quote = after_prefix.checked_add(u32::try_from(quote_length).ok()?.into())?;

let edit = Edit::insertion(".".to_string(), after_leading_quote);
InSyncWithFoo marked this conversation as resolved.
Show resolved Hide resolved

Some(Fix::safe_edit(edit))
InSyncWithFoo marked this conversation as resolved.
Show resolved Hide resolved
}
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 @@ -5,6 +5,7 @@ pub(crate) use asyncio_dangling_task::*;
pub(crate) use collection_literal_concatenation::*;
pub(crate) use decimal_from_float_literal::*;
pub(crate) use default_factory_kwarg::*;
pub(crate) use dotless_with_suffix::*;
pub(crate) use explicit_f_string_type_conversion::*;
pub(crate) use function_call_in_dataclass_default::*;
pub(crate) use implicit_optional::*;
Expand Down Expand Up @@ -51,6 +52,7 @@ mod collection_literal_concatenation;
mod confusables;
mod decimal_from_float_literal;
mod default_factory_kwarg;
mod dotless_with_suffix;
mod explicit_f_string_type_conversion;
mod function_call_in_dataclass_default;
mod helpers;
Expand Down
Loading
Loading