[pylint] Add add_argument utility and autofix for PLW1514 (#8928)

## Summary

- Adds `add_argument` similar to existing `remove_argument` utility to
safely add arguments to functions.
- Adds autofix for `PLW1514` as per specs requested in
#8883 as a test

## Test Plan

Checks on existing fixtures as well as additional test and fixture for
Python 3.9 and lower fix

## Issue Link

Closes: #8883

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

@@ -42,3 +42,30 @@ def func(*args, **kwargs):
 tempfile.SpooledTemporaryFile(0, "w", -1, "utf-8")
 tempfile.SpooledTemporaryFile(0, "wb")
 tempfile.SpooledTemporaryFile(0, )
+
+open("test.txt",)
+open()
+open(
+    "test.txt",  # comment
+)
+open(
+    "test.txt",
+    # comment
+)
+open(("test.txt"),)
+open(
+    ("test.txt"),  # comment
+)
+open(
+    ("test.txt"),
+    # comment
+)
+
+open((("test.txt")),)
+open(
+    (("test.txt")),  # comment
+)
+open(
+    (("test.txt")),
+    # comment
+)

crates/ruff_linter/src/fix/edits.rs

@@ -3,12 +3,14 @@
 use anyhow::{Context, Result};
 
 use ruff_diagnostics::Edit;
-use ruff_python_ast::AnyNodeRef;
+use ruff_python_ast::parenthesize::parenthesized_range;
 use ruff_python_ast::{self as ast, Arguments, ExceptHandler, Stmt};
+use ruff_python_ast::{AnyNodeRef, ArgOrKeyword};
 use ruff_python_codegen::Stylist;
 use ruff_python_index::Indexer;
 use ruff_python_trivia::{
-    has_leading_content, is_python_whitespace, PythonWhitespace, SimpleTokenKind, SimpleTokenizer,
+    has_leading_content, is_python_whitespace, CommentRanges, PythonWhitespace, SimpleTokenKind,
+    SimpleTokenizer,
 };
 use ruff_source_file::{Locator, NewlineWithTrailingNewline, UniversalNewlines};
 use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
@@ -138,6 +140,32 @@ pub(crate) fn remove_argument<T: Ranged>(
     }
 }
 
+/// Generic function to add arguments or keyword arguments to function calls.
+pub(crate) fn add_argument(
+    argument: &str,
+    arguments: &Arguments,
+    comment_ranges: &CommentRanges,
+    source: &str,
+) -> Edit {
+    if let Some(last) = arguments.arguments_source_order().last() {
+        // Case 1: existing arguments, so append after the last argument.
+        let last = parenthesized_range(
+            match last {
+                ArgOrKeyword::Arg(arg) => arg.into(),
+                ArgOrKeyword::Keyword(keyword) => (&keyword.value).into(),
+            },
+            arguments.into(),
+            comment_ranges,
+            source,
+        )
+        .unwrap_or(last.range());
+        Edit::insertion(format!(", {argument}"), last.end())
+    } else {
+        // Case 2: no arguments. Add argument, without any trailing comma.
+        Edit::insertion(argument.to_string(), arguments.start() + TextSize::from(1))
+    }
+}
+
 /// Determine if a vector contains only one, specific element.
 fn is_only<T: PartialEq>(vec: &[T], value: &T) -> bool {
     vec.len() == 1 && vec[0] == *value

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

@@ -328,4 +328,15 @@ mod tests {
         assert_messages!(diagnostics);
         Ok(())
     }
+
+    #[test]
+    fn unspecified_encoding_python39_or_lower() -> Result<()> {
+        let diagnostics = test_path(
+            Path::new("pylint/unspecified_encoding.py"),
+            &LinterSettings::for_rule(Rule::UnspecifiedEncoding)
+                .with_target_version(PythonVersion::Py39),
+        )?;
+        assert_messages!(diagnostics);
+        Ok(())
+    }
 }

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

@@ -1,10 +1,16 @@
-use ruff_diagnostics::{Diagnostic, Violation};
+use anyhow::Result;
+
+use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast as ast;
 use ruff_python_ast::call_path::{format_call_path, CallPath};
-use ruff_text_size::Ranged;
+use ruff_python_ast::Expr;
+use ruff_text_size::{Ranged, TextRange};
 
 use crate::checkers::ast::Checker;
+use crate::fix::edits::add_argument;
+use crate::importer::ImportRequest;
+use crate::settings::types::PythonVersion;
 
 /// ## What it does
 /// Checks for uses of `open` and related calls without an explicit `encoding`
@@ -15,7 +21,9 @@ use crate::checkers::ast::Checker;
 /// non-portable code, with differing behavior across platforms.
 ///
 /// Instead, consider using the `encoding` parameter to enforce a specific
-/// encoding.
+/// encoding. [PEP 597] recommends using `locale.getpreferredencoding(False)`
+/// as the default encoding on versions earlier than Python 3.10, and
+/// `encoding="locale"` on Python 3.10 and later.
 ///
 /// ## Example
 /// ```python
@@ -29,13 +37,15 @@ use crate::checkers::ast::Checker;
 ///
 /// ## References
 /// - [Python documentation: `open`](https://docs.python.org/3/library/functions.html#open)
+///
+/// [PEP 597]: https://peps.python.org/pep-0597/
 #[violation]
 pub struct UnspecifiedEncoding {
     function_name: String,
     mode: Mode,
 }
 
-impl Violation for UnspecifiedEncoding {
+impl AlwaysFixableViolation for UnspecifiedEncoding {
     #[derive_message_formats]
     fn message(&self) -> String {
         let UnspecifiedEncoding {
@@ -52,6 +62,10 @@ impl Violation for UnspecifiedEncoding {
             }
         }
     }
+
+    fn fix_title(&self) -> String {
+        format!("Add explicit `encoding` argument")
+    }
 }
 
 /// PLW1514
@@ -70,17 +84,63 @@ pub(crate) fn unspecified_encoding(checker: &mut Checker, call: &ast::ExprCall)
         return;
     };
 
-    checker.diagnostics.push(Diagnostic::new(
+    let mut diagnostic = Diagnostic::new(
         UnspecifiedEncoding {
             function_name,
             mode,
         },
         call.func.range(),
-    ));
+    );
+
+    if checker.settings.target_version >= PythonVersion::Py310 {
+        diagnostic.set_fix(generate_keyword_fix(checker, call));
+    } else {
+        diagnostic.try_set_fix(|| generate_import_fix(checker, call));
+    }
+
+    checker.diagnostics.push(diagnostic);
+}
+
+/// Generate an [`Edit`] for Python 3.10 and later.
+fn generate_keyword_fix(checker: &Checker, call: &ast::ExprCall) -> Fix {
+    Fix::unsafe_edit(add_argument(
+        &format!(
+            "encoding={}",
+            checker
+                .generator()
+                .expr(&Expr::StringLiteral(ast::ExprStringLiteral {
+                    value: ast::StringLiteralValue::single(ast::StringLiteral {
+                        value: "locale".to_string(),
+                        unicode: false,
+                        range: TextRange::default(),
+                    }),
+                    range: TextRange::default(),
+                }))
+        ),
+        &call.arguments,
+        checker.indexer().comment_ranges(),
+        checker.locator().contents(),
+    ))
+}
+
+/// Generate an [`Edit`] for Python 3.9 and earlier.
+fn generate_import_fix(checker: &Checker, call: &ast::ExprCall) -> Result<Fix> {
+    let (import_edit, binding) = checker.importer().get_or_import_symbol(
+        &ImportRequest::import("locale", "getpreferredencoding"),
+        call.start(),
+        checker.semantic(),
+    )?;
+    let argument_edit = add_argument(
+        &format!("encoding={binding}(False)"),
+        &call.arguments,
+        checker.indexer().comment_ranges(),
+        checker.locator().contents(),
+    );
+    Ok(Fix::unsafe_edits(import_edit, [argument_edit]))
 }
 
 /// Returns `true` if the given expression is a string literal containing a `b` character.
-fn is_binary_mode(expr: &ast::Expr) -> Option<bool> {
+fn is_binary_mode(expr: &Expr) -> Option<bool> {
     Some(
         expr.as_string_literal_expr()?
             .value
@@ -92,12 +152,7 @@ fn is_binary_mode(expr: &ast::Expr) -> Option<bool> {
 /// Returns `true` if the given call lacks an explicit `encoding`.
 fn is_violation(call: &ast::ExprCall, call_path: &CallPath) -> bool {
     // If we have something like `*args`, which might contain the encoding argument, abort.
-    if call
-        .arguments
-        .args
-        .iter()
-        .any(ruff_python_ast::Expr::is_starred_expr)
-    {
+    if call.arguments.args.iter().any(Expr::is_starred_expr) {
         return false;
     }
     // If we have something like `**kwargs`, which might contain the encoding argument, abort.