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

feat: Add ExternalLinter API #1384

Merged
merged 9 commits into from
Jan 28, 2025
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
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
Loading