Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions lsp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,19 @@
"off",
"verbose"
]
},
"python.analysis.diagnosticMode": {
"type": "string",
"description": "Controls which files are analyzed for diagnostics. 'openFilesOnly' (default) shows type errors only in open files. 'workspace' shows type errors in all files within the workspace.",
"default": "openFilesOnly",
"enum": [
"openFilesOnly",
"workspace"
],
"enumDescriptions": [
"Show diagnostics only for files that are currently open in the editor",
"Show diagnostics for all files in the workspace, similar to Pyright's workspace mode"
]
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion lsp/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,8 @@ export async function activate(context: ExtensionContext) {

context.subscriptions.push(
workspace.onDidChangeConfiguration(async event => {
if (event.affectsConfiguration('python.pyrefly')) {
if (event.affectsConfiguration('python.pyrefly') ||
event.affectsConfiguration('python.analysis')) {
client.sendNotification(DidChangeConfigurationNotification.type, {
settings: {},
});
Expand Down
24 changes: 19 additions & 5 deletions pyrefly/lib/lsp/non_wasm/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ use crate::lsp::non_wasm::queue::HeavyTaskQueue;
use crate::lsp::non_wasm::queue::LspEvent;
use crate::lsp::non_wasm::queue::LspQueue;
use crate::lsp::non_wasm::transaction_manager::TransactionManager;
use crate::lsp::non_wasm::workspace::DiagnosticMode;
use crate::lsp::non_wasm::workspace::LspAnalysisConfig;
use crate::lsp::non_wasm::workspace::Workspace;
use crate::lsp::non_wasm::workspace::Workspaces;
Expand Down Expand Up @@ -1081,7 +1082,17 @@ impl Server {
.state
.config_finder()
.python_file(ModuleName::unknown(), e.path());
if open_files.contains_key(&path)

// Get diagnostic mode for this file's workspace
let diagnostic_mode = self.workspaces.get_diagnostic_mode(&path);

// Check if we should show this diagnostic based on mode
let should_show_based_on_mode = match diagnostic_mode {
DiagnosticMode::Workspace => true, // Show all files in workspace mode
DiagnosticMode::OpenFilesOnly => open_files.contains_key(&path), // Only open files
};

if should_show_based_on_mode
&& config.project_includes.covers(&path)
&& !config.project_excludes.covers(&path)
&& self
Expand Down Expand Up @@ -1178,14 +1189,17 @@ impl Server {
let publish = |transaction: &Transaction| {
let mut diags: SmallMap<PathBuf, Vec<Diagnostic>> = SmallMap::new();
let open_files = self.open_files.read();
for x in open_files.keys() {
diags.insert(x.as_path().to_owned(), Vec::new());
}
for e in transaction.get_errors(&handles).collect_errors().shown {

// Collect errors from transaction for open files
// The filtering by diagnostic mode is handled in get_diag_if_shown
let errors = transaction.get_errors(&handles);

for e in errors.collect_errors().shown {
if let Some((path, diag)) = self.get_diag_if_shown(&e, &open_files) {
diags.entry(path.to_owned()).or_default().push(diag);
}
}

for (path, diagnostics) in diags.iter_mut() {
let handle = make_open_handle(&self.state, path);
Self::append_unreachable_diagnostics(transaction, &handle, diagnostics);
Expand Down
12 changes: 11 additions & 1 deletion pyrefly/lib/lsp/non_wasm/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ pub enum DiagnosticMode {
#[derive(Clone, Copy, Debug, Default, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct LspAnalysisConfig {
#[allow(dead_code)]
pub diagnostic_mode: Option<DiagnosticMode>,
pub import_format: Option<ImportFormat>,
pub inlay_hints: Option<InlayHintConfig>,
Expand Down Expand Up @@ -399,6 +398,17 @@ impl Workspaces {
}
}
}

/// Get the diagnostic mode for a given path.
/// Returns the configured diagnostic mode for the workspace containing the path,
/// or `OpenFilesOnly` as the default if not configured.
pub fn get_diagnostic_mode(&self, path: &std::path::Path) -> DiagnosticMode {
self.get_with(path.to_path_buf(), |(_, w)| {
w.lsp_analysis_config
.and_then(|config| config.diagnostic_mode)
.unwrap_or(DiagnosticMode::OpenFilesOnly)
})
}
}

#[cfg(test)]
Expand Down
1 change: 1 addition & 0 deletions pyrefly/lib/test/lsp/lsp_interaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,5 @@ mod rename;
mod type_definition;
mod util;
mod will_rename_files;
mod workspace_diagnostic_mode;
mod workspace_symbol;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# File with intentional type error for testing workspace diagnostic mode

def add_numbers(x: int, y: int) -> int:
return x + y

# This should cause a type error: passing string to int parameter
result = add_numbers("hello", "world")
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# File that will be opened in tests (should always show diagnostics)

def greet(name: str) -> str:
return f"Hello, {name}"

# No errors in this file
message = greet("World")
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Pyrefly config for workspace diagnostic mode tests
241 changes: 241 additions & 0 deletions pyrefly/lib/test/lsp/lsp_interaction/workspace_diagnostic_mode.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,241 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

use lsp_server::RequestId;
use lsp_server::Response;

use crate::test::lsp::lsp_interaction::object_model::InitializeSettings;
use crate::test::lsp::lsp_interaction::object_model::LspInteraction;
use crate::test::lsp::lsp_interaction::util::get_test_files_root;

/// Test that workspace mode uses get_all_errors (shows all analyzed files)
/// This verifies the filtering logic respects workspace diagnostic mode
#[test]
fn test_workspace_mode_uses_get_all_errors() {
let test_files_root = get_test_files_root();
let mut interaction = LspInteraction::new();
interaction.set_root(test_files_root.path().to_path_buf());
interaction.initialize(InitializeSettings {
configuration: Some(None),
..Default::default()
});

// Send configuration change to enable workspace diagnostic mode
interaction.server.did_change_configuration();
interaction.client.expect_configuration_request(2, None);
interaction.server.send_configuration_response(
2,
serde_json::json!([
{
"pyrefly": {
"displayTypeErrors": "force-on"
},
"analysis": {
"diagnosticMode": "workspace"
}
},
{
"pyrefly": {
"displayTypeErrors": "force-on"
},
"analysis": {
"diagnosticMode": "workspace"
}
}
]),
);

// Open a file - in workspace mode, this should work normally
// The real test is that the code path uses get_all_errors() instead of get_errors(&handles)
interaction
.server
.did_open("workspace_diagnostic_mode/opened_file.py");

// Request diagnostics - should work in workspace mode
interaction
.server
.diagnostic("workspace_diagnostic_mode/opened_file.py");

// File has no errors
Copy link
Contributor

@kinto0 kinto0 Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make sure this test tests that errors appear for files that aren't yet opened? I don't think it would pass (I tested this manually).

I think we need to adjust the arguments to this to include every file that is included in the project. otherwise, we might not end up checking them

it gets a little confusing because files can be in the workspace but not covered by a pyrefly config. or they can be in a config but not covered by a workspace. I think a reasonable approximation is to include all files covered by any config of any opened file. if we actually use the workspace files themselves, anything that's ignored will still appear.

it might make sense to abstract out and reuse the logic in did_open that will get the config to populate_project_files_if_necessary. then you can do what populate_all_project_files_in_config does to get all files in the project paths and validate those files.

if you think we also want to use any workspace file, you can copy what populate_workspace_files_if_necessary (but I would skip that tbh, I think project files are a good approximation)

Copy link
Contributor Author

@AryanBagade AryanBagade Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for detailed feedback @kinto0

Implementation

textDocument/diagnostic request handler

When a diagnostic request comes in for a file in workspace mode:

  • Check if the file is in workspace mode and not currently open
  • Create a handle using handle_from_module_path with a filesystem path (not make_open_handle which expects in-memory content)
  • Run the transaction on that specific file to analyze it on-demand

document_diagnostics function

  • For open files → use make_open_handle (in-memory handle)
  • For unopened files in workspace mode → use handle_from_module_path (filesystem handle)
  • For files neither open nor in workspace mode → return empty diagnostics

Test Results

  • ✅ All 4 workspace diagnostic mode tests pass
  • ✅ The test that checks errors appear for unopened files now passes with actual type errors

Question About Implementation Approach

I've implemented a "pull" model where unopened files are analyzed when textDocument/diagnostic is explicitly requested for them.

This means:

  • The tests pass (because they explicitly request diagnostics for unopened files)
  • But in actual VS Code usage, unopened files don't show errors automatically

Should I also implement a "push" model where validate_in_memory_for_possibly_committable_transaction proactively publishes diagnostics for all workspace files when in workspace mode? Or is the current "pull" model approach acceptable?
Let me know your thoughts!

interaction.client.expect_response(Response {
id: RequestId::from(2),
result: Some(serde_json::json!({
"items": [],
"kind": "full"
})),
error: None,
});

interaction.shutdown();
}

/// Test that openFilesOnly mode only shows errors for files that are opened
#[test]
fn test_open_files_only_mode_filters_correctly() {
let test_files_root = get_test_files_root();
let mut interaction = LspInteraction::new();
interaction.set_root(test_files_root.path().to_path_buf());
interaction.initialize(InitializeSettings {
configuration: Some(None),
..Default::default()
});

// Send configuration with openFilesOnly mode (explicit)
interaction.server.did_change_configuration();
interaction.client.expect_configuration_request(2, None);
interaction.server.send_configuration_response(
2,
serde_json::json!([
{
"pyrefly": {
"displayTypeErrors": "force-on"
},
"analysis": {
"diagnosticMode": "openFilesOnly"
}
},
{
"pyrefly": {
"displayTypeErrors": "force-on"
},
"analysis": {
"diagnosticMode": "openFilesOnly"
}
}
]),
);

// Open a file without errors
interaction
.server
.did_open("workspace_diagnostic_mode/opened_file.py");

// Request diagnostics for an UNOPENED file with errors
// In openFilesOnly mode, we should NOT get diagnostics for unopened files
interaction
.server
.diagnostic("workspace_diagnostic_mode/file_with_error.py");

// Expect NO errors because the file is not opened and we're in openFilesOnly mode
interaction.client.expect_response(Response {
id: RequestId::from(2),
result: Some(serde_json::json!({
"items": [],
"kind": "full"
})),
error: None,
});

interaction.shutdown();
}

/// Test default behavior (should be openFilesOnly for backward compatibility)
#[test]
fn test_default_mode_is_open_files_only() {
let test_files_root = get_test_files_root();
let mut interaction = LspInteraction::new();
interaction.set_root(test_files_root.path().to_path_buf());
interaction.initialize(InitializeSettings {
configuration: Some(None),
..Default::default()
});

// Don't set diagnosticMode - should default to openFilesOnly
interaction.server.did_change_configuration();
interaction.client.expect_configuration_request(2, None);
interaction.server.send_configuration_response(
2,
serde_json::json!([
{"pyrefly": {"displayTypeErrors": "force-on"}},
{"pyrefly": {"displayTypeErrors": "force-on"}}
]),
);

// Open a file without errors
interaction
.server
.did_open("workspace_diagnostic_mode/opened_file.py");

// Request diagnostics for an UNOPENED file with errors
// Default mode should be openFilesOnly, so no diagnostics for unopened files
interaction
.server
.diagnostic("workspace_diagnostic_mode/file_with_error.py");

// Expect NO errors because default mode is openFilesOnly
interaction.client.expect_response(Response {
id: RequestId::from(2),
result: Some(serde_json::json!({
"items": [],
"kind": "full"
})),
error: None,
});

interaction.shutdown();
}

/// Test that workspace mode does not show errors for files outside the workspace folder
#[test]
fn test_workspace_mode_excludes_files_outside_workspace() {
let test_files_root = get_test_files_root();
let mut interaction = LspInteraction::new();
interaction.set_root(test_files_root.path().to_path_buf());
interaction.initialize(InitializeSettings {
configuration: Some(None),
..Default::default()
});

// Send configuration with workspace mode
interaction.server.did_change_configuration();
interaction.client.expect_configuration_request(2, None);
interaction.server.send_configuration_response(
2,
serde_json::json!([
{
"pyrefly": {
"displayTypeErrors": "force-on"
},
"analysis": {
"diagnosticMode": "workspace"
}
},
{
"pyrefly": {
"displayTypeErrors": "force-on"
},
"analysis": {
"diagnosticMode": "workspace"
}
}
]),
);

// Open a file in the workspace
interaction
.server
.did_open("workspace_diagnostic_mode/opened_file.py");

// Request diagnostics - workspace mode should only show errors from files within the workspace
// Files outside the workspace (like dependencies) should not be shown
interaction
.server
.diagnostic("workspace_diagnostic_mode/opened_file.py");

// Expect NO errors because the file itself has no errors
// More importantly, we should NOT see errors from dependencies or files outside workspace
interaction.client.expect_response(Response {
id: RequestId::from(2),
result: Some(serde_json::json!({
"items": [],
"kind": "full"
})),
error: None,
});

interaction.shutdown();
}
Loading