fix(fast_check): improve overload handling in dts transform

src/fast_check/transform.rs

@@ -171,8 +171,11 @@ pub fn transform(
   })?;
 
   let dts = if let Some(dts_comments) = dts_comments {
-    let mut dts_transformer =
-      FastCheckDtsTransformer::new(parsed_source.text_info_lazy(), specifier);
+    let mut dts_transformer = FastCheckDtsTransformer::new(
+      parsed_source.text_info_lazy(),
+      public_ranges,
+      specifier,
+    );
 
     let module = dts_transformer.transform(program.expect_module())?;
 

src/fast_check/transform_dts.rs

@@ -8,6 +8,7 @@ use deno_ast::SourceTextInfo;
 use crate::FastCheckDiagnostic;
 use crate::FastCheckDiagnosticRange;
 
+use super::range_finder::ModulePublicRanges;
 use super::swc_helpers::any_type_ann;
 use super::swc_helpers::maybe_lit_to_ts_type;
 use super::swc_helpers::maybe_lit_to_ts_type_const;
@@ -66,6 +67,7 @@ impl FastCheckDtsDiagnostic {
 pub struct FastCheckDtsTransformer<'a> {
   id_counter: usize,
   text_info: &'a SourceTextInfo,
+  public_ranges: &'a ModulePublicRanges,
   pub diagnostics: Vec<FastCheckDtsDiagnostic>,
   specifier: &'a ModuleSpecifier,
   is_top_level: bool,
@@ -74,12 +76,14 @@ pub struct FastCheckDtsTransformer<'a> {
 impl<'a> FastCheckDtsTransformer<'a> {
   pub fn new(
     text_info: &'a SourceTextInfo,
+    public_ranges: &'a ModulePublicRanges,
     specifier: &'a ModuleSpecifier,
   ) -> Self {
     Self {
       id_counter: 0,
       text_info,
       specifier,
+      public_ranges,
       diagnostics: vec![],
       is_top_level: true,
     }
@@ -139,26 +143,21 @@ impl<'a> FastCheckDtsTransformer<'a> {
     body: Vec<ModuleItem>,
   ) -> Vec<ModuleItem> {
     let mut new_items: Vec<ModuleItem> = vec![];
-    let mut prev_is_overload = false;
 
     for item in body {
       match item {
         ModuleItem::ModuleDecl(module_decl) => match module_decl {
           ModuleDecl::Import(_) => {
-            prev_is_overload = false;
             new_items.push(ModuleItem::ModuleDecl(module_decl));
           }
           ModuleDecl::ExportDecl(export_decl) => {
-            let is_overload = if let Decl::Fn(fn_decl) = &export_decl.decl {
-              fn_decl.function.body.is_none()
-            } else {
-              false
-            };
-
-            let should_keep = prev_is_overload && !is_overload;
-            prev_is_overload = is_overload;
-            if should_keep {
-              continue;
+            if let Decl::Fn(_) = &export_decl.decl {
+              if self
+                .public_ranges
+                .is_impl_with_overloads(&export_decl.range())
+              {
+                continue; // skip implementation signature
+              }
             }
 
             if let Some(decl) = self.decl_to_type_decl(export_decl.decl.clone())
@@ -176,8 +175,6 @@ impl<'a> FastCheckDtsTransformer<'a> {
             }
           }
           ModuleDecl::ExportDefaultDecl(export_decl) => {
-            let mut is_overload = false;
-
             let value = match export_decl.decl {
               DefaultDecl::Class(mut class_expr) => {
                 class_expr.class.body =
@@ -188,7 +185,12 @@ impl<'a> FastCheckDtsTransformer<'a> {
                 }
               }
               DefaultDecl::Fn(mut fn_expr) => {
-                is_overload = fn_expr.function.body.is_none();
+                if self
+                  .public_ranges
+                  .is_impl_with_overloads(&export_decl.span.range())
+                {
+                  continue; // skip implementation signature
+                }
 
                 fn_expr.function.body = None;
                 ExportDefaultDecl {
@@ -199,29 +201,11 @@ impl<'a> FastCheckDtsTransformer<'a> {
               DefaultDecl::TsInterfaceDecl(_) => export_decl,
             };
 
-            let should_keep = prev_is_overload && !is_overload;
-            prev_is_overload = is_overload;
-            if should_keep {
-              continue;
-            }
-
             new_items.push(ModuleItem::ModuleDecl(
               ModuleDecl::ExportDefaultDecl(value),
             ))
           }
           ModuleDecl::ExportDefaultExpr(export_default_expr) => {
-            let is_overload =
-              if let Expr::Fn(fn_expr) = &*export_default_expr.expr {
-                fn_expr.function.body.is_none()
-              } else {
-                false
-              };
-            let should_keep = prev_is_overload && !is_overload;
-            prev_is_overload = is_overload;
-            if should_keep {
-              continue;
-            }
-
             let name = self.gen_unique_name();
             let name_ident = Ident::new(name.into(), DUMMY_SP);
             let type_ann = self
@@ -267,13 +251,16 @@ impl<'a> FastCheckDtsTransformer<'a> {
           | ModuleDecl::TsExportAssignment(_)
           | ModuleDecl::ExportNamed(_)
           | ModuleDecl::ExportAll(_) => {
-            prev_is_overload = false;
             new_items.push(ModuleItem::ModuleDecl(module_decl));
           }
         },
         ModuleItem::Stmt(stmt) => {
-          prev_is_overload = false;
           if let Stmt::Decl(decl) = stmt {
+            if let Decl::Fn(_) = &decl {
+              if self.public_ranges.is_impl_with_overloads(&decl.range()) {
+                continue; // skip implementation signature
+              }
+            }
             match decl {
               Decl::TsEnum(_)
               | Decl::Class(_)
@@ -812,46 +799,22 @@ impl<'a> FastCheckDtsTransformer<'a> {
   }
 
   fn class_body_to_type(&mut self, body: Vec<ClassMember>) -> Vec<ClassMember> {
-    // Track if the previous member was an overload signature or not.
-    // When overloads are present the last item has the implementation
-    // body. For declaration files the implementation always needs to
-    // be dropped. Needs to be unique for each class because another
-    // class could be created inside a class method.
-    let mut prev_is_overload = false;
-
     body
       .into_iter()
       .filter(|member| match member {
-        ClassMember::Constructor(class_constructor) => {
-          let is_overload = class_constructor.body.is_none();
-          if !prev_is_overload || is_overload {
-            prev_is_overload = is_overload;
-            true
-          } else {
-            prev_is_overload = false;
-            false
-          }
-        }
+        ClassMember::Constructor(constructor) => !self
+          .public_ranges
+          .is_impl_with_overloads(&constructor.range()),
         ClassMember::Method(method) => {
-          let is_overload = method.function.body.is_none();
-          if !prev_is_overload || is_overload {
-            prev_is_overload = is_overload;
-            true
-          } else {
-            prev_is_overload = false;
-            false
-          }
+          !self.public_ranges.is_impl_with_overloads(&method.range())
         }
         ClassMember::TsIndexSignature(_)
         | ClassMember::ClassProp(_)
         | ClassMember::PrivateProp(_)
         | ClassMember::Empty(_)
         | ClassMember::StaticBlock(_)
         | ClassMember::AutoAccessor(_)
-        | ClassMember::PrivateMethod(_) => {
-          prev_is_overload = false;
-          true
-        }
+        | ClassMember::PrivateMethod(_) => true,
       })
       .filter_map(|member| match member {
         ClassMember::Constructor(mut class_constructor) => {
@@ -927,6 +890,9 @@ impl<'a> FastCheckDtsTransformer<'a> {
 
 #[cfg(test)]
 mod tests {
+  use std::collections::VecDeque;
+
+  use crate::fast_check::range_finder::find_public_ranges;
   use crate::fast_check::transform_dts::FastCheckDtsTransformer;
   use crate::source::MemoryLoader;
   use crate::source::Source;
@@ -935,10 +901,14 @@ mod tests {
   use crate::CapturingModuleAnalyzer;
   use crate::GraphKind;
   use crate::ModuleGraph;
+  use crate::WorkspaceMember;
   use deno_ast::emit;
   use deno_ast::EmitOptions;
   use deno_ast::EmittedSourceText;
+  use deno_ast::ModuleSpecifier;
   use deno_ast::SourceMap;
+  use deno_semver::package::PackageNv;
+  use indexmap::IndexMap;
   use url::Url;
 
   async fn transform_dts_test(source: &str, expected: &str) {
@@ -978,9 +948,30 @@ mod tests {
 
     let parsed_source = module_info.source();
     let module = parsed_source.module().to_owned();
+    let nv = PackageNv::from_str("[email protected]").unwrap();
+    let public_ranges = find_public_ranges(
+      None,
+      Default::default(),
+      &graph,
+      &root_sym,
+      &[WorkspaceMember {
+        base: ModuleSpecifier::parse("file:///").unwrap(),
+        nv: nv.clone(),
+        exports: IndexMap::from([(".".to_string(), "./mod.ts".to_string())]),
+      }],
+      VecDeque::from([nv.clone()]),
+    )
+    .remove(&nv)
+    .unwrap()
+    .module_ranges
+    .shift_remove(&specifier)
+    .unwrap();
 
-    let mut transformer =
-      FastCheckDtsTransformer::new(parsed_source.text_info_lazy(), &specifier);
+    let mut transformer = FastCheckDtsTransformer::new(
+      parsed_source.text_info_lazy(),
+      &public_ranges,
+      &specifier,
+    );
     let module = transformer.transform(module).unwrap();
 
     let comments = parsed_source.comments().as_single_threaded();

tests/specs/graph/fast_check/class_optional_method.txt

@@ -0,0 +1,76 @@
+# https://jsr.io/@scope/a/meta.json
+{"versions": { "1.0.0": {} } }
+
+# https://jsr.io/@scope/a/1.0.0_meta.json
+{ "exports": { ".": "./mod.ts" } }
+
+# https://jsr.io/@scope/a/1.0.0/mod.ts
+export abstract class Value {
+  protected body?(): string | undefined | Promise<string | undefined>;
+
+  protected footer(): string | undefined {
+    return "";
+  }
+}
+
+# mod.ts
+import 'jsr:@scope/a'
+
+# output
+{
+  "roots": [
+    "file:///mod.ts"
+  ],
+  "modules": [
+    {
+      "kind": "esm",
+      "dependencies": [
+        {
+          "specifier": "jsr:@scope/a",
+          "code": {
+            "specifier": "jsr:@scope/a",
+            "span": {
+              "start": {
+                "line": 0,
+                "character": 7
+              },
+              "end": {
+                "line": 0,
+                "character": 21
+              }
+            }
+          }
+        }
+      ],
+      "size": 22,
+      "mediaType": "TypeScript",
+      "specifier": "file:///mod.ts"
+    },
+    {
+      "kind": "esm",
+      "size": 166,
+      "mediaType": "TypeScript",
+      "specifier": "https://jsr.io/@scope/a/1.0.0/mod.ts"
+    }
+  ],
+  "redirects": {
+    "jsr:@scope/a": "https://jsr.io/@scope/a/1.0.0/mod.ts"
+  },
+  "packages": {
+    "@scope/a": "@scope/[email protected]"
+  }
+}
+
+Fast check https://jsr.io/@scope/a/1.0.0/mod.ts:
+  {}
+  export abstract class Value {
+    protected body?(): string | undefined | Promise<string | undefined>;
+    protected footer(): string | undefined {
+      return {} as never;
+    }
+  }
+  --- DTS ---
+  export declare abstract class Value {
+    protected body?(): string | undefined | Promise<string | undefined>;
+    protected footer(): string | undefined;
+  }

tests/specs/graph/fast_check/functions.txt

@@ -194,7 +194,6 @@ Fast check https://jsr.io/@scope/a/1.0.0/mod.ts:
   export declare function test7(param?: number, param2?: PublicOther2): number;
   declare function test8(param: number): number;
   declare function test8(param: string): string;
-  declare function test8(param0?: any): any;
   export { test8 };
   export default function test9(param: number): number;
   export default function test9(param: string): string;