summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBartek IwaƄczuk <biwanczuk@gmail.com>2021-10-12 00:02:33 +0200
committerGitHub <noreply@github.com>2021-10-12 00:02:33 +0200
commitf332d72f1653ec03b64a80d8d4949dce5564cc99 (patch)
tree500e6d5d523f1cf66898985d5b7134c32cc270d8
parent5bad8e17734ef8cc1f19df292d553cc1327638f3 (diff)
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>
-rw-r--r--cli/lsp/analysis.rs9
-rw-r--r--cli/lsp/diagnostics.rs6
-rw-r--r--cli/lsp/language_server.rs84
-rw-r--r--cli/tests/integration/lsp_tests.rs36
-rw-r--r--cli/tests/testdata/lsp/deno.lint.jsonc8
-rw-r--r--cli/tests/testdata/lsp/did_open_lint.json8
-rw-r--r--cli/tools/lint.rs2
7 files changed, 121 insertions, 32 deletions
diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs
index a0718e8b3..b5fca62f4 100644
--- a/cli/lsp/analysis.rs
+++ b/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,
diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs
index c106c9865..4bae048c0 100644
--- a/cli/lsp/diagnostics.rs
+++ b/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())
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index 524a59191..f92e974e8 100644
--- a/cli/lsp/language_server.rs
+++ b/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)
}
};
diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs
index 215ba1a24..3126eb31c 100644
--- a/cli/tests/integration/lsp_tests.rs
+++ b/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);
+}
diff --git a/cli/tests/testdata/lsp/deno.lint.jsonc b/cli/tests/testdata/lsp/deno.lint.jsonc
new file mode 100644
index 000000000..51db1b5c7
--- /dev/null
+++ b/cli/tests/testdata/lsp/deno.lint.jsonc
@@ -0,0 +1,8 @@
+{
+ "lint": {
+ "rules": {
+ "exclude": ["camelcase"],
+ "include": ["ban-untagged-todo"]
+ }
+ }
+}
diff --git a/cli/tests/testdata/lsp/did_open_lint.json b/cli/tests/testdata/lsp/did_open_lint.json
new file mode 100644
index 000000000..51cef9807
--- /dev/null
+++ b/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}"
+ }
+}
diff --git a/cli/tools/lint.rs b/cli/tools/lint.rs
index 5630067ae..a565fee1f 100644
--- a/cli/tools/lint.rs
+++ b/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>,