fix(lsp): correctly infer file source from un-named files (#1413)

biomejs · Jan 3, 2024 · d869a33 · d869a33
1 parent 7fa4369
commit d869a33
Show file tree

Hide file tree

Showing 8 changed files with 294 additions and 163 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -279,6 +279,7 @@ Biome now scores 97% compatibility with Prettier and features more than 180 lint
 #### Bug fixes
 
 - Fix [#933](https://github.com/biomejs/biome/issues/933). Some files are properly ignored in the LSP too. E.g. `package.json`, `tsconfig.json`, etc.
+- Fix [#1394](https://github.com/biomejs/biome/issues/1394), by inferring the language extension from the internal saved files. Now newly created files JavaScript correctly show diagnostics.
 
 ### Formatter
 

diff --git a/crates/biome_lsp/tests/server.rs b/crates/biome_lsp/tests/server.rs
@@ -204,6 +204,21 @@ impl Server {
         .await
     }
 
+    async fn open_untitled_document(&mut self, text: impl Display) -> Result<()> {
+        self.notify(
+            "textDocument/didOpen",
+            DidOpenTextDocumentParams {
+                text_document: TextDocumentItem {
+                    uri: url!("untitled-1"),
+                    language_id: String::from("javascript"),
+                    version: 0,
+                    text: text.to_string(),
+                },
+            },
+        )
+        .await
+    }
+
     /// Opens a document with given contents and given name. The name must contain the extension too
     async fn open_named_document(
         &mut self,
@@ -624,6 +639,88 @@ async fn pull_diagnostics() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test]
+async fn pull_diagnostics_from_new_file() -> Result<()> {
+    let factory = ServerFactory::default();
+    let (service, client) = factory.create(None).into_inner();
+    let (stream, sink) = client.split();
+    let mut server = Server::new(service);
+
+    let (sender, mut receiver) = channel(CHANNEL_BUFFER_SIZE);
+    let reader = tokio::spawn(client_handler(stream, sink, sender));
+
+    server.initialize().await?;
+    server.initialized().await?;
+
+    server.open_untitled_document("if(a == b) {}").await?;
+
+    let notification = tokio::select! {
+        msg = receiver.next() => msg,
+        _ = sleep(Duration::from_secs(1)) => {
+            panic!("timed out waiting for the server to send diagnostics")
+        }
+    };
+
+    assert_eq!(
+        notification,
+        Some(ServerNotification::PublishDiagnostics(
+            PublishDiagnosticsParams {
+                uri: url!("untitled-1"),
+                version: Some(0),
+                diagnostics: vec![lsp::Diagnostic {
+                    range: lsp::Range {
+                        start: lsp::Position {
+                            line: 0,
+                            character: 5,
+                        },
+                        end: lsp::Position {
+                            line: 0,
+                            character: 7,
+                        },
+                    },
+                    severity: Some(lsp::DiagnosticSeverity::ERROR),
+                    code: Some(lsp::NumberOrString::String(String::from(
+                        "lint/suspicious/noDoubleEquals",
+                    ))),
+                    code_description: Some(CodeDescription {
+                        href: Url::parse("https://biomejs.dev/linter/rules/no-double-equals")
+                            .unwrap()
+                    }),
+                    source: Some(String::from("biome")),
+                    message: String::from(
+                        "Use === instead of ==.\n== is only allowed when comparing against `null`",
+                    ),
+                    related_information: Some(vec![lsp::DiagnosticRelatedInformation {
+                        location: lsp::Location {
+                            uri: url!("untitled-1"),
+                            range: lsp::Range {
+                                start: lsp::Position {
+                                    line: 0,
+                                    character: 5,
+                                },
+                                end: lsp::Position {
+                                    line: 0,
+                                    character: 7,
+                                },
+                            },
+                        },
+                        message: String::new(),
+                    }]),
+                    tags: None,
+                    data: None,
+                }],
+            }
+        ))
+    );
+
+    server.close_document().await?;
+
+    server.shutdown().await?;
+    reader.abort();
+
+    Ok(())
+}
+
 fn fixable_diagnostic(line: u32) -> Result<lsp::Diagnostic> {
     Ok(lsp::Diagnostic {
         range: lsp::Range {

diff --git a/crates/biome_service/src/file_handlers/javascript.rs b/crates/biome_service/src/file_handlers/javascript.rs
@@ -43,7 +43,7 @@ use biome_rowan::{AstNode, BatchMutationExt, Direction, FileSource, NodeCache};
 use std::borrow::Cow;
 use std::fmt::Debug;
 use std::path::PathBuf;
-use tracing::{debug, error, info, trace};
+use tracing::{debug, debug_span, error, info, trace};
 
 #[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)]
 #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
@@ -267,89 +267,101 @@ fn debug_formatter_ir(
 }
 
 fn lint(params: LintParams) -> LintResults {
-    let Ok(file_source) = params.parse.file_source(params.path) else {
-        return LintResults {
-            errors: 0,
-            diagnostics: vec![],
-            skipped_diagnostics: 0,
-        };
-    };
-    let tree = params.parse.tree();
-    let mut diagnostics = params.parse.into_diagnostics();
+    debug_span!("Linting JavaScript file", path =? params.path, language =? params.language)
+        .in_scope(move || {
+            let file_source = match params.parse.file_source(params.path) {
+                Ok(file_source) => file_source,
+                Err(_) => {
+                    if let Some(file_source) = params.language.as_js_file_source() {
+                        file_source
+                    } else {
+                        return LintResults {
+                            errors: 0,
+                            diagnostics: vec![],
+                            skipped_diagnostics: 0,
+                        };
+                    }
+                }
+            };
+            let tree = params.parse.tree();
+            let mut diagnostics = params.parse.into_diagnostics();
 
-    let analyzer_options =
-        compute_analyzer_options(&params.settings, PathBuf::from(params.path.as_path()));
+            let analyzer_options =
+                compute_analyzer_options(&params.settings, PathBuf::from(params.path.as_path()));
 
-    let mut diagnostic_count = diagnostics.len() as u64;
-    let mut errors = diagnostics
-        .iter()
-        .filter(|diag| diag.severity() <= Severity::Error)
-        .count();
+            let mut diagnostic_count = diagnostics.len() as u64;
+            let mut errors = diagnostics
+                .iter()
+                .filter(|diag| diag.severity() <= Severity::Error)
+                .count();
+
+            let has_lint = params.filter.categories.contains(RuleCategories::LINT);
+
+            info!("Analyze file {}", params.path.display());
+            let (_, analyze_diagnostics) = analyze(
+                &tree,
+                params.filter,
+                &analyzer_options,
+                file_source,
+                |signal| {
+                    if let Some(mut diagnostic) = signal.diagnostic() {
+                        // Do not report unused suppression comment diagnostics if this is a syntax-only analyzer pass
+                        if !has_lint
+                            && diagnostic.category() == Some(category!("suppressions/unused"))
+                        {
+                            return ControlFlow::<Never>::Continue(());
+                        }
 
-    let has_lint = params.filter.categories.contains(RuleCategories::LINT);
+                        diagnostic_count += 1;
+
+                        // We do now check if the severity of the diagnostics should be changed.
+                        // The configuration allows to change the severity of the diagnostics emitted by rules.
+                        let severity = diagnostic
+                            .category()
+                            .filter(|category| category.name().starts_with("lint/"))
+                            .map(|category| {
+                                params
+                                    .rules
+                                    .and_then(|rules| rules.get_severity_from_code(category))
+                                    .unwrap_or(Severity::Warning)
+                            })
+                            .unwrap_or_else(|| diagnostic.severity());
+
+                        if severity >= Severity::Error {
+                            errors += 1;
+                        }
 
-    info!("Analyze file {}", params.path.display());
-    let (_, analyze_diagnostics) = analyze(
-        &tree,
-        params.filter,
-        &analyzer_options,
-        file_source,
-        |signal| {
-            if let Some(mut diagnostic) = signal.diagnostic() {
-                // Do not report unused suppression comment diagnostics if this is a syntax-only analyzer pass
-                if !has_lint && diagnostic.category() == Some(category!("suppressions/unused")) {
-                    return ControlFlow::<Never>::Continue(());
-                }
+                        if diagnostic_count <= params.max_diagnostics {
+                            for action in signal.actions() {
+                                if !action.is_suppression() {
+                                    diagnostic = diagnostic.add_code_suggestion(action.into());
+                                }
+                            }
 
-                diagnostic_count += 1;
-
-                // We do now check if the severity of the diagnostics should be changed.
-                // The configuration allows to change the severity of the diagnostics emitted by rules.
-                let severity = diagnostic
-                    .category()
-                    .filter(|category| category.name().starts_with("lint/"))
-                    .map(|category| {
-                        params
-                            .rules
-                            .and_then(|rules| rules.get_severity_from_code(category))
-                            .unwrap_or(Severity::Warning)
-                    })
-                    .unwrap_or_else(|| diagnostic.severity());
-
-                if severity >= Severity::Error {
-                    errors += 1;
-                }
+                            let error = diagnostic.with_severity(severity);
 
-                if diagnostic_count <= params.max_diagnostics {
-                    for action in signal.actions() {
-                        if !action.is_suppression() {
-                            diagnostic = diagnostic.add_code_suggestion(action.into());
+                            diagnostics.push(biome_diagnostics::serde::Diagnostic::new(error));
                         }
                     }
 
-                    let error = diagnostic.with_severity(severity);
-
-                    diagnostics.push(biome_diagnostics::serde::Diagnostic::new(error));
-                }
+                    ControlFlow::<Never>::Continue(())
+                },
+            );
+
+            diagnostics.extend(
+                analyze_diagnostics
+                    .into_iter()
+                    .map(biome_diagnostics::serde::Diagnostic::new)
+                    .collect::<Vec<_>>(),
+            );
+            let skipped_diagnostics = diagnostic_count.saturating_sub(diagnostics.len() as u64);
+
+            LintResults {
+                diagnostics,
+                errors,
+                skipped_diagnostics,
             }
-
-            ControlFlow::<Never>::Continue(())
-        },
-    );
-
-    diagnostics.extend(
-        analyze_diagnostics
-            .into_iter()
-            .map(biome_diagnostics::serde::Diagnostic::new)
-            .collect::<Vec<_>>(),
-    );
-    let skipped_diagnostics = diagnostic_count.saturating_sub(diagnostics.len() as u64);
-
-    LintResults {
-        diagnostics,
-        errors,
-        skipped_diagnostics,
-    }
+        })
 }
 
 struct ActionsVisitor<'a> {
@@ -380,7 +392,7 @@ impl RegistryVisitor<JsLanguage> for ActionsVisitor<'_> {
     }
 }
 
-#[tracing::instrument(level = "trace", skip(parse))]
+#[tracing::instrument(level = "debug", skip(parse, settings))]
 fn code_actions(
     parse: AnyParse,
     range: TextRange,
@@ -555,7 +567,7 @@ fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceError> {
     }
 }
 
-#[tracing::instrument(level = "trace", skip(parse))]
+#[tracing::instrument(level = "trace", skip(parse, settings))]
 fn format(
     rome_path: &RomePath,
     parse: AnyParse,
@@ -576,7 +588,7 @@ fn format(
         }
     }
 }
-#[tracing::instrument(level = "trace", skip(parse))]
+#[tracing::instrument(level = "trace", skip(parse, settings))]
 fn format_range(
     rome_path: &RomePath,
     parse: AnyParse,
@@ -590,7 +602,7 @@ fn format_range(
     Ok(printed)
 }
 
-#[tracing::instrument(level = "trace", skip(parse))]
+#[tracing::instrument(level = "trace", skip(parse, settings))]
 fn format_on_type(
     rome_path: &RomePath,
     parse: AnyParse,