refactor: remove LinterBuilder (#1296)

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::<HashSet<_>>();
@@ -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()

src/context.rs

@@ -49,7 +49,7 @@ impl<'a> Context<'a> {
     default_jsx_fragment_factory: Option<String>,
   ) -> 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);

src/lib.rs

@@ -40,11 +40,14 @@ mod lint_tests {
   fn lint(
     source: &str,
     rules: Vec<Box<dyn LintRule>>,
-    all_rule_names: HashSet<&'static str>,
+    all_rule_codes: HashSet<&'static str>,
   ) -> Vec<LintDiagnostic> {
-    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<Box<dyn LintRule>>,
-    all_rule_names: HashSet<&'static str>,
+    all_rule_codes: HashSet<&'static str>,
   ) -> Vec<LintDiagnostic> {
-    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 {

src/linter.rs

@@ -10,103 +10,56 @@ 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<Box<dyn LintRule>>,
-  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<Box<dyn LintRule>>,
-    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<Box<dyn LintRule>>,
+  /// 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<LinterContext>,
+  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<Box<dyn LintRule>>,
   pub all_rule_codes: HashSet<&'static str>,
 }
 
 impl LinterContext {
-  fn new(
-    ignore_file_directive: String,
-    ignore_diagnostic_directive: String,
-    mut rules: Vec<Box<dyn LintRule>>,
-    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<Box<dyn LintRule>>,
-    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![];

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<LintDiagnostic>) {
-  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);