fix(codegen): do not generate sourcemap for empty length spans (#11892)

Upstream may have modified and generated an incorrect span.

Author: Boshen

crates/oxc_codegen/src/lib.rs

@@ -18,7 +18,7 @@ use std::borrow::Cow;
 use oxc_ast::ast::*;
 use oxc_data_structures::{code_buffer::CodeBuffer, stack::Stack};
 use oxc_semantic::Scoping;
-use oxc_span::{GetSpan, SPAN, Span};
+use oxc_span::{GetSpan, Span};
 use oxc_syntax::{
     identifier::{is_identifier_part, is_identifier_part_ascii},
     operator::{BinaryOperator, UnaryOperator, UpdateOperator},
@@ -690,29 +690,26 @@ impl<'a> Codegen<'a> {
     }
 
     fn add_source_mapping(&mut self, span: Span) {
-        if span == SPAN {
-            return;
-        }
         if let Some(sourcemap_builder) = self.sourcemap_builder.as_mut() {
-            sourcemap_builder.add_source_mapping(self.code.as_bytes(), span.start, None);
+            if !span.is_empty() {
+                sourcemap_builder.add_source_mapping(self.code.as_bytes(), span.start, None);
+            }
         }
     }
 
     fn add_source_mapping_end(&mut self, span: Span) {
-        if span == SPAN {
-            return;
-        }
         if let Some(sourcemap_builder) = self.sourcemap_builder.as_mut() {
-            sourcemap_builder.add_source_mapping(self.code.as_bytes(), span.end, None);
+            if !span.is_empty() {
+                sourcemap_builder.add_source_mapping(self.code.as_bytes(), span.end, None);
+            }
         }
     }
 
     fn add_source_mapping_for_name(&mut self, span: Span, name: &str) {
-        if span == SPAN {
-            return;
-        }
         if let Some(sourcemap_builder) = self.sourcemap_builder.as_mut() {
-            sourcemap_builder.add_source_mapping_for_name(self.code.as_bytes(), span, name);
+            if !span.is_empty() {
+                sourcemap_builder.add_source_mapping_for_name(self.code.as_bytes(), span, name);
+            }
         }
     }
 }

crates/oxc_codegen/tests/integration/main.rs

@@ -2,45 +2,9 @@
 pub mod comments;
 pub mod esbuild;
 pub mod js;
-pub mod tester;
+pub mod sourcemap;
 pub mod ts;
 
-use oxc_allocator::Allocator;
-use oxc_codegen::{Codegen, CodegenOptions, CodegenReturn};
-use oxc_parser::Parser;
-use oxc_span::SourceType;
+mod tester;
 
-#[track_caller]
-pub fn codegen(source_text: &str) -> String {
-    codegen_options(source_text, &CodegenOptions::default()).code
-}
-
-#[track_caller]
-pub fn codegen_options(source_text: &str, options: &CodegenOptions) -> CodegenReturn {
-    let allocator = Allocator::default();
-    let source_type = SourceType::ts();
-    let ret = Parser::new(&allocator, source_text, source_type).parse();
-    let mut options = options.clone();
-    options.single_quote = true;
-    Codegen::new().with_options(options).build(&ret.program)
-}
-
-#[track_caller]
-pub fn snapshot(name: &str, cases: &[&str]) {
-    snapshot_options(name, cases, &CodegenOptions::default());
-}
-
-#[track_caller]
-pub fn snapshot_options(name: &str, cases: &[&str], options: &CodegenOptions) {
-    use std::fmt::Write;
-
-    let snapshot = cases.iter().enumerate().fold(String::new(), |mut w, (i, case)| {
-        let result = codegen_options(case, options).code;
-        write!(w, "########## {i}\n{case}\n----------\n{result}\n",).unwrap();
-        w
-    });
-
-    insta::with_settings!({ prepend_module_to_snapshot => false, snapshot_suffix => "", omit_expression => true }, {
-        insta::assert_snapshot!(name, snapshot);
-    });
-}
+pub use tester::*;

crates/oxc_codegen/tests/integration/sourcemap.rs

@@ -0,0 +1,29 @@
+use oxc_allocator::Allocator;
+use oxc_ast::ast::{Expression, Statement};
+use oxc_codegen::Codegen;
+use oxc_parser::Parser;
+use oxc_span::{SourceType, Span};
+
+use crate::tester::default_options;
+
+/// Upstream may have modified the AST to include incorrect spans.
+/// e.g. <https://github.com/rolldown/rolldown/blob/v1.0.0-beta.19/crates/rolldown/src/utils/ecma_visitors/mod.rs>
+#[test]
+fn incorrect_ast() {
+    let allocator = Allocator::default();
+    let source_type = SourceType::ts();
+    let source_text = "foo\nvar bar = '测试'";
+    let ret = Parser::new(&allocator, source_text, source_type).parse();
+
+    let mut program = ret.program;
+    program.span = Span::new(0, 0);
+    if let Statement::ExpressionStatement(s) = &mut program.body[0] {
+        s.span = Span::new(17, 17);
+        if let Expression::Identifier(ident) = &mut s.expression {
+            ident.span = Span::new(17, 17);
+        }
+    }
+
+    let ret = Codegen::new().with_options(default_options()).build(&program);
+    assert!(ret.map.is_some(), "sourcemap exists");
+}

crates/oxc_codegen/tests/integration/tester.rs

@@ -1,20 +1,25 @@
 use oxc_allocator::Allocator;
-use oxc_codegen::{Codegen, CodegenOptions};
+use oxc_codegen::{Codegen, CodegenOptions, CodegenReturn};
 use oxc_parser::{ParseOptions, Parser};
 use oxc_span::SourceType;
 
+pub fn default_options() -> CodegenOptions {
+    // Ensure sourcemap do not crash.
+    CodegenOptions { source_map_path: Some("test.js".into()), ..CodegenOptions::default() }
+}
+
 #[track_caller]
 pub fn test_with_parse_options(source_text: &str, expected: &str, parse_options: ParseOptions) {
     let allocator = Allocator::default();
     let ret =
         Parser::new(&allocator, source_text, SourceType::tsx()).with_options(parse_options).parse();
-    let result = Codegen::new().build(&ret.program).code;
+    let result = Codegen::new().with_options(default_options()).build(&ret.program).code;
     assert_eq!(result, expected, "\nfor source: {source_text}");
 }
 
 #[track_caller]
 pub fn test(source_text: &str, expected: &str) {
-    test_options(source_text, expected, CodegenOptions::default());
+    test_options(source_text, expected, default_options());
 }
 
 #[track_caller]
@@ -29,12 +34,7 @@ pub fn test_options(source_text: &str, expected: &str, options: CodegenOptions)
 
 #[track_caller]
 pub fn test_tsx(source_text: &str, expected: &str) {
-    test_options_with_source_type(
-        source_text,
-        expected,
-        SourceType::tsx(),
-        CodegenOptions::default(),
-    );
+    test_options_with_source_type(source_text, expected, SourceType::tsx(), default_options());
 }
 
 #[track_caller]
@@ -66,3 +66,38 @@ pub fn test_minify(source_text: &str, expected: &str) {
 pub fn test_minify_same(source_text: &str) {
     test_minify(source_text, source_text);
 }
+
+#[track_caller]
+pub fn codegen(source_text: &str) -> String {
+    codegen_options(source_text, &default_options()).code
+}
+
+#[track_caller]
+pub fn codegen_options(source_text: &str, options: &CodegenOptions) -> CodegenReturn {
+    let allocator = Allocator::default();
+    let source_type = SourceType::ts();
+    let ret = Parser::new(&allocator, source_text, source_type).parse();
+    let mut options = options.clone();
+    options.single_quote = true;
+    Codegen::new().with_options(options).build(&ret.program)
+}
+
+#[track_caller]
+pub fn snapshot(name: &str, cases: &[&str]) {
+    snapshot_options(name, cases, &default_options());
+}
+
+#[track_caller]
+pub fn snapshot_options(name: &str, cases: &[&str], options: &CodegenOptions) {
+    use std::fmt::Write;
+
+    let snapshot = cases.iter().enumerate().fold(String::new(), |mut w, (i, case)| {
+        let result = codegen_options(case, options).code;
+        write!(w, "########## {i}\n{case}\n----------\n{result}\n",).unwrap();
+        w
+    });
+
+    insta::with_settings!({ prepend_module_to_snapshot => false, snapshot_suffix => "", omit_expression => true }, {
+        insta::assert_snapshot!(name, snapshot);
+    });
+}