Skip to content
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.

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

refactor: remove LinterBuilder #1296

Merged
merged 2 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 9 additions & 5 deletions examples/dlint/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<_>>();
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
22 changes: 14 additions & 8 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
115 changes: 29 additions & 86 deletions src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
}
Expand All @@ -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.
Expand Down Expand Up @@ -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![];
Expand Down
21 changes: 11 additions & 10 deletions src/test_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Loading