From 057faabcddc94fc7fd32ade811b287134dfc0cfc Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 22 Jul 2023 22:59:31 -0400 Subject: [PATCH] Use `Flags::intersects` rather than `Flags::contains` (#6007) ## Summary This is equivalent for a single flag, but I think it's more likely to be correct when the bitflags are modified -- the primary reason being that we sometimes define flags as the union of other flags, e.g.: ```rust const ANNOTATION = Self::TYPING_ONLY_ANNOTATION.bits() | Self::RUNTIME_ANNOTATION.bits(); ``` In this case, `flags.contains(Flag::ANNOTATION)` requires that _both_ flags in the union are set, whereas `flags.intersects(Flag::ANNOTATION)` requires that _at least one_ flag is set. --- crates/ruff/src/checkers/ast/mod.rs | 4 ++-- crates/ruff/src/directives.rs | 4 ++-- crates/ruff/src/message/text.rs | 6 +++--- .../rules/printf_string_formatting.rs | 14 +++++++------- crates/ruff_cli/src/printer.rs | 18 +++++++++--------- crates/ruff_python_semantic/src/binding.rs | 14 +++++++------- crates/ruff_python_semantic/src/scope.rs | 2 +- 7 files changed, 31 insertions(+), 31 deletions(-) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 2d55489894..d952f21ace 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -4513,10 +4513,10 @@ impl<'a> Checker<'a> { extract_all_names(parent, |name| self.semantic.is_builtin(name)); let mut flags = BindingFlags::empty(); - if all_flags.contains(DunderAllFlags::INVALID_OBJECT) { + if all_flags.intersects(DunderAllFlags::INVALID_OBJECT) { flags |= BindingFlags::INVALID_ALL_OBJECT; } - if all_flags.contains(DunderAllFlags::INVALID_FORMAT) { + if all_flags.intersects(DunderAllFlags::INVALID_FORMAT) { flags |= BindingFlags::INVALID_ALL_FORMAT; } diff --git a/crates/ruff/src/directives.rs b/crates/ruff/src/directives.rs index 4fc919a91c..06e8c09272 100644 --- a/crates/ruff/src/directives.rs +++ b/crates/ruff/src/directives.rs @@ -71,12 +71,12 @@ pub fn extract_directives( indexer: &Indexer, ) -> Directives { Directives { - noqa_line_for: if flags.contains(Flags::NOQA) { + noqa_line_for: if flags.intersects(Flags::NOQA) { extract_noqa_line_for(lxr, locator, indexer) } else { NoqaMapping::default() }, - isort: if flags.contains(Flags::ISORT) { + isort: if flags.intersects(Flags::ISORT) { extract_isort_directives(lxr, locator) } else { IsortDirectives::default() diff --git a/crates/ruff/src/message/text.rs b/crates/ruff/src/message/text.rs index 237b304d28..ee27ae28fe 100644 --- a/crates/ruff/src/message/text.rs +++ b/crates/ruff/src/message/text.rs @@ -109,11 +109,11 @@ impl Emitter for TextEmitter { sep = ":".cyan(), code_and_body = RuleCodeAndBody { message, - show_fix_status: self.flags.contains(EmitterFlags::SHOW_FIX_STATUS) + show_fix_status: self.flags.intersects(EmitterFlags::SHOW_FIX_STATUS) } )?; - if self.flags.contains(EmitterFlags::SHOW_SOURCE) { + if self.flags.intersects(EmitterFlags::SHOW_SOURCE) { writeln!( writer, "{}", @@ -124,7 +124,7 @@ impl Emitter for TextEmitter { )?; } - if self.flags.contains(EmitterFlags::SHOW_FIX_DIFF) { + if self.flags.intersects(EmitterFlags::SHOW_FIX_DIFF) { if let Some(diff) = Diff::from_message(message) { writeln!(writer, "{diff}")?; } diff --git a/crates/ruff/src/rules/pyupgrade/rules/printf_string_formatting.rs b/crates/ruff/src/rules/pyupgrade/rules/printf_string_formatting.rs index 2e02ad3ba9..88fea3a7fc 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/printf_string_formatting.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/printf_string_formatting.rs @@ -56,22 +56,22 @@ impl AlwaysAutofixableViolation for PrintfStringFormatting { fn simplify_conversion_flag(flags: CConversionFlags) -> String { let mut flag_string = String::new(); - if flags.contains(CConversionFlags::LEFT_ADJUST) { + if flags.intersects(CConversionFlags::LEFT_ADJUST) { flag_string.push('<'); } - if flags.contains(CConversionFlags::SIGN_CHAR) { + if flags.intersects(CConversionFlags::SIGN_CHAR) { flag_string.push('+'); } - if flags.contains(CConversionFlags::ALTERNATE_FORM) { + if flags.intersects(CConversionFlags::ALTERNATE_FORM) { flag_string.push('#'); } - if flags.contains(CConversionFlags::BLANK_SIGN) { - if !flags.contains(CConversionFlags::SIGN_CHAR) { + if flags.intersects(CConversionFlags::BLANK_SIGN) { + if !flags.intersects(CConversionFlags::SIGN_CHAR) { flag_string.push(' '); } } - if flags.contains(CConversionFlags::ZERO_PAD) { - if !flags.contains(CConversionFlags::LEFT_ADJUST) { + if flags.intersects(CConversionFlags::ZERO_PAD) { + if !flags.intersects(CConversionFlags::LEFT_ADJUST) { flag_string.push('0'); } } diff --git a/crates/ruff_cli/src/printer.rs b/crates/ruff_cli/src/printer.rs index 0c8070e6e4..c1a0608890 100644 --- a/crates/ruff_cli/src/printer.rs +++ b/crates/ruff_cli/src/printer.rs @@ -99,7 +99,7 @@ impl Printer { fn write_summary_text(&self, writer: &mut dyn Write, diagnostics: &Diagnostics) -> Result<()> { if self.log_level >= LogLevel::Default { - if self.flags.contains(Flags::SHOW_VIOLATIONS) { + if self.flags.intersects(Flags::SHOW_VIOLATIONS) { let fixed = diagnostics .fixed .values() @@ -160,12 +160,12 @@ impl Printer { return Ok(()); } - if !self.flags.contains(Flags::SHOW_VIOLATIONS) { + if !self.flags.intersects(Flags::SHOW_VIOLATIONS) { if matches!( self.format, SerializationFormat::Text | SerializationFormat::Grouped ) { - if self.flags.contains(Flags::SHOW_FIX_SUMMARY) { + if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) { if !diagnostics.fixed.is_empty() { writeln!(writer)?; print_fix_summary(writer, &diagnostics.fixed)?; @@ -192,11 +192,11 @@ impl Printer { SerializationFormat::Text => { TextEmitter::default() .with_show_fix_status(show_fix_status(self.autofix_level)) - .with_show_fix_diff(self.flags.contains(Flags::SHOW_FIX_DIFF)) - .with_show_source(self.flags.contains(Flags::SHOW_SOURCE)) + .with_show_fix_diff(self.flags.intersects(Flags::SHOW_FIX_DIFF)) + .with_show_source(self.flags.intersects(Flags::SHOW_SOURCE)) .emit(writer, &diagnostics.messages, &context)?; - if self.flags.contains(Flags::SHOW_FIX_SUMMARY) { + if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) { if !diagnostics.fixed.is_empty() { writeln!(writer)?; print_fix_summary(writer, &diagnostics.fixed)?; @@ -208,11 +208,11 @@ impl Printer { } SerializationFormat::Grouped => { GroupedEmitter::default() - .with_show_source(self.flags.contains(Flags::SHOW_SOURCE)) + .with_show_source(self.flags.intersects(Flags::SHOW_SOURCE)) .with_show_fix_status(show_fix_status(self.autofix_level)) .emit(writer, &diagnostics.messages, &context)?; - if self.flags.contains(Flags::SHOW_FIX_SUMMARY) { + if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) { if !diagnostics.fixed.is_empty() { writeln!(writer)?; print_fix_summary(writer, &diagnostics.fixed)?; @@ -367,7 +367,7 @@ impl Printer { let context = EmitterContext::new(&diagnostics.source_kind); TextEmitter::default() .with_show_fix_status(show_fix_status(self.autofix_level)) - .with_show_source(self.flags.contains(Flags::SHOW_SOURCE)) + .with_show_source(self.flags.intersects(Flags::SHOW_SOURCE)) .emit(writer, &diagnostics.messages, &context)?; } writer.flush()?; diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 1f47db394f..77bf59416a 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -45,44 +45,44 @@ impl<'a> Binding<'a> { /// Return `true` if this [`Binding`] represents an explicit re-export /// (e.g., `FastAPI` in `from fastapi import FastAPI as FastAPI`). pub const fn is_explicit_export(&self) -> bool { - self.flags.contains(BindingFlags::EXPLICIT_EXPORT) + self.flags.intersects(BindingFlags::EXPLICIT_EXPORT) } /// Return `true` if this [`Binding`] represents an external symbol /// (e.g., `FastAPI` in `from fastapi import FastAPI`). pub const fn is_external(&self) -> bool { - self.flags.contains(BindingFlags::EXTERNAL) + self.flags.intersects(BindingFlags::EXTERNAL) } /// Return `true` if this [`Binding`] represents an aliased symbol /// (e.g., `app` in `from fastapi import FastAPI as app`). pub const fn is_alias(&self) -> bool { - self.flags.contains(BindingFlags::ALIAS) + self.flags.intersects(BindingFlags::ALIAS) } /// Return `true` if this [`Binding`] represents a `nonlocal`. A [`Binding`] is a `nonlocal` /// if it's declared by a `nonlocal` statement, or shadows a [`Binding`] declared by a /// `nonlocal` statement. pub const fn is_nonlocal(&self) -> bool { - self.flags.contains(BindingFlags::NONLOCAL) + self.flags.intersects(BindingFlags::NONLOCAL) } /// Return `true` if this [`Binding`] represents a `global`. A [`Binding`] is a `global` if it's /// declared by a `global` statement, or shadows a [`Binding`] declared by a `global` statement. pub const fn is_global(&self) -> bool { - self.flags.contains(BindingFlags::GLOBAL) + self.flags.intersects(BindingFlags::GLOBAL) } /// Return `true` if this [`Binding`] represents an assignment to `__all__` with an invalid /// value (e.g., `__all__ = "Foo"`). pub const fn is_invalid_all_format(&self) -> bool { - self.flags.contains(BindingFlags::INVALID_ALL_FORMAT) + self.flags.intersects(BindingFlags::INVALID_ALL_FORMAT) } /// Return `true` if this [`Binding`] represents an assignment to `__all__` that includes an /// invalid member (e.g., `__all__ = ["Foo", 1]`). pub const fn is_invalid_all_object(&self) -> bool { - self.flags.contains(BindingFlags::INVALID_ALL_OBJECT) + self.flags.intersects(BindingFlags::INVALID_ALL_OBJECT) } /// Return `true` if this [`Binding`] represents an unbound variable diff --git a/crates/ruff_python_semantic/src/scope.rs b/crates/ruff_python_semantic/src/scope.rs index 04efe0e92f..431f22ab99 100644 --- a/crates/ruff_python_semantic/src/scope.rs +++ b/crates/ruff_python_semantic/src/scope.rs @@ -163,7 +163,7 @@ impl<'a> Scope<'a> { /// Returns `true` if this scope uses locals (e.g., `locals()`). pub const fn uses_locals(&self) -> bool { - self.flags.contains(ScopeFlags::USES_LOCALS) + self.flags.intersects(ScopeFlags::USES_LOCALS) } }