Skip to content

Commit

Permalink
[pylint] Implement bad-open-mode (W1501)
Browse files Browse the repository at this point in the history
  • Loading branch information
harupy committed Oct 28, 2023
1 parent f5e8507 commit 866b8e0
Show file tree
Hide file tree
Showing 8 changed files with 310 additions and 0 deletions.
35 changes: 35 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pylint/bad_open_mode.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Original code: https://github.com/pylint-dev/pylint/blob/3de196c91bcf67e92e5413dea09eb65c4d15c37e/tests/functional/b/bad_open_mode.py
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
Original file line number Diff line number Diff line change
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, func, args, keywords);
}
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
156 changes: 156 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
use ruff_python_ast::{self as ast, Constant, Expr, Keyword};
use rustc_hash::FxHashSet;

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_text_size::Ranged;

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

/// ## What it does
/// Check for an invalid `mode` value in `open` calls.
///
/// ## Why is this bad?
/// Python supports: `r`, `w`, `a`[, `x`] modes with `b`, `+`, and `U` (only with `r`) options.
///
/// ## Example
/// ```python
/// with open("file", "rwx") as f:
/// return f.read()
/// ```
///
/// Use instead:
/// ```python
/// with open("file", "r") as f:
/// return f.read()
/// ```
///
/// ## References
/// - 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`")
}
}
enum Kind {
Builtin,
Pathlib,
}

fn is_open(checker: &mut Checker, func: &Expr) -> Option<Kind> {
match func {
// pathlib.Path(...).open(...)
Expr::Attribute(ast::ExprAttribute { attr, value, .. }) if attr.as_str() == "open" => {
match value.as_ref() {
Expr::Call(ast::ExprCall { func, .. }) => checker
.semantic()
.resolve_call_path(func)
.is_some_and(|call_path| matches!(call_path.as_slice(), ["pathlib", "Path"]))
.then_some(Kind::Pathlib),
_ => None,
}
}
// open(...)
Expr::Name(ast::ExprName { id, .. }) => (id.as_str() == "open"
&& checker.semantic().is_builtin("open"))
.then_some(Kind::Builtin),
_ => None,
}
}

fn extract_mode_positional(args: &[Expr], kind: Kind) -> Option<&Expr> {
match (args, kind) {
([_, mode, ..], Kind::Builtin) | ([mode, ..], Kind::Pathlib) => Some(mode),
_ => None,
}
}

fn extract_mode_keyword(keywords: &[Keyword]) -> Option<&Expr> {
keywords.iter().find_map(|keyword| {
keyword
.arg
.as_ref()
.filter(|a| a.as_str() == "mode")
.map(|_| &keyword.value)
})
}

/// [pylint implementation](https://github.com/pylint-dev/pylint/blob/3de196c91bcf67e92e5413dea09eb65c4d15c37e/pylint/checkers/stdlib.py#L335C1-L362C16)
fn is_valid_mode(mode: &str) -> bool {
// check syntax
let mut seen = FxHashSet::default();
for c in mode.chars() {
if !seen.insert(c) {
return false;
}
if !matches!(c, 'r' | 'w' | 'a' | 't' | 'b' | '+' | 'U' | 'x') {
return false;
}
}

// check logic
let creating = mode.contains('x');
let mut reading = mode.contains('r');
let writing = mode.contains('w');
let appending = mode.contains('a');
let text = mode.contains('t');
let binary = mode.contains('b');
if mode.contains('U') {
if writing || appending || creating {
return false;
}
reading = true;
}
if text && binary {
return false;
}
[reading, writing, appending, creating]
.iter()
.filter(|&x| *x)
.count()
== 1
}

/// PLW1501
pub(crate) fn bad_open_mode(
checker: &mut Checker,
func: &Expr,
args: &[Expr],
keywords: &[Keyword],
) {
let Some(kind) = is_open(checker, func) else {
return;
};

let Some(mode) = extract_mode_positional(args, kind).or_else(|| extract_mode_keyword(keywords))
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(),
));
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
bad_open_mode.py:12:12: PLW1501 `rwx` is not a valid mode for `open`
|
10 | open(NAME, "+r", encoding="utf-8")
11 | open(NAME, "xb")
12 | open(NAME, "rwx") # [bad-open-mode]
| ^^^^^ PLW1501
13 | open(NAME, mode="rwx") # [bad-open-mode]
14 | open(NAME, "rwx", encoding="utf-8") # [bad-open-mode]
|

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

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

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

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

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

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

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

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

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

bad_open_mode.py:35:25: PLW1501 `rwx` is not a valid mode for `open`
|
33 | pathlib.Path(NAME).open("rwx") # [bad-open-mode]
34 | pathlib.Path(NAME).open(mode="rwx") # [bad-open-mode]
35 | 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.

0 comments on commit 866b8e0

Please sign in to comment.