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 @@ -91,9 +91,16 @@ def func(*args, **kwargs):
Path("foo.txt").write_text(text, *args)
Path("foo.txt").write_text(text, **kwargs)

# Violation but not detectable
# https://github.com/astral-sh/ruff/issues/19294
x = Path("foo.txt")
x.open()

# https://github.com/astral-sh/ruff/issues/18107
codecs.open("plw1514.py", "r", "utf-8").close() # this is fine

# function argument annotated as Path
from pathlib import Path

def format_file(file: Path):
with file.open() as f:
contents = f.read()
33 changes: 24 additions & 9 deletions crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_python_ast::name::QualifiedName;
use ruff_python_ast::{self as ast, Expr};
use ruff_python_semantic::SemanticModel;
use ruff_python_semantic::analyze::typing;
use ruff_text_size::{Ranged, TextRange};

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -111,20 +112,34 @@ enum Callee<'a> {
}

impl<'a> Callee<'a> {
fn is_pathlib_path_call(expr: &Expr, semantic: &SemanticModel) -> bool {
if let Expr::Call(ast::ExprCall { func, .. }) = expr {
semantic
.resolve_qualified_name(func)
.is_some_and(|qualified_name| {
matches!(qualified_name.segments(), ["pathlib", "Path"])
})
} else {
false
}
}

fn try_from_call_expression(
call: &'a ast::ExprCall,
semantic: &'a SemanticModel,
) -> Option<Self> {
if let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = call.func.as_ref() {
// Check for `pathlib.Path(...).open(...)` or equivalent
if let Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() {
if semantic
.resolve_qualified_name(func)
.is_some_and(|qualified_name| {
matches!(qualified_name.segments(), ["pathlib", "Path"])
})
{
return Some(Callee::Pathlib(attr));
// Direct: Path(...).open()
if Self::is_pathlib_path_call(value, semantic) {
return Some(Callee::Pathlib(attr));
}
// Indirect: x.open() where x = Path(...)
else if let Expr::Name(name) = value.as_ref() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this would be a good use of our typing module:

/// Test whether the given binding can be considered a `pathlib.PurePath`
/// or an instance of a subclass thereof.
pub fn is_pathlib_path(binding: &Binding, semantic: &SemanticModel) -> bool {
check_type::<PathlibPathChecker>(binding, semantic)
}

It looks like we already have one for Path. I think this will also allow us to cover the annotated function argument case from the issue, whereas I don't think this implementation would.

def format_file(file: Path):
    with file.open() as f:
        contents = f.read()

It might be good to add this as a test case if it does work.

if let Some(binding_id) = semantic.only_binding(name) {
let binding = semantic.binding(binding_id);
if typing::is_pathlib_path(binding, semantic) {
return Some(Callee::Pathlib(attr));
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -435,3 +435,41 @@ unspecified_encoding.py:80:1: PLW1514 [*] `pathlib.Path(...).write_text` without
81 81 |
82 82 | # Non-errors.
83 83 | Path("foo.txt").open(encoding="utf-8")

unspecified_encoding.py:96:1: PLW1514 [*] `pathlib.Path(...).open` in text mode without explicit `encoding` argument
|
94 | # https://github.com/astral-sh/ruff/issues/19294
95 | x = Path("foo.txt")
96 | x.open()
| ^^^^^^ PLW1514
97 |
98 | # https://github.com/astral-sh/ruff/issues/18107
|
= help: Add explicit `encoding` argument

ℹ Unsafe fix
93 93 |
94 94 | # https://github.com/astral-sh/ruff/issues/19294
95 95 | x = Path("foo.txt")
96 |-x.open()
96 |+x.open(encoding="utf-8")
97 97 |
98 98 | # https://github.com/astral-sh/ruff/issues/18107
99 99 | codecs.open("plw1514.py", "r", "utf-8").close() # this is fine

unspecified_encoding.py:105:10: PLW1514 [*] `pathlib.Path(...).open` in text mode without explicit `encoding` argument
|
104 | def format_file(file: Path):
105 | with file.open() as f:
| ^^^^^^^^^ PLW1514
106 | contents = f.read()
|
= help: Add explicit `encoding` argument

ℹ Unsafe fix
102 102 | from pathlib import Path
103 103 |
104 104 | def format_file(file: Path):
105 |- with file.open() as f:
105 |+ with file.open(encoding="utf-8") as f:
106 106 | contents = f.read()
Loading