Skip to content

Commit

Permalink
feat: Add ExternalLinter API (#1384)
Browse files Browse the repository at this point in the history
This commit adds an "external linter" API, that allows to
pass a callback with a parsed source that allows to contribute
additional rules and diagnostics.
  • Loading branch information
bartlomieju authored Jan 28, 2025
1 parent 757c636 commit 33d5e07
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 43 deletions.
8 changes: 4 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions examples/dlint/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use deno_lint::rules::get_all_rules;
use deno_lint::rules::{filtered_rules, recommended_rules};
use log::debug;
use rayon::prelude::*;
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::collections::HashSet;
use std::path::PathBuf;
Expand Down Expand Up @@ -91,6 +92,7 @@ fn run_linter(
let all_rule_codes = all_rules
.iter()
.map(|rule| rule.code())
.map(Cow::from)
.collect::<HashSet<_>>();
let rules = if let Some(config) = maybe_config {
config.get_rules()
Expand Down Expand Up @@ -133,6 +135,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();
Expand Down
39 changes: 24 additions & 15 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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<Arc<Box<Expr>>>,
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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<dyn LintRule>],
known_rules_codes: &HashSet<Cow<'static, str>>,
) -> Vec<LintDiagnostic> {
const CODE: &str = "ban-unused-ignore";

Expand All @@ -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
};

Expand Down Expand Up @@ -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<LintDiagnostic> {
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<Cow<'static, str>>,
) -> Vec<LintDiagnostic> {
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())),
Expand All @@ -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())),
Expand Down Expand Up @@ -451,6 +452,14 @@ impl<'a> Context<'a> {
.push(self.create_diagnostic(maybe_range, details));
}

/// Add fully constructed diagnostics.
///
/// This function can be used by the "external linter" to provide its own
/// diagnostics.
pub fn add_external_diagnostics(&mut self, diagnostics: &[LintDiagnostic]) {
self.diagnostics.extend_from_slice(diagnostics);
}

pub(crate) fn create_diagnostic(
&self,
maybe_range: Option<LintDiagnosticRange>,
Expand Down
34 changes: 16 additions & 18 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub use deno_ast::view::ProgramRef;

#[cfg(test)]
mod lint_tests {
use std::borrow::Cow;
use std::collections::HashSet;

use crate::diagnostic::LintDiagnostic;
Expand All @@ -41,7 +42,7 @@ mod lint_tests {
fn lint(
source: &str,
rules: Vec<Box<dyn LintRule>>,
all_rule_codes: HashSet<&'static str>,
all_rule_codes: HashSet<Cow<'static, str>>,
) -> Vec<LintDiagnostic> {
let linter = Linter::new(LinterOptions {
rules,
Expand All @@ -59,6 +60,7 @@ mod lint_tests {
default_jsx_factory: None,
default_jsx_fragment_factory: None,
},
external_linter: None,
})
.expect("Failed to lint");
diagnostics
Expand All @@ -67,7 +69,7 @@ mod lint_tests {
fn lint_with_ast(
parsed_source: &ParsedSource,
rules: Vec<Box<dyn LintRule>>,
all_rule_codes: HashSet<&'static str>,
all_rule_codes: HashSet<Cow<'static, str>>,
) -> Vec<LintDiagnostic> {
let linter = Linter::new(LinterOptions {
rules,
Expand All @@ -81,17 +83,23 @@ mod lint_tests {
default_jsx_factory: None,
default_jsx_fragment_factory: None,
},
None,
)
}

fn get_all_rules_codes() -> HashSet<Cow<'static, str>> {
get_all_rules()
.into_iter()
.map(|rule| rule.code())
.map(Cow::from)
.collect()
}

fn lint_recommended_rules(source: &str) -> Vec<LintDiagnostic> {
lint(
source,
recommended_rules(get_all_rules()),
get_all_rules()
.into_iter()
.map(|rule| rule.code())
.collect(),
get_all_rules_codes(),
)
}

Expand All @@ -101,25 +109,15 @@ mod lint_tests {
lint_with_ast(
parsed_source,
recommended_rules(get_all_rules()),
get_all_rules()
.into_iter()
.map(|rule| rule.code())
.collect(),
get_all_rules_codes(),
)
}

fn lint_specified_rule(
rule: Box<dyn LintRule>,
source: &str,
) -> Vec<LintDiagnostic> {
lint(
source,
vec![rule],
get_all_rules()
.into_iter()
.map(|rule| rule.code())
.collect(),
)
lint(source, vec![rule], get_all_rules_codes())
}

#[test]
Expand Down
54 changes: 48 additions & 6 deletions src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Box<dyn LintRule>>,
/// Collection of all the lint rule codes.
pub all_rule_codes: HashSet<&'static str>,
pub all_rule_codes: HashSet<Cow<'static, str>>,
/// Defaults to "deno-lint-ignore-file"
pub custom_ignore_file_directive: Option<&'static str>,
/// Defaults to "deno-lint-ignore"
Expand All @@ -40,7 +42,7 @@ pub(crate) struct LinterContext {
pub check_unknown_rules: bool,
/// Rules are sorted by priority
pub rules: Vec<Box<dyn LintRule>>,
pub all_rule_codes: HashSet<&'static str>,
pub all_rule_codes: HashSet<Cow<'static, str>>,
}

impl LinterContext {
Expand All @@ -65,11 +67,27 @@ impl LinterContext {
}
}

#[derive(Default)]
pub struct ExternalLinterResult {
pub diagnostics: Vec<LintDiagnostic>,
pub rules: Vec<Cow<'static, str>>,
}

/// Perform a run of "external linter" on a parsed source file.
///
/// Since we are working on an already parsed file, this callback
/// is infallible. If an error handling needs to be performed by the
/// external linter, it should be handled externally bt that linter,
/// and an empty [`ExternalLinterResult`] should be returned.
pub type ExternalLinterCb =
Arc<dyn Fn(ParsedSource) -> Option<ExternalLinterResult>>;

pub struct LintFileOptions {
pub specifier: ModuleSpecifier,
pub source_code: String,
pub media_type: MediaType,
pub config: LintConfig,
pub external_linter: Option<ExternalLinterCb>,
}

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -107,6 +125,7 @@ impl Linter {
&parsed_source,
options.config.default_jsx_factory,
options.config.default_jsx_fragment_factory,
options.external_linter,
);

Ok((parsed_source, diagnostics))
Expand All @@ -120,26 +139,40 @@ impl Linter {
&self,
parsed_source: &ParsedSource,
config: LintConfig,
maybe_external_linter: Option<ExternalLinterCb>,
) -> Vec<LintDiagnostic> {
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<LintDiagnostic> {
fn collect_diagnostics(
&self,
mut context: Context,
external_rule_codes: Vec<Cow<'static, str>>,
) -> Vec<LintDiagnostic> {
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().cloned());
let enabled_rules: HashSet<Cow<'static, str>> = external_rule_codes
.into_iter()
.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| {
Expand All @@ -159,6 +192,7 @@ impl Linter {
parsed_source: &ParsedSource,
default_jsx_factory: Option<String>,
default_jsx_fragment_factory: Option<String>,
maybe_external_linter: Option<ExternalLinterCb>,
) -> Vec<LintDiagnostic> {
let _mark = PerformanceMark::new("Linter::lint_inner");

Expand Down Expand Up @@ -200,7 +234,15 @@ 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 {
if let Some(external_linter_result) = cb(parsed_source.clone()) {
context.add_external_diagnostics(&external_linter_result.diagnostics);
external_rule_codes = external_linter_result.rules;
}
}

self.collect_diagnostics(context, external_rule_codes)
});

diagnostics
Expand Down
4 changes: 4 additions & 0 deletions src/test_util.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use std::borrow::Cow;

use crate::ast_parser;
use crate::diagnostic::LintDiagnostic;
use crate::linter::LintConfig;
Expand Down Expand Up @@ -326,6 +328,7 @@ fn lint(
all_rule_codes: get_all_rules()
.into_iter()
.map(|rule| rule.code())
.map(Cow::from)
.collect(),
custom_ignore_diagnostic_directive: None,
custom_ignore_file_directive: None,
Expand All @@ -341,6 +344,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),
Expand Down

0 comments on commit 33d5e07

Please sign in to comment.