From c80ab399aab767738cc46d7413ebf795a27800e9 Mon Sep 17 00:00:00 2001 From: Connor Skees Date: Sun, 4 Aug 2024 18:27:24 +0000 Subject: [PATCH] clippy --- crates/compiler/Cargo.toml | 2 +- crates/compiler/src/ast/stmt.rs | 19 +++++----- crates/compiler/src/evaluate/visitor.rs | 46 ++++++++++++------------ crates/compiler/src/lib.rs | 15 +++----- crates/compiler/src/parse/keyframes.rs | 2 +- crates/compiler/src/value/arglist.rs | 6 ++-- crates/compiler/src/value/calculation.rs | 1 + 7 files changed, 43 insertions(+), 48 deletions(-) diff --git a/crates/compiler/Cargo.toml b/crates/compiler/Cargo.toml index c94b918..eb692fe 100644 --- a/crates/compiler/Cargo.toml +++ b/crates/compiler/Cargo.toml @@ -23,7 +23,7 @@ bench = false rustdoc-args = ["--cfg", "doc_cfg"] [dependencies] -# todo: use lazy_static +# todo: replace with std::cell::LazyCell (msrv 1.80.0) once_cell = "1.15.0" # todo: use xorshift for random numbers rand = { version = "0.8", optional = true } diff --git a/crates/compiler/src/ast/stmt.rs b/crates/compiler/src/ast/stmt.rs index fe3a3a8..95d38bc 100644 --- a/crates/compiler/src/ast/stmt.rs +++ b/crates/compiler/src/ast/stmt.rs @@ -2,6 +2,7 @@ use std::{ cell::RefCell, collections::{BTreeMap, HashSet}, path::PathBuf, + rc::Rc, sync::Arc, }; @@ -301,17 +302,17 @@ pub struct ConfiguredVariable { pub struct Configuration { pub(crate) values: Arc>, #[allow(unused)] - pub(crate) original_config: Option>>, + pub(crate) original_config: Option>>, pub(crate) span: Option, } impl Configuration { pub fn through_forward( - config: Arc>, + config: Rc>, forward: &AstForwardRule, - ) -> Arc> { + ) -> Rc> { if (*config).borrow().is_empty() { - return Arc::new(RefCell::new(Configuration::empty())); + return Rc::new(RefCell::new(Configuration::empty())); } let mut new_values = Arc::clone(&(*config).borrow().values); @@ -330,14 +331,14 @@ impl Configuration { new_values = Arc::new(LimitedMapView::blocklist(new_values, hidden_variables)); } - Arc::new(RefCell::new(Self::with_values( + Rc::new(RefCell::new(Self::with_values( config, Arc::clone(&new_values), ))) } fn with_values( - config: Arc>, + config: Rc>, values: Arc>, ) -> Self { Self { @@ -394,10 +395,10 @@ impl Configuration { } #[allow(unused)] - pub fn original_config(config: Arc>) -> Arc> { + pub fn original_config(config: Rc>) -> Rc> { match (*config).borrow().original_config.as_ref() { - Some(v) => Arc::clone(v), - None => Arc::clone(&config), + Some(v) => Rc::clone(v), + None => Rc::clone(&config), } } } diff --git a/crates/compiler/src/evaluate/visitor.rs b/crates/compiler/src/evaluate/visitor.rs index e95ce20..e3a8017 100644 --- a/crates/compiler/src/evaluate/visitor.rs +++ b/crates/compiler/src/evaluate/visitor.rs @@ -6,6 +6,7 @@ use std::{ iter::FromIterator, mem, path::{Path, PathBuf}, + rc::Rc, sync::Arc, }; @@ -120,7 +121,7 @@ pub struct Visitor<'a> { pub(crate) active_modules: BTreeSet, css_tree: CssTree, parent: Option, - configuration: Arc>, + configuration: Rc>, import_nodes: Vec, pub options: &'a Options<'a>, pub(crate) map: &'a mut CodeMap, @@ -159,7 +160,7 @@ impl<'a> Visitor<'a> { css_tree: CssTree::new(), parent: None, current_import_path, - configuration: Arc::new(RefCell::new(Configuration::empty())), + configuration: Rc::new(RefCell::new(Configuration::empty())), is_plain_css: false, import_nodes: Vec::new(), modules: BTreeMap::new(), @@ -257,17 +258,16 @@ impl<'a> Visitor<'a> { } fn visit_forward_rule(&mut self, forward_rule: AstForwardRule) -> SassResult<()> { - let old_config = Arc::clone(&self.configuration); - let adjusted_config = - Configuration::through_forward(Arc::clone(&old_config), &forward_rule); + let old_config = Rc::clone(&self.configuration); + let adjusted_config = Configuration::through_forward(Rc::clone(&old_config), &forward_rule); if !forward_rule.configuration.is_empty() { let new_configuration = - self.add_forward_configuration(Arc::clone(&adjusted_config), &forward_rule)?; + self.add_forward_configuration(Rc::clone(&adjusted_config), &forward_rule)?; self.load_module( forward_rule.url.as_path(), - Some(Arc::clone(&new_configuration)), + Some(Rc::clone(&new_configuration)), false, forward_rule.span, |visitor, module, _| { @@ -334,9 +334,9 @@ impl<'a> Visitor<'a> { #[allow(clippy::unnecessary_unwrap)] fn add_forward_configuration( &mut self, - config: Arc>, + config: Rc>, forward_rule: &AstForwardRule, - ) -> SassResult>> { + ) -> SassResult>> { let mut new_values = BTreeMap::from_iter((*config).borrow().values.iter()); for variable in &forward_rule.configuration { @@ -367,7 +367,7 @@ impl<'a> Visitor<'a> { ); } - Ok(Arc::new(RefCell::new( + Ok(Rc::new(RefCell::new( if !(*config).borrow().is_implicit() || (*config).borrow().is_empty() { Configuration::explicit(new_values, forward_rule.span) } else { @@ -379,8 +379,8 @@ impl<'a> Visitor<'a> { /// Remove configured values from [upstream] that have been removed from /// [downstream], unless they match a name in [except]. fn remove_used_configuration( - upstream: &Arc>, - downstream: &Arc>, + upstream: &Rc>, + downstream: &Rc>, except: &HashSet, ) { let mut names_to_remove = Vec::new(); @@ -542,7 +542,7 @@ impl<'a> Visitor<'a> { fn execute( &mut self, stylesheet: StyleSheet, - configuration: Option>>, + configuration: Option>>, // todo: different errors based on this _names_in_errors: bool, ) -> SassResult>> { @@ -551,7 +551,7 @@ impl<'a> Visitor<'a> { // todo: use canonical url for modules if let Some(already_loaded) = self.modules.get(&stylesheet.url) { let current_configuration = - configuration.unwrap_or_else(|| Arc::clone(&self.configuration)); + configuration.unwrap_or_else(|| Rc::clone(&self.configuration)); if !current_configuration.borrow().is_implicit() { // if (!_moduleConfigurations[url]!.sameOriginal(currentConfiguration) && @@ -643,7 +643,7 @@ impl<'a> Visitor<'a> { pub(crate) fn load_module( &mut self, url: &Path, - configuration: Option>>, + configuration: Option>>, names_in_errors: bool, span: Span, callback: impl Fn(&mut Self, Arc>, StyleSheet) -> SassResult<()>, @@ -714,7 +714,7 @@ impl<'a> Visitor<'a> { fn visit_use_rule(&mut self, use_rule: AstUseRule) -> SassResult<()> { let configuration = if use_rule.configuration.is_empty() { - Arc::new(RefCell::new(Configuration::empty())) + Rc::new(RefCell::new(Configuration::empty())) } else { let mut values = BTreeMap::new(); @@ -727,7 +727,7 @@ impl<'a> Visitor<'a> { ); } - Arc::new(RefCell::new(Configuration::explicit(values, use_rule.span))) + Rc::new(RefCell::new(Configuration::explicit(values, use_rule.span))) }; let span = use_rule.span; @@ -739,7 +739,7 @@ impl<'a> Visitor<'a> { self.load_module( &use_rule.url, - Some(Arc::clone(&configuration)), + Some(Rc::clone(&configuration)), false, span, |visitor, module, _| { @@ -755,7 +755,7 @@ impl<'a> Visitor<'a> { } pub(crate) fn assert_configuration_is_empty( - config: &Arc>, + config: &Rc>, name_in_error: bool, ) -> SassResult<()> { let config = (**config).borrow(); @@ -960,7 +960,7 @@ impl<'a> Visitor<'a> { self.with_environment::, _>(env.clone(), |visitor| { let old_parent = visitor.parent; - let old_configuration = Arc::clone(&visitor.configuration); + let old_configuration = Rc::clone(&visitor.configuration); if loads_user_defined_modules { visitor.parent = Some(CssTree::ROOT); @@ -969,7 +969,7 @@ impl<'a> Visitor<'a> { // This configuration is only used if it passes through a `@forward` // rule, so we avoid creating unnecessary ones for performance reasons. if !stylesheet.forwards.is_empty() { - visitor.configuration = Arc::new(RefCell::new(env.to_implicit_configuration())); + visitor.configuration = Rc::new(RefCell::new(env.to_implicit_configuration())); } visitor.visit_stylesheet(stylesheet)?; @@ -2284,7 +2284,7 @@ impl<'a> Visitor<'a> { visitor.env.scopes_mut().insert_var_last(name, value); } - let were_keywords_accessed = Arc::new(Cell::new(false)); + let were_keywords_accessed = Rc::new(Cell::new(false)); let num_named_args = evaluated.named.len(); @@ -2297,7 +2297,7 @@ impl<'a> Visitor<'a> { let arg_list = Value::ArgList(ArgList::new( rest, - Arc::clone(&were_keywords_accessed), + Rc::clone(&were_keywords_accessed), // todo: superfluous clone evaluated.named.clone(), if evaluated.separator == ListSeparator::Undecided { diff --git a/crates/compiler/src/lib.rs b/crates/compiler/src/lib.rs index d801063..cb31238 100644 --- a/crates/compiler/src/lib.rs +++ b/crates/compiler/src/lib.rs @@ -47,21 +47,14 @@ grass input.scss clippy::multiple_crate_versions, clippy::wrong_self_convention, clippy::comparison_chain, - - // these features are only available on nightly - clippy::unnested_or_patterns, - clippy::uninlined_format_args, + clippy::unwrap_or_default, + clippy::manual_unwrap_or_default, // todo: these should be enabled - clippy::cast_sign_loss, - clippy::cast_lossless, - clippy::cast_precision_loss, - clippy::float_cmp, + clippy::arc_with_non_send_sync, // todo: unignore once we bump MSRV - clippy::format_push_string, - clippy::unnecessary_unwrap, - clippy::needless_late_init, + clippy::assigning_clones, unknown_lints, )] diff --git a/crates/compiler/src/parse/keyframes.rs b/crates/compiler/src/parse/keyframes.rs index de11160..b09c966 100644 --- a/crates/compiler/src/parse/keyframes.rs +++ b/crates/compiler/src/parse/keyframes.rs @@ -18,7 +18,7 @@ pub(crate) struct KeyframesSelectorParser { toks: Lexer, } -impl<'a> BaseParser for KeyframesSelectorParser { +impl BaseParser for KeyframesSelectorParser { fn toks(&self) -> &Lexer { &self.toks } diff --git a/crates/compiler/src/value/arglist.rs b/crates/compiler/src/value/arglist.rs index 25ea16d..0009102 100644 --- a/crates/compiler/src/value/arglist.rs +++ b/crates/compiler/src/value/arglist.rs @@ -1,4 +1,4 @@ -use std::{cell::Cell, collections::BTreeMap, sync::Arc}; +use std::{cell::Cell, collections::BTreeMap, rc::Rc}; use crate::common::{Identifier, ListSeparator}; @@ -7,7 +7,7 @@ use super::Value; #[derive(Debug, Clone)] pub struct ArgList { pub elems: Vec, - were_keywords_accessed: Arc>, + were_keywords_accessed: Rc>, // todo: special wrapper around this field to avoid having to make it private? keywords: BTreeMap, pub separator: ListSeparator, @@ -26,7 +26,7 @@ impl Eq for ArgList {} impl ArgList { pub fn new( elems: Vec, - were_keywords_accessed: Arc>, + were_keywords_accessed: Rc>, keywords: BTreeMap, separator: ListSeparator, ) -> Self { diff --git a/crates/compiler/src/value/calculation.rs b/crates/compiler/src/value/calculation.rs index edf73b2..7f62e2c 100644 --- a/crates/compiler/src/value/calculation.rs +++ b/crates/compiler/src/value/calculation.rs @@ -363,6 +363,7 @@ impl SassCalculation { BinaryOp::Plus } } else { + // todo: do we need this branch? } right = CalculationArg::Number(n); }