Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,11 @@
fd = os.open(".", os.O_RDONLY)
os.symlink("source.txt", "link.txt", dir_fd=fd) # Ok: dir_fd is not supported by pathlib
os.close(fd)

os.symlink(src="usr/bin/python", dst="tmp/python", unknown=True)
os.symlink("usr/bin/python", dst="tmp/python", target_is_directory=False)

os.symlink(src="usr/bin/python", dst="tmp/python", dir_fd=None)

os.symlink("usr/bin/python", dst="tmp/python", target_is_directory= True )
os.symlink("usr/bin/python", dst="tmp/python", target_is_directory="nonboolean")
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 @@ -1124,6 +1124,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
if checker.is_rule_enabled(Rule::OsMakedirs) {
flake8_use_pathlib::rules::os_makedirs(checker, call, segments);
}
if checker.is_rule_enabled(Rule::OsSymlink) {
flake8_use_pathlib::rules::os_symlink(checker, call, segments);
}
if checker.is_rule_enabled(Rule::PathConstructorCurrentDirectory) {
flake8_use_pathlib::rules::path_constructor_current_directory(
checker, call, segments,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -954,7 +954,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8UsePathlib, "207") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::Glob),
(Flake8UsePathlib, "208") => (RuleGroup::Stable, rules::flake8_use_pathlib::violations::OsListdir),
(Flake8UsePathlib, "210") => (RuleGroup::Stable, rules::flake8_use_pathlib::rules::InvalidPathlibWithSuffix),
(Flake8UsePathlib, "211") => (RuleGroup::Preview, rules::flake8_use_pathlib::violations::OsSymlink),
(Flake8UsePathlib, "211") => (RuleGroup::Preview, rules::flake8_use_pathlib::rules::OsSymlink),

// flake8-logging-format
(Flake8LoggingFormat, "001") => (RuleGroup::Stable, rules::flake8_logging_format::violations::LoggingStringFormat),
Expand Down
5 changes: 5 additions & 0 deletions crates/ruff_linter/src/preview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,11 @@ pub(crate) const fn is_fix_os_makedirs_enabled(settings: &LinterSettings) -> boo
settings.preview.is_enabled()
}

// https://github.com/astral-sh/ruff/pull/20009
pub(crate) const fn is_fix_os_symlink_enabled(settings: &LinterSettings) -> bool {
settings.preview.is_enabled()
}

// https://github.com/astral-sh/ruff/pull/11436
// https://github.com/astral-sh/ruff/pull/11168
pub(crate) const fn is_dunder_init_fix_unused_import_enabled(settings: &LinterSettings) -> bool {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/flake8_use_pathlib/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ mod tests {
#[test_case(Rule::OsPathGetatime, Path::new("PTH203.py"))]
#[test_case(Rule::OsPathGetmtime, Path::new("PTH204.py"))]
#[test_case(Rule::OsPathGetctime, Path::new("PTH205.py"))]
#[test_case(Rule::OsSymlink, Path::new("PTH211.py"))]
fn preview_flake8_use_pathlib(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/flake8_use_pathlib/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub(crate) use os_rename::*;
pub(crate) use os_replace::*;
pub(crate) use os_rmdir::*;
pub(crate) use os_sep_split::*;
pub(crate) use os_symlink::*;
pub(crate) use os_unlink::*;
pub(crate) use path_constructor_current_directory::*;
pub(crate) use replaceable_by_pathlib::*;
Expand Down Expand Up @@ -54,6 +55,7 @@ mod os_rename;
mod os_replace;
mod os_rmdir;
mod os_sep_split;
mod os_symlink;
mod os_unlink;
mod path_constructor_current_directory;
mod replaceable_by_pathlib;
148 changes: 148 additions & 0 deletions crates/ruff_linter/src/rules/flake8_use_pathlib/rules/os_symlink.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
use anyhow::anyhow;
use ruff_diagnostics::{Applicability, Edit, Fix};
use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_python_ast::ExprCall;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::importer::ImportRequest;
use crate::preview::is_fix_os_symlink_enabled;
use crate::rules::flake8_use_pathlib::helpers::{
has_unknown_keywords_or_starred_expr, is_keyword_only_argument_non_default,
is_pathlib_path_call,
};
use crate::{FixAvailability, Violation};

/// ## What it does
/// Checks for uses of `os.symlink`.
///
/// ## Why is this bad?
/// `pathlib` offers a high-level API for path manipulation, as compared to
/// the lower-level API offered by `os.symlink`.
///
/// ## Example
/// ```python
/// import os
///
/// os.symlink("usr/bin/python", "tmp/python", target_is_directory=False)
/// ```
///
/// Use instead:
/// ```python
/// from pathlib import Path
///
/// Path("tmp/python").symlink_to("usr/bin/python")
/// ```
///
/// ## Known issues
/// While using `pathlib` can improve the readability and type safety of your code,
/// it can be less performant than the lower-level alternatives that work directly with strings,
/// especially on older versions of Python.
///
/// ## Fix Safety
/// This rule's fix is marked as unsafe if the replacement would remove comments attached to the original expression.
///
/// ## References
/// - [Python documentation: `Path.symlink_to`](https://docs.python.org/3/library/pathlib.html#pathlib.Path.symlink_to)
/// - [PEP 428 – The pathlib module – object-oriented filesystem paths](https://peps.python.org/pep-0428/)
/// - [Correspondence between `os` and `pathlib`](https://docs.python.org/3/library/pathlib.html#correspondence-to-tools-in-the-os-module)
/// - [Why you should be using pathlib](https://treyhunner.com/2018/12/why-you-should-be-using-pathlib/)
/// - [No really, pathlib is great](https://treyhunner.com/2019/01/no-really-pathlib-is-great/)
#[derive(ViolationMetadata)]
pub(crate) struct OsSymlink;

impl Violation for OsSymlink {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
"`os.symlink` should be replaced by `Path.symlink_to`".to_string()
}

fn fix_title(&self) -> Option<String> {
Some("Replace with `Path(...).symlink_to(...)`".to_string())
}
}

/// PTH211
pub(crate) fn os_symlink(checker: &Checker, call: &ExprCall, segments: &[&str]) {
if segments != ["os", "symlink"] {
return;
}

// `dir_fd` is not supported by pathlib, so check if there are non-default values.
// Signature as of Python 3.13 (https://docs.python.org/3/library/os.html#os.symlink)
// ```text
// 0 1 2 3
// os.symlink(src, dst, target_is_directory=False, *, dir_fd=None)
// ```
if is_keyword_only_argument_non_default(&call.arguments, "dir_fd") {
return;
}

let range = call.range();
let mut diagnostic = checker.report_diagnostic(OsSymlink, call.func.range());

if !is_fix_os_symlink_enabled(checker.settings()) {
return;
}

if call.arguments.len() > 3 {
return;
}

if has_unknown_keywords_or_starred_expr(
&call.arguments,
&["src", "dst", "target_is_directory", "dir_fd"],
) {
return;
}

let (Some(src), Some(dst)) = (
call.arguments.find_argument_value("src", 0),
call.arguments.find_argument_value("dst", 1),
) else {
return;
};

diagnostic.try_set_fix(|| {
let (import_edit, binding) = checker.importer().get_or_import_symbol(
&ImportRequest::import("pathlib", "Path"),
call.start(),
checker.semantic(),
)?;

let applicability = if checker.comment_ranges().intersects(range) {
Applicability::Unsafe
} else {
Applicability::Safe
};

let locator = checker.locator();
let src_code = locator.slice(src.range());
let dst_code = locator.slice(dst.range());

let target_is_directory = call
.arguments
.find_argument_value("target_is_directory", 2)
.and_then(|expr| {
let code = locator.slice(expr.range());
expr.as_boolean_literal_expr()
.is_some_and(|bl| !bl.value)
.then_some(format!(", target_is_directory={code}"))
})
.ok_or_else(|| anyhow!("Non-boolean value passed for `target_is_directory`."))?;

let replacement = if is_pathlib_path_call(checker, dst) {
format!("{dst_code}.symlink_to({src_code}{target_is_directory})")
} else {
format!("{binding}({dst_code}).symlink_to({src_code}{target_is_directory})")
};

Ok(Fix::applicable_edits(
Edit::range_replacement(replacement, range),
[import_edit],
applicability,
))
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ use crate::rules::flake8_use_pathlib::helpers::{
};
use crate::rules::flake8_use_pathlib::{
rules::Glob,
violations::{
BuiltinOpen, Joiner, OsListdir, OsPathJoin, OsPathSplitext, OsStat, OsSymlink, PyPath,
},
violations::{BuiltinOpen, Joiner, OsListdir, OsPathJoin, OsPathSplitext, OsStat, PyPath},
};

pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {
Expand Down Expand Up @@ -62,20 +60,6 @@ pub(crate) fn replaceable_by_pathlib(checker: &Checker, call: &ExprCall) {
),
// PTH122
["os", "path", "splitext"] => checker.report_diagnostic_if_enabled(OsPathSplitext, range),
// PTH211
["os", "symlink"] => {
// `dir_fd` is not supported by pathlib, so check if there are non-default values.
// Signature as of Python 3.13 (https://docs.python.org/3/library/os.html#os.symlink)
// ```text
// 0 1 2 3
// os.symlink(src, dst, target_is_directory=False, *, dir_fd=None)
// ```
if is_keyword_only_argument_non_default(&call.arguments, "dir_fd") {
return;
}
checker.report_diagnostic_if_enabled(OsSymlink, range)
}

// PTH123
["" | "builtins", "open"] => {
// `closefd` and `opener` are not supported by pathlib, so check if they
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ PTH211 `os.symlink` should be replaced by `Path.symlink_to`
6 | os.symlink(b"usr/bin/python", b"tmp/python")
7 | Path("tmp/python").symlink_to("usr/bin/python") # Ok
|
help: Replace with `Path(...).symlink_to(...)`

PTH211 `os.symlink` should be replaced by `Path.symlink_to`
--> PTH211.py:6:1
Expand All @@ -18,6 +19,7 @@ PTH211 `os.symlink` should be replaced by `Path.symlink_to`
| ^^^^^^^^^^
7 | Path("tmp/python").symlink_to("usr/bin/python") # Ok
|
help: Replace with `Path(...).symlink_to(...)`

PTH211 `os.symlink` should be replaced by `Path.symlink_to`
--> PTH211.py:9:1
Expand All @@ -29,6 +31,7 @@ PTH211 `os.symlink` should be replaced by `Path.symlink_to`
10 | os.symlink(b"usr/bin/python", b"tmp/python", target_is_directory=True)
11 | Path("tmp/python").symlink_to("usr/bin/python", target_is_directory=True) # Ok
|
help: Replace with `Path(...).symlink_to(...)`

PTH211 `os.symlink` should be replaced by `Path.symlink_to`
--> PTH211.py:10:1
Expand All @@ -38,3 +41,58 @@ PTH211 `os.symlink` should be replaced by `Path.symlink_to`
| ^^^^^^^^^^
11 | Path("tmp/python").symlink_to("usr/bin/python", target_is_directory=True) # Ok
|
help: Replace with `Path(...).symlink_to(...)`

PTH211 `os.symlink` should be replaced by `Path.symlink_to`
--> PTH211.py:17:1
|
15 | os.close(fd)
16 |
17 | os.symlink(src="usr/bin/python", dst="tmp/python", unknown=True)
| ^^^^^^^^^^
18 | os.symlink("usr/bin/python", dst="tmp/python", target_is_directory=False)
|
help: Replace with `Path(...).symlink_to(...)`

PTH211 `os.symlink` should be replaced by `Path.symlink_to`
--> PTH211.py:18:1
|
17 | os.symlink(src="usr/bin/python", dst="tmp/python", unknown=True)
18 | os.symlink("usr/bin/python", dst="tmp/python", target_is_directory=False)
| ^^^^^^^^^^
19 |
20 | os.symlink(src="usr/bin/python", dst="tmp/python", dir_fd=None)
|
help: Replace with `Path(...).symlink_to(...)`

PTH211 `os.symlink` should be replaced by `Path.symlink_to`
--> PTH211.py:20:1
|
18 | os.symlink("usr/bin/python", dst="tmp/python", target_is_directory=False)
19 |
20 | os.symlink(src="usr/bin/python", dst="tmp/python", dir_fd=None)
| ^^^^^^^^^^
21 |
22 | os.symlink("usr/bin/python", dst="tmp/python", target_is_directory= True )
|
help: Replace with `Path(...).symlink_to(...)`

PTH211 `os.symlink` should be replaced by `Path.symlink_to`
--> PTH211.py:22:1
|
20 | os.symlink(src="usr/bin/python", dst="tmp/python", dir_fd=None)
21 |
22 | os.symlink("usr/bin/python", dst="tmp/python", target_is_directory= True )
| ^^^^^^^^^^
23 | os.symlink("usr/bin/python", dst="tmp/python", target_is_directory="nonboolean")
|
help: Replace with `Path(...).symlink_to(...)`

PTH211 `os.symlink` should be replaced by `Path.symlink_to`
--> PTH211.py:23:1
|
22 | os.symlink("usr/bin/python", dst="tmp/python", target_is_directory= True )
23 | os.symlink("usr/bin/python", dst="tmp/python", target_is_directory="nonboolean")
| ^^^^^^^^^^
|
help: Replace with `Path(...).symlink_to(...)`
Loading
Loading