Skip to content

Commit

Permalink
[refurb] Implement hashlib-digest-hex (FURB181) (#9077)
Browse files Browse the repository at this point in the history
## Summary

Implementation of  Refurb FURB181
Part of #1348

## Test Plan

Test cases from Refurb
  • Loading branch information
sbrugman authored Dec 10, 2023
1 parent febc69a commit 6e36dcf
Show file tree
Hide file tree
Showing 8 changed files with 527 additions and 0 deletions.
57 changes: 57 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB181.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import hashlib
from hashlib import (
blake2b,
blake2s,
md5,
sha1,
sha3_224,
sha3_256,
sha3_384,
sha3_512,
sha224,
)
from hashlib import sha256
from hashlib import sha256 as hash_algo
from hashlib import sha384, sha512, shake_128, shake_256

# these will match

blake2b().digest().hex()
blake2s().digest().hex()
md5().digest().hex()
sha1().digest().hex()
sha224().digest().hex()
sha256().digest().hex()
sha384().digest().hex()
sha3_224().digest().hex()
sha3_256().digest().hex()
sha3_384().digest().hex()
sha3_512().digest().hex()
sha512().digest().hex()
shake_128().digest(10).hex()
shake_256().digest(10).hex()

hashlib.sha256().digest().hex()

sha256(b"text").digest().hex()

hash_algo().digest().hex()

# not yet supported
h = sha256()
h.digest().hex()


# these will not

sha256().digest()
sha256().digest().hex("_")
sha256().digest().hex(bytes_per_sep=4)
sha256().hexdigest()

class Hash:
def digest(self) -> bytes:
return b""


Hash().digest().hex()
5 changes: 5 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
Rule::FString,
// flynt
Rule::StaticJoinToFString,
// refurb
Rule::HashlibDigestHex,
]) {
if let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func.as_ref() {
let attr = attr.as_str();
Expand Down Expand Up @@ -581,6 +583,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::HashlibInsecureHashFunction) {
flake8_bandit::rules::hashlib_insecure_hash_functions(checker, call);
}
if checker.enabled(Rule::HashlibDigestHex) {
refurb::rules::hashlib_digest_hex(checker, call);
}
if checker.enabled(Rule::RequestWithoutTimeout) {
flake8_bandit::rules::request_without_timeout(checker, call);
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Refurb, "169") => (RuleGroup::Preview, rules::refurb::rules::TypeNoneComparison),
(Refurb, "171") => (RuleGroup::Preview, rules::refurb::rules::SingleItemMembershipTest),
(Refurb, "177") => (RuleGroup::Preview, rules::refurb::rules::ImplicitCwd),
(Refurb, "181") => (RuleGroup::Preview, rules::refurb::rules::HashlibDigestHex),

// flake8-logging
(Flake8Logging, "001") => (RuleGroup::Preview, rules::flake8_logging::rules::DirectLoggerInstantiation),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/refurb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ mod tests {
#[test_case(Rule::IsinstanceTypeNone, Path::new("FURB168.py"))]
#[test_case(Rule::TypeNoneComparison, Path::new("FURB169.py"))]
#[test_case(Rule::RedundantLogBase, Path::new("FURB163.py"))]
#[test_case(Rule::HashlibDigestHex, Path::new("FURB181.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
120 changes: 120 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/hashlib_digest_hex.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Expr, ExprAttribute, ExprCall};
use ruff_text_size::{Ranged, TextRange};

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for the use of `.digest().hex()` on a hashlib hash, like `sha512`.
///
/// ## Why is this bad?
/// When generating a hex digest from a hash, it's preferable to use the
/// `.hexdigest()` method, rather than calling `.digest()` and then `.hex()`,
/// as the former is more concise and readable.
///
/// ## Example
/// ```python
/// from hashlib import sha512
///
/// hashed = sha512(b"some data").digest().hex()
/// ```
///
/// Use instead:
/// ```python
/// from hashlib import sha512
///
/// hashed = sha512(b"some data").hexdigest()
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as unsafe, as the target of the `.digest()` call
/// could be a user-defined class that implements a `.hex()` method, rather
/// than a hashlib hash object.
///
/// ## References
/// - [Python documentation: `hashlib`](https://docs.python.org/3/library/hashlib.html)
#[violation]
pub struct HashlibDigestHex;

impl Violation for HashlibDigestHex {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
format!("Use of hashlib's `.digest().hex()`")
}

fn fix_title(&self) -> Option<String> {
Some("Replace with `.hexdigest()`".to_string())
}
}

/// FURB181
pub(crate) fn hashlib_digest_hex(checker: &mut Checker, call: &ExprCall) {
if !call.arguments.is_empty() {
return;
}

let Expr::Attribute(ExprAttribute { attr, value, .. }) = call.func.as_ref() else {
return;
};

if attr.as_str() != "hex" {
return;
}

let Expr::Call(ExprCall {
func, arguments, ..
}) = value.as_ref()
else {
return;
};

let Expr::Attribute(ExprAttribute { attr, value, .. }) = func.as_ref() else {
return;
};

if attr.as_str() != "digest" {
return;
}

let Expr::Call(ExprCall { func, .. }) = value.as_ref() else {
return;
};

if checker.semantic().resolve_call_path(func).is_some_and(
|call_path: smallvec::SmallVec<[&str; 8]>| {
matches!(
call_path.as_slice(),
[
"hashlib",
"md5"
| "sha1"
| "sha224"
| "sha256"
| "sha384"
| "sha512"
| "blake2b"
| "blake2s"
| "sha3_224"
| "sha3_256"
| "sha3_384"
| "sha3_512"
| "shake_128"
| "shake_256"
| "_Hash"
]
)
},
) {
let mut diagnostic = Diagnostic::new(HashlibDigestHex, call.range());
if arguments.is_empty() {
diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
".hexdigest".to_string(),
TextRange::new(value.end(), call.func.end()),
)));
}
checker.diagnostics.push(diagnostic);
}
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub(crate) use check_and_remove_from_set::*;
pub(crate) use delete_full_slice::*;
pub(crate) use hashlib_digest_hex::*;
pub(crate) use if_expr_min_max::*;
pub(crate) use implicit_cwd::*;
pub(crate) use isinstance_type_none::*;
Expand All @@ -16,6 +17,7 @@ pub(crate) use unnecessary_enumerate::*;

mod check_and_remove_from_set;
mod delete_full_slice;
mod hashlib_digest_hex;
mod if_expr_min_max;
mod implicit_cwd;
mod isinstance_type_none;
Expand Down
Loading

0 comments on commit 6e36dcf

Please sign in to comment.