From 28b0a25015ccb2c49066fbc045a4315ca0f94b39 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 23 Jul 2024 18:47:11 -0400 Subject: [PATCH] refactor: remove `LinterBuilder` (#1296) --- examples/dlint/main.rs | 14 +++-- src/context.rs | 2 +- src/lib.rs | 22 +++++--- src/linter.rs | 115 +++++++++++------------------------------ src/test_util.rs | 21 ++++---- 5 files changed, 64 insertions(+), 110 deletions(-) diff --git a/examples/dlint/main.rs b/examples/dlint/main.rs index fb870008..67c7a876 100644 --- a/examples/dlint/main.rs +++ b/examples/dlint/main.rs @@ -10,7 +10,8 @@ use deno_ast::MediaType; use deno_ast::ModuleSpecifier; use deno_lint::linter::LintConfig; use deno_lint::linter::LintFileOptions; -use deno_lint::linter::LinterBuilder; +use deno_lint::linter::Linter; +use deno_lint::linter::LinterOptions; use deno_lint::rules::get_all_rules; use deno_lint::rules::{filtered_rules, recommended_rules}; use log::debug; @@ -88,7 +89,7 @@ fn run_linter( let error_counts = Arc::new(AtomicUsize::new(0)); let all_rules = get_all_rules(); - let all_rule_names = all_rules + let all_rule_codes = all_rules .iter() .map(|rule| rule.code()) .collect::>(); @@ -106,9 +107,12 @@ fn run_linter( debug!("Configured rules: {}", rules.len()); } let file_diagnostics = Arc::new(Mutex::new(BTreeMap::new())); - let linter_builder = LinterBuilder::default().rules(rules, all_rule_names); - - let linter = linter_builder.build(); + let linter = Linter::new(LinterOptions { + rules, + all_rule_codes, + custom_ignore_file_directive: None, + custom_ignore_diagnostic_directive: None, + }); paths .par_iter() diff --git a/src/context.rs b/src/context.rs index 0fcb06d9..be54e64f 100644 --- a/src/context.rs +++ b/src/context.rs @@ -49,7 +49,7 @@ impl<'a> Context<'a> { default_jsx_fragment_factory: Option, ) -> Self { let line_ignore_directives = parse_line_ignore_directives( - &linter_ctx.ignore_diagnostic_directive, + linter_ctx.ignore_diagnostic_directive, program, ); let scope = Scope::analyze(program); diff --git a/src/lib.rs b/src/lib.rs index e2ab6c57..ad2e550c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -40,11 +40,14 @@ mod lint_tests { fn lint( source: &str, rules: Vec>, - all_rule_names: HashSet<&'static str>, + all_rule_codes: HashSet<&'static str>, ) -> Vec { - let linter = LinterBuilder::default() - .rules(rules, all_rule_names) - .build(); + let linter = Linter::new(LinterOptions { + rules, + all_rule_codes, + custom_ignore_diagnostic_directive: None, + custom_ignore_file_directive: None, + }); let (_, diagnostics) = linter .lint_file(LintFileOptions { @@ -63,11 +66,14 @@ mod lint_tests { fn lint_with_ast( parsed_source: &ParsedSource, rules: Vec>, - all_rule_names: HashSet<&'static str>, + all_rule_codes: HashSet<&'static str>, ) -> Vec { - let linter = LinterBuilder::default() - .rules(rules, all_rule_names) - .build(); + let linter = Linter::new(LinterOptions { + rules, + all_rule_codes, + custom_ignore_diagnostic_directive: None, + custom_ignore_file_directive: None, + }); linter.lint_with_ast( parsed_source, LintConfig { diff --git a/src/linter.rs b/src/linter.rs index ceb06cff..4ed4ff21 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -10,79 +10,32 @@ use deno_ast::MediaType; use deno_ast::ParsedSource; use deno_ast::{ModuleSpecifier, ParseDiagnostic}; use std::collections::HashSet; -use std::sync::Arc; -pub struct LinterBuilder { - ignore_file_directive: String, - ignore_diagnostic_directive: String, - rules: Vec>, - all_codes: HashSet<&'static str>, -} - -impl Default for LinterBuilder { - fn default() -> Self { - Self { - ignore_file_directive: "deno-lint-ignore-file".to_string(), - ignore_diagnostic_directive: "deno-lint-ignore".to_string(), - rules: Vec::new(), - all_codes: Default::default(), - } - } -} - -impl LinterBuilder { - pub fn build(self) -> Linter { - Linter::new( - self.ignore_file_directive, - self.ignore_diagnostic_directive, - self.rules, - self.all_codes, - ) - } - - /// Set name for directive that can be used to skip linting file. - /// - /// Defaults to "deno-lint-ignore-file". - pub fn ignore_file_directive(mut self, directive: &str) -> Self { - directive.clone_into(&mut self.ignore_file_directive); - self - } - - /// Set name for directive that can be used to ignore next line. - /// - /// Defaults to "deno-lint-ignore". - pub fn ignore_diagnostic_directive(mut self, directive: &str) -> Self { - directive.clone_into(&mut self.ignore_diagnostic_directive); - self - } - - /// Set a list of rules that will be used for linting. - /// - /// Defaults to empty list (no rules will be run by default). - pub fn rules( - mut self, - rules: Vec>, - all_codes: HashSet<&'static str>, - ) -> Self { - self.rules = rules; - self.all_codes = all_codes; - self - } +pub struct LinterOptions { + /// Rules to lint with. + pub rules: Vec>, + /// Collection of all the lint rule codes. + pub all_rule_codes: HashSet<&'static str>, + /// Defaults to "deno-lint-ignore-file" + pub custom_ignore_file_directive: Option<&'static str>, + /// Defaults to "deno-lint-ignore" + pub custom_ignore_diagnostic_directive: Option<&'static str>, } -/// A linter instance. It can be cheaply cloned and shared between threads. -#[derive(Clone)] +/// A linter instance. +#[derive(Debug)] pub struct Linter { - ctx: Arc, + ctx: LinterContext, } /// A struct defining configuration of a `Linter` instance. /// /// This struct is passed along and used to construct a specific file context, /// just before a particular file is linted. -pub struct LinterContext { - pub ignore_file_directive: String, - pub ignore_diagnostic_directive: String, +#[derive(Debug)] +pub(crate) struct LinterContext { + pub ignore_file_directive: &'static str, + pub ignore_diagnostic_directive: &'static str, pub check_unknown_rules: bool, /// Rules are sorted by priority pub rules: Vec>, @@ -90,23 +43,23 @@ pub struct LinterContext { } impl LinterContext { - fn new( - ignore_file_directive: String, - ignore_diagnostic_directive: String, - mut rules: Vec>, - all_rule_codes: HashSet<&'static str>, - ) -> Self { + fn new(options: LinterOptions) -> Self { + let mut rules = options.rules; crate::rules::sort_rules_by_priority(&mut rules); let check_unknown_rules = rules .iter() .any(|a| a.code() == (BanUnknownRuleCode).code()); LinterContext { - ignore_file_directive, - ignore_diagnostic_directive, + ignore_file_directive: options + .custom_ignore_file_directive + .unwrap_or("deno-lint-ignore-file"), + ignore_diagnostic_directive: options + .custom_ignore_file_directive + .unwrap_or("deno-lint-ignore"), check_unknown_rules, rules, - all_rule_codes, + all_rule_codes: options.all_rule_codes, } } } @@ -125,20 +78,10 @@ pub struct LintConfig { } impl Linter { - fn new( - ignore_file_directive: String, - ignore_diagnostic_directive: String, - rules: Vec>, - all_rule_codes: HashSet<&'static str>, - ) -> Self { - let ctx = LinterContext::new( - ignore_file_directive, - ignore_diagnostic_directive, - rules, - all_rule_codes, - ); + pub fn new(options: LinterOptions) -> Self { + let ctx = LinterContext::new(options); - Linter { ctx: Arc::new(ctx) } + Linter { ctx } } /// Lint a single file. @@ -226,7 +169,7 @@ impl Linter { // we're gonna check if the file should be ignored, before performing // other expensive work like scope or control-flow analysis. let file_ignore_directive = - parse_file_ignore_directives(&self.ctx.ignore_file_directive, pg); + parse_file_ignore_directives(self.ctx.ignore_file_directive, pg); if let Some(ignore_directive) = file_ignore_directive.as_ref() { if ignore_directive.ignore_all() { return vec![]; diff --git a/src/test_util.rs b/src/test_util.rs index eb3707d1..b685911c 100644 --- a/src/test_util.rs +++ b/src/test_util.rs @@ -4,7 +4,8 @@ use crate::ast_parser; use crate::diagnostic::LintDiagnostic; use crate::linter::LintConfig; use crate::linter::LintFileOptions; -use crate::linter::LinterBuilder; +use crate::linter::Linter; +use crate::linter::LinterOptions; use crate::rules::get_all_rules; use crate::rules::LintRule; use deno_ast::diagnostics::Diagnostic; @@ -320,15 +321,15 @@ fn lint( source: &str, specifier: &str, ) -> (ParsedSource, Vec) { - let linter = LinterBuilder::default() - .rules( - vec![rule], - get_all_rules() - .into_iter() - .map(|rule| rule.code()) - .collect(), - ) - .build(); + let linter = Linter::new(LinterOptions { + rules: vec![rule], + all_rule_codes: get_all_rules() + .into_iter() + .map(|rule| rule.code()) + .collect(), + custom_ignore_diagnostic_directive: None, + custom_ignore_file_directive: None, + }); let specifier = ModuleSpecifier::parse(specifier).unwrap(); let media_type = MediaType::from_specifier(&specifier);