From b7de97ba396028c4e6b3a98af970f3994ce508ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 2 Jan 2025 23:06:49 +0100 Subject: [PATCH] feat: Add ExternalLinter API --- examples/dlint/main.rs | 3 ++- src/context.rs | 35 ++++++++++++++++------------ src/lib.rs | 13 +++++++---- src/linter.rs | 52 +++++++++++++++++++++++++++++++++++++----- src/test_util.rs | 3 ++- 5 files changed, 78 insertions(+), 28 deletions(-) diff --git a/examples/dlint/main.rs b/examples/dlint/main.rs index 67c7a876..c47774fb 100644 --- a/examples/dlint/main.rs +++ b/examples/dlint/main.rs @@ -91,7 +91,7 @@ fn run_linter( let all_rules = get_all_rules(); let all_rule_codes = all_rules .iter() - .map(|rule| rule.code()) + .map(|rule| rule.code().into()) .collect::>(); let rules = if let Some(config) = maybe_config { config.get_rules() @@ -134,6 +134,7 @@ fn run_linter( default_jsx_factory: Some("React.createElement".to_string()), default_jsx_fragment_factory: Some("React.Fragment".to_string()), }, + external_linter: None, })?; let mut number_of_errors = diagnostics.len(); diff --git a/src/context.rs b/src/context.rs index 23812967..f2a43e2c 100644 --- a/src/context.rs +++ b/src/context.rs @@ -9,7 +9,7 @@ use crate::ignore_directives::{ LineIgnoreDirective, }; use crate::linter::LinterContext; -use crate::rules::{self, LintRule}; +use crate::rules; use deno_ast::swc::ast::Expr; use deno_ast::swc::common::comments::Comment; use deno_ast::swc::common::util::take::Take; @@ -20,6 +20,7 @@ use deno_ast::{ }; use deno_ast::{MediaType, ModuleSpecifier}; use deno_ast::{MultiThreadedComments, Scope}; +use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::sync::Arc; @@ -33,7 +34,6 @@ pub struct Context<'a> { scope: Scope, control_flow: ControlFlow, traverse_flow: TraverseFlow, - all_rule_codes: &'a HashSet<&'static str>, check_unknown_rules: bool, #[allow(clippy::redundant_allocation)] // This type comes from SWC. jsx_factory: Option>>, @@ -112,7 +112,6 @@ impl<'a> Context<'a> { diagnostics: Vec::new(), traverse_flow: TraverseFlow::default(), check_unknown_rules: linter_ctx.check_unknown_rules, - all_rule_codes: &linter_ctx.all_rule_codes, jsx_factory, jsx_fragment_factory, } @@ -266,7 +265,7 @@ impl<'a> Context<'a> { /// works for diagnostics reported by other rules. pub(crate) fn ban_unused_ignore( &self, - specified_rules: &[Box], + known_rules_codes: &HashSet>, ) -> Vec { const CODE: &str = "ban-unused-ignore"; @@ -280,10 +279,8 @@ impl<'a> Context<'a> { return vec![]; } - let executed_builtin_codes: HashSet<&'static str> = - specified_rules.iter().map(|r| r.code()).collect(); let is_unused_code = |&(code, status): &(&String, &CodeStatus)| { - let is_unknown = !executed_builtin_codes.contains(code.as_str()); + let is_unknown = !known_rules_codes.contains(code.as_str()); !status.used && !is_unknown }; @@ -334,15 +331,17 @@ impl<'a> Context<'a> { // struct. /// Lint rule implementation for `ban-unknown-rule-code`. /// This should be run after all normal rules. - pub(crate) fn ban_unknown_rule_code(&mut self) -> Vec { - let is_unknown_rule = - |code: &&String| !self.all_rule_codes.contains(code.as_str()); - + pub(crate) fn ban_unknown_rule_code( + &mut self, + enabled_rules: &HashSet>, + ) -> Vec { let mut diagnostics = Vec::new(); if let Some(file_ignore) = self.file_ignore_directive.as_ref() { - for unknown_rule_code in - file_ignore.codes().keys().filter(is_unknown_rule) + for unknown_rule_code in file_ignore + .codes() + .keys() + .filter(|code| !enabled_rules.contains(code.as_str())) { let d = self.create_diagnostic( Some(self.create_diagnostic_range(file_ignore.range())), @@ -358,8 +357,10 @@ impl<'a> Context<'a> { } for line_ignore in self.line_ignore_directives.values() { - for unknown_rule_code in - line_ignore.codes().keys().filter(is_unknown_rule) + for unknown_rule_code in line_ignore + .codes() + .keys() + .filter(|code| !enabled_rules.contains(code.as_str())) { let d = self.create_diagnostic( Some(self.create_diagnostic_range(line_ignore.range())), @@ -451,6 +452,10 @@ impl<'a> Context<'a> { .push(self.create_diagnostic(maybe_range, details)); } + pub fn add_external_diagnostics(&mut self, diagnostics: &[LintDiagnostic]) { + self.diagnostics.extend_from_slice(diagnostics); + } + pub(crate) fn create_diagnostic( &self, maybe_range: Option, diff --git a/src/lib.rs b/src/lib.rs index ad2e550c..2f07c973 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,6 +28,7 @@ pub use deno_ast::view::ProgramRef; #[cfg(test)] mod lint_tests { + use std::borrow::Cow; use std::collections::HashSet; use crate::diagnostic::LintDiagnostic; @@ -40,7 +41,7 @@ mod lint_tests { fn lint( source: &str, rules: Vec>, - all_rule_codes: HashSet<&'static str>, + all_rule_codes: HashSet>, ) -> Vec { let linter = Linter::new(LinterOptions { rules, @@ -58,6 +59,7 @@ mod lint_tests { default_jsx_factory: None, default_jsx_fragment_factory: None, }, + external_linter: None, }) .expect("Failed to lint"); diagnostics @@ -66,7 +68,7 @@ mod lint_tests { fn lint_with_ast( parsed_source: &ParsedSource, rules: Vec>, - all_rule_codes: HashSet<&'static str>, + all_rule_codes: HashSet>, ) -> Vec { let linter = Linter::new(LinterOptions { rules, @@ -80,6 +82,7 @@ mod lint_tests { default_jsx_factory: None, default_jsx_fragment_factory: None, }, + None, ) } @@ -89,7 +92,7 @@ mod lint_tests { recommended_rules(get_all_rules()), get_all_rules() .into_iter() - .map(|rule| rule.code()) + .map(|rule| rule.code().into()) .collect(), ) } @@ -102,7 +105,7 @@ mod lint_tests { recommended_rules(get_all_rules()), get_all_rules() .into_iter() - .map(|rule| rule.code()) + .map(|rule| rule.code().into()) .collect(), ) } @@ -116,7 +119,7 @@ mod lint_tests { vec![rule], get_all_rules() .into_iter() - .map(|rule| rule.code()) + .map(|rule| rule.code().into()) .collect(), ) } diff --git a/src/linter.rs b/src/linter.rs index 2c5ebb79..a5a459cc 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -10,13 +10,15 @@ use deno_ast::diagnostics::Diagnostic; use deno_ast::MediaType; use deno_ast::ParsedSource; use deno_ast::{ModuleSpecifier, ParseDiagnostic}; +use std::borrow::Cow; use std::collections::HashSet; +use std::sync::Arc; pub struct LinterOptions { /// Rules to lint with. pub rules: Vec>, /// Collection of all the lint rule codes. - pub all_rule_codes: HashSet<&'static str>, + pub all_rule_codes: HashSet>, /// Defaults to "deno-lint-ignore-file" pub custom_ignore_file_directive: Option<&'static str>, /// Defaults to "deno-lint-ignore" @@ -40,7 +42,7 @@ pub(crate) struct LinterContext { pub check_unknown_rules: bool, /// Rules are sorted by priority pub rules: Vec>, - pub all_rule_codes: HashSet<&'static str>, + pub all_rule_codes: HashSet>, } impl LinterContext { @@ -65,11 +67,19 @@ impl LinterContext { } } +pub struct ExternalLinterResult { + pub diagnostics: Vec, + pub rules: Vec, +} +pub type ExternalLinterCb = + Arc Result>; + pub struct LintFileOptions { pub specifier: ModuleSpecifier, pub source_code: String, pub media_type: MediaType, pub config: LintConfig, + pub external_linter: Option, } #[derive(Debug, Clone)] @@ -107,6 +117,7 @@ impl Linter { &parsed_source, options.config.default_jsx_factory, options.config.default_jsx_fragment_factory, + options.external_linter, ); Ok((parsed_source, diagnostics)) @@ -120,26 +131,45 @@ impl Linter { &self, parsed_source: &ParsedSource, config: LintConfig, + maybe_external_linter: Option, ) -> Vec { let _mark = PerformanceMark::new("Linter::lint_with_ast"); self.lint_inner( parsed_source, config.default_jsx_factory, config.default_jsx_fragment_factory, + maybe_external_linter, ) } // TODO(bartlomieju): this struct does too much - not only it checks for ignored // lint rules, it also runs 2 additional rules. These rules should be rewritten // to use a regular way of writing a rule and not live on the `Context` struct. - fn collect_diagnostics(&self, mut context: Context) -> Vec { + fn collect_diagnostics( + &self, + mut context: Context, + external_rule_codes: Vec, + ) -> Vec { let _mark = PerformanceMark::new("Linter::collect_diagnostics"); let mut diagnostics = context.check_ignore_directive_usage(); + + let mut all_rules = self.ctx.all_rule_codes.clone(); + all_rules.extend( + external_rule_codes + .iter() + .map(|code| code.to_string().into()), + ); + let enabled_rules: HashSet> = external_rule_codes + .into_iter() + .map(|code| code.into()) + .chain(self.ctx.rules.iter().map(|r| r.code().into())) + .collect(); + // Run `ban-unknown-rule-code` - diagnostics.extend(context.ban_unknown_rule_code()); + diagnostics.extend(context.ban_unknown_rule_code(&all_rules)); // Run `ban-unused-ignore` - diagnostics.extend(context.ban_unused_ignore(&self.ctx.rules)); + diagnostics.extend(context.ban_unused_ignore(&enabled_rules)); // Finally sort by position the diagnostics originates on then by code diagnostics.sort_by(|a, b| { @@ -159,6 +189,7 @@ impl Linter { parsed_source: &ParsedSource, default_jsx_factory: Option, default_jsx_fragment_factory: Option, + maybe_external_linter: Option, ) -> Vec { let _mark = PerformanceMark::new("Linter::lint_inner"); @@ -200,7 +231,16 @@ impl Linter { rule.lint_program_with_ast_view(&mut context, pg); } - self.collect_diagnostics(context) + let mut external_rule_codes = vec![]; + if let Some(cb) = maybe_external_linter { + let result = cb(parsed_source.clone()); + // TODO(bartlomijue): get rid of this unwrap + let external_linter_result = result.unwrap(); + context.add_external_diagnostics(&external_linter_result.diagnostics); + external_rule_codes = external_linter_result.rules; + } + + self.collect_diagnostics(context, external_rule_codes) }); diagnostics diff --git a/src/test_util.rs b/src/test_util.rs index 9fed7ef7..0e20ef63 100644 --- a/src/test_util.rs +++ b/src/test_util.rs @@ -325,7 +325,7 @@ fn lint( rules: vec![rule], all_rule_codes: get_all_rules() .into_iter() - .map(|rule| rule.code()) + .map(|rule| rule.code().into()) .collect(), custom_ignore_diagnostic_directive: None, custom_ignore_file_directive: None, @@ -341,6 +341,7 @@ fn lint( default_jsx_factory: Some("React.createElement".to_owned()), default_jsx_fragment_factory: Some("React.Fragment".to_owned()), }, + external_linter: None, }); match lint_result { Ok((source, diagnostics)) => (source, diagnostics),