Skip to content

Commit

Permalink
refactor: store LintRule in an Arc instead of Box (#901)
Browse files Browse the repository at this point in the history
  • Loading branch information
bartlomieju committed Sep 22, 2021
1 parent f7cab1e commit e4ccf17
Show file tree
Hide file tree
Showing 100 changed files with 320 additions and 235 deletions.
4 changes: 2 additions & 2 deletions examples/dlint/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub struct Config {
}

impl Config {
pub fn get_rules(&self) -> Arc<Vec<Box<dyn LintRule>>> {
pub fn get_rules(&self) -> Vec<Arc<dyn LintRule>> {
get_filtered_rules(
Some(self.rules.tags.clone()),
Some(self.rules.exclude.clone()),
Expand Down Expand Up @@ -158,7 +158,7 @@ mod tests {
}}
}

fn into_codes(rules: Arc<Vec<Box<dyn LintRule>>>) -> HashSet<&'static str> {
fn into_codes(rules: Vec<Arc<dyn LintRule>>) -> HashSet<&'static str> {
rules.iter().map(|rule| rule.code()).collect()
}

Expand Down
5 changes: 3 additions & 2 deletions examples/dlint/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use serde::{Deserialize, Serialize};
use serde_json::Value;
use std::collections::{HashMap, HashSet};
use std::rc::Rc;
use std::sync::Arc;

#[derive(Deserialize)]
struct DiagnosticsFromJs {
Expand Down Expand Up @@ -109,8 +110,8 @@ pub struct PluginRunner {
}

impl PluginRunner {
pub fn new(plugin_path: &str) -> Box<Self> {
Box::new(Self {
pub fn new(plugin_path: &str) -> Arc<Self> {
Arc::new(Self {
plugin_path: plugin_path.to_string(),
})
}
Expand Down
14 changes: 6 additions & 8 deletions examples/dlint/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,10 @@ fn run_linter(
} else {
get_recommended_rules()
};
let plugins = Arc::new(
plugin_paths
.into_iter()
.map(|p| js::PluginRunner::new(p) as Box<dyn Plugin>)
.collect::<Vec<_>>(),
);
let plugins = plugin_paths
.into_iter()
.map(|p| js::PluginRunner::new(p) as Arc<dyn Plugin>)
.collect::<Vec<_>>();

let file_diagnostics = Arc::new(Mutex::new(BTreeMap::new()));
paths
Expand All @@ -113,8 +111,8 @@ fn run_linter(
}

let linter_builder = LinterBuilder::default()
.rules(Arc::clone(&rules))
.plugins(Arc::clone(&plugins))
.rules(rules.clone())
.plugins(plugins.clone())
.syntax(determine_syntax(file_path));

let linter = linter_builder.build();
Expand Down
3 changes: 2 additions & 1 deletion src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use deno_ast::swc::common::{Span, SyntaxContext};
use deno_ast::view as ast_view;
use deno_ast::view::{BytePos, RootNode, SourceFile};
use std::collections::{HashMap, HashSet};
use std::sync::Arc;
use std::time::Instant;

/// `Context` stores data needed while performing all lint rules to a file.
Expand Down Expand Up @@ -199,7 +200,7 @@ impl<'view> Context<'view> {
/// works for diagnostics reported by other rules.
pub(crate) fn ban_unused_ignore(
&self,
specified_rules: &[Box<dyn LintRule>],
specified_rules: &[Arc<dyn LintRule>],
) -> Vec<LintDiagnostic> {
const CODE: &str = "ban-unused-ignore";

Expand Down
9 changes: 3 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,7 @@ mod lint_tests {
use deno_ast::ParsedSource;
use std::sync::Arc;

fn lint(
source: &str,
rules: Arc<Vec<Box<dyn LintRule>>>,
) -> Vec<LintDiagnostic> {
fn lint(source: &str, rules: Vec<Arc<dyn LintRule>>) -> Vec<LintDiagnostic> {
let linter = LinterBuilder::default().rules(rules).build();

let (_, diagnostics) = linter
Expand All @@ -48,7 +45,7 @@ mod lint_tests {

fn lint_with_ast(
parsed_source: &ParsedSource,
rules: Arc<Vec<Box<dyn LintRule>>>,
rules: Vec<Arc<dyn LintRule>>,
) -> Vec<LintDiagnostic> {
let linter = LinterBuilder::default().rules(rules).build();

Expand All @@ -68,7 +65,7 @@ mod lint_tests {
fn lint_specified_rule<T: LintRule + 'static>(
source: &str,
) -> Vec<LintDiagnostic> {
lint(source, Arc::new(vec![T::new()]))
lint(source, vec![T::new()])
}

#[test]
Expand Down
16 changes: 8 additions & 8 deletions src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ pub struct LinterBuilder {
ignore_file_directive: String,
ignore_diagnostic_directive: String,
syntax: deno_ast::swc::parser::Syntax,
rules: Arc<Vec<Box<dyn LintRule>>>,
plugins: Arc<Vec<Box<dyn Plugin>>>,
rules: Vec<Arc<dyn LintRule>>,
plugins: Vec<Arc<dyn Plugin>>,
}

impl LinterBuilder {
Expand Down Expand Up @@ -63,12 +63,12 @@ impl LinterBuilder {
self
}

pub fn rules(mut self, rules: Arc<Vec<Box<dyn LintRule>>>) -> Self {
pub fn rules(mut self, rules: Vec<Arc<dyn LintRule>>) -> Self {
self.rules = rules;
self
}

pub fn plugins(mut self, plugins: Arc<Vec<Box<dyn Plugin>>>) -> Self {
pub fn plugins(mut self, plugins: Vec<Arc<dyn Plugin>>) -> Self {
self.plugins = plugins;
self
}
Expand All @@ -79,17 +79,17 @@ pub struct Linter {
ignore_file_directive: String,
ignore_diagnostic_directive: String,
syntax: Syntax,
rules: Arc<Vec<Box<dyn LintRule>>>,
plugins: Arc<Vec<Box<dyn Plugin>>>,
rules: Vec<Arc<dyn LintRule>>,
plugins: Vec<Arc<dyn Plugin>>,
}

impl Linter {
fn new(
ignore_file_directive: String,
ignore_diagnostic_directive: String,
syntax: Syntax,
rules: Arc<Vec<Box<dyn LintRule>>>,
plugins: Arc<Vec<Box<dyn Plugin>>>,
rules: Vec<Arc<dyn LintRule>>,
plugins: Vec<Arc<dyn Plugin>>,
) -> Self {
Linter {
ast_parser: AstParser::new(),
Expand Down
32 changes: 13 additions & 19 deletions src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ const DUMMY_NODE: () = ();

pub trait LintRule: std::fmt::Debug + Send + Sync {
/// Creates an instance of this rule.
fn new() -> Box<Self>
fn new() -> Arc<Self>
where
Self: Sized;

Expand Down Expand Up @@ -141,17 +141,15 @@ pub trait LintRule: std::fmt::Debug + Send + Sync {
fn docs(&self) -> &'static str;
}

pub fn get_all_rules() -> Arc<Vec<Box<dyn LintRule>>> {
Arc::new(get_all_rules_raw())
pub fn get_all_rules() -> Vec<Arc<dyn LintRule>> {
get_all_rules_raw()
}

pub fn get_recommended_rules() -> Arc<Vec<Box<dyn LintRule>>> {
Arc::new(
get_all_rules_raw()
.into_iter()
.filter(|r| r.tags().contains(&"recommended"))
.collect(),
)
pub fn get_recommended_rules() -> Vec<Arc<dyn LintRule>> {
get_all_rules_raw()
.into_iter()
.filter(|r| r.tags().contains(&"recommended"))
.collect()
}

/// Returns a list of rules after filtering.
Expand All @@ -173,7 +171,7 @@ pub fn get_filtered_rules(
maybe_tags: Option<Vec<String>>,
maybe_exclude: Option<Vec<String>>,
maybe_include: Option<Vec<String>>,
) -> Arc<Vec<Box<dyn LintRule>>> {
) -> Vec<Arc<dyn LintRule>> {
let tags_set =
maybe_tags.map(|tags| tags.into_iter().collect::<HashSet<_>>());

Expand Down Expand Up @@ -207,10 +205,10 @@ pub fn get_filtered_rules(

rules.sort_by_key(|r| r.code());

Arc::new(rules)
rules
}

fn get_all_rules_raw() -> Vec<Box<dyn LintRule>> {
fn get_all_rules_raw() -> Vec<Arc<dyn LintRule>> {
vec![
adjacent_overload_signatures::AdjacentOverloadSignatures::new(),
ban_ts_comment::BanTsComment::new(),
Expand Down Expand Up @@ -311,12 +309,8 @@ mod tests {

#[test]
fn recommended_rules_sorted_alphabetically() {
let sorted_recommended_rules = {
let mut recommended_rules =
Arc::try_unwrap(get_recommended_rules()).unwrap();
recommended_rules.sort_by_key(|r| r.code());
Arc::new(recommended_rules)
};
let mut sorted_recommended_rules = get_recommended_rules();
sorted_recommended_rules.sort_by_key(|r| r.code());

for (sorted, unsorted) in sorted_recommended_rules
.iter()
Expand Down
5 changes: 3 additions & 2 deletions src/rules/adjacent_overload_signatures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use deno_ast::view as ast_view;
use deno_ast::view::Spanned;
use derive_more::Display;
use std::collections::HashSet;
use std::sync::Arc;

#[derive(Debug)]
pub struct AdjacentOverloadSignatures;
Expand All @@ -26,8 +27,8 @@ enum AdjacentOverloadSignaturesHint {
}

impl LintRule for AdjacentOverloadSignatures {
fn new() -> Box<Self> {
Box::new(AdjacentOverloadSignatures)
fn new() -> Arc<Self> {
Arc::new(AdjacentOverloadSignatures)
}

fn tags(&self) -> &'static [&'static str] {
Expand Down
5 changes: 3 additions & 2 deletions src/rules/ban_ts_comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use deno_ast::swc::common::comments::CommentKind;
use deno_ast::swc::common::Span;
use once_cell::sync::Lazy;
use regex::Regex;
use std::sync::Arc;

/// This rule differs from typescript-eslint. In typescript-eslint the following
/// defaults apply:
Expand Down Expand Up @@ -61,8 +62,8 @@ impl BanTsComment {
}

impl LintRule for BanTsComment {
fn new() -> Box<Self> {
Box::new(BanTsComment)
fn new() -> Arc<Self> {
Arc::new(BanTsComment)
}

fn tags(&self) -> &'static [&'static str] {
Expand Down
5 changes: 3 additions & 2 deletions src/rules/ban_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use deno_ast::view as ast_view;
use deno_ast::view::{Spanned, TsEntityName, TsKeywordTypeKind};
use if_chain::if_chain;
use std::convert::TryFrom;
use std::sync::Arc;

#[derive(Debug)]
pub struct BanTypes;
Expand Down Expand Up @@ -74,8 +75,8 @@ impl TryFrom<&str> for BannedType {
}

impl LintRule for BanTypes {
fn new() -> Box<Self> {
Box::new(BanTypes)
fn new() -> Arc<Self> {
Arc::new(BanTypes)
}

fn tags(&self) -> &'static [&'static str] {
Expand Down
5 changes: 3 additions & 2 deletions src/rules/ban_unknown_rule_code.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
// Copyright 2020-2021 the Deno authors. All rights reserved. MIT license.
use super::{Context, LintRule};
use crate::{Program, ProgramRef};
use std::sync::Arc;

/// This is a dummy struct just for having the docs.
/// The actual implementation resides in [`Context`].
#[derive(Debug)]
pub struct BanUnknownRuleCode;

impl LintRule for BanUnknownRuleCode {
fn new() -> Box<Self> {
Box::new(BanUnknownRuleCode)
fn new() -> Arc<Self> {
Arc::new(BanUnknownRuleCode)
}

fn tags(&self) -> &'static [&'static str] {
Expand Down
5 changes: 3 additions & 2 deletions src/rules/ban_untagged_ignore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@
use super::{Context, LintRule};
use crate::{Program, ProgramRef};
use deno_ast::swc::common::Span;
use std::sync::Arc;

#[derive(Debug)]
pub struct BanUntaggedIgnore;

const CODE: &str = "ban-untagged-ignore";

impl LintRule for BanUntaggedIgnore {
fn new() -> Box<Self> {
Box::new(BanUntaggedIgnore)
fn new() -> Arc<Self> {
Arc::new(BanUntaggedIgnore)
}

fn tags(&self) -> &'static [&'static str] {
Expand Down
5 changes: 3 additions & 2 deletions src/rules/ban_untagged_todo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use deno_ast::swc::common::comments::Comment;
use deno_ast::swc::common::comments::CommentKind;
use once_cell::sync::Lazy;
use regex::Regex;
use std::sync::Arc;

#[derive(Debug)]
pub struct BanUntaggedTodo;
Expand All @@ -14,8 +15,8 @@ const MESSAGE: &str = "TODO should be tagged with (@username) or (#issue)";
const HINT: &str = "Add a user tag or issue reference to the TODO comment, e.g. TODO(@djones), TODO(djones), TODO(#123)";

impl LintRule for BanUntaggedTodo {
fn new() -> Box<Self> {
Box::new(BanUntaggedTodo)
fn new() -> Arc<Self> {
Arc::new(BanUntaggedTodo)
}

fn code(&self) -> &'static str {
Expand Down
5 changes: 3 additions & 2 deletions src/rules/ban_unused_ignore.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
// Copyright 2020-2021 the Deno authors. All rights reserved. MIT license.
use super::{Context, LintRule};
use crate::{Program, ProgramRef};
use std::sync::Arc;

/// This is a dummy struct just for having the docs.
/// The actual implementation resides in [`Context`].
#[derive(Debug)]
pub struct BanUnusedIgnore;

impl LintRule for BanUnusedIgnore {
fn new() -> Box<Self> {
Box::new(BanUnusedIgnore)
fn new() -> Arc<Self> {
Arc::new(BanUnusedIgnore)
}

fn tags(&self) -> &'static [&'static str] {
Expand Down
5 changes: 3 additions & 2 deletions src/rules/camelcase.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@ use deno_ast::view::Spanned;
use once_cell::sync::Lazy;
use regex::{Captures, Regex};
use std::collections::{BTreeMap, BTreeSet};
use std::sync::Arc;

#[derive(Debug)]
pub struct Camelcase;

const CODE: &str = "camelcase";

impl LintRule for Camelcase {
fn new() -> Box<Self> {
Box::new(Camelcase)
fn new() -> Arc<Self> {
Arc::new(Camelcase)
}

fn tags(&self) -> &'static [&'static str] {
Expand Down
Loading

0 comments on commit e4ccf17

Please sign in to comment.