Repository: denoland/deno
Pull request: fix(lsp): inherit missing fmt and lint config from parent scopes

<!--
Before submitting a PR, please read https://docs.deno.com/runtime/manual/references/contributing

1. Give the PR a descriptive title.

  Examples of good title:
    - fix(std/http): Fix race condition in server
    - docs(console): Update docstrings
    - feat(doc): Handle nested reexports

  Examples of bad title:
    - fix #7123
    - update docs
    - fix bugs

2. Ensure there is a related issue and it is referenced in the PR text.
3. Ensure there are tests that cover the changes.
4. Ensure `cargo test` passes.
5. Ensure `./tools/format.js` passes without changing files.
6. Ensure `./tools/lint.js` passes.
7. Open as a draft PR if your work is still in progress. The CI won't run
   all steps, but you can add '[ci]' to a commit message to force it to.
8. If you would like to run the benchmarks on the CI, add the 'ci-bench' label.
-->


Changed files:
filename: cli/lsp/config.rs
patch: @@ -1169,6 +1169,7 @@ impl ConfigData {
   async fn load(
     config_file_specifier: Option<&ModuleSpecifier>,
     scope: &ModuleSpecifier,
+    parent: Option<(&ModuleSpecifier, &ConfigData)>,
     settings: &Settings,
     file_fetcher: Option<&FileFetcher>,
   ) -> Self {
@@ -1182,8 +1183,14 @@ impl ConfigData {
             "  Resolved Deno configuration file: \"{}\"",
             config_file.specifier.as_str()
           );
-          Self::load_inner(Some(config_file), scope, settings, file_fetcher)
-            .await
+          Self::load_inner(
+            Some(config_file),
+            scope,
+            parent,
+            settings,
+            file_fetcher,
+          )
+          .await
         }
         Err(err) => {
           lsp_warn!(
@@ -1192,7 +1199,7 @@ impl ConfigData {
             err
           );
           let mut data =
-            Self::load_inner(None, scope, settings, file_fetcher).await;
+            Self::load_inner(None, scope, parent, settings, file_fetcher).await;
           data
             .watched_files
             .insert(specifier.clone(), ConfigWatchedFileType::DenoJson);
@@ -1210,13 +1217,14 @@ impl ConfigData {
         }
       }
     } else {
-      Self::load_inner(None, scope, settings, file_fetcher).await
+      Self::load_inner(None, scope, parent, settings, file_fetcher).await
     }
   }
 
   async fn load_inner(
     config_file: Option<ConfigFile>,
     scope: &ModuleSpecifier,
+    parent: Option<(&ModuleSpecifier, &ConfigData)>,
     settings: &Settings,
     file_fetcher: Option<&FileFetcher>,
   ) -> Self {
@@ -1238,45 +1246,76 @@ impl ConfigData {
         .or_insert(ConfigWatchedFileType::DenoJson);
     }
 
-    // Resolve some config file fields ahead of time
-    let fmt_options = config_file
-      .as_ref()
-      .and_then(|config_file| {
-        config_file
-          .to_fmt_config()
-          .and_then(|o| {
-            let base_path = config_file
-              .specifier
-              .to_file_path()
-              .map_err(|_| anyhow!("Invalid base path."))?;
-            FmtOptions::resolve(o, None, &base_path)
-          })
-          .inspect_err(|err| {
-            lsp_warn!("  Couldn't read formatter configuration: {}", err)
-          })
-          .ok()
-      })
-      .unwrap_or_default();
-    let lint_options = config_file
-      .as_ref()
-      .and_then(|config_file| {
-        config_file
-          .to_lint_config()
-          .and_then(|o| {
-            let base_path = config_file
-              .specifier
-              .to_file_path()
-              .map_err(|_| anyhow!("Invalid base path."))?;
-            LintOptions::resolve(o, None, &base_path)
-          })
-          .inspect_err(|err| {
-            lsp_warn!("  Couldn't read lint configuration: {}", err)
-          })
-          .ok()
-      })
-      .unwrap_or_default();
-    let lint_rules =
-      get_configured_rules(lint_options.rules.clone(), config_file.as_ref());
+    let mut fmt_options = None;
+    if let Some((_, parent_data)) = parent {
+      let has_own_fmt_options = config_file
+        .as_ref()
+        .is_some_and(|config_file| config_file.json.fmt.is_some());
+      if !has_own_fmt_options {
+        fmt_options = Some(parent_data.fmt_options.clone())
+      }
+    }
+    let fmt_options = fmt_options.unwrap_or_else(|| {
+      config_file
+        .as_ref()
+        .and_then(|config_file| {
+          config_file
+            .to_fmt_config()
+            .and_then(|o| {
+              let base_path = config_file
+                .specifier
+                .to_file_path()
+                .map_err(|_| anyhow!("Invalid base path."))?;
+              FmtOptions::resolve(o, None, &base_path)
+            })
+            .inspect_err(|err| {
+              lsp_warn!("  Couldn't read formatter configuration: {}", err)
+            })
+            .ok()
+        })
+        .map(Arc::new)
+        .unwrap_or_default()
+    });
+
+    let mut lint_options_rules = None;
+    if let Some((_, parent_data)) = parent {
+      let has_own_lint_options = config_file
+        .as_ref()
+        .is_some_and(|config_file| config_file.json.lint.is_some());
+      if !has_own_lint_options {
+        lint_options_rules = Some((
+          parent_data.lint_options.clone(),
+          parent_data.lint_rules.clone(),
+        ))
+      }
+    }
+    let (lint_options, lint_rules) = lint_options_rules.unwrap_or_else(|| {
+      let lint_options = config_file
+        .as_ref()
+        .and_then(|config_file| {
+          config_file
+            .to_lint_config()
+            .and_then(|o| {
+              let base_path = config_file
+                .specifier
+                .to_file_path()
+                .map_err(|_| anyhow!("Invalid base path."))?;
+              LintOptions::resolve(o, None, &base_path)
+            })
+            .inspect_err(|err| {
+              lsp_warn!("  Couldn't read lint configuration: {}", err)
+            })
+            .ok()
+        })
+        .map(Arc::new)
+        .unwrap_or_default();
+      let lint_rules = Arc::new(get_configured_rules(
+        lint_options.rules.clone(),
+        config_file.as_ref(),
+      ));
+      (lint_options, lint_rules)
+    });
+
     let vendor_dir = config_file.as_ref().and_then(|c| c.vendor_dir_path());
 
     // Load lockfile
@@ -1449,9 +1488,9 @@ impl ConfigData {
 
     ConfigData {
       config_file: config_file.map(Arc::new),
-      fmt_options: Arc::new(fmt_options),
-      lint_options: Arc::new(lint_options),
-      lint_rules: Arc::new(lint_rules),
+      fmt_options,
+      lint_options,
+      lint_rules,
       ts_config: Arc::new(ts_config),
       byonm,
       node_modules_dir,
@@ -1601,6 +1640,7 @@ impl ConfigTree {
               ConfigData::load(
                 Some(&config_uri),
                 folder_uri,
+                None,
                 settings,
                 Some(file_fetcher),
               )
@@ -1616,17 +1656,20 @@ impl ConfigTree {
         || specifier.path().ends_with("/deno.jsonc")
       {
         if let Ok(scope) = specifier.join(".") {
-          let entry = scopes.entry(scope.clone());
-          #[allow(clippy::map_entry)]
-          if matches!(entry, std::collections::btree_map::Entry::Vacant(_)) {
+          if !scopes.contains_key(&scope) {
+            let parent = scopes
+              .iter()
+              .rev()
+              .find(|(s, _)| scope.as_str().starts_with(s.as_str()));
             let data = ConfigData::load(
               Some(specifier),
               &scope,
+              parent,
               settings,
               Some(file_fetcher),
             )
             .await;
-            entry.or_insert(data);
+            scopes.insert(scope, data);
           }
         }
       }
@@ -1639,8 +1682,14 @@ impl ConfigTree {
       {
         scopes.insert(
           folder_uri.clone(),
-          ConfigData::load(None, folder_uri, settings, Some(file_fetcher))
-            .await,
+          ConfigData::load(
+            None,
+            folder_uri,
+            None,
+            settings,
+            Some(file_fetcher),
+          )
+          .await,
         );
       }
     }
@@ -1654,6 +1703,7 @@ impl ConfigTree {
     let data = ConfigData::load_inner(
       Some(config_file),
       &scope,
+      None,
       &Default::default(),
       None,
     )

filename: tests/integration/lsp_tests.rs
patch: @@ -11468,6 +11468,8 @@ fn lsp_deno_json_scopes_fmt_config() {
     })
     .to_string(),
   );
+  temp_dir.create_dir_all("project2/project3");
+  temp_dir.write("project2/project3/deno.json", json!({}).to_string());
   let mut client = context.new_lsp_command().build();
   client.initialize_default();
   client.did_open(json!({
@@ -11530,6 +11532,38 @@ fn lsp_deno_json_scopes_fmt_config() {
       "newText": "''",
     }])
   );
+  // `project2/project3/file.ts` should use the fmt settings from
+  // `project2/deno.json`, since `project2/project3/deno.json` has no fmt field.
+  client.did_open(json!({
+    "textDocument": {
+      "uri": temp_dir.uri().join("project2/project3/file.ts").unwrap(),
+      "languageId": "typescript",
+      "version": 1,
+      "text": "console.log(\"\");\n",
+    },
+  }));
+  let res = client.write_request(
+    "textDocument/formatting",
+    json!({
+      "textDocument": {
+        "uri": temp_dir.uri().join("project2/project3/file.ts").unwrap(),
+      },
+      "options": {
+        "tabSize": 2,
+        "insertSpaces": true,
+      },
+    }),
+  );
+  assert_eq!(
+    res,
+    json!([{
+      "range": {
+        "start": { "line": 0, "character": 12 },
+        "end": { "line": 0, "character": 14 },
+      },
+      "newText": "''",
+    }])
+  );
   client.shutdown();
 }
 
@@ -11561,6 +11595,8 @@ fn lsp_deno_json_scopes_lint_config() {
     })
     .to_string(),
   );
+  temp_dir.create_dir_all("project2/project3");
+  temp_dir.write("project2/project3/deno.json", json!({}).to_string());
   let mut client = context.new_lsp_command().build();
   client.initialize_default();
   let diagnostics = client.did_open(json!({
@@ -11629,6 +11665,46 @@ fn lsp_deno_json_scopes_lint_config() {
       "version": 1,
     })
   );
+  client.write_notification(
+    "textDocument/didClose",
+    json!({
+      "textDocument": {
+        "uri": temp_dir.uri().join("project2/file.ts").unwrap(),
+      },
+    }),
+  );
+  // `project2/project3/file.ts` should use the lint settings from
+  // `project2/deno.json`, since `project2/project3/deno.json` has no lint
+  // field.
+  let diagnostics = client.did_open(json!({
+    "textDocument": {
+      "uri": temp_dir.uri().join("project2/project3/file.ts").unwrap(),
+      "languageId": "typescript",
+      "version": 1,
+      "text": r#"
+        // TODO: Unused var
+        const snake_case_var = 1;
+        console.log(snake_case_var);
+      "#,
+    },
+  }));
+  assert_eq!(
+    json!(diagnostics.messages_with_source("deno-lint")),
+    json!({
+      "uri": temp_dir.uri().join("project2/project3/file.ts").unwrap(),
+      "diagnostics": [{
+        "range": {
+          "start": { "line": 1, "character": 8 },
+          "end": { "line": 1, "character": 27 },
+        },
+        "severity": 2,
+        "code": "ban-untagged-todo",
+        "source": "deno-lint",
+        "message": "TODO should be tagged with (@username) or (#issue)\nAdd a user tag or issue reference to the TODO comment, e.g. TODO(@djones), TODO(djones), TODO(#123)",
+      }],
+      "version": 1,
+    })
+  );
   client.shutdown();
 }