Support IfExp with dual string arms in invalid-envvar-default (#9734)

## Summary

Just like #6537 and #6538 but for the `default` second parameter to
`getenv()`.

Also rename "BAD" to "BAR" in the tests, since those strings shouldn't
trigger the rule.

## Test Plan

Added passing and failing examples to `invalid_envvar_default.py`.
This commit is contained in:
Christopher Covington 2024-01-31 15:41:24 +00:00 committed by GitHub
parent 9e3ff01ce8
commit 7ae7bf6e30
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 25 additions and 47 deletions

View file

@ -6,8 +6,9 @@ dictVarBad = os.getenv("AAA", {"a", 7}) # [invalid-envvar-default]
print(os.getenv("TEST", False)) # [invalid-envvar-default] print(os.getenv("TEST", False)) # [invalid-envvar-default]
os.getenv("AA", "GOOD") os.getenv("AA", "GOOD")
os.getenv("AA", f"GOOD") os.getenv("AA", f"GOOD")
os.getenv("AA", "GOOD" + "BAD") os.getenv("AA", "GOOD" + "BAR")
os.getenv("AA", "GOOD" + 1) os.getenv("AA", "GOOD" + 1)
os.getenv("AA", "GOOD %s" % "BAD") os.getenv("AA", "GOOD %s" % "BAR")
os.getenv("B", Z) os.getenv("B", Z)
os.getenv("AA", "GOOD" if Z else "BAR")
os.getenv("AA", 1 if Z else "BAR") # [invalid-envvar-default]

View file

@ -1,6 +1,7 @@
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Expr, Operator}; use ruff_python_ast as ast;
use ruff_python_semantic::analyze::type_inference::{PythonType, ResolvedPythonType};
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
@ -40,43 +41,6 @@ impl Violation for InvalidEnvvarDefault {
} }
} }
fn is_valid_default(expr: &Expr) -> bool {
// We can't infer the types of these defaults, so assume they're valid.
if matches!(
expr,
Expr::Name(_) | Expr::Attribute(_) | Expr::Subscript(_) | Expr::Call(_)
) {
return true;
}
// Allow string concatenation.
if let Expr::BinOp(ast::ExprBinOp {
left,
right,
op: Operator::Add,
range: _,
}) = expr
{
return is_valid_default(left) && is_valid_default(right);
}
// Allow string formatting.
if let Expr::BinOp(ast::ExprBinOp {
left,
op: Operator::Mod,
..
}) = expr
{
return is_valid_default(left);
}
// Otherwise, the default must be a string or `None`.
matches!(
expr,
Expr::StringLiteral(_) | Expr::NoneLiteral(_) | Expr::FString(_)
)
}
/// PLW1508 /// PLW1508
pub(crate) fn invalid_envvar_default(checker: &mut Checker, call: &ast::ExprCall) { pub(crate) fn invalid_envvar_default(checker: &mut Checker, call: &ast::ExprCall) {
if checker if checker
@ -89,10 +53,15 @@ pub(crate) fn invalid_envvar_default(checker: &mut Checker, call: &ast::ExprCall
return; return;
}; };
if !is_valid_default(expr) { if matches!(
ResolvedPythonType::from(expr),
ResolvedPythonType::Unknown
| ResolvedPythonType::Atom(PythonType::String | PythonType::None)
) {
return;
}
checker checker
.diagnostics .diagnostics
.push(Diagnostic::new(InvalidEnvvarDefault, expr.range())); .push(Diagnostic::new(InvalidEnvvarDefault, expr.range()));
} }
}
} }

View file

@ -34,11 +34,19 @@ invalid_envvar_default.py:6:25: PLW1508 Invalid type for environment variable de
invalid_envvar_default.py:10:17: PLW1508 Invalid type for environment variable default; expected `str` or `None` invalid_envvar_default.py:10:17: PLW1508 Invalid type for environment variable default; expected `str` or `None`
| |
8 | os.getenv("AA", f"GOOD") 8 | os.getenv("AA", f"GOOD")
9 | os.getenv("AA", "GOOD" + "BAD") 9 | os.getenv("AA", "GOOD" + "BAR")
10 | os.getenv("AA", "GOOD" + 1) 10 | os.getenv("AA", "GOOD" + 1)
| ^^^^^^^^^^ PLW1508 | ^^^^^^^^^^ PLW1508
11 | os.getenv("AA", "GOOD %s" % "BAD") 11 | os.getenv("AA", "GOOD %s" % "BAR")
12 | os.getenv("B", Z) 12 | os.getenv("B", Z)
| |
invalid_envvar_default.py:14:17: PLW1508 Invalid type for environment variable default; expected `str` or `None`
|
12 | os.getenv("B", Z)
13 | os.getenv("AA", "GOOD" if Z else "BAR")
14 | os.getenv("AA", 1 if Z else "BAR") # [invalid-envvar-default]
| ^^^^^^^^^^^^^^^^^ PLW1508
|