Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pylint: E1507 invalid-envvar-value #3467

Merged
merged 9 commits into from
Mar 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions crates/ruff/resources/test/fixtures/pylint/invalid_envvar_value.py
Original file line number Diff line number Diff line change
@@ -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)
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
73 changes: 73 additions & 0 deletions crates/ruff/src/rules/pylint/rules/invalid_envvar_value.rs
Original file line number Diff line number Diff line change
@@ -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)));
}
}
}
}
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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: ~

3 changes: 3 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.