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: better diagnostic report for harmony dependency #7337

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
2 changes: 2 additions & 0 deletions crates/node_binding/binding.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,7 @@ export interface JsRspackError {
name: string
message: string
moduleIdentifier?: string
loc?: string
file?: string
stack?: string
hideStack?: boolean
Expand Down Expand Up @@ -654,6 +655,7 @@ export interface JsStatsError {
chunkName?: string
chunkEntry?: boolean
chunkInitial?: boolean
loc?: string
file?: string
moduleIdentifier?: string
moduleName?: string
Expand Down
2 changes: 2 additions & 0 deletions crates/rspack_binding_values/src/rspack_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pub struct JsRspackError {
pub name: String,
pub message: String,
pub module_identifier: Option<String>,
pub loc: Option<String>,
pub file: Option<String>,
pub stack: Option<String>,
pub hide_stack: Option<bool>,
Expand All @@ -50,6 +51,7 @@ impl JsRspackError {
}),
message: diagnostic.render_report(colored)?,
module_identifier: diagnostic.module_identifier().map(|d| d.to_string()),
loc: diagnostic.format_location(),
file: diagnostic.file().map(|f| f.to_string_lossy().to_string()),
stack: diagnostic.stack(),
hide_stack: diagnostic.hide_stack(),
Expand Down
2 changes: 2 additions & 0 deletions crates/rspack_binding_values/src/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub struct JsStatsError {
pub chunk_name: Option<String>,
pub chunk_entry: Option<bool>,
pub chunk_initial: Option<bool>,
pub loc: Option<String>,
pub file: Option<String>,
pub module_identifier: Option<&'static str>,
pub module_name: Option<String>,
Expand All @@ -34,6 +35,7 @@ impl From<rspack_core::StatsError<'_>> for JsStatsError {
module_identifier: stats.module_identifier,
module_name: stats.module_name.map(|i| i.into_owned()),
module_id: stats.module_id.map(|i| i.to_owned()),
loc: stats.loc,
file: stats.file.map(|f| f.to_string_lossy().to_string()),
chunk_name: stats.chunk_name,
chunk_entry: stats.chunk_entry,
Expand Down
2 changes: 1 addition & 1 deletion crates/rspack_core/src/compiler/make/repair/factorize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl Task<MakeTaskContext> for FactorizeTask {
return Err(e);
}
let mut diagnostics = Vec::with_capacity(create_data.diagnostics.len() + 1);
diagnostics.push(e.into());
diagnostics.push(Into::<Diagnostic>::into(e).with_loc(create_data.dependency.loc()));
diagnostics.append(&mut create_data.diagnostics);
// Continue bundling if `options.bail` set to `false`.
Ok(vec![Box::new(
Expand Down
5 changes: 5 additions & 0 deletions crates/rspack_core/src/dependency/dependency_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::{any::Any, fmt::Debug};
use dyn_clone::{clone_trait_object, DynClone};
use rspack_collections::IdentifierSet;
use rspack_error::Diagnostic;
use rspack_error::ErrorLocation;
use rspack_util::ext::AsAny;
use swc_core::ecma::atoms::Atom;

Expand Down Expand Up @@ -66,6 +67,10 @@ pub trait Dependency:
ConnectionState::Bool(true)
}

fn loc(&self) -> Option<ErrorLocation> {
None
}

fn span(&self) -> Option<ErrorSpan> {
None
}
Expand Down
4 changes: 4 additions & 0 deletions crates/rspack_core/src/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,7 @@ impl Stats<'_> {
module_identifier: module_identifier.map(|i| i.as_str()),
module_name,
module_id: module_id.flatten(),
loc: d.format_location(),
file: d.file().map(ToOwned::to_owned),

chunk_name: chunk.and_then(|c| c.name.clone()),
Expand Down Expand Up @@ -654,6 +655,7 @@ impl Stats<'_> {
module_identifier: module_identifier.map(|i| i.as_str()),
module_name,
module_id: module_id.flatten(),
loc: d.format_location(),
file: d.file().map(ToOwned::to_owned),

chunk_name: chunk.and_then(|c| c.name.clone()),
Expand Down Expand Up @@ -1254,6 +1256,7 @@ pub struct StatsError<'s> {
pub module_identifier: Option<&'static str>,
pub module_name: Option<Cow<'s, str>>,
pub module_id: Option<&'s str>,
pub loc: Option<String>,
pub file: Option<PathBuf>,

pub chunk_name: Option<String>,
Expand All @@ -1271,6 +1274,7 @@ pub struct StatsWarning<'s> {
pub module_identifier: Option<&'static str>,
pub module_name: Option<Cow<'s, str>>,
pub module_id: Option<&'s str>,
pub loc: Option<String>,
pub file: Option<PathBuf>,

pub chunk_name: Option<String>,
Expand Down
66 changes: 66 additions & 0 deletions crates/rspack_error/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::{

use miette::{GraphicalTheme, IntoDiagnostic, MietteDiagnostic};
use rspack_collections::Identifier;
use swc_core::common::{SourceMap, Span};

use crate::{graphical::GraphicalReportHandler, Error};

Expand Down Expand Up @@ -61,10 +62,41 @@ impl fmt::Display for RspackSeverity {
}
}

#[derive(Debug, Clone, Copy)]
pub struct SourcePosition {
pub line: usize,
pub column: usize,
}

#[derive(Debug, Clone, Copy)]
pub struct ErrorLocation {
pub start: SourcePosition,
pub end: SourcePosition,
}

impl ErrorLocation {
pub fn new(span: Span, source_map: &SourceMap) -> Self {
let lo = source_map.lookup_char_pos(span.lo());
let hi = source_map.lookup_char_pos(span.hi());

ErrorLocation {
start: SourcePosition {
line: lo.line,
column: lo.col_display,
},
end: SourcePosition {
line: hi.line,
column: hi.col_display,
},
}
}
}

#[derive(Debug, Clone)]
pub struct Diagnostic {
inner: Arc<miette::Error>,
module_identifier: Option<Identifier>,
loc: Option<ErrorLocation>,
file: Option<PathBuf>,
hide_stack: Option<bool>,
chunk: Option<u32>,
Expand All @@ -82,6 +114,7 @@ impl From<miette::Error> for Diagnostic {
Self {
inner: Arc::new(value),
module_identifier: None,
loc: None,
file: None,
hide_stack: None,
chunk: None,
Expand All @@ -108,6 +141,7 @@ impl Diagnostic {
)
.into(),
module_identifier: None,
loc: None,
file: None,
hide_stack: None,
chunk: None,
Expand All @@ -124,6 +158,7 @@ impl Diagnostic {
)
.into(),
module_identifier: None,
loc: None,
file: None,
hide_stack: None,
chunk: None,
Expand Down Expand Up @@ -164,6 +199,37 @@ impl Diagnostic {
self
}

pub fn format_location(&self) -> Option<String> {
if let Some(loc) = &self.loc {
if loc.start.line == loc.end.line {
if loc.start.column == loc.end.column {
return Some(format!("{}:{}", loc.start.line, loc.start.column));
}

return Some(format!(
"{}:{}-{}",
loc.start.line, loc.start.column, loc.end.column
));
}

return Some(format!(
"{}:{}-{}:{}",
loc.start.line, loc.start.column, loc.end.line, loc.end.column
));
}

None
}

pub fn loc(&self) -> Option<ErrorLocation> {
self.loc
}

pub fn with_loc(mut self, loc: Option<ErrorLocation>) -> Self {
self.loc = loc;
self
}

pub fn file(&self) -> Option<&Path> {
self.file.as_deref()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ use itertools::Itertools;
use rspack_collections::{Identifier, IdentifierSet};
use rspack_core::{
property_access, AsContextDependency, AsModuleDependency, Compilation, Dependency,
DependencyLocation, DependencyType, ExportNameOrSpec, ExportsOfExportsSpec, ExportsSpec,
DependencyType, ErrorSpan, ExportNameOrSpec, ExportsOfExportsSpec, ExportsSpec,
HarmonyExportInitFragment, ModuleGraph, RuntimeGlobals, RuntimeSpec, UsedName, DEFAULT_EXPORT,
};
use rspack_core::{DependencyId, DependencyTemplate};
use rspack_core::{TemplateContext, TemplateReplaceSource};
use rspack_error::ErrorLocation;
use swc_core::atoms::Atom;

use crate::parser_plugin::JS_DEFAULT_KEYWORD;
Expand All @@ -19,26 +20,39 @@ pub enum DeclarationId {

#[derive(Debug, Clone)]
pub struct DeclarationInfo {
pub range: DependencyLocation,
pub prefix: String,
pub suffix: String,
range: ErrorSpan,
prefix: String,
suffix: String,
}

impl DeclarationInfo {
pub fn new(range: ErrorSpan, prefix: String, suffix: String) -> Self {
Self {
range,
prefix,
suffix,
}
}
}

#[derive(Debug, Clone)]
pub struct HarmonyExportExpressionDependency {
pub range: DependencyLocation,
pub range_stmt: DependencyLocation,
pub declaration: Option<DeclarationId>,
pub id: DependencyId,
id: DependencyId,
loc: ErrorLocation,
range: ErrorSpan,
range_stmt: ErrorSpan,
declaration: Option<DeclarationId>,
}

impl HarmonyExportExpressionDependency {
pub fn new(
range: DependencyLocation,
range_stmt: DependencyLocation,
loc: ErrorLocation,
range: ErrorSpan,
range_stmt: ErrorSpan,
declaration: Option<DeclarationId>,
) -> Self {
Self {
loc,
range,
range_stmt,
declaration,
Expand All @@ -56,6 +70,10 @@ impl Dependency for HarmonyExportExpressionDependency {
&self.id
}

fn loc(&self) -> Option<ErrorLocation> {
Some(self.loc)
}

fn get_exports(&self, _mg: &ModuleGraph) -> Option<ExportsSpec> {
Some(ExportsSpec {
exports: ExportsOfExportsSpec::Array(vec![ExportNameOrSpec::String(
Expand Down Expand Up @@ -116,8 +134,8 @@ impl DependencyTemplate for HarmonyExportExpressionDependency {
DeclarationId::Id(id) => id,
DeclarationId::Func(func) => {
source.replace(
func.range.start(),
func.range.end(),
func.range.start,
func.range.end,
&format!("{}{}{}", func.prefix, DEFAULT_EXPORT, func.suffix),
None,
);
Expand Down Expand Up @@ -151,8 +169,8 @@ impl DependencyTemplate for HarmonyExportExpressionDependency {
}

source.replace(
self.range_stmt.start(),
self.range.start(),
self.range_stmt.start,
self.range.start,
"/* harmony default export */ ",
None,
);
Expand Down Expand Up @@ -207,12 +225,12 @@ impl DependencyTemplate for HarmonyExportExpressionDependency {
};

source.replace(
self.range_stmt.start(),
self.range.start(),
self.range_stmt.start,
self.range.start,
&format!("{}(", content),
None,
);
source.replace(self.range.end(), self.range_stmt.end(), ");", None);
source.replace(self.range.end, self.range_stmt.end, ");", None);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,24 @@
use rspack_core::{
AsContextDependency, AsModuleDependency, Dependency, DependencyId, DependencyLocation,
DependencyTemplate, DependencyType, TemplateContext, TemplateReplaceSource,
AsContextDependency, AsModuleDependency, Dependency, DependencyId, DependencyTemplate,
DependencyType, ErrorSpan, TemplateContext, TemplateReplaceSource,
};
use rspack_error::ErrorLocation;

// Remove `export` label.
// Before: `export const a = 1`
// After: `const a = 1`
#[derive(Debug, Clone)]
pub struct HarmonyExportHeaderDependency {
pub range: Option<DependencyLocation>,
pub range_stmt: DependencyLocation,
pub id: DependencyId,
id: DependencyId,
loc: ErrorLocation,
range: Option<ErrorSpan>,
range_stmt: ErrorSpan,
}

impl HarmonyExportHeaderDependency {
pub fn new(range: Option<DependencyLocation>, range_stmt: DependencyLocation) -> Self {
pub fn new(loc: ErrorLocation, range: Option<ErrorSpan>, range_stmt: ErrorSpan) -> Self {
Self {
loc,
range,
range_stmt,
id: DependencyId::default(),
Expand All @@ -24,12 +27,17 @@ impl HarmonyExportHeaderDependency {
}

impl Dependency for HarmonyExportHeaderDependency {
fn dependency_type(&self) -> &DependencyType {
&DependencyType::EsmExportHeader
}
fn id(&self) -> &rspack_core::DependencyId {
&self.id
}

fn loc(&self) -> Option<ErrorLocation> {
Some(self.loc)
}

fn dependency_type(&self) -> &DependencyType {
&DependencyType::EsmExportHeader
}
}

impl DependencyTemplate for HarmonyExportHeaderDependency {
Expand All @@ -39,11 +47,11 @@ impl DependencyTemplate for HarmonyExportHeaderDependency {
_code_generatable_context: &mut TemplateContext,
) {
source.replace(
self.range_stmt.start(),
if let Some(range) = self.range.clone() {
range.start()
self.range_stmt.start,
if let Some(range) = self.range {
range.start
} else {
self.range_stmt.end()
self.range_stmt.end
},
"",
None,
Expand Down
Loading
Loading