refactor: separate imports from symbols (#780)

Author: crowlKats

examples/ddoc/main.rs

@@ -13,7 +13,6 @@ use deno_doc::html::HrefResolver;
 use deno_doc::html::UrlResolveKind;
 use deno_doc::html::UsageComposer;
 use deno_doc::html::UsageComposerEntry;
-use deno_doc::node::DeclarationDef;
 use deno_graph::BuildOptions;
 use deno_graph::GraphKind;
 use deno_graph::ModuleGraph;
@@ -190,7 +189,7 @@ async fn run() -> anyhow::Result<()> {
       filter,
       private,
     } => {
-      let doc_nodes_by_url = if json_input {
+      let mut doc_nodes_by_url = if json_input {
         assert_eq!(files.len(), 1);
         serde_json::from_reader(std::fs::File::open(&files[0])?)?
       } else {
@@ -210,27 +209,25 @@ async fn run() -> anyhow::Result<()> {
         return Ok(());
       }
 
-      let mut merged_doc = deno_doc::Document::default();
-      for doc in doc_nodes_by_url.into_values() {
-        if merged_doc.module_doc.is_empty() {
-          merged_doc.module_doc = doc.module_doc;
+      for (_, doc) in &mut doc_nodes_by_url {
+        if let Some(filter) = &filter {
+          let symbols = std::mem::take(&mut doc.symbols);
+          doc.symbols = find_nodes_by_name_recursively(symbols, filter);
         }
-        merged_doc.symbols.extend(doc.symbols);
-      }
-
-      merged_doc.symbols.retain(|doc_node| {
-        !matches!(doc_node.declarations[0].def, DeclarationDef::Import(..))
-      });
-
-      if let Some(filter) = filter {
-        merged_doc.symbols =
-          find_nodes_by_name_recursively(merged_doc.symbols, &filter);
       }
 
       if json {
-        serde_json::to_writer_pretty(std::io::stdout(), &merged_doc)?;
+        serde_json::to_writer_pretty(std::io::stdout(), &doc_nodes_by_url)?;
         println!();
       } else {
+        let mut merged_doc = deno_doc::Document::default();
+        for doc in doc_nodes_by_url.into_values() {
+          if merged_doc.module_doc.is_empty() {
+            merged_doc.module_doc = doc.module_doc;
+          }
+          merged_doc.symbols.extend(doc.symbols);
+        }
+
         let result = DocPrinter::new(&merged_doc, true, false);
         println!("{result}");
       }

js/types.d.ts

@@ -2,9 +2,17 @@
 
 export interface Document {
   moduleDoc?: JsDoc;
+  imports?: Import[];
   symbols: DocNode[];
 }
 
+export interface Import {
+  importedName: string;
+  originalName?: string;
+  jsDoc?: JsDoc;
+  src: string;
+}
+
 export interface DocNode {
   name: string;
   isDefault?: true;
@@ -25,7 +33,6 @@ export type Declaration =
   | DeclarationTypeAlias
   | DeclarationNamespace
   | DeclarationInterface
-  | DeclarationImport
   | DeclarationReference;
 
 interface DeclarationBase {
@@ -44,7 +51,6 @@ export type DocNodeKind =
   | "typeAlias"
   | "namespace"
   | "interface"
-  | "import"
   | "reference";
 
 export interface DeclarationModuleDoc extends DeclarationBase {
@@ -87,11 +93,6 @@ export interface DeclarationInterface extends DeclarationBase {
   def: InterfaceDef;
 }
 
-export interface DeclarationImport extends DeclarationBase {
-  kind: "import";
-  def: ImportDef;
-}
-
 export interface DeclarationReference extends DeclarationBase {
   kind: "reference";
   reference_def: ReferenceDef;
@@ -190,11 +191,6 @@ export interface FunctionDef {
   decorators?: DecoratorDef[];
 }
 
-export interface ImportDef {
-  src: string;
-  imported?: string;
-}
-
 export interface InterfaceDef {
   defName?: string;
   extends: TsTypeDef[];

src/diagnostics.rs

@@ -402,7 +402,7 @@ impl DiagnosticDocNodeVisitor<'_, '_> {
         | DeclarationDef::Namespace(..)
         | DeclarationDef::TypeAlias(..)
         | DeclarationDef::Variable(..) => true,
-        DeclarationDef::Import(..) | DeclarationDef::Reference(..) => false,
+        DeclarationDef::Reference(..) => false,
       }
     }
 

src/diff/mod.rs

@@ -469,7 +469,7 @@ pub enum DeclarationDefDiff {
   TypeAlias(TypeAliasDiff),
   Namespace(NamespaceDiff),
   Interface(InterfaceDiff),
-  // ignore Import & Reference
+  // ignore Reference
 }
 
 impl DeclarationDefDiff {
@@ -600,7 +600,6 @@ impl DeclarationDefDiff {
         InterfaceDiff::diff(old_def, new_def).map(DeclarationDefDiff::Interface)
       }
 
-      (DeclarationDef::Import(..), DeclarationDef::Import(..)) => None,
       (DeclarationDef::Reference(..), DeclarationDef::Reference(..)) => None,
 
       _ => unreachable!(),

src/html/jsdoc.rs

@@ -982,6 +982,7 @@ mod test {
           ModuleSpecifier::parse("file:///a.ts").unwrap(),
           Document {
             module_doc: Default::default(),
+            imports: vec![],
             symbols: vec![
               Symbol::interface(
                 "foo".into(),
@@ -1024,6 +1025,7 @@ mod test {
           ModuleSpecifier::parse("file:///b.ts").unwrap(),
           Document {
             module_doc: Default::default(),
+            imports: vec![],
             symbols: vec![Symbol::interface(
               "baz".into(),
               false,

src/html/mod.rs

@@ -287,6 +287,7 @@ pub struct GenerateCtx {
   pub package_name: Option<String>,
   pub common_ancestor: Option<PathBuf>,
   pub module_docs: IndexMap<Rc<ShortPath>, crate::js_doc::JsDoc>,
+  pub imports: IndexMap<Rc<ShortPath>, Vec<crate::node::Import>>,
   pub doc_nodes: IndexMap<Rc<ShortPath>, Vec<DocNodeWithContext>>,
   pub href_resolver: Rc<dyn HrefResolver>,
   pub usage_composer: Option<Rc<dyn UsageComposer>>,
@@ -323,6 +324,7 @@ impl GenerateCtx {
 
     let mut main_entrypoint = None;
     let mut module_docs = IndexMap::new();
+    let mut imports = IndexMap::new();
 
     let mut doc_nodes = doc_nodes_by_url
       .into_iter()
@@ -339,6 +341,7 @@ impl GenerateCtx {
         }
 
         module_docs.insert(short_path.clone(), document.module_doc);
+        imports.insert(short_path.clone(), document.imports);
 
         let nodes = document
           .symbols
@@ -529,6 +532,7 @@ impl GenerateCtx {
       package_name: options.package_name,
       common_ancestor,
       module_docs,
+      imports,
       doc_nodes,
       href_resolver: options.href_resolver,
       usage_composer: options.usage_composer,
@@ -938,7 +942,6 @@ impl DocNodeWithContext {
         .inner
         .declarations
         .iter()
-        .filter(|d| !matches!(d.def, crate::node::DeclarationDef::Import(..)))
         .map(|d| DocNodeKindCtx::from(crate::node::DocNodeKind::from(&d.def)))
         .collect()
     }

src/html/partition.rs

@@ -31,10 +31,6 @@ where
   {
     'outer: for node in doc_nodes {
       for decl in &node.declarations {
-        if matches!(decl.def, DeclarationDef::Import(..)) {
-          continue 'outer;
-        }
-
         if flatten_namespaces
           && matches!(decl.def, DeclarationDef::Namespace(..))
         {
@@ -223,10 +219,6 @@ pub fn flatten_namespace<'a>(
 
     'outer: for node in &nodes {
       for decl in &node.declarations {
-        if matches!(decl.def, DeclarationDef::Import(..)) {
-          continue 'outer;
-        }
-
         if matches!(decl.def, DeclarationDef::Namespace(..)) {
           let children: Vec<_> =
             node.namespace_children.as_ref().unwrap().clone();

src/html/render_context.rs

@@ -1,4 +1,3 @@
-use crate::DeclarationDef;
 use crate::html::DocNodeWithContext;
 use crate::html::GenerateCtx;
 use crate::html::UrlResolveKind;
@@ -36,10 +35,16 @@ impl<'ctx> RenderContext<'ctx> {
     doc_nodes: &[DocNodeWithContext],
     current_resolve: UrlResolveKind<'ctx>,
   ) -> Self {
+    let current_imports = current_resolve
+      .get_file()
+      .and_then(|file| ctx.imports.get(file))
+      .map(|imports| get_current_imports(imports))
+      .unwrap_or_default();
+
     Self {
       ctx,
       scoped_symbols: NamespacedSymbols::new(ctx, doc_nodes),
-      current_imports: Rc::new(get_current_imports(doc_nodes)),
+      current_imports: Rc::new(current_imports),
       current_type_params: Default::default(),
       current_resolve,
       namespace_parts: Rc::new([]),
@@ -460,36 +465,30 @@ fn split_with_brackets(s: &str) -> Vec<String> {
 }
 
 fn get_current_imports(
-  symbols: &[DocNodeWithContext],
+  imports: &[crate::node::Import],
 ) -> HashMap<String, String> {
-  let mut imports = HashMap::new();
-
-  for symbol in symbols {
-    for decl in &symbol.declarations {
-      if let DeclarationDef::Import(import_def) = &decl.def {
-        // TODO: handle import aliasing
-        if import_def.imported.as_deref() == Some(symbol.get_name()) {
-          imports.insert(symbol.get_name().to_string(), import_def.src.clone());
-        }
-      }
+  let mut imports_out = HashMap::new();
+
+  for import in imports {
+    // TODO: handle import aliasing
+    if import.original_name.as_deref() == Some(&*import.imported_name) {
+      imports_out.insert(import.imported_name.to_string(), import.src.clone());
     }
   }
 
-  imports
+  imports_out
 }
 
 #[cfg(test)]
 mod test {
   use super::*;
+  use crate::Location;
   use crate::html::HrefResolver;
   use crate::html::{
     GenerateOptions, UsageComposer, UsageComposerEntry, UsageToMd,
   };
-  use crate::node::DeclarationKind;
   use crate::node::Document;
-  use crate::node::ImportDef;
-  use crate::node::Symbol;
-  use crate::{Declaration, Location};
+  use crate::node::Import;
   use indexmap::IndexMap;
 
   struct TestResolver;
@@ -563,24 +562,13 @@ mod test {
       ModuleSpecifier::parse("file:///mod.ts").unwrap(),
       Document {
         module_doc: Default::default(),
-        symbols: vec![Symbol {
-          name: "foo".into(),
-          is_default: false,
-          declarations: vec![Declaration {
-            location: Location {
-              filename: "a".into(),
-              line: 0,
-              col: 0,
-              byte_index: 0,
-            },
-            declaration_kind: DeclarationKind::Private,
-            js_doc: Default::default(),
-            def: crate::node::DeclarationDef::Import(ImportDef {
-              src: "b".to_string(),
-              imported: Some("foo".to_string()),
-            }),
-          }],
+        imports: vec![Import {
+          imported_name: "foo".into(),
+          js_doc: Default::default(),
+          src: "b".to_string(),
+          original_name: Some("foo".to_string()),
         }],
+        symbols: vec![],
       },
     )]);
 
@@ -616,10 +604,7 @@ mod test {
     let render_ctx = RenderContext::new(&ctx, doc_nodes, UrlResolveKind::Root);
     assert_eq!(render_ctx.lookup_symbol_href("bar").unwrap(), "global$bar");
 
-    // imports
-    let render_ctx = RenderContext::new(&ctx, doc_nodes, UrlResolveKind::Root);
-    assert_eq!(render_ctx.lookup_symbol_href("foo").unwrap(), "b/foo");
-
+    // imports (only available when current resolve is a file)
     let render_ctx = RenderContext::new(
       &ctx,
       doc_nodes,

src/html/symbols/mod.rs

@@ -57,10 +57,6 @@ impl SymbolGroupCtx {
       IndexMap::<crate::node::DocNodeKind, Vec<&Declaration>>::default();
 
     for decl in &symbol.declarations {
-      if matches!(decl.def, DeclarationDef::Import(..)) {
-        continue;
-      }
-
       declarations
         .entry(decl.def.to_kind())
         .or_insert(vec![])
@@ -584,7 +580,7 @@ impl SymbolInnerCtx {
             },
           ))
         }
-        DeclarationDef::Import(..) | DeclarationDef::Reference(..) => {
+        DeclarationDef::Reference(..) => {
           unreachable!()
         }
       });

src/html/symbols/namespace.rs

@@ -439,9 +439,7 @@ fn summary_for_symbol(
         info: None,
       })
     }
-    DeclarationDef::Reference(..)
-    | DeclarationDef::Namespace(..)
-    | DeclarationDef::Import(..) => None,
+    DeclarationDef::Reference(..) | DeclarationDef::Namespace(..) => None,
   }
 }