mirror of
https://github.com/astral-sh/ruff
synced 2026-01-21 13:30:49 -05:00
[ty] Add option to disable syntax errors in the language server (#22217)
Co-authored-by: Micha Reiser <micha@reiser.io>
This commit is contained in:
@@ -18,8 +18,8 @@ use ty_project::{Db as _, ProjectDatabase};
|
||||
|
||||
use crate::capabilities::ResolvedClientCapabilities;
|
||||
use crate::document::{FileRangeExt, ToRangeExt};
|
||||
use crate::session::DocumentHandle;
|
||||
use crate::session::client::Client;
|
||||
use crate::session::{DocumentHandle, GlobalSettings};
|
||||
use crate::system::{AnySystemPath, file_to_url};
|
||||
use crate::{DIAGNOSTIC_NAME, Db, DiagnosticMode};
|
||||
use crate::{PositionEncoding, Session};
|
||||
@@ -61,6 +61,7 @@ impl Diagnostics {
|
||||
&self,
|
||||
db: &ProjectDatabase,
|
||||
client_capabilities: ResolvedClientCapabilities,
|
||||
global_settings: &GlobalSettings,
|
||||
) -> LspDiagnostics {
|
||||
if let Some(notebook_document) = db.notebook_document(self.file_or_notebook) {
|
||||
let mut cell_diagnostics: FxHashMap<Url, Vec<Diagnostic>> = FxHashMap::default();
|
||||
@@ -72,8 +73,15 @@ impl Diagnostics {
|
||||
}
|
||||
|
||||
for diagnostic in &self.items {
|
||||
let (url, lsp_diagnostic) =
|
||||
to_lsp_diagnostic(db, diagnostic, self.encoding, client_capabilities);
|
||||
let Some((url, lsp_diagnostic)) = to_lsp_diagnostic(
|
||||
db,
|
||||
diagnostic,
|
||||
self.encoding,
|
||||
client_capabilities,
|
||||
global_settings,
|
||||
) else {
|
||||
continue;
|
||||
};
|
||||
|
||||
let Some(url) = url else {
|
||||
tracing::warn!("Unable to find notebook cell");
|
||||
@@ -91,8 +99,17 @@ impl Diagnostics {
|
||||
LspDiagnostics::TextDocument(
|
||||
self.items
|
||||
.iter()
|
||||
.map(|diagnostic| {
|
||||
to_lsp_diagnostic(db, diagnostic, self.encoding, client_capabilities).1
|
||||
.filter_map(|diagnostic| {
|
||||
Some(
|
||||
to_lsp_diagnostic(
|
||||
db,
|
||||
diagnostic,
|
||||
self.encoding,
|
||||
client_capabilities,
|
||||
global_settings,
|
||||
)?
|
||||
.1,
|
||||
)
|
||||
})
|
||||
.collect(),
|
||||
)
|
||||
@@ -197,7 +214,11 @@ pub(super) fn publish_diagnostics(document: &DocumentHandle, session: &Session,
|
||||
});
|
||||
};
|
||||
|
||||
match diagnostics.to_lsp_diagnostics(db, session.client_capabilities()) {
|
||||
match diagnostics.to_lsp_diagnostics(
|
||||
db,
|
||||
session.client_capabilities(),
|
||||
session.global_settings(),
|
||||
) {
|
||||
LspDiagnostics::TextDocument(diagnostics) => {
|
||||
publish_diagnostics_notification(document.url().clone(), diagnostics);
|
||||
}
|
||||
@@ -232,49 +253,71 @@ pub(crate) fn publish_settings_diagnostics(
|
||||
|
||||
let session_encoding = session.position_encoding();
|
||||
let client_capabilities = session.client_capabilities();
|
||||
let state = session.project_state_mut(&AnySystemPath::System(path));
|
||||
let db = &state.db;
|
||||
let project = db.project();
|
||||
let settings_diagnostics = project.check_settings(db);
|
||||
|
||||
// We need to send diagnostics if we have non-empty ones, or we have ones to clear.
|
||||
// These will both almost always be empty so this function will almost always be a no-op.
|
||||
if settings_diagnostics.is_empty() && state.untracked_files_with_pushed_diagnostics.is_empty() {
|
||||
return;
|
||||
}
|
||||
let project_path = AnySystemPath::System(path);
|
||||
|
||||
// Group diagnostics by URL
|
||||
let mut diagnostics_by_url: FxHashMap<Url, Vec<_>> = FxHashMap::default();
|
||||
for diagnostic in settings_diagnostics {
|
||||
if let Some(span) = diagnostic.primary_span() {
|
||||
let file = span.expect_ty_file();
|
||||
let Some(url) = file_to_url(db, file) else {
|
||||
tracing::debug!("Failed to convert file to URL at {}", file.path(db));
|
||||
continue;
|
||||
};
|
||||
diagnostics_by_url.entry(url).or_default().push(diagnostic);
|
||||
let (mut diagnostics_by_url, old_untracked) = {
|
||||
let state = session.project_state_mut(&project_path);
|
||||
let db = &state.db;
|
||||
let project = db.project();
|
||||
let settings_diagnostics = project.check_settings(db);
|
||||
|
||||
// We need to send diagnostics if we have non-empty ones, or we have ones to clear.
|
||||
// These will both almost always be empty so this function will almost always be a no-op.
|
||||
if settings_diagnostics.is_empty()
|
||||
&& state.untracked_files_with_pushed_diagnostics.is_empty()
|
||||
{
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Record the URLs we're sending non-empty diagnostics for, so we know to clear them
|
||||
// the next time we publish settings diagnostics!
|
||||
let old_untracked = std::mem::replace(
|
||||
&mut state.untracked_files_with_pushed_diagnostics,
|
||||
diagnostics_by_url.keys().cloned().collect(),
|
||||
);
|
||||
// Group diagnostics by URL
|
||||
let mut diagnostics_by_url: FxHashMap<Url, Vec<_>> = FxHashMap::default();
|
||||
for diagnostic in settings_diagnostics {
|
||||
if let Some(span) = diagnostic.primary_span() {
|
||||
let file = span.expect_ty_file();
|
||||
let Some(url) = file_to_url(db, file) else {
|
||||
tracing::debug!("Failed to convert file to URL at {}", file.path(db));
|
||||
continue;
|
||||
};
|
||||
diagnostics_by_url.entry(url).or_default().push(diagnostic);
|
||||
}
|
||||
}
|
||||
|
||||
// Record the URLs we're sending non-empty diagnostics for, so we know to clear them
|
||||
// the next time we publish settings diagnostics!
|
||||
let old_untracked = std::mem::replace(
|
||||
&mut state.untracked_files_with_pushed_diagnostics,
|
||||
diagnostics_by_url.keys().cloned().collect(),
|
||||
);
|
||||
|
||||
(diagnostics_by_url, old_untracked)
|
||||
};
|
||||
|
||||
// Add empty diagnostics for any files that had diagnostics before but don't now.
|
||||
// This will clear them (either the file is no longer relevant to us or fixed!)
|
||||
for url in old_untracked {
|
||||
diagnostics_by_url.entry(url).or_default();
|
||||
}
|
||||
|
||||
let db = session.project_db(&project_path);
|
||||
let global_settings = session.global_settings();
|
||||
|
||||
// Send the settings diagnostics!
|
||||
for (url, file_diagnostics) in diagnostics_by_url {
|
||||
// Convert diagnostics to LSP format
|
||||
let lsp_diagnostics = file_diagnostics
|
||||
.into_iter()
|
||||
.map(|diagnostic| {
|
||||
to_lsp_diagnostic(db, &diagnostic, session_encoding, client_capabilities).1
|
||||
.filter_map(|diagnostic| {
|
||||
Some(
|
||||
to_lsp_diagnostic(
|
||||
db,
|
||||
&diagnostic,
|
||||
session_encoding,
|
||||
client_capabilities,
|
||||
global_settings,
|
||||
)?
|
||||
.1,
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
@@ -315,7 +358,12 @@ pub(super) fn to_lsp_diagnostic(
|
||||
diagnostic: &ruff_db::diagnostic::Diagnostic,
|
||||
encoding: PositionEncoding,
|
||||
client_capabilities: ResolvedClientCapabilities,
|
||||
) -> (Option<lsp_types::Url>, Diagnostic) {
|
||||
global_settings: &GlobalSettings,
|
||||
) -> Option<(Option<lsp_types::Url>, Diagnostic)> {
|
||||
if diagnostic.is_invalid_syntax() && !global_settings.show_syntax_errors() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let supports_related_information =
|
||||
client_capabilities.supports_diagnostic_related_information();
|
||||
|
||||
@@ -388,7 +436,7 @@ pub(super) fn to_lsp_diagnostic(
|
||||
|
||||
let data = DiagnosticData::try_from_diagnostic(db, diagnostic, encoding);
|
||||
|
||||
(
|
||||
Some((
|
||||
url,
|
||||
Diagnostic {
|
||||
range,
|
||||
@@ -414,7 +462,7 @@ pub(super) fn to_lsp_diagnostic(
|
||||
related_information,
|
||||
data: serde_json::to_value(data).ok(),
|
||||
},
|
||||
)
|
||||
))
|
||||
}
|
||||
|
||||
/// Converts an [`Annotation`] to a [`DiagnosticRelatedInformation`].
|
||||
|
||||
@@ -66,7 +66,11 @@ impl BackgroundDocumentRequestHandler for DocumentDiagnosticRequestHandler {
|
||||
// SAFETY: Pull diagnostic requests are only called for text documents, not for
|
||||
// notebook documents.
|
||||
items: diagnostics
|
||||
.to_lsp_diagnostics(db, snapshot.resolved_client_capabilities())
|
||||
.to_lsp_diagnostics(
|
||||
db,
|
||||
snapshot.resolved_client_capabilities(),
|
||||
snapshot.global_settings(),
|
||||
)
|
||||
.expect_text_document(),
|
||||
},
|
||||
})
|
||||
|
||||
@@ -29,7 +29,7 @@ use crate::server::lazy_work_done_progress::LazyWorkDoneProgress;
|
||||
use crate::server::{Action, Result};
|
||||
use crate::session::client::Client;
|
||||
use crate::session::index::Index;
|
||||
use crate::session::{SessionSnapshot, SuspendedWorkspaceDiagnosticRequest};
|
||||
use crate::session::{GlobalSettings, SessionSnapshot, SuspendedWorkspaceDiagnosticRequest};
|
||||
use crate::system::file_to_url;
|
||||
|
||||
/// Handler for [Workspace diagnostics](workspace-diagnostics)
|
||||
@@ -324,6 +324,7 @@ struct ResponseWriter<'a> {
|
||||
// `file_to_url` isn't guaranteed to return the exact same URL as the one provided
|
||||
// by the client.
|
||||
previous_result_ids: FxHashMap<DocumentKey, (Url, String)>,
|
||||
global_settings: &'a GlobalSettings,
|
||||
}
|
||||
|
||||
impl<'a> ResponseWriter<'a> {
|
||||
@@ -361,6 +362,7 @@ impl<'a> ResponseWriter<'a> {
|
||||
position_encoding,
|
||||
client_capabilities: snapshot.resolved_client_capabilities(),
|
||||
previous_result_ids,
|
||||
global_settings: snapshot.global_settings(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -409,14 +411,17 @@ impl<'a> ResponseWriter<'a> {
|
||||
new_id => {
|
||||
let lsp_diagnostics = diagnostics
|
||||
.iter()
|
||||
.map(|diagnostic| {
|
||||
to_lsp_diagnostic(
|
||||
db,
|
||||
diagnostic,
|
||||
self.position_encoding,
|
||||
self.client_capabilities,
|
||||
.filter_map(|diagnostic| {
|
||||
Some(
|
||||
to_lsp_diagnostic(
|
||||
db,
|
||||
diagnostic,
|
||||
self.position_encoding,
|
||||
self.client_capabilities,
|
||||
self.global_settings,
|
||||
)?
|
||||
.1,
|
||||
)
|
||||
.1
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
|
||||
@@ -124,6 +124,12 @@ impl ClientOptions {
|
||||
self
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_show_syntax_errors(mut self, show_syntax_errors: bool) -> Self {
|
||||
self.global.show_syntax_errors = Some(show_syntax_errors);
|
||||
self
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_unknown(mut self, unknown: HashMap<String, Value>) -> Self {
|
||||
self.unknown = unknown;
|
||||
@@ -143,6 +149,12 @@ pub struct GlobalOptions {
|
||||
|
||||
/// Experimental features that the server provides on an opt-in basis.
|
||||
pub(crate) experimental: Option<Experimental>,
|
||||
|
||||
/// If `true` or [`None`], show syntax errors as diagnostics.
|
||||
///
|
||||
/// This is useful when using ty with other language servers, allowing the user to refer
|
||||
/// to syntax errors from only one source.
|
||||
pub(crate) show_syntax_errors: Option<bool>,
|
||||
}
|
||||
|
||||
impl GlobalOptions {
|
||||
@@ -155,6 +167,7 @@ impl GlobalOptions {
|
||||
GlobalSettings {
|
||||
diagnostic_mode: self.diagnostic_mode.unwrap_or_default(),
|
||||
experimental,
|
||||
show_syntax_errors: self.show_syntax_errors.unwrap_or(true),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,12 +7,17 @@ use ty_project::metadata::options::ProjectOptionsOverrides;
|
||||
pub(crate) struct GlobalSettings {
|
||||
pub(super) diagnostic_mode: DiagnosticMode,
|
||||
pub(super) experimental: ExperimentalSettings,
|
||||
pub(super) show_syntax_errors: bool,
|
||||
}
|
||||
|
||||
impl GlobalSettings {
|
||||
pub(crate) fn diagnostic_mode(&self) -> DiagnosticMode {
|
||||
self.diagnostic_mode
|
||||
}
|
||||
|
||||
pub(crate) fn show_syntax_errors(&self) -> bool {
|
||||
self.show_syntax_errors
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Default, Debug, PartialEq)]
|
||||
|
||||
@@ -432,6 +432,44 @@ b: Litera
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_syntax_with_syntax_errors_disabled() -> anyhow::Result<()> {
|
||||
let mut server = TestServerBuilder::new()?
|
||||
.with_workspace(
|
||||
SystemPath::new("src"),
|
||||
Some(ClientOptions::default().with_show_syntax_errors(false)),
|
||||
)?
|
||||
.build()
|
||||
.wait_until_workspaces_are_initialized();
|
||||
|
||||
server.initialization_result().unwrap();
|
||||
|
||||
let mut builder = NotebookBuilder::virtual_file("src/test.ipynb");
|
||||
|
||||
builder.add_python_cell(
|
||||
r#"def foo(
|
||||
"#,
|
||||
);
|
||||
|
||||
builder.add_python_cell(
|
||||
r#"x = 1 +
|
||||
"#,
|
||||
);
|
||||
|
||||
builder.open(&mut server);
|
||||
|
||||
let diagnostics = server.collect_publish_diagnostic_notifications(2);
|
||||
|
||||
assert_json_snapshot!(diagnostics, @r###"
|
||||
{
|
||||
"vscode-notebook-cell://src/test.ipynb#0": [],
|
||||
"vscode-notebook-cell://src/test.ipynb#1": []
|
||||
}
|
||||
"###);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn semantic_tokens_full_for_cell(
|
||||
server: &mut TestServer,
|
||||
cell_uri: &lsp_types::Url,
|
||||
|
||||
@@ -334,3 +334,30 @@ def foo() -> str:
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_syntax_with_syntax_errors_disabled() -> Result<()> {
|
||||
let workspace_root = SystemPath::new("src");
|
||||
let foo = SystemPath::new("src/foo.py");
|
||||
let foo_content = "\
|
||||
def foo(
|
||||
";
|
||||
|
||||
let mut server = TestServerBuilder::new()?
|
||||
.with_workspace(
|
||||
workspace_root,
|
||||
Some(ClientOptions::default().with_show_syntax_errors(false)),
|
||||
)?
|
||||
.with_file(foo, foo_content)?
|
||||
.enable_pull_diagnostics(false)
|
||||
.build()
|
||||
.wait_until_workspaces_are_initialized();
|
||||
|
||||
server.open_text_document(foo, foo_content, 1);
|
||||
|
||||
let diagnostics = server.await_notification::<PublishDiagnostics>();
|
||||
|
||||
insta::assert_debug_snapshot!(diagnostics);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -68,6 +68,54 @@ def foo() -> str:
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_syntax_with_syntax_errors_disabled() -> Result<()> {
|
||||
let _filter = filter_result_id();
|
||||
|
||||
let workspace_root = SystemPath::new("src");
|
||||
let foo = SystemPath::new("src/foo.py");
|
||||
let foo_content = "\
|
||||
def foo(
|
||||
";
|
||||
|
||||
let mut server = TestServerBuilder::new()?
|
||||
.with_workspace(
|
||||
workspace_root,
|
||||
Some(ClientOptions::default().with_show_syntax_errors(false)),
|
||||
)?
|
||||
.with_file(foo, foo_content)?
|
||||
.with_initialization_options(
|
||||
ClientOptions::default()
|
||||
.with_show_syntax_errors(false)
|
||||
.with_diagnostic_mode(DiagnosticMode::Workspace),
|
||||
)
|
||||
.enable_pull_diagnostics(true)
|
||||
.build()
|
||||
.wait_until_workspaces_are_initialized();
|
||||
|
||||
let workspace_diagnostics = server.workspace_diagnostic_request(None, None);
|
||||
assert_compact_json_snapshot!(workspace_diagnostics, @r#"
|
||||
{
|
||||
"items": [
|
||||
{
|
||||
"kind": "full",
|
||||
"uri": "file://<temp_dir>/src/foo.py",
|
||||
"version": null,
|
||||
"resultId": "[RESULT_ID]",
|
||||
"items": []
|
||||
}
|
||||
]
|
||||
}
|
||||
"#);
|
||||
|
||||
server.open_text_document(foo, foo_content, 1);
|
||||
let diagnostics = server.document_diagnostic_request(foo, None);
|
||||
|
||||
assert_compact_json_snapshot!(diagnostics, @r#"{"kind": "full", "resultId": "[RESULT_ID]", "items": []}"#);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn document_diagnostic_caching_unchanged() -> Result<()> {
|
||||
let _filter = filter_result_id();
|
||||
|
||||
@@ -9,6 +9,7 @@ Position encoding: UTF16
|
||||
Global settings: GlobalSettings {
|
||||
diagnostic_mode: OpenFilesOnly,
|
||||
experimental: ExperimentalSettings,
|
||||
show_syntax_errors: true,
|
||||
}
|
||||
Open text documents: 0
|
||||
|
||||
|
||||
@@ -0,0 +1,21 @@
|
||||
---
|
||||
source: crates/ty_server/tests/e2e/publish_diagnostics.rs
|
||||
expression: diagnostics
|
||||
---
|
||||
PublishDiagnosticsParams {
|
||||
uri: Url {
|
||||
scheme: "file",
|
||||
cannot_be_a_base: false,
|
||||
username: "",
|
||||
password: None,
|
||||
host: None,
|
||||
port: None,
|
||||
path: "<temp_dir>/src/foo.py",
|
||||
query: None,
|
||||
fragment: None,
|
||||
},
|
||||
diagnostics: [],
|
||||
version: Some(
|
||||
1,
|
||||
),
|
||||
}
|
||||
Reference in New Issue
Block a user