Skip to content

Commit

Permalink
refactor: remove unnecessary function span_to_location
Browse files Browse the repository at this point in the history
  • Loading branch information
shulaoda committed Dec 7, 2024
1 parent f0462ea commit 11f668d
Show file tree
Hide file tree
Showing 21 changed files with 89 additions and 77 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/rspack_core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ once_cell = { workspace = true }
paste = { workspace = true }
rayon = { workspace = true }
regex = { workspace = true }
ropey = { workspace = true }
rspack_ast = { workspace = true }
rspack_cacheable = { workspace = true }
rspack_collections = { workspace = true }
Expand Down
86 changes: 63 additions & 23 deletions crates/rspack_core/src/dependency/dependency_location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::{

use derivative::Derivative;
use rspack_cacheable::cacheable;
use rspack_util::itoa;

/// Represents a range in a dependency, typically used for tracking the span of code in a source file.
/// It stores the start and end positions (as offsets) of the range, typically using base-0 indexing.
Expand Down Expand Up @@ -41,25 +42,16 @@ impl DependencyRange {

/// Converts the `DependencyRange` into a `DependencyLocation`.
/// The `source` parameter is an optional source map used to resolve the exact position in the source file.
pub fn to_loc(&self, source: Option<&Arc<dyn SourceLocation>>) -> DependencyLocation {
DependencyLocation::Real(match source {
Some(source) => {
let (start, end) = source.look_up_range_pos(self.start, self.end);

if start.line == end.line && start.column == end.column {
pub fn to_loc<T: AsLoc>(&self, source: Option<T>) -> Option<DependencyLocation> {
source
.and_then(|s| s.as_loc().look_up_range_pos(self.start, self.end))
.map(|(start, end)| {
DependencyLocation::Real(if start.line == end.line && start.column == end.column {
RealDependencyLocation::new(start, None)
} else {
RealDependencyLocation::new(start, Some(end))
}
}
None => RealDependencyLocation::new(
SourcePosition {
line: self.start as usize,
column: self.end as usize,
},
None,
),
})
})
})
}
}

Expand All @@ -85,17 +77,22 @@ impl fmt::Display for RealDependencyLocation {
write!(
f,
"{}:{}-{}",
self.start.line, self.start.column, end.column
itoa!(self.start.line),
itoa!(self.start.column),
itoa!(end.column)
)
} else {
write!(
f,
"{}:{}-{}:{}",
self.start.line, self.start.column, end.line, end.column
itoa!(self.start.line),
itoa!(self.start.column),
itoa!(end.line),
itoa!(end.column)
)
}
} else {
write!(f, "{}:{}", self.start.line, self.start.column)
write!(f, "{}:{}", itoa!(self.start.line), itoa!(self.start.column))
}
}
}
Expand Down Expand Up @@ -157,7 +154,7 @@ impl From<(u32, u32)> for SourcePosition {

/// Trait representing a source map that can resolve the positions of code ranges to source file positions.
pub trait SourceLocation: Send + Sync {
fn look_up_range_pos(&self, start: u32, end: u32) -> (SourcePosition, SourcePosition);
fn look_up_range_pos(&self, start: u32, end: u32) -> Option<(SourcePosition, SourcePosition)>;
}

impl Debug for dyn SourceLocation {
Expand All @@ -167,11 +164,11 @@ impl Debug for dyn SourceLocation {
}

impl SourceLocation for swc_core::common::SourceMap {
fn look_up_range_pos(&self, start: u32, end: u32) -> (SourcePosition, SourcePosition) {
fn look_up_range_pos(&self, start: u32, end: u32) -> Option<(SourcePosition, SourcePosition)> {
let lo = self.lookup_char_pos(swc_core::common::BytePos(start + 1));
let hi = self.lookup_char_pos(swc_core::common::BytePos(end + 1));

(
Some((
SourcePosition {
line: lo.line,
column: lo.col_display,
Expand All @@ -180,9 +177,52 @@ impl SourceLocation for swc_core::common::SourceMap {
line: hi.line,
column: hi.col_display,
},
)
))
}
}

impl SourceLocation for &str {
fn look_up_range_pos(&self, start: u32, end: u32) -> Option<(SourcePosition, SourcePosition)> {
let r = ropey::Rope::from_str(self);
let start_char_offset = r.try_byte_to_char(start as usize).ok()?;
let end_char_offset = r.try_byte_to_char(end as usize).ok()?;

let start_line = r.char_to_line(start_char_offset);
let start_column = start_char_offset - r.line_to_char(start_line);
let end_line = r.char_to_line(end_char_offset);
let end_column = end_char_offset - r.line_to_char(end_line);

Some((
SourcePosition {
line: start_line,
column: start_column,
},
SourcePosition {
line: end_line,
column: end_column,
},
))
}
}

/// Type alias for a shared reference to a `SourceLocation` trait object, typically used for source maps.
pub type SharedSourceMap = Arc<dyn SourceLocation>;

pub trait AsLoc {
fn as_loc(&self) -> &dyn SourceLocation;
}

impl AsLoc for &Arc<dyn SourceLocation> {
#[inline]
fn as_loc(&self) -> &dyn SourceLocation {
self.as_ref()
}
}

impl AsLoc for &str {
#[inline]
fn as_loc(&self) -> &dyn SourceLocation {
let loc: &dyn SourceLocation = self;
loc
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl Dependency for CommonJsFullRequireDependency {
}

fn loc(&self) -> Option<DependencyLocation> {
Some(self.range.to_loc(self.source_map.as_ref()))
self.range.to_loc(self.source_map.as_ref())
}

fn range(&self) -> Option<&DependencyRange> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl Dependency for CommonJsRequireDependency {
}

fn loc(&self) -> Option<DependencyLocation> {
Some(self.range.to_loc(self.source_map.as_ref()))
self.range.to_loc(self.source_map.as_ref())
}

fn category(&self) -> &DependencyCategory {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ impl Dependency for RequireHeaderDependency {
}

fn loc(&self) -> Option<DependencyLocation> {
Some(self.range.to_loc(self.source_map.as_ref()))
self.range.to_loc(self.source_map.as_ref())
}

fn could_affect_referencing_module(&self) -> rspack_core::AffectType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl Dependency for RequireResolveHeaderDependency {
}

fn loc(&self) -> Option<DependencyLocation> {
Some(self.range.to_loc(self.source_map.as_ref()))
self.range.to_loc(self.source_map.as_ref())
}

fn could_affect_referencing_module(&self) -> AffectType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl Dependency for ESMExportExpressionDependency {
}

fn loc(&self) -> Option<DependencyLocation> {
Some(self.range.to_loc(self.source_map.as_ref()))
self.range.to_loc(self.source_map.as_ref())
}

fn get_exports(&self, _mg: &ModuleGraph) -> Option<ExportsSpec> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl Dependency for ESMExportHeaderDependency {
}

fn loc(&self) -> Option<DependencyLocation> {
Some(self.range.to_loc(self.source_map.as_ref()))
self.range.to_loc(self.source_map.as_ref())
}

fn dependency_type(&self) -> &DependencyType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1053,7 +1053,7 @@ impl Dependency for ESMExportImportedSpecifierDependency {
}

fn loc(&self) -> Option<DependencyLocation> {
Some(self.range.to_loc(self.source_map.as_ref()))
self.range.to_loc(self.source_map.as_ref())
}

fn range(&self) -> Option<&DependencyRange> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl Dependency for ESMExportSpecifierDependency {
}

fn loc(&self) -> Option<DependencyLocation> {
Some(self.range.to_loc(self.source_map.as_ref()))
self.range.to_loc(self.source_map.as_ref())
}

fn category(&self) -> &DependencyCategory {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ impl Dependency for ESMImportSideEffectDependency {
}

fn loc(&self) -> Option<DependencyLocation> {
Some(self.range.to_loc(self.source_map.as_ref()))
self.range.to_loc(self.source_map.as_ref())
}

fn range(&self) -> Option<&DependencyRange> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ impl Dependency for ESMImportSpecifierDependency {
}

fn loc(&self) -> Option<DependencyLocation> {
Some(self.range.to_loc(self.source_map.as_ref()))
self.range.to_loc(self.source_map.as_ref())
}

fn range(&self) -> Option<&DependencyRange> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl Dependency for ProvideDependency {
}

fn loc(&self) -> Option<DependencyLocation> {
Some(self.range.to_loc(self.source_map.as_ref()))
self.range.to_loc(self.source_map.as_ref())
}

fn category(&self) -> &DependencyCategory {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl ESMAcceptDependency {
}

pub fn loc(&self) -> Option<DependencyLocation> {
Some(self.range.to_loc(self.source_map.as_ref()))
self.range.to_loc(self.source_map.as_ref())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl ModuleArgumentDependency {
}

pub fn loc(&self) -> Option<DependencyLocation> {
Some(self.range.to_loc(self.source_map.as_ref()))
self.range.to_loc(self.source_map.as_ref())
}
}

Expand Down
43 changes: 7 additions & 36 deletions crates/rspack_plugin_javascript/src/parser_and_generator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,15 @@ use rspack_core::diagnostics::map_box_diagnostics_to_module_parse_diagnostics;
use rspack_core::rspack_sources::{BoxSource, ReplaceSource, Source, SourceExt};
use rspack_core::{
render_init_fragments, AsyncDependenciesBlockIdentifier, BuildMetaExportsType, ChunkGraph,
Compilation, DependenciesBlock, DependencyId, GenerateContext, Module, ModuleGraph, ModuleType,
ParseContext, ParseResult, ParserAndGenerator, SideEffectsBailoutItem, SourceType, SpanExt,
TemplateContext, TemplateReplaceSource,
Compilation, DependenciesBlock, DependencyId, DependencyRange, GenerateContext, Module,
ModuleGraph, ModuleType, ParseContext, ParseResult, ParserAndGenerator, SideEffectsBailoutItem,
SourceType, TemplateContext, TemplateReplaceSource,
};
use rspack_error::miette::Diagnostic;
use rspack_error::{DiagnosticExt, IntoTWithDiagnosticArray, Result, TWithDiagnosticArray};
use rspack_util::itoa;
use swc_core::common::comments::Comments;
use swc_core::common::input::SourceFileInput;
use swc_core::common::{FileName, Span, SyntaxContext};
use swc_core::common::{FileName, SyntaxContext};
use swc_core::ecma::ast;
use swc_core::ecma::parser::{lexer::Lexer, EsSyntax, Syntax};
use swc_node_comments::SwcComments;
Expand Down Expand Up @@ -241,7 +240,9 @@ impl ParserAndGenerator for JavaScriptParserAndGenerator {
.side_effects_item
.take()
.and_then(|item| -> Option<_> {
let msg = span_to_location(item.span, &source.source())?;
let range: DependencyRange = item.span.into();
let source = source.source();
let msg = range.to_loc(Some(source.as_ref()))?.to_string();
Some(SideEffectsBailoutItem { msg, ty: item.ty })
})
});
Expand Down Expand Up @@ -376,36 +377,6 @@ impl ParserAndGenerator for JavaScriptParserAndGenerator {
}
}

// Todo(shulaoda): check if this can be removed
fn span_to_location(span: Span, source: &str) -> Option<String> {
let r = ropey::Rope::from_str(source);
let start = span.real_lo();
let end = span.real_hi();
let start_char_offset = r.try_byte_to_char(start as usize).ok()?;
let start_line = r.char_to_line(start_char_offset);
let start_column = start_char_offset - r.line_to_char(start_line);

let end_char_offset = r.try_byte_to_char(end as usize).ok()?;
let end_line = r.char_to_line(end_char_offset);
let end_column = end_char_offset - r.line_to_char(end_line);
if start_line == end_line {
Some(format!(
"{}:{}-{}",
itoa!(start_line + 1),
itoa!(start_column),
itoa!(end_column)
))
} else {
Some(format!(
"{}:{}-{}:{}",
itoa!(start_line + 1),
itoa!(start_column),
itoa!(end_line + 1),
itoa!(end_column)
))
}
}

fn remove_bom(s: Arc<dyn Source>) -> Arc<dyn Source> {
if s.source().starts_with('\u{feff}') {
let mut s = ReplaceSource::new(s);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,7 @@ impl AMDRequireDependenciesBlockParserPlugin {
));

let source_map: SharedSourceMap = parser.source_map.clone();
let block_loc =
Some(Into::<DependencyRange>::into(call_expr.span).to_loc(Some(source_map).as_ref()));
let block_loc = Into::<DependencyRange>::into(call_expr.span).to_loc(Some(&source_map));

if call_expr.args.len() == 1 {
let mut block_deps: Vec<BoxDependency> = vec![dep];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ impl JavascriptParserPlugin for ImportParserPlugin {
let source_map: SharedSourceMap = parser.source_map.clone();
let mut block = AsyncDependenciesBlock::new(
*parser.module_identifier,
Some(Into::<DependencyRange>::into(node.span).to_loc(Some(source_map).as_ref())),
Into::<DependencyRange>::into(node.span).to_loc(Some(&source_map)),
None,
vec![dep],
Some(param.string().clone()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ impl JavascriptParserPlugin for RequireEnsureDependenciesBlockParserPlugin {
let source_map: SharedSourceMap = parser.source_map.clone();
let mut block = AsyncDependenciesBlock::new(
*parser.module_identifier,
Some(Into::<DependencyRange>::into(expr.span).to_loc(Some(source_map).as_ref())),
Into::<DependencyRange>::into(expr.span).to_loc(Some(&source_map)),
None,
deps,
None,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ fn add_dependencies(
let source_map: SharedSourceMap = parser.source_map.clone();
let mut block = AsyncDependenciesBlock::new(
*parser.module_identifier,
Some(Into::<DependencyRange>::into(span).to_loc(Some(source_map).as_ref())),
Into::<DependencyRange>::into(span).to_loc(Some(&source_map)),
None,
vec![dep],
None,
Expand Down

0 comments on commit 11f668d

Please sign in to comment.