Skip to content

Commit

Permalink
feat!(linter): enforce rule severity from the cli and configuration f…
Browse files Browse the repository at this point in the history
…ile (oxc-project#3337)

closes oxc-project#2059

Breaking change:

* `--deny` / `-D` from the CLI and `error` from configuration file will
report diagnostics as severity "error".
* `--warn` / `-W` from the CLI and `warn` from configuration file will
report diagnostics as severity "warning".
  • Loading branch information
Boshen authored May 18, 2024
1 parent 8388c7b commit f13fe8a
Show file tree
Hide file tree
Showing 16 changed files with 217 additions and 136 deletions.
2 changes: 1 addition & 1 deletion crates/oxc_cli/fixtures/eslintrc_env/eslintrc_no_env.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"rules": {
"no-undef": "error"
"no-undef": "warn"
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"rules": {
"no-empty": ["error", { "allowEmptyCatch": false }]
"no-empty": ["warn", { "allowEmptyCatch": false }]
}
}
6 changes: 6 additions & 0 deletions crates/oxc_cli/src/command/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ pub enum LintFilter {
#[bpaf(short('A'), long("allow"), argument("NAME"))]
String,
),
Warn(
/// Deny the rule or category (emit a warning)
#[bpaf(short('W'), long("warn"), argument("NAME"))]
String,
),
Deny(
/// Deny the rule or category (emit an error)
#[bpaf(short('D'), long("deny"), argument("NAME"))]
Expand All @@ -109,6 +114,7 @@ impl LintFilter {
fn into_tuple(self) -> (AllowWarnDeny, String) {
match self {
Self::Allow(s) => (AllowWarnDeny::Allow, s),
Self::Warn(s) => (AllowWarnDeny::Warn, s),
Self::Deny(s) => (AllowWarnDeny::Deny, s),
}
}
Expand Down
10 changes: 5 additions & 5 deletions crates/oxc_cli/src/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ mod test {

#[test]
fn filter_allow_one() {
let args = &["-D", "correctness", "-A", "no-debugger", "fixtures/linter/debugger.js"];
let args = &["-W", "correctness", "-A", "no-debugger", "fixtures/linter/debugger.js"];
let result = test(args);
assert_eq!(result.number_of_files, 1);
assert_eq!(result.number_of_warnings, 0);
Expand All @@ -314,7 +314,7 @@ mod test {
#[test]
fn eslintrc_no_undef() {
let args = &[
"-D",
"-W",
"no-undef",
"-c",
"fixtures/no_undef/eslintrc.json",
Expand All @@ -329,7 +329,7 @@ mod test {
#[test]
fn eslintrc_no_env() {
let args = &[
"-D",
"-W",
"no-undef",
"-c",
"fixtures/eslintrc_env/eslintrc_no_env.json",
Expand Down Expand Up @@ -359,7 +359,7 @@ mod test {
let args = &[
"-c",
"fixtures/no_empty_allow_empty_catch/eslintrc.json",
"-D",
"-W",
"no-empty",
"fixtures/no_empty_allow_empty_catch/test.js",
];
Expand All @@ -374,7 +374,7 @@ mod test {
let args = &[
"-c",
"fixtures/no_empty_disallow_empty_catch/eslintrc.json",
"-D",
"-W",
"no-empty",
"fixtures/no_empty_disallow_empty_catch/test.js",
];
Expand Down
6 changes: 6 additions & 0 deletions crates/oxc_diagnostics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,12 @@ impl OxcDiagnostic {
}
}

#[must_use]
pub fn with_severity(mut self, severity: Severity) -> Self {
self.inner.severity = severity;
self
}

#[must_use]
pub fn with_help<T: Into<String>>(mut self, help: T) -> Self {
self.inner.help = Some(help.into());
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_diagnostics/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl DiagnosticService {
for diagnostic in diagnostics {
let severity = diagnostic.severity();
let is_warning = severity == Some(Severity::Warning);
let is_error = severity.is_none() || severity == Some(Severity::Error);
let is_error = severity == Some(Severity::Error) || severity.is_none();
if is_warning || is_error {
if is_warning {
let warnings_count = self.warnings_count() + 1;
Expand Down
2 changes: 0 additions & 2 deletions crates/oxc_language_server/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ pub struct FixedContent {
pub range: Range,
}

#[derive(Debug)]
pub struct IsolatedLintHandler {
linter: Arc<Linter>,
}
Expand Down Expand Up @@ -382,7 +381,6 @@ fn offset_to_position(offset: usize, source_text: &str) -> Option<Position> {
Some(Position::new(line as u32, column as u32))
}

#[derive(Debug)]
pub struct ServerLinter {
linter: Arc<Linter>,
}
Expand Down
1 change: 0 additions & 1 deletion crates/oxc_language_server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ use tower_lsp::lsp_types::{
};
use tower_lsp::{Client, LanguageServer, LspService, Server};

#[derive(Debug)]
struct Backend {
client: Client,
root_uri: OnceCell<Option<Url>>,
Expand Down
20 changes: 12 additions & 8 deletions crates/oxc_linter/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use oxc_diagnostics::OxcDiagnostic;
use rustc_hash::FxHashSet;
use serde::Deserialize;

use crate::{rules::RuleEnum, AllowWarnDeny};
use crate::{rules::RuleEnum, AllowWarnDeny, RuleWithSeverity};

pub use self::{
env::ESLintEnv, globals::ESLintGlobals, rules::ESLintRules,
Expand Down Expand Up @@ -66,12 +66,12 @@ impl ESLintConfig {
#[allow(clippy::option_if_let_else)]
pub fn override_rules(
&self,
rules_for_override: &mut FxHashSet<RuleEnum>,
rules_for_override: &mut FxHashSet<RuleWithSeverity>,
all_rules: &[RuleEnum],
) {
use itertools::Itertools;
let mut rules_to_replace = vec![];
let mut rules_to_remove = vec![];
let mut rules_to_replace: Vec<RuleWithSeverity> = vec![];
let mut rules_to_remove: Vec<RuleWithSeverity> = vec![];

// Rules can have the same name but different plugin names
let lookup = self.rules.iter().into_group_map_by(|r| r.rule_name.as_str());
Expand All @@ -83,22 +83,25 @@ impl ESLintConfig {
let rule_config = &rule_configs[0];
let rule_name = &rule_config.rule_name;
let plugin_name = &rule_config.plugin_name;
match rule_config.severity {
let severity = rule_config.severity;
match severity {
AllowWarnDeny::Warn | AllowWarnDeny::Deny => {
if let Some(rule) = all_rules
.iter()
.find(|r| r.name() == rule_name && r.plugin_name() == plugin_name)
{
let config = rule_config.config.clone().unwrap_or_default();
rules_to_replace.push(rule.read_json(config));
let rule = rule.read_json(config);
rules_to_replace.push(RuleWithSeverity::new(rule, severity));
}
}
AllowWarnDeny::Allow => {
if let Some(rule) = rules_for_override
.iter()
.find(|r| r.name() == rule_name && r.plugin_name() == plugin_name)
{
rules_to_remove.push(rule.clone());
let rule = rule.clone();
rules_to_remove.push(rule);
}
}
}
Expand All @@ -112,7 +115,8 @@ impl ESLintConfig {
{
if let Some(rule) = rules_for_override.iter().find(|r| r.name() == *name) {
let config = rule_config.config.clone().unwrap_or_default();
rules_to_replace.push(rule.read_json(config));
rules_to_replace
.push(RuleWithSeverity::new(rule.read_json(config), rule.severity));
}
} else if rule_configs.iter().all(|r| r.severity.is_allow()) {
if let Some(rule) = rules_for_override.iter().find(|r| r.name() == *name) {
Expand Down
36 changes: 25 additions & 11 deletions crates/oxc_linter/src/context.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use std::{cell::RefCell, path::Path, rc::Rc, sync::Arc};

use oxc_codegen::{Codegen, CodegenOptions};
use oxc_diagnostics::OxcDiagnostic;
use oxc_diagnostics::{OxcDiagnostic, Severity};
use oxc_semantic::{AstNodes, JSDocFinder, ScopeTree, Semantic, SymbolTable};
use oxc_span::SourceType;

use crate::{
disable_directives::{DisableDirectives, DisableDirectivesBuilder},
fixer::{Fix, Message},
javascript_globals::GLOBALS,
ESLintConfig, ESLintEnv, ESLintGlobals, ESLintSettings,
AllowWarnDeny, ESLintConfig, ESLintEnv, ESLintGlobals, ESLintSettings,
};

#[derive(Clone)]
Expand All @@ -23,11 +23,14 @@ pub struct LintContext<'a> {
/// Whether or not to apply code fixes during linting.
fix: bool,

current_rule_name: &'static str,

file_path: Rc<Path>,

eslint_config: Arc<ESLintConfig>,

// states
current_rule_name: &'static str,

severity: Severity,
}

impl<'a> LintContext<'a> {
Expand All @@ -39,9 +42,10 @@ impl<'a> LintContext<'a> {
diagnostics: RefCell::new(vec![]),
disable_directives: Rc::new(disable_directives),
fix: false,
current_rule_name: "",
file_path: file_path.into(),
eslint_config: Arc::new(ESLintConfig::default()),
current_rule_name: "",
severity: Severity::Warning,
}
}

Expand All @@ -57,6 +61,18 @@ impl<'a> LintContext<'a> {
self
}

#[must_use]
pub fn with_rule_name(mut self, name: &'static str) -> Self {
self.current_rule_name = name;
self
}

#[must_use]
pub fn with_severity(mut self, severity: AllowWarnDeny) -> Self {
self.severity = Severity::from(severity);
self
}

pub fn semantic(&self) -> &Rc<Semantic<'a>> {
&self.semantic
}
Expand Down Expand Up @@ -100,12 +116,6 @@ impl<'a> LintContext<'a> {
false
}

#[must_use]
pub fn with_rule_name(mut self, name: &'static str) -> Self {
self.current_rule_name = name;
self
}

/* Diagnostics */

pub fn into_message(self) -> Vec<Message<'a>> {
Expand All @@ -114,6 +124,10 @@ impl<'a> LintContext<'a> {

fn add_diagnostic(&self, message: Message<'a>) {
if !self.disable_directives.contains(self.current_rule_name, message.start()) {
let mut message = message;
if message.error.severity != self.severity {
message.error = message.error.with_severity(self.severity);
}
self.diagnostics.borrow_mut().push(message);
}
}
Expand Down
11 changes: 7 additions & 4 deletions crates/oxc_linter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub use crate::{
config::ESLintConfig,
context::LintContext,
options::{AllowWarnDeny, LintOptions},
rule::RuleWithSeverity,
service::{LintService, LintServiceOptions},
};
use crate::{
Expand All @@ -50,9 +51,8 @@ fn size_asserts() {
assert_eq_size!(RuleEnum, [u8; 16]);
}

#[derive(Debug)]
pub struct Linter {
rules: Vec<RuleEnum>,
rules: Vec<RuleWithSeverity>,
options: LintOptions,
eslint_config: Arc<ESLintConfig>,
}
Expand All @@ -72,8 +72,9 @@ impl Linter {
Ok(Self { rules, options, eslint_config: Arc::new(eslint_config) })
}

#[cfg(test)]
#[must_use]
pub fn with_rules(mut self, rules: Vec<RuleEnum>) -> Self {
pub fn with_rules(mut self, rules: Vec<RuleWithSeverity>) -> Self {
self.rules = rules;
self
}
Expand Down Expand Up @@ -105,7 +106,9 @@ impl Linter {
let rules = self
.rules
.iter()
.map(|rule| (rule, ctx.clone().with_rule_name(rule.name())))
.map(|rule| {
(rule, ctx.clone().with_rule_name(rule.name()).with_severity(rule.severity))
})
.collect::<Vec<_>>();

for (rule, ctx) in &rules {
Expand Down
Loading

0 comments on commit f13fe8a

Please sign in to comment.