fix(lsp): lint diagnostics respect config file (#12338)

This commit fixes problem with LSP where diagnostics coming
from "deno lint" don't respect configuration file.

LSP was changed to store "Option<ConfigFile>", "Option<LintConfig>"
and "Option<FmtConfig>" on "Inner"; as well as storing "Option<LintConfig>"
and "Option<FmtConfig>" on "StateSnapshot".

Co-authored-by: Kitson Kelly <me@kitsonkelly.com>

cli/lsp/analysis.rs

@@ -5,8 +5,10 @@ use super::tsc;
 
 use crate::ast;
 use crate::ast::Location;
+use crate::config_file::LintConfig;
 use crate::lsp::documents::DocumentData;
 use crate::tools::lint::create_linter;
+use crate::tools::lint::get_configured_rules;
 
 use deno_ast::swc::ast as swc_ast;
 use deno_ast::swc::common::comments::Comment;
@@ -28,7 +30,6 @@ use deno_core::serde_json::json;
 use deno_core::url;
 use deno_core::ModuleResolutionError;
 use deno_core::ModuleSpecifier;
-use deno_lint::rules;
 use import_map::ImportMap;
 use lspower::lsp;
 use lspower::lsp::Position;
@@ -196,9 +197,11 @@ fn as_lsp_range(range: &deno_lint::diagnostic::Range) -> Range {
 
 pub fn get_lint_references(
   parsed_source: &deno_ast::ParsedSource,
+  maybe_lint_config: Option<&LintConfig>,
 ) -> Result<Vec<Reference>, AnyError> {
   let syntax = deno_ast::get_syntax(parsed_source.media_type());
-  let lint_rules = rules::get_recommended_rules();
+  let lint_rules =
+    get_configured_rules(maybe_lint_config, vec![], vec![], vec![])?;
   let linter = create_linter(syntax, lint_rules);
   // TODO(dsherret): do not re-parse here again
   let (_, lint_diagnostics) = linter.lint(
@@ -1350,7 +1353,7 @@ mod tests {
       MediaType::TypeScript,
     )
     .unwrap();
-    let actual = get_lint_references(&parsed_module).unwrap();
+    let actual = get_lint_references(&parsed_module, None).unwrap();
 
     assert_eq!(
       actual,

cli/lsp/diagnostics.rs

@@ -314,6 +314,7 @@ async fn generate_lint_diagnostics(
 ) -> Result<DiagnosticVec, AnyError> {
   let documents = snapshot.documents.clone();
   let workspace_settings = snapshot.config.settings.workspace.clone();
+  let maybe_lint_config = snapshot.maybe_lint_config.clone();
   tokio::task::spawn(async move {
     let mut diagnostics_vec = Vec::new();
     if workspace_settings.lint {
@@ -333,7 +334,10 @@ async fn generate_lint_diagnostics(
             .flatten();
           let diagnostics = match module {
             Some(Ok(module)) => {
-              if let Ok(references) = analysis::get_lint_references(module) {
+              if let Ok(references) = analysis::get_lint_references(
+                module,
+                maybe_lint_config.as_ref(),
+              ) {
                 references
                   .into_iter()
                   .map(|r| r.to_diagnostic())

cli/lsp/language_server.rs

@@ -54,6 +54,8 @@ use super::tsc::Assets;
 use super::tsc::TsServer;
 use super::urls;
 use crate::config_file::ConfigFile;
+use crate::config_file::FmtConfig;
+use crate::config_file::LintConfig;
 use crate::config_file::TsConfig;
 use crate::deno_dir;
 use crate::file_fetcher::get_source_from_data_url;
@@ -73,6 +75,8 @@ pub struct StateSnapshot {
   pub assets: Assets,
   pub config: ConfigSnapshot,
   pub documents: DocumentCache,
+  pub maybe_lint_config: Option<LintConfig>,
+  pub maybe_fmt_config: Option<FmtConfig>,
   pub maybe_config_uri: Option<ModuleSpecifier>,
   pub module_registries: registries::ModuleRegistry,
   pub performance: Performance,
@@ -104,6 +108,10 @@ pub(crate) struct Inner {
   /// An optional configuration file which has been specified in the client
   /// options.
   maybe_config_file: Option<ConfigFile>,
+  /// An optional configuration for linter which has been taken from specified config file.
+  maybe_lint_config: Option<LintConfig>,
+  /// An optional configuration for formatter which has been taken from specified config file.
+  maybe_fmt_config: Option<FmtConfig>,
   /// An optional URL which provides the location of a TypeScript configuration
   /// file which will be used by the Deno LSP.
   maybe_config_uri: Option<Url>,
@@ -151,6 +159,8 @@ impl Inner {
       diagnostics_server,
       documents: Default::default(),
       maybe_cache_path: None,
+      maybe_lint_config: None,
+      maybe_fmt_config: None,
       maybe_cache_server: None,
       maybe_config_file: None,
       maybe_config_uri: None,
@@ -388,16 +398,9 @@ impl Inner {
     &mut self,
     tsconfig: &mut TsConfig,
   ) -> Result<(), AnyError> {
-    self.maybe_config_file = None;
-    self.maybe_config_uri = None;
-
-    let maybe_file_and_url = self.get_config_file_and_url()?;
-
-    if let Some((config_file, config_url)) = maybe_file_and_url {
+    if let Some(config_file) = self.maybe_config_file.as_ref() {
       let (value, maybe_ignored_options) = config_file.to_compiler_options()?;
       tsconfig.merge(&value);
-      self.maybe_config_file = Some(config_file);
-      self.maybe_config_uri = Some(config_url);
       if let Some(ignored_options) = maybe_ignored_options {
         // TODO(@kitsonk) turn these into diagnostics that can be sent to the
         // client
@@ -416,6 +419,8 @@ impl Inner {
         LspError::internal_error()
       })?,
       documents: self.documents.clone(),
+      maybe_lint_config: self.maybe_lint_config.clone(),
+      maybe_fmt_config: self.maybe_fmt_config.clone(),
       maybe_config_uri: self.maybe_config_uri.clone(),
       module_registries: self.module_registries.clone(),
       performance: self.performance.clone(),
@@ -579,6 +584,37 @@ impl Inner {
     Ok(())
   }
 
+  fn update_config_file(&mut self) -> Result<(), AnyError> {
+    self.maybe_config_file = None;
+    self.maybe_config_uri = None;
+    self.maybe_fmt_config = None;
+    self.maybe_lint_config = None;
+
+    let maybe_file_and_url = self.get_config_file_and_url()?;
+
+    if let Some((config_file, config_url)) = maybe_file_and_url {
+      let lint_config = config_file
+        .to_lint_config()
+        .map_err(|err| {
+          anyhow!("Unable to update lint configuration: {:?}", err)
+        })?
+        .unwrap_or_default();
+      let fmt_config = config_file
+        .to_fmt_config()
+        .map_err(|err| {
+          anyhow!("Unable to update formatter configuration: {:?}", err)
+        })?
+        .unwrap_or_default();
+
+      self.maybe_config_file = Some(config_file);
+      self.maybe_config_uri = Some(config_url);
+      self.maybe_lint_config = Some(lint_config);
+      self.maybe_fmt_config = Some(fmt_config);
+    }
+
+    Ok(())
+  }
+
   async fn update_tsconfig(&mut self) -> Result<(), AnyError> {
     let mark = self.performance.mark("update_tsconfig", None::<()>);
     let mut tsconfig = TsConfig::new(json!({
@@ -694,6 +730,9 @@ impl Inner {
     if let Err(err) = self.update_cache() {
       self.client.show_message(MessageType::Warning, err).await;
     }
+    if let Err(err) = self.update_config_file() {
+      self.client.show_message(MessageType::Warning, err).await;
+    }
     if let Err(err) = self.update_tsconfig().await {
       self.client.show_message(MessageType::Warning, err).await;
     }
@@ -918,6 +957,9 @@ impl Inner {
     if let Err(err) = self.update_registries().await {
       self.client.show_message(MessageType::Warning, err).await;
     }
+    if let Err(err) = self.update_config_file() {
+      self.client.show_message(MessageType::Warning, err).await;
+    }
     if let Err(err) = self.update_tsconfig().await {
       self.client.show_message(MessageType::Warning, err).await;
     }
@@ -948,6 +990,9 @@ impl Inner {
     // if the current tsconfig has changed, we need to reload it
     if let Some(config_uri) = &self.maybe_config_uri {
       if params.changes.iter().any(|fe| *config_uri == fe.uri) {
+        if let Err(err) = self.update_config_file() {
+          self.client.show_message(MessageType::Warning, err).await;
+        }
         if let Err(err) = self.update_tsconfig().await {
           self.client.show_message(MessageType::Warning, err).await;
         }
@@ -1031,19 +1076,8 @@ impl Inner {
         PathBuf::from(params.text_document.uri.path())
       };
 
-    let maybe_file_and_url = self.get_config_file_and_url().map_err(|err| {
-      error!("Unable to parse configuration file: {}", err);
-      LspError::internal_error()
-    })?;
-
-    let fmt_options = if let Some((config_file, _)) = maybe_file_and_url {
-      config_file
-        .to_fmt_config()
-        .map_err(|err| {
-          error!("Unable to parse fmt configuration: {}", err);
-          LspError::internal_error()
-        })?
-        .unwrap_or_default()
+    let fmt_options = if let Some(fmt_config) = self.maybe_fmt_config.as_ref() {
+      fmt_config.options.clone()
     } else {
       Default::default()
     };
@@ -1052,16 +1086,12 @@ impl Inner {
     let text_edits = tokio::task::spawn_blocking(move || {
       let format_result = match source.module() {
         Some(Ok(parsed_module)) => {
-          Ok(format_parsed_module(parsed_module, fmt_options.options))
+          Ok(format_parsed_module(parsed_module, fmt_options))
         }
         Some(Err(err)) => Err(err.to_string()),
         None => {
           // it's not a js/ts file, so attempt to format its contents
-          format_file(
-            &file_path,
-            source.text_info().text_str(),
-            fmt_options.options,
-          )
+          format_file(&file_path, source.text_info().text_str(), fmt_options)
         }
       };
 

cli/tests/integration/lsp_tests.rs

@@ -3560,3 +3560,39 @@ console.log(snake_case);
   );
   shutdown(&mut client);
 }
+
+#[test]
+fn lsp_lint_with_config() {
+  let temp_dir = TempDir::new().expect("could not create temp dir");
+  let mut params: lsp::InitializeParams =
+    serde_json::from_value(load_fixture("initialize_params.json")).unwrap();
+  let deno_lint_jsonc =
+    serde_json::to_vec_pretty(&load_fixture("deno.lint.jsonc")).unwrap();
+  fs::write(temp_dir.path().join("deno.lint.jsonc"), deno_lint_jsonc).unwrap();
+
+  params.root_uri = Some(Url::from_file_path(temp_dir.path()).unwrap());
+  if let Some(Value::Object(mut map)) = params.initialization_options {
+    map.insert("config".to_string(), json!("./deno.lint.jsonc"));
+    params.initialization_options = Some(Value::Object(map));
+  }
+
+  let deno_exe = deno_exe_path();
+  let mut client = LspClient::new(&deno_exe).unwrap();
+  client
+    .write_request::<_, _, Value>("initialize", params)
+    .unwrap();
+
+  let diagnostics = did_open(&mut client, load_fixture("did_open_lint.json"));
+  let diagnostics = diagnostics
+    .into_iter()
+    .flat_map(|x| x.diagnostics)
+    .collect::<Vec<_>>();
+  assert_eq!(diagnostics.len(), 3);
+  for diagnostic in diagnostics {
+    assert_eq!(
+      diagnostic.code,
+      Some(lsp::NumberOrString::String("ban-untagged-todo".to_string()))
+    );
+  }
+  shutdown(&mut client);
+}

cli/tests/testdata/lsp/deno.lint.jsonc

@@ -0,0 +1,8 @@
+{
+  "lint": {
+    "rules": {
+      "exclude": ["camelcase"],
+      "include": ["ban-untagged-todo"]
+    }
+  }
+}

cli/tests/testdata/lsp/did_open_lint.json

@@ -0,0 +1,8 @@
+{
+  "textDocument": {
+    "uri": "file:///a/file.ts",
+    "languageId": "typescript",
+    "version": 1,
+    "text": "// TODO: fixme\nexport async function non_camel_case() {\nconsole.log(\"finished!\")\n}"
+  }
+}

cli/tools/lint.rs

@@ -484,7 +484,7 @@ fn sort_diagnostics(diagnostics: &mut Vec<LintDiagnostic>) {
   });
 }
 
-fn get_configured_rules(
+pub(crate) fn get_configured_rules(
   maybe_lint_config: Option<&LintConfig>,
   rules_tags: Vec<String>,
   rules_include: Vec<String>,