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`
This commit is contained in:
Dhruv Manilawala 2024-06-07 11:53:22 +05:30 committed by GitHub
parent ea27445479
commit d22f3402e1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 60 additions and 57 deletions

View file

@ -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)

View file

@ -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);
}

View file

@ -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,
}

View file

@ -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<bool> 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<bool> for Cache {
fn from(value: bool) -> Self {
if value {
Cache::Enabled
} else {
Cache::Disabled
}
}
}