Skip to content

Commit

Permalink
refactor(linter,diagnostic): one diagnostic struct to eliminate monom…
Browse files Browse the repository at this point in the history
…orphization of generic types

part of #3213
  • Loading branch information
Boshen committed May 11, 2024
1 parent a23ba71 commit 5af55c3
Show file tree
Hide file tree
Showing 355 changed files with 5,930 additions and 6,437 deletions.
20 changes: 19 additions & 1 deletion crates/oxc_diagnostics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ pub type Report = miette::Report;

pub type Result<T> = std::result::Result<T, OxcDiagnostic>;

pub use miette::LabeledSpan;
use miette::{Diagnostic, SourceCode};
pub use miette::{LabeledSpan, NamedSource};
use thiserror::Error;

#[derive(Debug, Error, Diagnostic)]
Expand Down Expand Up @@ -60,6 +60,7 @@ pub struct OxcDiagnosticInner {
pub message: String,
pub labels: Option<Vec<LabeledSpan>>,
pub help: Option<String>,
pub severity: Severity,
}

impl fmt::Display for OxcDiagnostic {
Expand All @@ -75,6 +76,10 @@ impl Diagnostic for OxcDiagnostic {
self.help.as_ref().map(Box::new).map(|c| c as Box<dyn Display>)
}

fn severity(&self) -> Option<Severity> {
Some(self.severity)
}

fn labels(&self) -> Option<Box<dyn Iterator<Item = LabeledSpan> + '_>> {
self.labels
.as_ref()
Expand All @@ -92,6 +97,19 @@ impl OxcDiagnostic {
message: message.into(),
labels: None,
help: None,
severity: Severity::Error,
}),
}
}

#[must_use]
pub fn warning<T: Into<String>>(message: T) -> Self {
Self {
inner: Box::new(OxcDiagnosticInner {
message: message.into(),
labels: None,
help: None,
severity: Severity::Warning,
}),
}
}
Expand Down
4 changes: 3 additions & 1 deletion crates/oxc_linter/src/ast_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,9 @@ pub fn is_method_call<'a>(
}

if let Some(methods) = methods {
let Some(static_property_name) = member_expr.static_property_name() else { return false };
let Some(static_property_name) = member_expr.static_property_name() else {
return false;
};
if !methods.contains(&static_property_name) {
return false;
}
Expand Down
10 changes: 3 additions & 7 deletions crates/oxc_linter/src/context.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{cell::RefCell, path::Path, rc::Rc, sync::Arc};

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

Expand Down Expand Up @@ -116,15 +116,11 @@ impl<'a> LintContext<'a> {
}
}

pub fn diagnostic<T: Into<Error>>(&self, diagnostic: T) {
pub fn diagnostic(&self, diagnostic: OxcDiagnostic) {
self.add_diagnostic(Message::new(diagnostic.into(), None));
}

pub fn diagnostic_with_fix<T, F>(&self, diagnostic: T, fix: F)
where
T: Into<Error>,
F: FnOnce() -> Fix<'a>,
{
pub fn diagnostic_with_fix<F: FnOnce() -> Fix<'a>>(&self, diagnostic: OxcDiagnostic, fix: F) {
if self.fix {
self.add_diagnostic(Message::new(diagnostic.into(), Some(fix())));
} else {
Expand Down
94 changes: 57 additions & 37 deletions crates/oxc_linter/src/disable_directives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,36 +214,45 @@ fn test() {
// [Disabling Rules](https://eslint.org/docs/latest/use/configure/rules#disabling-rules)
// Using configuration comments
let pass = vec![
// To disable rule warnings in a part of a file, use block comments in the following format:
format!("
// To disable rule warnings in a part of a file, use block comments in the following format:
format!(
"
/* {prefix}-disable */
debugger;
/* {prefix}-enable */
"),
// You can also disable or enable warnings for specific rules:
format!("
"
),
// You can also disable or enable warnings for specific rules:
format!(
"
/* {prefix}-disable no-debugger, no-console */
debugger;
/* {prefix}-enable no-debugger, no-console */
"),
// To disable rule warnings in an entire file, put a /* eslint-disable */ block comment at the top of the file:
format!("
"
),
// To disable rule warnings in an entire file, put a /* eslint-disable */ block comment at the top of the file:
format!(
"
/* {prefix}-disable */
debugger;
"),
// You can also disable or enable specific rules for an entire file:
format!("
"
),
// You can also disable or enable specific rules for an entire file:
format!(
"
/* {prefix}-disable no-debugger */
debugger;
"),
// To ensure that a rule is never applied (regardless of any future enable/disable lines):
// This is not supported.
// "
// /* eslint no-debugger: \"off\" */
// debugger;
// "),
// To disable all rules on a specific line, use a line or block comment in one of the following formats:
format!("debugger; // {prefix}-disable-line
"
),
// To ensure that a rule is never applied (regardless of any future enable/disable lines):
// This is not supported.
// "
// /* eslint no-debugger: \"off\" */
// debugger;
// "),
// To disable all rules on a specific line, use a line or block comment in one of the following formats:
format!(
"debugger; // {prefix}-disable-line
debugger; // {prefix}-disable-line
// {prefix}-disable-next-line
Expand All @@ -253,9 +262,11 @@ fn test() {
debugger;
debugger; /* {prefix}-disable-line */
"),
// To disable a specific rule on a specific line:
format!("
"
),
// To disable a specific rule on a specific line:
format!(
"
debugger; // {prefix}-disable-line no-debugger
// {prefix}-disable-next-line no-debugger
Expand All @@ -265,9 +276,11 @@ fn test() {
/* {prefix}-disable-next-line no-debugger */
debugger;
"),
// To disable multiple rules on a specific line:
format!("
"
),
// To disable multiple rules on a specific line:
format!(
"
debugger; // {prefix}-disable-line no-debugger, quotes, semi
// {prefix}-disable-next-line no-debugger, quotes, semi
Expand All @@ -284,23 +297,29 @@ fn test() {
semi
*/
debugger;
"),
// To disable all rules twice:
format!("
"
),
// To disable all rules twice:
format!(
"
/* {prefix}-disable */
debugger;
/* {prefix}-disable */
debugger;
"),
// To disable a rule twice:
format!("
"
),
// To disable a rule twice:
format!(
"
/* {prefix}-disable no-debugger */
debugger;
/* {prefix}-disable no-debugger */
debugger;
"),
// Comment descriptions
format!("
"
),
// Comment descriptions
format!(
"
// {prefix}-disable-next-line no-debugger -- Here's a description about why this configuration is necessary.
debugger;
Expand All @@ -309,8 +328,9 @@ fn test() {
* along with some additional information
**/
debugger;
")
];
"
),
];

let fail = vec![
"debugger".to_string(),
Expand Down
6 changes: 1 addition & 5 deletions crates/oxc_linter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,7 @@ impl Linter {
} else {
("", 7)
};
writeln!(
writer,
"| {rule_name:<rule_width$} | {plugin_name:<plugin_width$} | {default:<default_width$} |"
)
.unwrap();
writeln!(writer, "| {rule_name:<rule_width$} | {plugin_name:<plugin_width$} | {default:<default_width$} |").unwrap();
}
writeln!(writer).unwrap();
}
Expand Down
8 changes: 6 additions & 2 deletions crates/oxc_linter/src/partial_loader/astro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,12 @@ impl<'a> AstroPartialLoader<'a> {

let start = offsets.first()?;
let end = offsets.last()?;
let Ok(start) = u32::try_from(*start) else { return None };
let Ok(end) = u32::try_from(*end) else { return None };
let Ok(start) = u32::try_from(*start) else {
return None;
};
let Ok(end) = u32::try_from(*end) else {
return None;
};

let js_code =
Span::new(start + ASTRO_SPLIT.len() as u32, end).source_text(self.source_text);
Expand Down
8 changes: 6 additions & 2 deletions crates/oxc_linter/src/partial_loader/vue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ impl<'a> VuePartialLoader<'a> {
/// <https://vuejs.org/api/sfc-spec.html#script>
fn parse_scripts(&self) -> Vec<JavaScriptSource<'a>> {
let mut pointer = 0;
let Some(result1) = self.parse_script(&mut pointer) else { return vec![] };
let Some(result2) = self.parse_script(&mut pointer) else { return vec![result1] };
let Some(result1) = self.parse_script(&mut pointer) else {
return vec![];
};
let Some(result2) = self.parse_script(&mut pointer) else {
return vec![result1];
};
vec![result1, result2]
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,20 @@ use oxc_ast::{
ast::{Expression, MemberExpression},
AstKind,
};
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::Error,
};
use oxc_diagnostics::OxcDiagnostic;

use oxc_macros::declare_oxc_lint;
use oxc_span::{CompactStr, Span};
use oxc_span::Span;

use crate::{context::LintContext, rule::Rule, AstNode};

#[derive(Debug, Error, Diagnostic)]
#[error("deepscan(bad-array-method-on-arguments): Bad array method on arguments")]
#[diagnostic(
severity(warning),
help(
"The 'arguments' object does not have '{0}()' method. If an array method was intended, consider converting the 'arguments' object to an array or using ES6 rest parameter instead."
)
)]
struct BadArrayMethodOnArgumentsDiagnostic(CompactStr, #[label] pub Span);
fn bad_array_method_on_arguments_diagnostic(x0: &str, span1: Span) -> OxcDiagnostic {
OxcDiagnostic::warning("deepscan(bad-array-method-on-arguments): Bad array method on arguments")
.with_help(format!(
"The 'arguments' object does not have '{x0}()' method. If an array method was intended, consider converting the 'arguments' object to an array or using ES6 rest parameter instead."
))
.with_labels([span1.into()])
}

/// `https://deepscan.io/docs/rules/bad-array-method-on-arguments`
#[derive(Debug, Default, Clone)]
Expand Down Expand Up @@ -51,17 +47,23 @@ impl Rule for BadArrayMethodOnArguments {
if !node.kind().is_specific_id_reference("arguments") {
return;
}
let Some(parent_node_id) = ctx.nodes().parent_id(node.id()) else { return };
let Some(parent_node_id) = ctx.nodes().parent_id(node.id()) else {
return;
};
let AstKind::MemberExpression(member_expr) = ctx.nodes().kind(parent_node_id) else {
return;
};
let Some(parent_node_id) = ctx.nodes().parent_id(parent_node_id) else { return };
let AstKind::CallExpression(_) = ctx.nodes().kind(parent_node_id) else { return };
let Some(parent_node_id) = ctx.nodes().parent_id(parent_node_id) else {
return;
};
let AstKind::CallExpression(_) = ctx.nodes().kind(parent_node_id) else {
return;
};
match member_expr {
MemberExpression::StaticMemberExpression(expr) => {
if ARRAY_METHODS.binary_search(&expr.property.name.as_str()).is_ok() {
ctx.diagnostic(BadArrayMethodOnArgumentsDiagnostic(
expr.property.name.to_compact_str(),
ctx.diagnostic(bad_array_method_on_arguments_diagnostic(
expr.property.name.as_str(),
expr.span,
));
}
Expand All @@ -70,8 +72,8 @@ impl Rule for BadArrayMethodOnArguments {
match &expr.expression {
Expression::StringLiteral(name) => {
if ARRAY_METHODS.binary_search(&name.value.as_str()).is_ok() {
ctx.diagnostic(BadArrayMethodOnArgumentsDiagnostic(
name.value.to_compact_str(),
ctx.diagnostic(bad_array_method_on_arguments_diagnostic(
name.value.as_str(),
expr.span,
));
}
Expand All @@ -85,9 +87,8 @@ impl Rule for BadArrayMethodOnArguments {
})
{
if ARRAY_METHODS.binary_search(&name).is_ok() {
ctx.diagnostic(BadArrayMethodOnArgumentsDiagnostic(
CompactStr::from(name),
expr.span,
ctx.diagnostic(bad_array_method_on_arguments_diagnostic(
name, expr.span,
));
}
}
Expand Down

0 comments on commit 5af55c3

Please sign in to comment.