[pylint] Implement bad-open-mode (W1501)

crates/ruff_linter/resources/test/fixtures/pylint/bad_open_mode.py

@@ -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]

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -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);
             }

crates/ruff_linter/src/codes.rs

@@ -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),

crates/ruff_linter/src/rules/pylint/mod.rs

@@ -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")

crates/ruff_linter/src/rules/pylint/rules/bad_open_mode.rs

@@ -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(),
+    ));
+}

crates/ruff_linter/src/rules/pylint/rules/mod.rs

@@ -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::*;
@@ -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;

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1501_bad_open_mode.py.snap.new

@@ -0,0 +1,112 @@
+---
+source: crates/ruff_linter/src/rules/pylint/mod.rs
+assertion_line: 152
+---
+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
+   |
+
+