Skip to content

WIP: Remove unused variables #58

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
devongovett wants to merge 1 commit into from
Closed

WIP: Remove unused variables #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions node/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ struct Config {
pub css_modules: Option<bool>,
pub analyze_dependencies: Option<bool>,
pub pseudo_classes: Option<OwnedPseudoClasses>,
pub unused_symbols: Option<HashSet<String>>
pub unused_symbols: Option<HashSet<String>>,
pub remove_unused_vars: Option<bool>
}

#[derive(Debug, Deserialize)]
Expand Down Expand Up @@ -154,7 +155,8 @@ fn compile<'i>(code: &'i str, config: &Config) -> Result<TransformResult, Compil
Some(o) => o.nesting,
None => false
},
css_modules: config.css_modules.unwrap_or(false)
css_modules: config.css_modules.unwrap_or(false),
collect_used_vars: config.remove_unused_vars.unwrap_or(false)
})?;
stylesheet.minify(MinifyOptions {
targets: config.targets,
Expand Down
22 changes: 18 additions & 4 deletions src/declaration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use crate::targets::Browsers;
use crate::parser::ParserOptions;
use crate::error::{ParserError, PrinterError};
use crate::logical::LogicalProperties;
use std::collections::HashSet;

#[derive(Debug, PartialEq)]
pub struct DeclarationBlock {
Expand All @@ -34,12 +35,13 @@ pub struct DeclarationBlock {
}

impl DeclarationBlock {
pub fn parse<'i, 't>(input: &mut Parser<'i, 't>, options: &ParserOptions) -> Result<Self, ParseError<'i, ParserError<'i>>> {
pub fn parse<'i, 't>(input: &mut Parser<'i, 't>, options: &ParserOptions, used_vars: &mut Option<HashSet<String>>) -> Result<Self, ParseError<'i, ParserError<'i>>> {
let mut important_declarations = DeclarationList::new();
let mut declarations = DeclarationList::new();
let mut parser = DeclarationListParser::new(input, PropertyDeclarationParser {
important_declarations: &mut important_declarations,
declarations: &mut declarations,
used_vars,
options
});
while let Some(res) = parser.next() {
Expand Down Expand Up @@ -91,14 +93,22 @@ impl DeclarationBlock {
&mut self,
handler: &mut DeclarationHandler,
important_handler: &mut DeclarationHandler,
logical_properties: &mut LogicalProperties
logical_properties: &mut LogicalProperties,
used_vars: &Option<HashSet<String>>
) {
macro_rules! handle {
($decls: expr, $handler: expr) => {
for decl in $decls.iter() {
let handled = $handler.handle_property(decl, logical_properties);

if !handled {
if let Some(used_vars) = &used_vars {
if let Property::Custom(custom) = &decl {
if custom.name.starts_with("--") && !used_vars.contains(&custom.name) {
continue
}
}
}
$handler.decls.push(decl.clone());
}
}
Expand All @@ -118,6 +128,7 @@ impl DeclarationBlock {
struct PropertyDeclarationParser<'a> {
important_declarations: &'a mut Vec<Property>,
declarations: &'a mut Vec<Property>,
used_vars: &'a mut Option<HashSet<String>>,
options: &'a ParserOptions
}

Expand All @@ -131,7 +142,7 @@ impl<'a, 'i> cssparser::DeclarationParser<'i> for PropertyDeclarationParser<'a>
name: CowRcStr<'i>,
input: &mut cssparser::Parser<'i, 't>,
) -> Result<Self::Declaration, cssparser::ParseError<'i, Self::Error>> {
parse_declaration(name, input, &mut self.declarations, &mut self.important_declarations, &self.options)
parse_declaration(name, input, &mut self.declarations, &mut self.important_declarations, self.used_vars, &self.options)
}
}

Expand All @@ -147,9 +158,12 @@ pub(crate) fn parse_declaration<'i, 't>(
input: &mut cssparser::Parser<'i, 't>,
declarations: &mut DeclarationList,
important_declarations: &mut DeclarationList,
used_vars: &mut Option<HashSet<String>>,
options: &ParserOptions
) -> Result<(), cssparser::ParseError<'i, ParserError<'i>>> {
let property = input.parse_until_before(Delimiter::Bang, |input| Property::parse(name, input, options))?;
let property = input.parse_until_before(Delimiter::Bang, |input|
Property::parse(name, input, options, used_vars)
)?;
let important = input.try_parse(|input| {
input.expect_delim('!')?;
input.expect_ident_matching("important")
Expand Down
52 changes: 52 additions & 0 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ mod tests {
assert_eq!(res.exports.unwrap(), expected_exports);
}

fn unused_vars_test(source: &str, expected: &str) {
let mut stylesheet = StyleSheet::parse("test.css".into(), source, ParserOptions { collect_used_vars: true, ..ParserOptions::default()}).unwrap();
stylesheet.minify(MinifyOptions::default());
let res = stylesheet.to_css(PrinterOptions { minify: true, ..PrinterOptions::default() }).unwrap();
assert_eq!(res.code, expected);
}

macro_rules! map(
{ $($key:expr => $name:literal $(referenced: $referenced: literal)? $($value:literal $(global: $global: literal)? $(from $from:literal)?)*),* } => {
{
Expand Down Expand Up @@ -9200,4 +9207,49 @@ mod tests {
width: device-width;
}"#, "@-ms-viewport{width:device-width}");
}

#[test]
fn test_removed_unused_vars() {
unused_vars_test(r#"
.foo {
--used: blue;
--unused: red;
}

.bar {
color: var(--used);
}
"#, ".foo{--used:blue}.bar{color:var(--used)}");
unused_vars_test(r#"
.foo {
--used: blue;
--unused: red;
}

.bar {
color: var(--unknown, var(--used));
}
"#, ".foo{--used:blue}.bar{color:var(--unknown, var(--used))}");
unused_vars_test(r#"
.foo {
--used: blue;
--unused: red;
}

.bar {
color: yo var(--unknown, yo var(--used));
}
"#, ".foo{--used:blue}.bar{color:yo var(--unknown, yo var(--used))}");
unused_vars_test(r#"
.foo {
--used: blue;
--unused: red;
--foo: var(--used);
}

.bar {
color: var(--foo);
}
"#, ".foo{--used:blue;--foo:var(--used)}.bar{color:var(--foo)}");
}
}
47 changes: 29 additions & 18 deletions src/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,27 +22,30 @@ use crate::rules::{
use crate::values::ident::CustomIdent;
use crate::declaration::{DeclarationBlock, DeclarationList, parse_declaration};
use crate::vendor_prefix::VendorPrefix;
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use crate::error::ParserError;

#[derive(Default)]
pub struct ParserOptions {
pub nesting: bool,
pub css_modules: bool
pub css_modules: bool,
pub collect_used_vars: bool
}

/// The parser for the top-level rules in a stylesheet.
pub struct TopLevelRuleParser<'a> {
default_namespace: Option<String>,
namespace_prefixes: HashMap<String, String>,
used_vars: &'a mut Option<HashSet<String>>,
options: &'a ParserOptions
}

impl<'a, 'b> TopLevelRuleParser<'a> {
pub fn new(options: &'a ParserOptions) -> TopLevelRuleParser<'a> {
pub fn new(options: &'a ParserOptions, used_vars: &'a mut Option<HashSet<String>>) -> TopLevelRuleParser<'a> {
TopLevelRuleParser {
default_namespace: None,
namespace_prefixes: HashMap::new(),
used_vars,
options
}
}
Expand All @@ -51,6 +54,7 @@ impl<'a, 'b> TopLevelRuleParser<'a> {
NestedRuleParser {
default_namespace: &mut self.default_namespace,
namespace_prefixes: &mut self.namespace_prefixes,
used_vars: self.used_vars,
options: &self.options
}
}
Expand Down Expand Up @@ -203,10 +207,10 @@ impl<'a, 'i> QualifiedRuleParser<'i> for TopLevelRuleParser<'a> {
}
}

#[derive(Clone)]
struct NestedRuleParser<'a> {
default_namespace: &'a Option<String>,
namespace_prefixes: &'a HashMap<String, String>,
used_vars: &'a mut Option<HashSet<String>>,
options: &'a ParserOptions
}

Expand All @@ -215,6 +219,7 @@ impl<'a, 'b> NestedRuleParser<'a> {
let nested_parser = NestedRuleParser {
default_namespace: self.default_namespace,
namespace_prefixes: self.namespace_prefixes,
used_vars: self.used_vars,
options: self.options
};

Expand Down Expand Up @@ -354,7 +359,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a> {
AtRulePrelude::CounterStyle(name) => {
Ok(CssRule::CounterStyle(CounterStyleRule {
name,
declarations: DeclarationBlock::parse(input, self.options)?,
declarations: DeclarationBlock::parse(input, self.options, &mut None)?,
loc
}))
},
Expand All @@ -377,12 +382,14 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a> {
vendor_prefix,
// TODO: parse viewport descriptors rather than properties
// https://drafts.csswg.org/css-device-adapt/#viewport-desc
declarations: DeclarationBlock::parse(input, self.options)?,
declarations: DeclarationBlock::parse(input, self.options, &mut None)?,
loc
}))
},
AtRulePrelude::Keyframes(name, vendor_prefix) => {
let iter = RuleListParser::new_for_nested_rule(input, KeyframeListParser);
let iter = RuleListParser::new_for_nested_rule(input, KeyframeListParser {
used_vars: self.used_vars
});
Ok(CssRule::Keyframes(KeyframesRule {
name,
keyframes: iter.filter_map(Result::ok).collect(),
Expand All @@ -393,7 +400,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a> {
AtRulePrelude::Page(selectors) => {
Ok(CssRule::Page(PageRule {
selectors,
declarations: DeclarationBlock::parse(input, self.options)?,
declarations: DeclarationBlock::parse(input, self.options, &mut None)?,
loc
}))
},
Expand Down Expand Up @@ -438,9 +445,9 @@ impl<'a, 'b, 'i> QualifiedRuleParser<'i> for NestedRuleParser<'a> {
) -> Result<CssRule, ParseError<'i, Self::Error>> {
let loc = start.source_location();
let (declarations, rules) = if self.options.nesting {
parse_declarations_and_nested_rules(input, self.default_namespace, self.namespace_prefixes, self.options)?
parse_declarations_and_nested_rules(input, self.default_namespace, self.namespace_prefixes, self.used_vars, self.options)?
} else {
(DeclarationBlock::parse(input, self.options)?, CssRuleList(vec![]))
(DeclarationBlock::parse(input, self.options, self.used_vars)?, CssRuleList(vec![]))
};
Ok(CssRule::Style(StyleRule {
selectors,
Expand All @@ -456,6 +463,7 @@ fn parse_declarations_and_nested_rules<'a, 'i, 't>(
input: &mut Parser<'i, 't>,
default_namespace: &'a Option<String>,
namespace_prefixes: &'a HashMap<String, String>,
used_vars: &'a mut Option<HashSet<String>>,
options: &'a ParserOptions
) -> Result<(DeclarationBlock, CssRuleList), ParseError<'i, ParserError<'i>>> {
let mut important_declarations = DeclarationList::new();
Expand All @@ -467,7 +475,8 @@ fn parse_declarations_and_nested_rules<'a, 'i, 't>(
options,
declarations: &mut declarations,
important_declarations: &mut important_declarations,
rules: &mut rules
rules: &mut rules,
used_vars
};

let mut declaration_parser = DeclarationListParser::new(input, parser);
Expand Down Expand Up @@ -500,7 +509,8 @@ pub struct StyleRuleParser<'a> {
options: &'a ParserOptions,
declarations: &'a mut DeclarationList,
important_declarations: &'a mut DeclarationList,
rules: &'a mut CssRuleList
rules: &'a mut CssRuleList,
used_vars: &'a mut Option<HashSet<String>>
}

/// Parse a declaration within {} block: `color: blue`
Expand All @@ -517,7 +527,7 @@ impl<'a, 'i> cssparser::DeclarationParser<'i> for StyleRuleParser<'a> {
// Declarations cannot come after nested rules.
return Err(input.new_custom_error(ParserError::InvalidNesting))
}
parse_declaration(name, input, &mut self.declarations, &mut self.important_declarations, &self.options)
parse_declaration(name, input, &mut self.declarations, &mut self.important_declarations, self.used_vars, &self.options)
}
}

Expand Down Expand Up @@ -565,21 +575,21 @@ impl<'a, 'i> AtRuleParser<'i> for StyleRuleParser<'a> {
AtRulePrelude::Media(query) => {
self.rules.0.push(CssRule::Media(MediaRule {
query,
rules: parse_nested_at_rule(input, self.default_namespace, self.namespace_prefixes, self.options)?,
rules: parse_nested_at_rule(input, self.default_namespace, self.namespace_prefixes, self.used_vars, self.options)?,
loc
}));
Ok(())
},
AtRulePrelude::Supports(condition) => {
self.rules.0.push(CssRule::Supports(SupportsRule {
condition,
rules: parse_nested_at_rule(input, self.default_namespace, self.namespace_prefixes, self.options)?,
rules: parse_nested_at_rule(input, self.default_namespace, self.namespace_prefixes, self.used_vars, self.options)?,
loc
}));
Ok(())
},
AtRulePrelude::Nest(selectors) => {
let (declarations, rules) = parse_declarations_and_nested_rules(input, self.default_namespace, self.namespace_prefixes, self.options)?;
let (declarations, rules) = parse_declarations_and_nested_rules(input, self.default_namespace, self.namespace_prefixes, self.used_vars, self.options)?;
self.rules.0.push(CssRule::Nesting(NestingRule {
style: StyleRule {
selectors,
Expand All @@ -605,13 +615,14 @@ fn parse_nested_at_rule<'a, 'i, 't>(
input: &mut Parser<'i, 't>,
default_namespace: &'a Option<String>,
namespace_prefixes: &'a HashMap<String, String>,
used_vars: &'a mut Option<HashSet<String>>,
options: &'a ParserOptions
) -> Result<CssRuleList, ParseError<'i, ParserError<'i>>> {
let loc = input.current_source_location();

// Declarations can be immediately within @media and @supports blocks that are nested within a parent style rule.
// These act the same way as if they were nested within a `& { ... }` block.
let (declarations, mut rules) = parse_declarations_and_nested_rules(input, default_namespace, namespace_prefixes, options)?;
let (declarations, mut rules) = parse_declarations_and_nested_rules(input, default_namespace, namespace_prefixes, used_vars, options)?;

if declarations.declarations.len() > 0 {
rules.0.insert(0, CssRule::Style(StyleRule {
Expand Down Expand Up @@ -651,7 +662,7 @@ impl<'a, 'b, 'i> QualifiedRuleParser<'i> for StyleRuleParser<'a> {
input: &mut Parser<'i, 't>,
) -> Result<(), ParseError<'i, Self::Error>> {
let loc = start.source_location();
let (declarations, rules) = parse_declarations_and_nested_rules(input, self.default_namespace, self.namespace_prefixes, self.options)?;
let (declarations, rules) = parse_declarations_and_nested_rules(input, self.default_namespace, self.namespace_prefixes, self.used_vars, self.options)?;
self.rules.0.push(CssRule::Style(StyleRule {
selectors,
vendor_prefix: VendorPrefix::empty(),
Expand Down
Loading