From d22f3402e12933489a40e345aa871561bf1fff81 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Fri, 7 Jun 2024 11:53:22 +0530 Subject: [PATCH] Remove `result_like` dependency (#11793) ## Summary This PR removes the `result-like` dependency and instead implement the required functionality. The motivation being that `noqa.is_enabled()` is easier to read than `noqa.into()`. For context, I was just trying to understand the syntax error workflow and I saw these flags which were being converted via `into`. I always find `into` confusing because you never know what's it being converted into unless you know the type. Later realized that it's just a boolean flag. After removing the usages from these two flags, it turns out that the dependency is only being used in one rule so I thought to remove that as well. ## Test Plan `cargo insta test` --- Cargo.lock | 33 ----------------- Cargo.toml | 1 - crates/ruff/src/commands/check.rs | 2 +- crates/ruff/src/diagnostics.rs | 2 +- crates/ruff_linter/Cargo.toml | 1 - crates/ruff_linter/src/checkers/ast/mod.rs | 2 +- crates/ruff_linter/src/linter.rs | 4 +-- .../rules/unused_loop_control_variable.rs | 36 +++++++++++-------- crates/ruff_linter/src/settings/flags.rs | 36 +++++++++++++++++-- 9 files changed, 60 insertions(+), 57 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f6e9b0be68..8a974b12ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1629,17 +1629,6 @@ version = "0.2.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bda66fc9667c18cb2758a2ac84d1167245054bcf85d5d1aaa6923f45801bdd02" -[[package]] -name = "pmutil" -version = "0.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "52a40bc70c2c58040d2d8b167ba9a5ff59fc9dab7ad44771cfde3dcfde7a09c6" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "portable-atomic" version = "1.6.0" @@ -1859,27 +1848,6 @@ version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "adad44e29e4c806119491a7f06f03de4d1af22c3a680dd47f1e6e179439d1f56" -[[package]] -name = "result-like" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "abf7172fef6a7d056b5c26bf6c826570267562d51697f4982ff3ba4aec68a9df" -dependencies = [ - "result-like-derive", -] - -[[package]] -name = "result-like-derive" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a8d6574c02e894d66370cfc681e5d68fedbc9a548fb55b30a96b3f0ae22d0fe5" -dependencies = [ - "pmutil", - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "ring" version = "0.17.8" @@ -2086,7 +2054,6 @@ dependencies = [ "pyproject-toml", "quick-junit", "regex", - "result-like", "ruff_cache", "ruff_diagnostics", "ruff_macros", diff --git a/Cargo.toml b/Cargo.toml index a0b74a5efb..841d2b17e3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -99,7 +99,6 @@ quote = { version = "1.0.23" } rand = { version = "0.8.5" } rayon = { version = "1.10.0" } regex = { version = "1.10.2" } -result-like = { version = "0.5.0" } rustc-hash = { version = "1.1.0" } schemars = { version = "0.8.16" } seahash = { version = "4.1.0" } diff --git a/crates/ruff/src/commands/check.rs b/crates/ruff/src/commands/check.rs index 0b421639a1..ea9058d795 100644 --- a/crates/ruff/src/commands/check.rs +++ b/crates/ruff/src/commands/check.rs @@ -59,7 +59,7 @@ pub(crate) fn check( ); // Load the caches. - let caches = if bool::from(cache) { + let caches = if cache.is_enabled() { Some(PackageCacheMap::init(&package_roots, &resolver)) } else { None diff --git a/crates/ruff/src/diagnostics.rs b/crates/ruff/src/diagnostics.rs index 9f32e4af8f..dcd5e2890e 100644 --- a/crates/ruff/src/diagnostics.rs +++ b/crates/ruff/src/diagnostics.rs @@ -186,7 +186,7 @@ pub(crate) fn lint_path( ) -> Result { // Check the cache. let caching = match cache { - Some(cache) if noqa.into() => { + Some(cache) if noqa.is_enabled() => { let relative_path = cache .relative_path(path) .expect("wrong package cache for file"); diff --git a/crates/ruff_linter/Cargo.toml b/crates/ruff_linter/Cargo.toml index 6a9e6931df..032c85f5ac 100644 --- a/crates/ruff_linter/Cargo.toml +++ b/crates/ruff_linter/Cargo.toml @@ -56,7 +56,6 @@ pep440_rs = { workspace = true, features = ["serde"] } pyproject-toml = { workspace = true } quick-junit = { workspace = true } regex = { workspace = true } -result-like = { workspace = true } rustc-hash = { workspace = true } schemars = { workspace = true, optional = true } serde = { workspace = true } diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 183c18f03b..a964a1c01d 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -281,7 +281,7 @@ impl<'a> Checker<'a> { // members from the fix that will eventually be excluded by a `noqa`. // Unfortunately, we _do_ want to register a `Diagnostic` for each // eventually-ignored import, so that our `noqa` counts are accurate. - if !self.noqa.to_bool() { + if !self.noqa.is_enabled() { return false; } noqa::rule_is_ignored(code, offset, self.noqa_line_for, self.locator) diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 7a36e67d5b..971644a800 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -299,7 +299,7 @@ pub fn check_path( } // Enforce `noqa` directives. - if (noqa.into() && !diagnostics.is_empty()) + if (noqa.is_enabled() && !diagnostics.is_empty()) || settings .rules .iter_enabled() @@ -315,7 +315,7 @@ pub fn check_path( &per_file_ignores, settings, ); - if noqa.into() { + if noqa.is_enabled() { for index in ignored.iter().rev() { diagnostics.swap_remove(*index); } diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs index b5fa0d4663..d7cc719121 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs @@ -8,12 +8,6 @@ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; -#[derive(Debug, Copy, Clone, PartialEq, Eq, result_like::BoolLike)] -enum Certainty { - Certain, - Uncertain, -} - /// ## What it does /// Checks for unused variables in loops (e.g., `for` and `while` statements). /// @@ -61,10 +55,13 @@ impl Violation for UnusedLoopControlVariable { let UnusedLoopControlVariable { name, certainty, .. } = self; - if certainty.to_bool() { - format!("Loop control variable `{name}` not used within loop body") - } else { - format!("Loop control variable `{name}` may not be used within loop body") + match certainty { + Certainty::Certain => { + format!("Loop control variable `{name}` not used within loop body") + } + Certainty::Uncertain => { + format!("Loop control variable `{name}` may not be used within loop body") + } } } @@ -105,10 +102,13 @@ pub(crate) fn unused_loop_control_variable(checker: &mut Checker, stmt_for: &ast } // Avoid fixing any variables that _may_ be used, but undetectably so. - let certainty = - Certainty::from(!helpers::uses_magic_variable_access(&stmt_for.body, |id| { - checker.semantic().has_builtin_binding(id) - })); + let certainty = if helpers::uses_magic_variable_access(&stmt_for.body, |id| { + checker.semantic().has_builtin_binding(id) + }) { + Certainty::Uncertain + } else { + Certainty::Certain + }; // Attempt to rename the variable by prepending an underscore, but avoid // applying the fix if doing so wouldn't actually cause us to ignore the @@ -129,7 +129,7 @@ pub(crate) fn unused_loop_control_variable(checker: &mut Checker, stmt_for: &ast expr.range(), ); if let Some(rename) = rename { - if certainty.into() { + if certainty == Certainty::Certain { // Avoid fixing if the variable, or any future bindings to the variable, are // used _after_ the loop. let scope = checker.semantic().current_scope(); @@ -149,3 +149,9 @@ pub(crate) fn unused_loop_control_variable(checker: &mut Checker, stmt_for: &ast checker.diagnostics.push(diagnostic); } } + +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +enum Certainty { + Certain, + Uncertain, +} diff --git a/crates/ruff_linter/src/settings/flags.rs b/crates/ruff_linter/src/settings/flags.rs index a1ce403194..5815e19b49 100644 --- a/crates/ruff_linter/src/settings/flags.rs +++ b/crates/ruff_linter/src/settings/flags.rs @@ -5,14 +5,46 @@ pub enum FixMode { Diff, } -#[derive(Debug, Copy, Clone, Hash, result_like::BoolLike)] +#[derive(Debug, Copy, Clone, Hash)] pub enum Noqa { Enabled, Disabled, } -#[derive(Debug, Copy, Clone, Hash, result_like::BoolLike)] +impl Noqa { + pub const fn is_enabled(self) -> bool { + matches!(self, Noqa::Enabled) + } +} + +impl From for Noqa { + fn from(value: bool) -> Self { + if value { + Noqa::Enabled + } else { + Noqa::Disabled + } + } +} + +#[derive(Debug, Copy, Clone, Hash)] pub enum Cache { Enabled, Disabled, } + +impl Cache { + pub const fn is_enabled(self) -> bool { + matches!(self, Cache::Enabled) + } +} + +impl From for Cache { + fn from(value: bool) -> Self { + if value { + Cache::Enabled + } else { + Cache::Disabled + } + } +}