pylint: E1507 invalid-envvar-value (#3467)

crates/ruff/resources/test/fixtures/pylint/invalid_envvar_value.py

@@ -0,0 +1,12 @@
+import os
+
+os.getenv(1)  # [invalid-envvar-value]
+os.getenv("a")
+os.getenv("test")
+os.getenv(key="testingAgain")
+os.getenv(key=11)  # [invalid-envvar-value]
+os.getenv(["hello"])  # [invalid-envvar-value]
+os.getenv(key="foo", default="bar")
+
+AA = "aa"
+os.getenv(AA)

crates/ruff/src/checkers/ast/mod.rs

@@ -2869,6 +2869,9 @@ where
                 if self.settings.rules.enabled(&Rule::InvalidEnvvarDefault) {
                     pylint::rules::invalid_envvar_default(self, func, args, keywords);
                 }
+                if self.settings.rules.enabled(&Rule::InvalidEnvvarValue) {
+                    pylint::rules::invalid_envvar_value(self, func, args, keywords);
+                }
 
                 // flake8-pytest-style
                 if self.settings.rules.enabled(&Rule::PatchWithLambda) {

crates/ruff/src/codes.rs

@@ -176,6 +176,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
         (Pylint, "E1206") => Rule::LoggingTooFewArgs,
         (Pylint, "E1307") => Rule::BadStringFormatType,
         (Pylint, "E1310") => Rule::BadStrStripCall,
+        (Pylint, "E1507") => Rule::InvalidEnvvarValue,
         (Pylint, "E2502") => Rule::BidirectionalUnicode,
         (Pylint, "R0133") => Rule::ComparisonOfConstant,
         (Pylint, "R0206") => Rule::PropertyWithParameters,

crates/ruff/src/registry.rs

@@ -147,6 +147,7 @@ ruff_macros::register_rules!(
     rules::pylint::rules::InvalidAllObject,
     rules::pylint::rules::InvalidAllFormat,
     rules::pylint::rules::InvalidEnvvarDefault,
+    rules::pylint::rules::InvalidEnvvarValue,
     rules::pylint::rules::BadStringFormatType,
     rules::pylint::rules::BidirectionalUnicode,
     rules::pylint::rules::BadStrStripCall,

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

@@ -44,6 +44,7 @@ mod tests {
     #[test_case(Rule::InvalidAllFormat, Path::new("invalid_all_format.py"); "PLE0605")]
     #[test_case(Rule::InvalidAllObject, Path::new("invalid_all_object.py"); "PLE0604")]
     #[test_case(Rule::InvalidEnvvarDefault, Path::new("invalid_envvar_default.py"); "PLW1508")]
+    #[test_case(Rule::InvalidEnvvarValue, Path::new("invalid_envvar_value.py"); "PLE1507")]
     #[test_case(Rule::TooManyReturnStatements, Path::new("too_many_return_statements.py"); "PLR0911")]
     #[test_case(Rule::TooManyArguments, Path::new("too_many_arguments.py"); "PLR0913")]
     #[test_case(Rule::TooManyBranches, Path::new("too_many_branches.py"); "PLR0912")]

crates/ruff/src/rules/pylint/rules/invalid_envvar_value.rs

@@ -0,0 +1,73 @@
+use rustpython_parser::ast::{Constant, Expr, ExprKind, Keyword};
+
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::types::Range;
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for `os.getenv` calls with an invalid `key` argument.
+///
+/// ## Why is this bad?
+/// `os.getenv` only supports strings as the first argument (`key`).
+///
+/// If the provided argument is not a string, `os.getenv` will throw a
+/// `TypeError` at runtime.
+///
+/// ## Example
+/// ```python
+/// os.getenv(1)
+/// ```
+///
+/// Use instead:
+/// ```python
+/// os.getenv("1")
+/// ```
+#[violation]
+pub struct InvalidEnvvarValue;
+
+impl Violation for InvalidEnvvarValue {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Invalid type for initial `os.getenv` argument; expected `str`")
+    }
+}
+
+/// PLE1507
+pub fn invalid_envvar_value(
+    checker: &mut Checker,
+    func: &Expr,
+    args: &[Expr],
+    keywords: &[Keyword],
+) {
+    if checker
+        .ctx
+        .resolve_call_path(func)
+        .map_or(false, |call_path| call_path.as_slice() == ["os", "getenv"])
+    {
+        // Get the first argument for `getenv`
+        if let Some(expr) = args.get(0).or_else(|| {
+            keywords
+                .iter()
+                .find(|keyword| keyword.node.arg.as_ref().map_or(false, |arg| arg == "key"))
+                .map(|keyword| &keyword.node.value)
+        }) {
+            // Ignoring types that are inferred, only do direct constants
+            if !matches!(
+                expr.node,
+                ExprKind::Constant {
+                    value: Constant::Str { .. },
+                    ..
+                } | ExprKind::Name { .. }
+                    | ExprKind::Attribute { .. }
+                    | ExprKind::Subscript { .. }
+                    | ExprKind::Call { .. }
+            ) {
+                checker
+                    .diagnostics
+                    .push(Diagnostic::new(InvalidEnvvarValue, Range::from(expr)));
+            }
+        }
+    }
+}

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

@@ -11,6 +11,7 @@ pub use global_variable_not_assigned::GlobalVariableNotAssigned;
 pub use invalid_all_format::{invalid_all_format, InvalidAllFormat};
 pub use invalid_all_object::{invalid_all_object, InvalidAllObject};
 pub use invalid_envvar_default::{invalid_envvar_default, InvalidEnvvarDefault};
+pub use invalid_envvar_value::{invalid_envvar_value, InvalidEnvvarValue};
 pub use logging::{logging_call, LoggingTooFewArgs, LoggingTooManyArgs};
 pub use magic_value_comparison::{magic_value_comparison, MagicValueComparison};
 pub use merge_isinstance::{merge_isinstance, ConsiderMergingIsinstance};
@@ -46,6 +47,7 @@ mod global_variable_not_assigned;
 mod invalid_all_format;
 mod invalid_all_object;
 mod invalid_envvar_default;
+mod invalid_envvar_value;
 mod logging;
 mod magic_value_comparison;
 mod merge_isinstance;

crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLE1507_invalid_envvar_value.py.snap

@@ -0,0 +1,44 @@
+---
+source: crates/ruff/src/rules/pylint/mod.rs
+expression: diagnostics
+---
+- kind:
+    name: InvalidEnvvarValue
+    body: "Invalid type for initial `os.getenv` argument; expected `str`"
+    suggestion: ~
+    fixable: false
+  location:
+    row: 3
+    column: 10
+  end_location:
+    row: 3
+    column: 11
+  fix: ~
+  parent: ~
+- kind:
+    name: InvalidEnvvarValue
+    body: "Invalid type for initial `os.getenv` argument; expected `str`"
+    suggestion: ~
+    fixable: false
+  location:
+    row: 7
+    column: 14
+  end_location:
+    row: 7
+    column: 16
+  fix: ~
+  parent: ~
+- kind:
+    name: InvalidEnvvarValue
+    body: "Invalid type for initial `os.getenv` argument; expected `str`"
+    suggestion: ~
+    fixable: false
+  location:
+    row: 8
+    column: 10
+  end_location:
+    row: 8
+    column: 19
+  fix: ~
+  parent: ~
+

ruff.schema.json

@@ -1851,6 +1851,9 @@
         "PLE1307",
         "PLE131",
         "PLE1310",
+        "PLE15",
+        "PLE150",
+        "PLE1507",
         "PLE2",
         "PLE25",
         "PLE250",