[red-knot] Watch search paths #12407

MichaReiser · 2024-07-19T15:29:27Z

Summary

This PR extends the CLI to watch not only the workspace root but also all search paths. The watcher automatically registers new search paths or stops watching search paths when the program's SearchPathSettings change.

Test Plan

Added integration tests.

github-actions · 2024-07-19T15:48:26Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

warning: Detected debug build without --no-cache.
error: Failed to parse examples/Chat_finetuning_data_prep.ipynb:6:18:25: Unparenthesized generator expression cannot be used here
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_sharepoint_doc.ipynb:28:1:5: Simple statements must be separated by newlines or semicolons
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_sharepoint_text.ipynb:28:1:5: Simple statements must be separated by newlines or semicolons
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_middleware_azure_function.ipynb:37:1:13: Simple statements must be separated by newlines or semicolons

Formatter (preview)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

ruff format --preview

warning: Detected debug build without --no-cache.
error: Failed to parse examples/Chat_finetuning_data_prep.ipynb:6:18:25: Unparenthesized generator expression cannot be used here
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_sharepoint_doc.ipynb:28:1:5: Simple statements must be separated by newlines or semicolons
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_sharepoint_text.ipynb:28:1:5: Simple statements must be separated by newlines or semicolons
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_middleware_azure_function.ipynb:37:1:13: Simple statements must be separated by newlines or semicolons

MichaReiser · 2024-07-23T12:51:28Z

crates/red_knot_python_semantic/src/semantic_index/builder.rs

I admit, adding countme fields to the different tracked structs isn't directly related to this PR but it seemed useful to have and it's a local enough change. But I can extract it if it is preferred.

MichaReiser · 2024-07-23T12:54:20Z

crates/red_knot/src/watch/workspace_watcher.rs

+
+    /// Paths that should be watched but setting up the watcher failed for some reason.
+    /// This should be rare.
+    errored_paths: Vec<SystemPathBuf>,


I use a Vec here because this should almost always be empty and searching small vecs tends to be faster than hash maps, not that it matters much 😆

8.py

codspeed-hq · 2024-07-23T12:54:47Z

CodSpeed Performance Report

Merging #12407 will not alter performance

_{Comparing watch-search-paths (0dcb095) with watch-search-paths (b396613)}

Summary

✅ 33 untouched benchmarks

AlexWaygood

The module-resolver crate bits look good (haven't looked at the other bits yet, but I will!)

AlexWaygood · 2024-07-23T13:00:09Z

crates/red_knot_module_resolver/src/lib.rs

+use crate::resolver::{module_resolution_settings, SearchPathIterator};
 pub use db::{Db, Jar};
 pub use module::{Module, ModuleKind};
 pub use module_name::ModuleName;
 pub use resolver::resolve_module;
+use ruff_db::system::SystemPath;
+use std::iter::FusedIterator;
 pub use typeshed::{
    vendored_typeshed_stubs, TypeshedVersionsParseError, TypeshedVersionsParseErrorKind,
 };


nit: this is inconsistent with the import style I've used for the rest of this crate

Suggested change

use crate::resolver::{module_resolution_settings, SearchPathIterator};

pub use db::{Db, Jar};

pub use module::{Module, ModuleKind};

pub use module_name::ModuleName;

pub use resolver::resolve_module;

use ruff_db::system::SystemPath;

use std::iter::FusedIterator;

pub use typeshed::{

vendored_typeshed_stubs, TypeshedVersionsParseError, TypeshedVersionsParseErrorKind,

};

use std::iter::FusedIterator;

use ruff_db::system::SystemPath;

pub use db::{Db, Jar};

pub use module::{Module, ModuleKind};

pub use module_name::ModuleName;

use resolver::{module_resolution_settings, SearchPathIterator};

pub use resolver::resolve_module;

pub use typeshed::{

vendored_typeshed_stubs, TypeshedVersionsParseError, TypeshedVersionsParseErrorKind,

};

I think we should either deploy tooling to standardize use sorting or not be opinionated about it. It otherwise is guaranteed to get out of sync.

Fair enough, I'll try to be less picky about it in the future :(

FWIW I'd be happy to have tooling that standardizes it, but I haven't looked into what the options are.

Rustfmt has an option for it but it isn't stabilized. https://rust-lang.github.io/rustfmt/?version=master&search=import#group_imports and it can't be enabled on non-nigthly rustfmt

Warning: can't set `group_imports = StdExternalCrate`, unstable features are only available in nightly channel. Warning: can't set `unstable_features = true`, unstable features are only available in nightly channel.

crates/red_knot_module_resolver/src/lib.rs

carljm

Nice!

crates/red_knot/src/workspace.rs

crates/red_knot/tests/file_watching.rs

crates/ruff_db/src/system/path.rs

MichaReiser force-pushed the watch-search-paths branch 2 times, most recently from 7cb5f50 to f969d0d Compare July 23, 2024 12:48

MichaReiser commented Jul 23, 2024

View reviewed changes

MichaReiser added the red-knot Multi-file analysis & type inference label Jul 23, 2024

MichaReiser marked this pull request as ready for review July 23, 2024 12:53

MichaReiser requested review from carljm and AlexWaygood as code owners July 23, 2024 12:53

MichaReiser commented Jul 23, 2024

View reviewed changes

AlexWaygood reviewed Jul 23, 2024

View reviewed changes

8.py Outdated Show resolved Hide resolved

MichaReiser force-pushed the watch-search-paths branch from f969d0d to d5ba5ce Compare July 23, 2024 12:55

AlexWaygood reviewed Jul 23, 2024

View reviewed changes

MichaReiser force-pushed the watch-search-paths branch from d5ba5ce to 98a3959 Compare July 23, 2024 13:13

[red-knot] Watch search paths

b396613

MichaReiser force-pushed the watch-search-paths branch from 98a3959 to b396613 Compare July 23, 2024 13:15

carljm approved these changes Jul 23, 2024

View reviewed changes

crates/red_knot/src/workspace.rs Outdated Show resolved Hide resolved

crates/red_knot/tests/file_watching.rs Outdated Show resolved Hide resolved

crates/ruff_db/src/system/path.rs Show resolved Hide resolved

Address code review feedback

0dcb095

MichaReiser enabled auto-merge (squash) July 24, 2024 07:35

MichaReiser merged commit eac965e into main Jul 24, 2024
16 checks passed

MichaReiser deleted the watch-search-paths branch July 24, 2024 07:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[red-knot] Watch search paths #12407

[red-knot] Watch search paths #12407

MichaReiser commented Jul 19, 2024 •

edited

Loading

github-actions bot commented Jul 19, 2024 •

edited

Loading

MichaReiser Jul 23, 2024

MichaReiser Jul 23, 2024

codspeed-hq bot commented Jul 23, 2024 •

edited

Loading

AlexWaygood left a comment

AlexWaygood Jul 23, 2024

MichaReiser Jul 23, 2024

AlexWaygood Jul 23, 2024

carljm Jul 23, 2024

MichaReiser Jul 24, 2024

carljm left a comment

[red-knot] Watch search paths #12407

[red-knot] Watch search paths #12407

Conversation

MichaReiser commented Jul 19, 2024 • edited Loading

Summary

Test Plan

github-actions bot commented Jul 19, 2024 • edited Loading

ruff-ecosystem results

Linter (stable)

Linter (preview)

Formatter (stable)

Formatter (preview)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codspeed-hq bot commented Jul 23, 2024 • edited Loading

Merging #12407 will not alter performance

Summary

AlexWaygood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carljm left a comment

Choose a reason for hiding this comment

MichaReiser commented Jul 19, 2024 •

edited

Loading

github-actions bot commented Jul 19, 2024 •

edited

Loading

`ruff-ecosystem` results

codspeed-hq bot commented Jul 23, 2024 •

edited

Loading