refactor: unify the logic of span_to_location into trait `SourcePos…

…ition` (#8640)

refactor: remove unnecessary function span_to_location

crates/rspack_core/Cargo.toml

@@ -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 }

crates/rspack_core/src/dependency/dependency_location.rs

@@ -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.
@@ -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,
-      ),
-    })
+        })
+      })
   }
 }
 
@@ -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))
     }
   }
 }
@@ -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 {
@@ -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,
@@ -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 + 1,
+        column: start_column,
+      },
+      SourcePosition {
+        line: end_line + 1,
+        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
+  }
+}

crates/rspack_plugin_javascript/src/dependency/commonjs/common_js_full_require_dependency.rs

@@ -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> {

crates/rspack_plugin_javascript/src/dependency/commonjs/common_js_require_dependency.rs

@@ -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 {

crates/rspack_plugin_javascript/src/dependency/commonjs/require_header_dependency.rs

@@ -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 {

crates/rspack_plugin_javascript/src/dependency/commonjs/require_resolve_header_dependency.rs

@@ -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 {

crates/rspack_plugin_javascript/src/dependency/esm/esm_export_expression_dependency.rs

@@ -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> {

crates/rspack_plugin_javascript/src/dependency/esm/esm_export_header_dependency.rs

@@ -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 {

crates/rspack_plugin_javascript/src/dependency/esm/esm_export_imported_specifier_dependency.rs

@@ -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> {

crates/rspack_plugin_javascript/src/dependency/esm/esm_export_specifier_dependency.rs

@@ -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 {

crates/rspack_plugin_javascript/src/dependency/esm/esm_import_dependency.rs

@@ -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> {

crates/rspack_plugin_javascript/src/dependency/esm/esm_import_specifier_dependency.rs

@@ -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> {

crates/rspack_plugin_javascript/src/dependency/esm/provide_dependency.rs

@@ -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 {

crates/rspack_plugin_javascript/src/dependency/hmr/esm_accept_dependency.rs

@@ -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())
   }
 }
 

crates/rspack_plugin_javascript/src/dependency/module_argument_dependency.rs

@@ -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())
   }
 }
 

crates/rspack_plugin_javascript/src/parser_and_generator/mod.rs

@@ -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;
@@ -241,7 +240,10 @@ impl ParserAndGenerator for JavaScriptParserAndGenerator {
           .side_effects_item
           .take()
           .and_then(|item| -> Option<_> {
-            let msg = span_to_location(item.span, &source.source())?;
+            let source = source.source();
+            let msg = Into::<DependencyRange>::into(item.span)
+              .to_loc(Some(source.as_ref()))?
+              .to_string();
             Some(SideEffectsBailoutItem { msg, ty: item.ty })
           })
       });
@@ -349,36 +351,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);

crates/rspack_plugin_javascript/src/parser_plugin/amd_require_dependencies_block_parser_plugin.rs

@@ -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];

crates/rspack_plugin_javascript/src/parser_plugin/import_parser_plugin.rs

@@ -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()),

crates/rspack_plugin_javascript/src/parser_plugin/require_ensure_dependencies_block_parse_plugin.rs

@@ -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,

crates/rspack_plugin_javascript/src/parser_plugin/worker_plugin.rs

@@ -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,