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] Implement bad-open-mode (W1501) #8294

Merged
merged 1 commit into from Oct 28, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 34 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pylint/bad_open_mode.py
@@ -0,0 +1,34 @@
import pathlib

NAME = "foo.bar"
open(NAME, "wb")
open(NAME, "w", encoding="utf-8")
open(NAME, "rb")
open(NAME, "x", encoding="utf-8")
open(NAME, "br")
open(NAME, "+r", encoding="utf-8")
open(NAME, "xb")
open(NAME, "rwx") # [bad-open-mode]
open(NAME, mode="rwx") # [bad-open-mode]
open(NAME, "rwx", encoding="utf-8") # [bad-open-mode]
open(NAME, "rr", encoding="utf-8") # [bad-open-mode]
open(NAME, "+", encoding="utf-8") # [bad-open-mode]
open(NAME, "xw", encoding="utf-8") # [bad-open-mode]
open(NAME, "ab+")
open(NAME, "a+b")
open(NAME, "+ab")
open(NAME, "+rUb")
open(NAME, "x+b")
open(NAME, "Ua", encoding="utf-8") # [bad-open-mode]
open(NAME, "Ur++", encoding="utf-8") # [bad-open-mode]
open(NAME, "Ut", encoding="utf-8")
open(NAME, "Ubr")

mode = "rw"
open(NAME, mode)

pathlib.Path(NAME).open("wb")
pathlib.Path(NAME).open(mode)
pathlib.Path(NAME).open("rwx") # [bad-open-mode]
pathlib.Path(NAME).open(mode="rwx") # [bad-open-mode]
pathlib.Path(NAME).open("rwx", encoding="utf-8") # [bad-open-mode]
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Expand Up @@ -749,6 +749,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::SysExitAlias) {
pylint::rules::sys_exit_alias(checker, func);
}
if checker.enabled(Rule::BadOpenMode) {
pylint::rules::bad_open_mode(checker, call);
}
if checker.enabled(Rule::BadStrStripCall) {
pylint::rules::bad_str_strip_call(checker, func, args);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Expand Up @@ -270,6 +270,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "W0604") => (RuleGroup::Preview, rules::pylint::rules::GlobalAtModuleLevel),
(Pylint, "W0603") => (RuleGroup::Stable, rules::pylint::rules::GlobalStatement),
(Pylint, "W0711") => (RuleGroup::Stable, rules::pylint::rules::BinaryOpException),
(Pylint, "W1501") => (RuleGroup::Preview, rules::pylint::rules::BadOpenMode),
(Pylint, "W1508") => (RuleGroup::Stable, rules::pylint::rules::InvalidEnvvarDefault),
(Pylint, "W1509") => (RuleGroup::Stable, rules::pylint::rules::SubprocessPopenPreexecFn),
(Pylint, "W1510") => (RuleGroup::Stable, rules::pylint::rules::SubprocessRunWithoutCheck),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Expand Up @@ -21,6 +21,7 @@ mod tests {
#[test_case(Rule::AndOrTernary, Path::new("and_or_ternary.py"))]
#[test_case(Rule::AssertOnStringLiteral, Path::new("assert_on_string_literal.py"))]
#[test_case(Rule::AwaitOutsideAsync, Path::new("await_outside_async.py"))]
#[test_case(Rule::BadOpenMode, Path::new("bad_open_mode.py"))]
#[test_case(
Rule::BadStringFormatCharacter,
Path::new("bad_string_format_character.py")
Expand Down
197 changes: 197 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs
@@ -0,0 +1,197 @@
use bitflags::bitflags;

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Constant, Expr};
use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged;

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

/// ## What it does
/// Check for an invalid `mode` argument in `open` calls.
///
/// ## Why is this bad?
/// The `open` function accepts a `mode` argument that specifies how the file
/// should be opened (e.g., read-only, write-only, append-only, etc.).
///
/// Python supports a variety of open modes: `r`, `w`, `a`, and `x`, to control
/// reading, writing, appending, and creating, respectively, along with
/// `b` (binary mode), `+` (read and write), and `U` (universal newlines),
/// the latter of which is only valid alongside `r`. This rule detects both
/// invalid combinations of modes and invalid characters in the mode string
/// itself.
///
/// ## Example
/// ```python
/// with open("file", "rwx") as f:
/// return f.read()
/// ```
///
/// Use instead:
/// ```python
/// with open("file", "r") as f:
/// return f.read()
/// ```
///
/// ## References
/// - [Python documentation: `open`](https://docs.python.org/3/library/functions.html#open)
#[violation]
pub struct BadOpenMode {
mode: String,
}

impl Violation for BadOpenMode {
#[derive_message_formats]
fn message(&self) -> String {
let BadOpenMode { mode } = self;
format!("`{mode}` is not a valid mode for `open`")
}
}

/// PLW1501
pub(crate) fn bad_open_mode(checker: &mut Checker, call: &ast::ExprCall) {
let Some(kind) = is_open(call.func.as_ref(), checker.semantic()) else {
return;
};

let Some(mode) = extract_mode(call, kind) else {
return;
};

let Expr::Constant(ast::ExprConstant {
value: Constant::Str(ast::StringConstant { value, .. }),
..
}) = mode
else {
return;
};

if is_valid_mode(value) {
return;
}

checker.diagnostics.push(Diagnostic::new(
BadOpenMode {
mode: value.to_string(),
},
mode.range(),
));
}

#[derive(Debug, Copy, Clone)]
enum Kind {
/// A call to the builtin `open(...)`.
Builtin,
/// A call to `pathlib.Path(...).open(...)`.
Pathlib,
}

/// If a function is a call to `open`, returns the kind of `open` call.
fn is_open(func: &Expr, semantic: &SemanticModel) -> Option<Kind> {
match func {
// Ex) `pathlib.Path(...).open(...)`
Expr::Attribute(ast::ExprAttribute { attr, value, .. }) if attr.as_str() == "open" => {
match value.as_ref() {
Expr::Call(ast::ExprCall { func, .. }) => semantic
.resolve_call_path(func)
.is_some_and(|call_path| matches!(call_path.as_slice(), ["pathlib", "Path"]))
.then_some(Kind::Pathlib),
_ => None,
}
}
// Ex) `open(...)`
Expr::Name(ast::ExprName { id, .. }) => {
(id.as_str() == "open" && semantic.is_builtin("open")).then_some(Kind::Builtin)
}
_ => None,
}
}

/// Returns the mode argument, if present.
fn extract_mode(call: &ast::ExprCall, kind: Kind) -> Option<&Expr> {
match kind {
Kind::Builtin => call.arguments.find_argument("mode", 1),
Kind::Pathlib => call.arguments.find_argument("mode", 0),
}
}

bitflags! {
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub(super) struct OpenMode: u8 {
/// `r`
const READ = 0b0001;
/// `w`
const WRITE = 0b0010;
/// `a`
const APPEND = 0b0100;
/// `x`
const CREATE = 0b1000;
/// `b`
const BINARY = 0b10000;
/// `t`
const TEXT = 0b10_0000;
/// `+`
const PLUS = 0b100_0000;
/// `U`
const UNIVERSAL_NEWLINES = 0b1000_0000;

}
}

impl TryFrom<char> for OpenMode {
type Error = ();

fn try_from(value: char) -> Result<Self, Self::Error> {
match value {
'r' => Ok(Self::READ),
'w' => Ok(Self::WRITE),
'a' => Ok(Self::APPEND),
'x' => Ok(Self::CREATE),
'b' => Ok(Self::BINARY),
't' => Ok(Self::TEXT),
'+' => Ok(Self::PLUS),
'U' => Ok(Self::UNIVERSAL_NEWLINES),
_ => Err(()),
}
}
}

/// Returns `true` if the open mode is valid.
fn is_valid_mode(mode: &str) -> bool {
// Flag duplicates and invalid characters.
let mut flags = OpenMode::empty();
for char in mode.chars() {
let Ok(flag) = OpenMode::try_from(char) else {
return false;
};
if flags.intersects(flag) {
return false;
}
flags.insert(flag);
}

// Both text and binary mode cannot be set at the same time.
if flags.contains(OpenMode::TEXT | OpenMode::BINARY) {
return false;
}

// The `U` mode is only valid with `r`.
if flags.contains(OpenMode::UNIVERSAL_NEWLINES)
&& flags.intersects(OpenMode::WRITE | OpenMode::APPEND | OpenMode::CREATE)
{
return false;
}

// Otherwise, reading, writing, creating, and appending are mutually exclusive.
[
OpenMode::READ | OpenMode::UNIVERSAL_NEWLINES,
OpenMode::WRITE,
OpenMode::CREATE,
OpenMode::APPEND,
]
.into_iter()
.filter(|flag| flags.intersects(*flag))
.count()
== 1
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Expand Up @@ -2,6 +2,7 @@ pub(crate) use and_or_ternary::*;
pub(crate) use assert_on_string_literal::*;
pub(crate) use await_outside_async::*;
pub(crate) use bad_dunder_method_name::*;
pub(crate) use bad_open_mode::*;
pub(crate) use bad_str_strip_call::*;
pub(crate) use bad_string_format_character::BadStringFormatCharacter;
pub(crate) use bad_string_format_type::*;
Expand Down Expand Up @@ -70,6 +71,7 @@ mod and_or_ternary;
mod assert_on_string_literal;
mod await_outside_async;
mod bad_dunder_method_name;
mod bad_open_mode;
mod bad_str_strip_call;
pub(crate) mod bad_string_format_character;
mod bad_string_format_type;
Expand Down
@@ -0,0 +1,111 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
bad_open_mode.py:11:12: PLW1501 `rwx` is not a valid mode for `open`
|
9 | open(NAME, "+r", encoding="utf-8")
10 | open(NAME, "xb")
11 | open(NAME, "rwx") # [bad-open-mode]
| ^^^^^ PLW1501
12 | open(NAME, mode="rwx") # [bad-open-mode]
13 | open(NAME, "rwx", encoding="utf-8") # [bad-open-mode]
|

bad_open_mode.py:12:17: PLW1501 `rwx` is not a valid mode for `open`
|
10 | open(NAME, "xb")
11 | open(NAME, "rwx") # [bad-open-mode]
12 | open(NAME, mode="rwx") # [bad-open-mode]
| ^^^^^ PLW1501
13 | open(NAME, "rwx", encoding="utf-8") # [bad-open-mode]
14 | open(NAME, "rr", encoding="utf-8") # [bad-open-mode]
|

bad_open_mode.py:13:12: PLW1501 `rwx` is not a valid mode for `open`
|
11 | open(NAME, "rwx") # [bad-open-mode]
12 | open(NAME, mode="rwx") # [bad-open-mode]
13 | open(NAME, "rwx", encoding="utf-8") # [bad-open-mode]
| ^^^^^ PLW1501
14 | open(NAME, "rr", encoding="utf-8") # [bad-open-mode]
15 | open(NAME, "+", encoding="utf-8") # [bad-open-mode]
|

bad_open_mode.py:14:12: PLW1501 `rr` is not a valid mode for `open`
|
12 | open(NAME, mode="rwx") # [bad-open-mode]
13 | open(NAME, "rwx", encoding="utf-8") # [bad-open-mode]
14 | open(NAME, "rr", encoding="utf-8") # [bad-open-mode]
| ^^^^ PLW1501
15 | open(NAME, "+", encoding="utf-8") # [bad-open-mode]
16 | open(NAME, "xw", encoding="utf-8") # [bad-open-mode]
|

bad_open_mode.py:15:12: PLW1501 `+` is not a valid mode for `open`
|
13 | open(NAME, "rwx", encoding="utf-8") # [bad-open-mode]
14 | open(NAME, "rr", encoding="utf-8") # [bad-open-mode]
15 | open(NAME, "+", encoding="utf-8") # [bad-open-mode]
| ^^^ PLW1501
16 | open(NAME, "xw", encoding="utf-8") # [bad-open-mode]
17 | open(NAME, "ab+")
|

bad_open_mode.py:16:12: PLW1501 `xw` is not a valid mode for `open`
|
14 | open(NAME, "rr", encoding="utf-8") # [bad-open-mode]
15 | open(NAME, "+", encoding="utf-8") # [bad-open-mode]
16 | open(NAME, "xw", encoding="utf-8") # [bad-open-mode]
| ^^^^ PLW1501
17 | open(NAME, "ab+")
18 | open(NAME, "a+b")
|

bad_open_mode.py:22:12: PLW1501 `Ua` is not a valid mode for `open`
|
20 | open(NAME, "+rUb")
21 | open(NAME, "x+b")
22 | open(NAME, "Ua", encoding="utf-8") # [bad-open-mode]
| ^^^^ PLW1501
23 | open(NAME, "Ur++", encoding="utf-8") # [bad-open-mode]
24 | open(NAME, "Ut", encoding="utf-8")
|

bad_open_mode.py:23:12: PLW1501 `Ur++` is not a valid mode for `open`
|
21 | open(NAME, "x+b")
22 | open(NAME, "Ua", encoding="utf-8") # [bad-open-mode]
23 | open(NAME, "Ur++", encoding="utf-8") # [bad-open-mode]
| ^^^^^^ PLW1501
24 | open(NAME, "Ut", encoding="utf-8")
25 | open(NAME, "Ubr")
|

bad_open_mode.py:32:25: PLW1501 `rwx` is not a valid mode for `open`
|
30 | pathlib.Path(NAME).open("wb")
31 | pathlib.Path(NAME).open(mode)
32 | pathlib.Path(NAME).open("rwx") # [bad-open-mode]
| ^^^^^ PLW1501
33 | pathlib.Path(NAME).open(mode="rwx") # [bad-open-mode]
34 | pathlib.Path(NAME).open("rwx", encoding="utf-8") # [bad-open-mode]
|

bad_open_mode.py:33:30: PLW1501 `rwx` is not a valid mode for `open`
|
31 | pathlib.Path(NAME).open(mode)
32 | pathlib.Path(NAME).open("rwx") # [bad-open-mode]
33 | pathlib.Path(NAME).open(mode="rwx") # [bad-open-mode]
| ^^^^^ PLW1501
34 | pathlib.Path(NAME).open("rwx", encoding="utf-8") # [bad-open-mode]
|

bad_open_mode.py:34:25: PLW1501 `rwx` is not a valid mode for `open`
|
32 | pathlib.Path(NAME).open("rwx") # [bad-open-mode]
33 | pathlib.Path(NAME).open(mode="rwx") # [bad-open-mode]
34 | pathlib.Path(NAME).open("rwx", encoding="utf-8") # [bad-open-mode]
| ^^^^^ PLW1501
|


1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.