From b135b87a69a4b14fe01faf5017e78e9134325263 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Sun, 7 Jun 2020 23:11:43 -0400 Subject: [PATCH] resolve clippy lints --- src/builtin/selector.rs | 8 +- src/lib.rs | 3 + src/selector/compound.rs | 2 +- src/selector/extend/extension.rs | 2 +- src/selector/extend/functions.rs | 4 +- src/selector/extend/mod.rs | 155 +++++++++++++------------------ src/selector/mod.rs | 2 - src/selector/parse.rs | 6 +- src/selector/simple.rs | 5 +- 9 files changed, 81 insertions(+), 106 deletions(-) diff --git a/src/builtin/selector.rs b/src/builtin/selector.rs index 923a56b..6f99801 100644 --- a/src/builtin/selector.rs +++ b/src/builtin/selector.rs @@ -1,5 +1,3 @@ -#![allow(unused_variables, unused_mut)] - use super::{Builtin, GlobalFunctionMap}; use crate::args::CallArgs; @@ -107,11 +105,7 @@ fn selector_nest(args: CallArgs, scope: &Scope, super_selector: &Selector) -> Sa .into_value()) } -fn selector_append( - mut args: CallArgs, - scope: &Scope, - super_selector: &Selector, -) -> SassResult { +fn selector_append(args: CallArgs, scope: &Scope, super_selector: &Selector) -> SassResult { let span = args.span(); let selectors = args.get_variadic(scope, super_selector)?; if selectors.is_empty() { diff --git a/src/lib.rs b/src/lib.rs index 82105d2..92aa6c7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -59,6 +59,9 @@ grass input.scss clippy::filter_map, clippy::else_if_without_else, clippy::new_ret_no_self, + renamed_and_removed_lints, + clippy::unknown_clippy_lints, + clippy::replace_consts, // temporarily allowed while under heavy development. // eventually these allows should be refactored away diff --git a/src/selector/compound.rs b/src/selector/compound.rs index 9b398d5..c3f3612 100644 --- a/src/selector/compound.rs +++ b/src/selector/compound.rs @@ -214,7 +214,7 @@ impl CompoundSelector { Self { components } } - simple => { + _ => { let mut components = vec![SimpleSelector::Parent(None)]; components.append(&mut self.components); Self { components } diff --git a/src/selector/extend/extension.rs b/src/selector/extend/extension.rs index 095a145..6560da3 100644 --- a/src/selector/extend/extension.rs +++ b/src/selector/extend/extension.rs @@ -35,7 +35,7 @@ pub(super) struct Extension { impl Extension { pub fn one_off(extender: ComplexSelector, specificity: Option, is_original: bool) -> Self { Self { - specificity: specificity.unwrap_or(extender.max_specificity()), + specificity: specificity.unwrap_or_else(|| extender.max_specificity()), extender, target: None, span: None, diff --git a/src/selector/extend/functions.rs b/src/selector/extend/functions.rs index 64a5d3c..5527dc5 100644 --- a/src/selector/extend/functions.rs +++ b/src/selector/extend/functions.rs @@ -729,11 +729,11 @@ fn complex_is_parent_superselector( /// /// For example, given `[[1, 2], [3, 4], [5]]`, this returns: /// -/// ```norun +/// ```no_run /// [[1, 3, 5], /// [2, 3, 5], /// [1, 4, 5], -/// [2, 4, 5]] +/// [2, 4, 5]]; /// ``` pub(crate) fn paths(choices: Vec>) -> Vec> { choices.into_iter().fold(vec![vec![]], |paths, choice| { diff --git a/src/selector/extend/mod.rs b/src/selector/extend/mod.rs index 70f7e90..b2afa00 100644 --- a/src/selector/extend/mod.rs +++ b/src/selector/extend/mod.rs @@ -38,7 +38,13 @@ enum ExtendMode { AllTargets, } -#[derive(Clone, Debug, Eq, PartialEq)] +impl Default for ExtendMode { + fn default() -> Self { + Self::Normal + } +} + +#[derive(Clone, Debug, Eq, PartialEq, Default)] pub(crate) struct Extender { /// A map from all simple selectors in the stylesheet to the selector lists /// that contain them. @@ -88,6 +94,7 @@ pub(crate) struct Extender { impl Extender { /// An `Extender` that contains no extensions and can have no extensions added. // TODO: empty extender + #[allow(dead_code)] const EMPTY: () = (); pub fn extend( @@ -112,7 +119,7 @@ impl Extender { targets: SelectorList, mode: ExtendMode, ) -> SelectorList { - let mut extenders: IndexMap = source + let extenders: IndexMap = source .components .clone() .into_iter() @@ -149,7 +156,7 @@ impl Extender { .extend(selector.components.clone().into_iter()); } - selector = extender.extend_list(selector, extensions, None); + selector = extender.extend_list(selector, &extensions, &None); } selector @@ -158,12 +165,7 @@ impl Extender { fn with_mode(mode: ExtendMode) -> Self { Self { mode, - selectors: Default::default(), - extensions: Default::default(), - extensions_by_extender: Default::default(), - media_contexts: Default::default(), - source_specificity: Default::default(), - originals: Default::default(), + ..Extender::default() } } @@ -171,8 +173,8 @@ impl Extender { fn extend_list( &mut self, list: SelectorList, - extensions: HashMap>, - media_query_context: Option>, + extensions: &HashMap>, + media_query_context: &Option>, ) -> SelectorList { // This could be written more simply using Vec>, but we want to avoid // any allocations in the common case where no extends apply. @@ -180,21 +182,15 @@ impl Extender { for i in 0..list.components.len() { let complex = list.components.get(i).unwrap().clone(); - if let Some(result) = self.extend_complex( - complex.clone(), - extensions.clone(), - media_query_context.clone(), - ) { - if extended.is_empty() { - if i != 0 { - extended = list.components[0..i].to_vec(); - } + if let Some(result) = + self.extend_complex(complex.clone(), extensions, media_query_context) + { + if extended.is_empty() && i != 0 { + extended = list.components[0..i].to_vec(); } extended.extend(result.into_iter()); - } else { - if !extended.is_empty() { - extended.push(complex); - } + } else if !extended.is_empty() { + extended.push(complex); } } @@ -212,8 +208,8 @@ impl Extender { fn extend_complex( &mut self, complex: ComplexSelector, - extensions: HashMap>, - media_query_context: Option>, + extensions: &HashMap>, + media_query_context: &Option>, ) -> Option> { // The complex selectors that each compound selector in `complex.components` // can expand to. @@ -236,16 +232,11 @@ impl Extender { let complex_has_line_break = complex.line_break; - let is_original = self.originals.contains(&complex); - for i in 0..complex.components.len() { if let Some(ComplexSelectorComponent::Compound(component)) = complex.components.get(i) { - if let Some(extended) = self.extend_compound( - component.clone(), - extensions.clone(), - media_query_context.clone(), - is_original, - ) { + if let Some(extended) = + self.extend_compound(component, extensions, media_query_context) + { if extended_not_expanded.is_empty() { extended_not_expanded = complex .components @@ -311,15 +302,11 @@ impl Extender { /// Extends `compound` using `extensions`, and returns the contents of a /// `SelectorList`. - /// - /// The `in_original` parameter indicates whether this is in an original - /// complex selector, meaning that `compound` should not be trimmed out. fn extend_compound( &mut self, - compound: CompoundSelector, - extensions: HashMap>, - media_query_context: Option>, - in_original: bool, + compound: &CompoundSelector, + extensions: &HashMap>, + media_query_context: &Option>, ) -> Option> { // If there's more than one target and they all need to match, we track // which targets are actually extended. @@ -332,16 +319,14 @@ impl Extender { if let Some(extended) = self.extend_simple( simple.clone(), - extensions.clone(), - media_query_context.clone(), + extensions, + media_query_context, &mut targets_used, ) { - if options.is_empty() { - if i != 0 { - options.push(vec![self.extension_for_compound( - compound.components.clone().into_iter().take(i).collect(), - )]); - } + if options.is_empty() && i != 0 { + options.push(vec![self.extension_for_compound( + compound.components.clone().into_iter().take(i).collect(), + )]); } options.extend(extended.into_iter()); @@ -356,7 +341,7 @@ impl Extender { // If `self.mode` isn't `ExtendMode::Normal` and we didn't use all the targets in // `extensions`, extension fails for `compound`. - if targets_used.len() > 0 && targets_used.len() != extensions.len() { + if !targets_used.is_empty() && targets_used.len() != extensions.len() { return None; } @@ -369,7 +354,7 @@ impl Extender { .clone() .into_iter() .map(|state| { - state.assert_compatible_media_context(&media_query_context); + state.assert_compatible_media_context(media_query_context); state.extender }) .collect(), @@ -405,15 +390,13 @@ impl Extender { let unified_paths: Vec>> = paths(options) .into_iter() .map(|path| { - let complexes: Vec>; - - if first { + let complexes: Vec> = if first { // The first path is always the original selector. We can't just // return `compound` directly because pseudo selectors may be // modified, but we don't have to do any unification. first = false; - complexes = vec![vec![ComplexSelectorComponent::Compound(CompoundSelector { + vec![vec![ComplexSelectorComponent::Compound(CompoundSelector { components: path .clone() .into_iter() @@ -425,7 +408,7 @@ impl Extender { } }) .collect(), - })]]; + })]] } else { let mut to_unify: VecDeque> = VecDeque::new(); let mut originals: Vec = Vec::new(); @@ -448,13 +431,13 @@ impl Extender { )]); } - complexes = unify_complex(Vec::from(to_unify))?; - } + unify_complex(Vec::from(to_unify))? + }; let mut line_break = false; for state in path { - state.assert_compatible_media_context(&media_query_context); + state.assert_compatible_media_context(media_query_context); line_break = line_break || state.extender.line_break; } @@ -482,8 +465,8 @@ impl Extender { fn extend_simple( &mut self, simple: SimpleSelector, - extensions: HashMap>, - media_query_context: Option>, + extensions: &HashMap>, + media_query_context: &Option>, targets_used: &mut HashSet, ) -> Option>> { if let SimpleSelector::Pseudo( @@ -492,16 +475,14 @@ impl Extender { }, ) = simple.clone() { - if let Some(extended) = - self.extend_pseudo(simple, extensions.clone(), media_query_context) - { + if let Some(extended) = self.extend_pseudo(simple, extensions, media_query_context) { return Some( extended .into_iter() .map(move |pseudo| { self.without_pseudo( SimpleSelector::Pseudo(pseudo.clone()), - extensions.clone(), + extensions, targets_used, self.mode, ) @@ -523,14 +504,11 @@ impl Extender { fn extend_pseudo( &mut self, pseudo: Pseudo, - extensions: HashMap>, - media_query_context: Option>, + extensions: &HashMap>, + media_query_context: &Option>, ) -> Option> { let extended = self.extend_list( - pseudo - .selector - .clone() - .unwrap_or_else(|| SelectorList::new()), + pseudo.selector.clone().unwrap_or_else(SelectorList::new), extensions, media_query_context, ); @@ -544,8 +522,7 @@ impl Extender { // writing. We can keep them if either the original selector had a complex // selector, or the result of extending has only complex selectors, because // either way we aren't breaking anything that isn't already broken. - let mut complexes = extended.components.clone(); - if pseudo.normalized_name == "not" + let mut complexes = if pseudo.normalized_name == "not" && !pseudo .selector .clone() @@ -558,13 +535,14 @@ impl Extender { .iter() .any(|complex| complex.components.len() == 1) { - complexes = extended + extended .components - .clone() .into_iter() .filter(|complex| complex.components.len() <= 1) - .collect(); - } + .collect() + } else { + extended.components + }; complexes = complexes .into_iter() @@ -598,10 +576,10 @@ impl Extender { // become `.foo:not(.bar)`. However, this is a narrow edge case and // supporting it properly would make this code and the code calling it // a lot more complicated, so it's not supported for now. - if inner_pseudo.normalized_name != "matches" { - Vec::new() - } else { + if inner_pseudo.normalized_name == "matches" { inner_pseudo.selector.clone().unwrap().components + } else { + Vec::new() } } "matches" | "any" | "current" | "nth-child" | "nth-last-child" => { @@ -657,7 +635,7 @@ impl Extender { fn without_pseudo( &self, simple: SimpleSelector, - extensions: HashMap>, + extensions: &HashMap>, targets_used: &mut HashSet, mode: ExtendMode, ) -> Option> { @@ -782,17 +760,19 @@ impl Extender { // ensures that we aren't comparing against a selector that's already been // trimmed, and thus that if there are two identical selectors only one is // trimmed. - if result.iter().any(|complex2| { + let should_continue = result.iter().any(|complex2| { complex2.min_specificity() >= max_specificity && complex2.is_super_selector(complex1) - }) { + }); + if should_continue { continue; } - if selectors.iter().take(i).any(|complex2| { + let should_continue = selectors.iter().take(i).any(|complex2| { complex2.min_specificity() >= max_specificity && complex2.is_super_selector(complex1) - }) { + }); + if should_continue { continue; } @@ -800,12 +780,11 @@ impl Extender { } if should_break_to_outer { continue; - } else { - break; } + break; } - return Vec::from(result); + Vec::from(result) } } diff --git a/src/selector/mod.rs b/src/selector/mod.rs index 53facb4..94becaa 100644 --- a/src/selector/mod.rs +++ b/src/selector/mod.rs @@ -1,5 +1,3 @@ -#![allow(dead_code, unused_variables, unused_mut)] - use std::fmt::{self, Display}; use peekmore::{PeekMore, PeekMoreIterator}; diff --git a/src/selector/parse.rs b/src/selector/parse.rs index e2ff283..e0bd091 100644 --- a/src/selector/parse.rs +++ b/src/selector/parse.rs @@ -335,7 +335,7 @@ impl<'a, I: Iterator> SelectorParser<'a, I> { devour_whitespace(self.toks); self.expect_closing_paren()?; } else { - argument = Some(self.declaration_value(true)?); + argument = Some(self.declaration_value()?); } } else if SELECTOR_PSEUDO_CLASSES.contains(&unvendored) { selector = Some(self.parse_selector_list()?); @@ -358,7 +358,7 @@ impl<'a, I: Iterator> SelectorParser<'a, I> { self.expect_closing_paren()?; argument = Some(this_arg); } else { - argument = Some(self.declaration_value(true)?.trim_end().to_string()); + argument = Some(self.declaration_value()?.trim_end().to_string()); } Ok(SimpleSelector::Pseudo(Pseudo { @@ -533,7 +533,7 @@ impl<'a, I: Iterator> SelectorParser<'a, I> { Ok(buf) } - fn declaration_value(&mut self, allow_empty: bool) -> SassResult { + fn declaration_value(&mut self) -> SassResult { // todo: this consumes the closing paren let mut tmp = read_until_closing_paren(self.toks)?; if let Some(Token { kind: ')', .. }) = tmp.pop() { diff --git a/src/selector/simple.rs b/src/selector/simple.rs index 2daac6e..1430de2 100644 --- a/src/selector/simple.rs +++ b/src/selector/simple.rs @@ -567,6 +567,7 @@ impl Pseudo { } } + #[allow(clippy::missing_const_for_fn)] pub fn with_selector(self, selector: Option) -> Self { Self { selector, ..self } } @@ -601,7 +602,7 @@ impl Pseudo { min = min.max(complex.min_specificity()); max = max.max(complex.max_specificity()); } - return Specificity { min, max }; + Specificity { min, max } } else { // This is higher than any selector's specificity can actually be. let mut min = BASE_SPECIFICITY.pow(3_u32); @@ -610,7 +611,7 @@ impl Pseudo { min = min.min(complex.min_specificity()); max = max.max(complex.max_specificity()); } - return Specificity { min, max }; + Specificity { min, max } } } }