refactor(lsp): remove DocumentInner (#23251)

cli/lsp/documents.rs

@@ -178,7 +178,7 @@ impl IndexValid {
 
 #[derive(Debug, Clone)]
 pub enum AssetOrDocument {
-  Document(Document),
+  Document(Arc<Document>),
   Asset(AssetDocument),
 }
 
@@ -190,7 +190,7 @@ impl AssetOrDocument {
     }
   }
 
-  pub fn document(&self) -> Option<&Document> {
+  pub fn document(&self) -> Option<&Arc<Document>> {
     match self {
       AssetOrDocument::Asset(_) => None,
       AssetOrDocument::Document(doc) => Some(doc),
@@ -200,7 +200,7 @@ impl AssetOrDocument {
   pub fn text(&self) -> Arc<str> {
     match self {
       AssetOrDocument::Asset(a) => a.text(),
-      AssetOrDocument::Document(d) => d.0.text_info.text(),
+      AssetOrDocument::Document(d) => d.text_info.text(),
     }
   }
 
@@ -276,7 +276,7 @@ type ModuleResult = Result<deno_graph::JsModule, deno_graph::ModuleGraphError>;
 type ParsedSourceResult = Result<ParsedSource, deno_ast::ParseDiagnostic>;
 
 #[derive(Debug)]
-struct DocumentInner {
+pub struct Document {
   /// Contains the last-known-good set of dependencies from parsing the module.
   dependencies: Arc<DocumentDependencies>,
   fs_version: String,
@@ -293,9 +293,6 @@ struct DocumentInner {
   text_info: SourceTextInfo,
 }
 
-#[derive(Debug, Clone)]
-pub struct Document(Arc<DocumentInner>);
-
 impl Document {
   fn new(
     specifier: ModuleSpecifier,
@@ -304,7 +301,7 @@ impl Document {
     text_info: SourceTextInfo,
     resolver: &dyn deno_graph::source::Resolver,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
-  ) -> Self {
+  ) -> Arc<Self> {
     // we only ever do `Document::new` on on disk resources that are supposed to
     // be diagnosable, unlike `Document::open`, so it is safe to unconditionally
     // parse the module.
@@ -318,7 +315,7 @@ impl Document {
     let dependencies =
       Arc::new(DocumentDependencies::from_maybe_module(&maybe_module));
     let line_index = Arc::new(LineIndex::new(text_info.text_str()));
-    Self(Arc::new(DocumentInner {
+    Arc::new(Document {
       dependencies,
       fs_version,
       line_index,
@@ -331,42 +328,42 @@ impl Document {
         .filter(|_| specifier.scheme() == "file"),
       text_info,
       specifier,
-    }))
+    })
   }
 
   fn maybe_with_new_resolver(
     &self,
     resolver: &dyn deno_graph::source::Resolver,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
-  ) -> Option<Self> {
-    let parsed_source_result = match &self.0.maybe_parsed_source {
+  ) -> Option<Arc<Self>> {
+    let parsed_source_result = match &self.maybe_parsed_source {
       Some(parsed_source_result) => parsed_source_result.clone(),
       None => return None, // nothing to change
     };
     let maybe_module = Some(analyze_module(
-      &self.0.specifier,
+      &self.specifier,
       &parsed_source_result,
-      self.0.maybe_headers.as_ref(),
+      self.maybe_headers.as_ref(),
       resolver,
       npm_resolver,
     ));
     let dependencies =
       Arc::new(DocumentDependencies::from_maybe_module(&maybe_module));
-    Some(Self(Arc::new(DocumentInner {
+    Some(Arc::new(Self {
       // updated properties
       dependencies,
       maybe_module,
       maybe_navigation_tree: Mutex::new(None),
       maybe_parsed_source: Some(parsed_source_result),
       // maintain - this should all be copies/clones
-      fs_version: self.0.fs_version.clone(),
-      line_index: self.0.line_index.clone(),
-      maybe_headers: self.0.maybe_headers.clone(),
-      maybe_language_id: self.0.maybe_language_id,
-      maybe_lsp_version: self.0.maybe_lsp_version,
-      text_info: self.0.text_info.clone(),
-      specifier: self.0.specifier.clone(),
-    })))
+      fs_version: self.fs_version.clone(),
+      line_index: self.line_index.clone(),
+      maybe_headers: self.maybe_headers.clone(),
+      maybe_language_id: self.maybe_language_id,
+      maybe_lsp_version: self.maybe_lsp_version,
+      text_info: self.text_info.clone(),
+      specifier: self.specifier.clone(),
+    }))
   }
 
   fn open(
@@ -377,7 +374,7 @@ impl Document {
     cache: &Arc<dyn HttpCache>,
     resolver: &dyn deno_graph::source::Resolver,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
-  ) -> Self {
+  ) -> Arc<Self> {
     let maybe_headers = language_id.as_headers();
     let text_info = SourceTextInfo::new(content);
     let (maybe_parsed_source, maybe_module) = if language_id.is_diagnosable() {
@@ -394,7 +391,7 @@ impl Document {
     let dependencies =
       Arc::new(DocumentDependencies::from_maybe_module(&maybe_module));
     let line_index = Arc::new(LineIndex::new(text_info.text_str()));
-    Self(Arc::new(DocumentInner {
+    Arc::new(Self {
       dependencies,
       fs_version: calculate_fs_version(cache, &specifier)
         .unwrap_or_else(|| "1".to_string()),
@@ -408,7 +405,7 @@ impl Document {
         .filter(|_| specifier.scheme() == "file"),
       text_info,
       specifier,
-    }))
+    })
   }
 
   fn with_change(
@@ -417,9 +414,9 @@ impl Document {
     changes: Vec<lsp::TextDocumentContentChangeEvent>,
     resolver: &dyn deno_graph::source::Resolver,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
-  ) -> Result<Document, AnyError> {
-    let mut content = self.0.text_info.text_str().to_string();
-    let mut line_index = self.0.line_index.clone();
+  ) -> Result<Arc<Self>, AnyError> {
+    let mut content = self.text_info.text_str().to_string();
+    let mut line_index = self.line_index.clone();
     let mut index_valid = IndexValid::All;
     for change in changes {
       if let Some(range) = change.range {
@@ -436,19 +433,17 @@ impl Document {
     }
     let text_info = SourceTextInfo::from_string(content);
     let (maybe_parsed_source, maybe_module) = if self
-      .0
       .maybe_language_id
       .as_ref()
       .map(|li| li.is_diagnosable())
       .unwrap_or(false)
     {
       let maybe_headers = self
-        .0
         .maybe_language_id
         .as_ref()
         .and_then(|li| li.as_headers());
       parse_and_analyze_module(
-        &self.0.specifier,
+        &self.specifier,
         text_info.clone(),
         maybe_headers,
         resolver,
@@ -460,64 +455,64 @@ impl Document {
     let dependencies = if let Some(Ok(module)) = &maybe_module {
       Arc::new(DocumentDependencies::from_module(module))
     } else {
-      self.0.dependencies.clone() // use the last known good
+      self.dependencies.clone() // use the last known good
     };
     let line_index = if index_valid == IndexValid::All {
       line_index
     } else {
       Arc::new(LineIndex::new(text_info.text_str()))
     };
-    Ok(Document(Arc::new(DocumentInner {
-      specifier: self.0.specifier.clone(),
-      fs_version: self.0.fs_version.clone(),
-      maybe_language_id: self.0.maybe_language_id,
+    Ok(Arc::new(Self {
+      specifier: self.specifier.clone(),
+      fs_version: self.fs_version.clone(),
+      maybe_language_id: self.maybe_language_id,
       dependencies,
       text_info,
       line_index,
-      maybe_headers: self.0.maybe_headers.clone(),
+      maybe_headers: self.maybe_headers.clone(),
       maybe_module,
       maybe_parsed_source: maybe_parsed_source
-        .filter(|_| self.0.specifier.scheme() == "file"),
+        .filter(|_| self.specifier.scheme() == "file"),
       maybe_lsp_version: Some(version),
       maybe_navigation_tree: Mutex::new(None),
-    })))
-  }
-
-  pub fn saved(&self, cache: &Arc<dyn HttpCache>) -> Document {
-    Document(Arc::new(DocumentInner {
-      specifier: self.0.specifier.clone(),
-      fs_version: calculate_fs_version(cache, &self.0.specifier)
-        .unwrap_or_else(|| "1".to_string()),
-      maybe_language_id: self.0.maybe_language_id,
-      dependencies: self.0.dependencies.clone(),
-      text_info: self.0.text_info.clone(),
-      line_index: self.0.line_index.clone(),
-      maybe_headers: self.0.maybe_headers.clone(),
-      maybe_module: self.0.maybe_module.clone(),
-      maybe_parsed_source: self.0.maybe_parsed_source.clone(),
-      maybe_lsp_version: self.0.maybe_lsp_version,
-      maybe_navigation_tree: Mutex::new(None),
     }))
   }
 
+  pub fn saved(&self, cache: &Arc<dyn HttpCache>) -> Arc<Self> {
+    Arc::new(Self {
+      specifier: self.specifier.clone(),
+      fs_version: calculate_fs_version(cache, &self.specifier)
+        .unwrap_or_else(|| "1".to_string()),
+      maybe_language_id: self.maybe_language_id,
+      dependencies: self.dependencies.clone(),
+      text_info: self.text_info.clone(),
+      line_index: self.line_index.clone(),
+      maybe_headers: self.maybe_headers.clone(),
+      maybe_module: self.maybe_module.clone(),
+      maybe_parsed_source: self.maybe_parsed_source.clone(),
+      maybe_lsp_version: self.maybe_lsp_version,
+      maybe_navigation_tree: Mutex::new(None),
+    })
+  }
+
   pub fn specifier(&self) -> &ModuleSpecifier {
-    &self.0.specifier
+    &self.specifier
   }
 
   pub fn content(&self) -> Arc<str> {
-    self.0.text_info.text()
+    self.text_info.text()
   }
 
   pub fn text_info(&self) -> SourceTextInfo {
-    self.0.text_info.clone()
+    self.text_info.clone()
   }
 
   pub fn line_index(&self) -> Arc<LineIndex> {
-    self.0.line_index.clone()
+    self.line_index.clone()
   }
 
   fn fs_version(&self) -> &str {
-    self.0.fs_version.as_str()
+    self.fs_version.as_str()
   }
 
   pub fn script_version(&self) -> String {
@@ -545,12 +540,11 @@ impl Document {
   }
 
   pub fn is_open(&self) -> bool {
-    self.0.maybe_lsp_version.is_some()
+    self.maybe_lsp_version.is_some()
   }
 
   pub fn maybe_types_dependency(&self) -> Resolution {
-    if let Some(types_dep) = self.0.dependencies.maybe_types_dependency.as_ref()
-    {
+    if let Some(types_dep) = self.dependencies.maybe_types_dependency.as_ref() {
       types_dep.dependency.clone()
     } else {
       Resolution::None
@@ -558,42 +552,41 @@ impl Document {
   }
 
   pub fn media_type(&self) -> MediaType {
-    if let Some(Ok(module)) = &self.0.maybe_module {
+    if let Some(Ok(module)) = &self.maybe_module {
       return module.media_type;
     }
-    let specifier_media_type = MediaType::from_specifier(&self.0.specifier);
+    let specifier_media_type = MediaType::from_specifier(&self.specifier);
     if specifier_media_type != MediaType::Unknown {
       return specifier_media_type;
     }
 
     self
-      .0
       .maybe_language_id
       .map(|id| id.as_media_type())
       .unwrap_or(MediaType::Unknown)
   }
 
   pub fn maybe_language_id(&self) -> Option<LanguageId> {
-    self.0.maybe_language_id
+    self.maybe_language_id
   }
 
   /// Returns the current language server client version if any.
   pub fn maybe_lsp_version(&self) -> Option<i32> {
-    self.0.maybe_lsp_version
+    self.maybe_lsp_version
   }
 
   fn maybe_js_module(&self) -> Option<&ModuleResult> {
-    self.0.maybe_module.as_ref()
+    self.maybe_module.as_ref()
   }
 
   pub fn maybe_parsed_source(
     &self,
   ) -> Option<Result<deno_ast::ParsedSource, deno_ast::ParseDiagnostic>> {
-    self.0.maybe_parsed_source.clone()
+    self.maybe_parsed_source.clone()
   }
 
   pub fn maybe_navigation_tree(&self) -> Option<Arc<tsc::NavigationTree>> {
-    self.0.maybe_navigation_tree.lock().clone()
+    self.maybe_navigation_tree.lock().clone()
   }
 
   pub fn update_navigation_tree_if_version(
@@ -606,12 +599,12 @@ impl Document {
     // and setting the navigation tree, because the document is immutable
     // and this is enforced by it being wrapped in an Arc.
     if self.script_version() == script_version {
-      *self.0.maybe_navigation_tree.lock() = Some(tree);
+      *self.maybe_navigation_tree.lock() = Some(tree);
     }
   }
 
   pub fn dependencies(&self) -> &IndexMap<String, deno_graph::Dependency> {
-    &self.0.dependencies.deps
+    &self.dependencies.deps
   }
 
   /// If the supplied position is within a dependency range, return the resolved
@@ -727,7 +720,7 @@ impl RedirectResolver {
 
 #[derive(Debug, Default)]
 struct FileSystemDocuments {
-  docs: HashMap<ModuleSpecifier, Document>,
+  docs: HashMap<ModuleSpecifier, Arc<Document>>,
   dirty: bool,
 }
 
@@ -738,7 +731,7 @@ impl FileSystemDocuments {
     resolver: &dyn deno_graph::source::Resolver,
     specifier: &ModuleSpecifier,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
-  ) -> Option<Document> {
+  ) -> Option<Arc<Document>> {
     let fs_version = if specifier.scheme() == "data" {
       Some("1".to_string())
     } else {
@@ -761,7 +754,7 @@ impl FileSystemDocuments {
     resolver: &dyn deno_graph::source::Resolver,
     specifier: &ModuleSpecifier,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
-  ) -> Option<Document> {
+  ) -> Option<Arc<Document>> {
     let doc = if specifier.scheme() == "file" {
       let path = specifier_to_file_path(specifier).ok()?;
       let fs_version = calculate_fs_version_at_path(&path)?;
@@ -845,7 +838,7 @@ pub struct Documents {
   /// that depend on the key.
   dependents_map: Arc<HashMap<ModuleSpecifier, HashSet<ModuleSpecifier>>>,
   /// A map of documents that are "open" in the language server.
-  open_docs: HashMap<ModuleSpecifier, Document>,
+  open_docs: HashMap<ModuleSpecifier, Arc<Document>>,
   /// Documents stored on the file system.
   file_system_docs: Arc<Mutex<FileSystemDocuments>>,
   /// Any imports to the context supplied by configuration files. This is like
@@ -923,7 +916,7 @@ impl Documents {
     version: i32,
     language_id: LanguageId,
     content: Arc<str>,
-  ) -> Document {
+  ) -> Arc<Document> {
     let resolver = self.get_resolver();
     let npm_resolver = self.get_npm_resolver();
     let document = Document::open(
@@ -952,7 +945,7 @@ impl Documents {
     specifier: &ModuleSpecifier,
     version: i32,
     changes: Vec<lsp::TextDocumentContentChangeEvent>,
-  ) -> Result<Document, AnyError> {
+  ) -> Result<Arc<Document>, AnyError> {
     let doc = self
       .open_docs
       .get(specifier)
@@ -1135,7 +1128,10 @@ impl Documents {
   }
 
   /// Return a document for the specifier.
-  pub fn get(&self, original_specifier: &ModuleSpecifier) -> Option<Document> {
+  pub fn get(
+    &self,
+    original_specifier: &ModuleSpecifier,
+  ) -> Option<Arc<Document>> {
     let specifier = self.resolve_specifier(original_specifier)?;
     if let Some(document) = self.open_docs.get(&specifier) {
       Some(document.clone())
@@ -1152,7 +1148,7 @@ impl Documents {
 
   /// Return a collection of documents that are contained in the document store
   /// based on the provided filter.
-  pub fn documents(&self, filter: DocumentsFilter) -> Vec<Document> {
+  pub fn documents(&self, filter: DocumentsFilter) -> Vec<Arc<Document>> {
     match filter {
       DocumentsFilter::OpenDiagnosable => self
         .open_docs
@@ -1203,7 +1199,7 @@ impl Documents {
     let referrer = referrer_doc.specifier();
     let dependencies = match referrer_doc {
       AssetOrDocument::Asset(_) => None,
-      AssetOrDocument::Document(doc) => Some(doc.0.dependencies.clone()),
+      AssetOrDocument::Document(doc) => Some(doc.dependencies.clone()),
     };
     let mut results = Vec::new();
     for specifier in specifiers {
@@ -1605,7 +1601,7 @@ fn node_resolve_npm_req_ref(
 /// Loader that will look at the open documents.
 pub struct OpenDocumentsGraphLoader<'a> {
   pub inner_loader: &'a mut dyn deno_graph::source::Loader,
-  pub open_docs: &'a HashMap<ModuleSpecifier, Document>,
+  pub open_docs: &'a HashMap<ModuleSpecifier, Arc<Document>>,
   pub unstable_sloppy_imports: bool,
 }
 

cli/lsp/language_server.rs

@@ -286,7 +286,7 @@ impl LanguageServer {
     async fn create_graph_for_caching(
       cli_options: CliOptions,
       roots: Vec<ModuleSpecifier>,
-      open_docs: Vec<Document>,
+      open_docs: Vec<Arc<Document>>,
     ) -> Result<(), AnyError> {
       let open_docs = open_docs
         .into_iter()
@@ -1193,7 +1193,7 @@ impl Inner {
     &mut self,
     specifier: &ModuleSpecifier,
     params: DidOpenTextDocumentParams,
-  ) -> Document {
+  ) -> Arc<Document> {
     let mark = self.performance.mark_with_args("lsp.did_open", &params);
     let language_id =
       params
@@ -3452,7 +3452,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
 struct PrepareCacheResult {
   cli_options: CliOptions,
   roots: Vec<ModuleSpecifier>,
-  open_docs: Vec<Document>,
+  open_docs: Vec<Arc<Document>>,
   mark: PerformanceMark,
 }
 

cli/lsp/tsc.rs

@@ -1013,54 +1013,48 @@ impl TsServer {
   }
 }
 
+/// An lsp representation of an asset in memory, that has either been retrieved
+/// from static assets built into Rust, or static assets built into tsc.
 #[derive(Debug, Clone)]
-struct AssetDocumentInner {
+pub struct AssetDocument {
   specifier: ModuleSpecifier,
   text: Arc<str>,
   line_index: Arc<LineIndex>,
   maybe_navigation_tree: Option<Arc<NavigationTree>>,
 }
 
-/// An lsp representation of an asset in memory, that has either been retrieved
-/// from static assets built into Rust, or static assets built into tsc.
-#[derive(Debug, Clone)]
-pub struct AssetDocument(Arc<AssetDocumentInner>);
-
 impl AssetDocument {
   pub fn new(specifier: ModuleSpecifier, text: impl AsRef<str>) -> Self {
     let text = text.as_ref();
-    Self(Arc::new(AssetDocumentInner {
+    Self {
       specifier,
       text: text.into(),
       line_index: Arc::new(LineIndex::new(text)),
       maybe_navigation_tree: None,
-    }))
+    }
   }
 
   pub fn specifier(&self) -> &ModuleSpecifier {
-    &self.0.specifier
+    &self.specifier
   }
 
-  pub fn with_navigation_tree(
-    &self,
-    tree: Arc<NavigationTree>,
-  ) -> AssetDocument {
-    AssetDocument(Arc::new(AssetDocumentInner {
+  pub fn with_navigation_tree(&self, tree: Arc<NavigationTree>) -> Self {
+    Self {
       maybe_navigation_tree: Some(tree),
-      ..(*self.0).clone()
-    }))
+      ..self.clone()
+    }
   }
 
   pub fn text(&self) -> Arc<str> {
-    self.0.text.clone()
+    self.text.clone()
   }
 
   pub fn line_index(&self) -> Arc<LineIndex> {
-    self.0.line_index.clone()
+    self.line_index.clone()
   }
 
   pub fn maybe_navigation_tree(&self) -> Option<Arc<NavigationTree>> {
-    self.0.maybe_navigation_tree.clone()
+    self.maybe_navigation_tree.clone()
   }
 }