diff --git a/Cargo.lock b/Cargo.lock index 328270f398..4f40060dad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -364,6 +364,7 @@ dependencies = [ "ruff_python_trivia", "ruff_source_file", "ruff_text_size", + "salsa", "tempfile", "thin-vec", "ty_project", @@ -4702,6 +4703,7 @@ dependencies = [ "tracing-flame", "tracing-subscriber", "ty_combine", + "ty_ide", "ty_module_resolver", "ty_project", "ty_python_core", @@ -4780,6 +4782,7 @@ dependencies = [ "ruff_python_parser", "ruff_python_stdlib", "ruff_python_trivia", + "ruff_ranged_value", "ruff_source_file", "ruff_text_size", "rustc-hash", diff --git a/_typos.toml b/_typos.toml index d9a85a6d8b..ec2875012d 100644 --- a/_typos.toml +++ b/_typos.toml @@ -25,6 +25,7 @@ BA = "BA" # acronym for "Bad Allowed", used in testing. jod = "jod" # e.g., `jod-thread` Numer = "Numer" # Library name 'NumerBlox' in "Who's Using Ruff?" CPY = "CPY" # it's a Ruff rule category +aadd = "aadd" # django's async `add`, alongside `acreate`, `aset` and the rest [default] extend-ignore-re = [ diff --git a/crates/by_transforms/Cargo.toml b/crates/by_transforms/Cargo.toml index 989c66cf93..1a6432d768 100644 --- a/crates/by_transforms/Cargo.toml +++ b/crates/by_transforms/Cargo.toml @@ -21,6 +21,7 @@ ruff_python_stdlib = { workspace = true } ruff_python_trivia = { workspace = true } ruff_source_file = { workspace = true } ruff_text_size = { workspace = true } +salsa = { workspace = true } thin-vec = { workspace = true } ty_project = { workspace = true, features = ["testing"] } ty_python_core = { workspace = true } diff --git a/crates/by_transforms/src/lib.rs b/crates/by_transforms/src/lib.rs index 4edc021204..c23ab0a00a 100644 --- a/crates/by_transforms/src/lib.rs +++ b/crates/by_transforms/src/lib.rs @@ -9,12 +9,13 @@ pub use config::{Config, PythonVersion, SoundnessPositions}; use std::collections::{BTreeSet, HashSet}; use ruff_db::files::{File, system_path_to_file}; -use ruff_db::system::{DbWithWritableSystem as _, SystemPathBuf}; -use ruff_diagnostics::{Edit, Fix, IsolationLevel}; +use ruff_db::system::{DbWithWritableSystem as _, SystemPath, SystemPathBuf}; +use ruff_diagnostics::{Edit, Fix, IsolationLevel, SourceMap}; use ruff_python_ast::Stmt; use ruff_python_ast::visitor::Visitor; use ruff_source_file::LineRanges; use ruff_text_size::{Ranged, TextSize}; +use salsa::Setter as _; use ty_project::{ProjectMetadata, TestDb}; use type_info::TypeInfo; @@ -34,6 +35,47 @@ pub(crate) fn make_in_memory_db(source: &str) -> (TestDb, File) { (db, file) } +/// A caller-supplied capability to build a *second* database over the same +/// project as the one it hands to [`transpile_typed`]. +/// +/// A pre-pass — erased-union reification, name qualification, enum lowering — +/// can rewrite the source before phase 0 runs, and phase 0's type-aware passes +/// must query a db whose contents are the source they walk. Given this +/// capability the transpiler keeps the real project (its metadata, search paths +/// and sibling files) and serves only the rewritten file from memory; without +/// it, phase 0 falls back to a single-file db that resolves nothing outside the +/// file — correct, but blind to imports. +/// +/// The database must be a *new* one, not a clone: salsa handles cloned from one +/// database share storage, so the source override would be visible through the +/// caller's own db. +pub type RebuildProject<'a> = &'a dyn Fn() -> Option>; + +/// A rebuilt project db in which one file reads as rewritten source. +struct Overlaid { + db: Box, + file: File, +} + +/// Rebuild the project db and serve `source` as the contents of `path`, so +/// type-aware passes see the rewritten source with the project still around it. +/// +/// Returns `None` when the caller supplied no [`RebuildProject`], the rebuild +/// failed, or the rebuilt db doesn't hold `path` — every one of which leaves the +/// single-file fallback in place. +fn overlay_rewritten_source( + rebuild: Option>, + path: &SystemPath, + source: &str, +) -> Option { + let mut db = rebuild?()?; + let file = system_path_to_file(&*db, path).ok()?; + let text = ruff_db::source::source_text(&*db, file) + .with_text(source.to_owned(), &SourceMap::default()); + file.set_source_text_override(&mut *db).to(Some(text)); + Some(Overlaid { db, file }) +} + /// Qualify every context-sensitively resolved name (`a: Color = Red` → /// `Color.Red`) against the *original* source — before the enum lowering /// rewrites `enum class` to python, so ty resolves the same source it checks. @@ -172,8 +214,9 @@ pub fn transpile_typed( db: &dyn ty_python_semantic::Db, file: File, config: &Config, + rebuild: Option>, ) -> Result { - transpile_typed_with_map(db, file, config).map(|(out, _)| out) + transpile_typed_with_map(db, file, config, rebuild).map(|(out, _)| out) } /// Like [`transpile_typed`] but also returns a line table mapping each output @@ -188,6 +231,7 @@ pub fn transpile_typed_with_map( db: &dyn ty_python_semantic::Db, file: File, config: &Config, + rebuild: Option>, ) -> Result<(String, Vec>), TranspileError> { let source_ref = ruff_db::source::source_text(db, file); let original_source = source_ref.as_str(); @@ -237,13 +281,22 @@ pub fn transpile_typed_with_map( let enum_changed = matches!(enum_lowered.output, std::borrow::Cow::Owned(_)); let source_changed = reified_changed || qualified_changed || enum_changed; - // phase 0: AST passes. with the project db (no enums) type-aware passes - // resolve cross-module imports; `phase0_map` maps spliced lines → working - // (post-enum) lines - let project = if source_changed { - None + // phase 0: AST passes. type-aware passes resolve cross-module imports from + // the project db; when a pre-pass rewrote the source, that db is rebuilt + // over the rewritten text rather than given up, so an enum or a qualified + // name elsewhere in the file doesn't blind them. `phase0_map` maps spliced + // lines → working (post-enum) lines + let overlaid = if source_changed { + file.path(db) + .as_system_path() + .and_then(|path| overlay_rewritten_source(rebuild, path, working_source)) } else { - Some((db, file)) + None + }; + let project = match &overlaid { + Some(overlaid) => Some((&*overlaid.db, overlaid.file)), + None if source_changed => None, + None => Some((db, file)), }; let (spliced, ast_errors, phase0_map) = transforms::ast_driver::run_against_source(working_source, config, project); @@ -1227,7 +1280,25 @@ mod cross_file { use ruff_db::files::system_path_to_file; use ty_project::{ProjectMetadata, TestDb}; - fn project_db(files: &[(&str, &str)]) -> TestDb { + /// a project db over `files`, keeping the file list so the project can be + /// built a second time — the [`RebuildProject`] capability a real caller + /// supplies, for when a pre-pass rewrites the source + struct Project { + files: Vec<(String, String)>, + db: TestDb, + } + + impl Project { + fn db(&self) -> &TestDb { + &self.db + } + + fn rebuild(&self) -> Box { + Box::new(build_db(&self.files)) + } + } + + fn build_db(files: &[(String, String)]) -> TestDb { let mut db = TestDb::new(ProjectMetadata::new( ruff_python_ast::name::Name::new_static(""), SystemPathBuf::from("/"), @@ -1239,9 +1310,19 @@ mod cross_file { db } - fn transpile_file(db: &TestDb, path: &str, config: &Config) -> String { - let file = system_path_to_file(db, path).expect("file not in db"); - transpile_typed(db, file, config).expect("transpile failed") + fn project_db(files: &[(&str, &str)]) -> Project { + let files: Vec<(String, String)> = files + .iter() + .map(|(path, src)| ((*path).to_owned(), (*src).to_owned())) + .collect(); + let db = build_db(&files); + Project { files, db } + } + + fn transpile_file(project: &Project, path: &str, config: &Config) -> String { + let file = system_path_to_file(project.db(), path).expect("file not in db"); + let rebuild = || Some(project.rebuild()); + transpile_typed(project.db(), file, config, Some(&rebuild)).expect("transpile failed") } /// `f[int](1)` must lower to `f(1)` only because ty resolves the imported @@ -1250,11 +1331,11 @@ mod cross_file { /// single-file path can't see `f` and would leave the broken `f[int](1)`. #[test] fn generic_call_stripped_via_imported_function() { - let db = project_db(&[ + let project = project_db(&[ ("/mod_a.by", "def f[T](t: T) -> T: ...\n"), ("/mod_b.by", "from mod_a import f\nresult = f[int](1)\n"), ]); - let out = transpile_file(&db, "/mod_b.by", &Config::test_default()); + let out = transpile_file(&project, "/mod_b.by", &Config::test_default()); assert!( out.contains("result = f(1)"), "imported generic function should strip type args, got:\n{out}" @@ -1265,13 +1346,74 @@ mod cross_file { ); } + /// the same call, in a file that also declares an `enum class`. the enum + /// lowering rewrites the source before phase 0, and phase 0 then drops the + /// project db because the working source no longer matches the file + /// (`transpile_typed_with_map`'s `source_changed` → `project = None`) — so + /// every type-aware pass loses cross-module *and* project resolution, and + /// this call is left as the broken `f[int](1)`. an unrelated declaration + /// elsewhere in the file must not change what a call lowers to. + /// + /// reordering cannot fix it: phase 0 depends on running against the + /// enum-lowered source (`inferred_annotation` skips the `__slots__ = ()` + /// the enum lowering re-feeds through the pipeline). the fix is to give + /// phase 0 a db that keeps the project's metadata and system while serving + /// the rewritten source for this one file — a `System` wrapper passed to + /// `ProjectDatabase::fallible`, which needs a capability threaded in from + /// the caller that owns the real db + #[test] + fn cross_module_resolution_survives_an_enum_in_the_same_file() { + let project = project_db(&[ + ("/mod_a.by", "def f[T](t: T) -> T: ...\n"), + ( + "/mod_b.by", + "from mod_a import f\n\nenum class Colour:\n case Red\n\nresult = f[int](1)\n", + ), + ]); + let out = transpile_file(&project, "/mod_b.by", &Config::test_default()); + assert!( + out.contains("result = f(1)"), + "an enum elsewhere in the file must not blind cross-module resolution, got:\n{out}" + ); + } + + /// the same defect reached by *qualification* rather than by an enum + /// declaration, which is the likelier trigger and the wider one: one + /// unqualified member of an enum imported from elsewhere — `return Red`, + /// the headline example of context-sensitive resolution — is enough, since + /// `qualified_changed` sets `source_changed` on its own + /// + /// so composing two shipped basedpython features in one file silently + /// breaks the second, with a clean `by check` and valid emitted python + #[test] + fn cross_module_resolution_survives_a_qualified_name_in_the_same_file() { + let project = project_db(&[ + ("/mod_a.by", "def f[T](t: T) -> T: ...\n"), + ("/colours.by", "enum class Colour:\n case Red, Green\n"), + ( + "/mod_b.by", + "from mod_a import f\nfrom colours import Colour\n\ndef pick() -> Colour:\n return Red\n\nresult = f[int](1)\n", + ), + ]); + let out = transpile_file(&project, "/mod_b.by", &Config::test_default()); + assert!( + out.contains("Colour.Red"), + "the qualification itself should still happen, got:\n{out}" + ); + assert!( + out.contains("result = f(1)"), + "a qualified name elsewhere in the file must not blind cross-module \ + resolution, got:\n{out}" + ); + } + /// an imported *reified* generic function keeps its `[int]` specialization /// — the `@generic` wrapper routes it through `__getitem__`. only the /// cross-module type tells us `f` reifies `T` (value-position use), so the /// single-file path can't make this call #[test] fn imported_reified_function_call_site_preserved() { - let db = project_db(&[ + let project = project_db(&[ ( "/mod_a.by", "def f[T](t: object) -> bool:\n return isinstance(t, T)\n", @@ -1282,7 +1424,7 @@ mod cross_file { min_version: PythonVersion::PY312, ..Config::test_default() }; - let out = transpile_file(&db, "/mod_b.by", &config); + let out = transpile_file(&project, "/mod_b.by", &config); assert!( out.contains("f[int](1)"), "reified call site must keep its type args, got:\n{out}" @@ -1294,14 +1436,14 @@ mod cross_file { /// a class, not a function. #[test] fn imported_class_constructor_preserved() { - let db = project_db(&[ + let project = project_db(&[ ( "/mod_a.by", "class Box[T]:\n def __init__(self, t: T): ...\n", ), ("/mod_b.by", "from mod_a import Box\nb = Box[int](1)\n"), ]); - let out = transpile_file(&db, "/mod_b.by", &Config::test_default()); + let out = transpile_file(&project, "/mod_b.by", &Config::test_default()); assert!( out.contains("Box[int](1)"), "imported generic constructor must keep its type args, got:\n{out}" @@ -1313,7 +1455,7 @@ mod cross_file { /// function and emits its precise import. the surface stays `import ext` #[test] fn imported_extension_rewrites_call_and_adds_import() { - let db = project_db(&[ + let project = project_db(&[ ( "/ext.by", "extension list:\n def second(self) -> Element:\n return self[1]\n", @@ -1323,7 +1465,7 @@ mod cross_file { "import ext\n\nxs = [1, 2, 3]\nprint(xs.second())\n", ), ]); - let out = transpile_file(&db, "/main.by", &Config::test_default()); + let out = transpile_file(&project, "/main.by", &Config::test_default()); assert!( out.contains("from ext import _by_ext__list__second"), "backing-function import should be emitted, got:\n{out}" @@ -1333,7 +1475,7 @@ mod cross_file { "call should be rewritten, got:\n{out}" ); // the defining module lowers the block itself - let ext_out = transpile_file(&db, "/ext.by", &Config::test_default()); + let ext_out = transpile_file(&project, "/ext.by", &Config::test_default()); assert!( ext_out.contains("def _by_ext__list__second(self):"), "defining module should lower the block, got:\n{ext_out}" @@ -1346,7 +1488,7 @@ mod cross_file { /// the using module constructs it — so the two files must agree exactly #[test] fn imported_implementation_wraps_argument_and_adds_import() { - let db = project_db(&[ + let project = project_db(&[ ( "/iface.by", "abstract class A:\n abstract def f(self) -> int: ...\n\nclass B:\n a: int = 3\n", @@ -1360,7 +1502,7 @@ mod cross_file { "import adapters\nfrom iface import A, B\n\ndef takes_a(a: A) -> int:\n return a.f()\n\nb = B()\ntakes_a(b)\n", ), ]); - let out = transpile_file(&db, "/main.by", &Config::test_default()); + let out = transpile_file(&project, "/main.by", &Config::test_default()); assert!( out.contains("from adapters import _by_impl__A__B"), "witness import should be emitted, got:\n{out}" @@ -1370,7 +1512,7 @@ mod cross_file { "argument should be wrapped, got:\n{out}" ); // the defining module emits the class under the same name - let adapters_out = transpile_file(&db, "/adapters.by", &Config::test_default()); + let adapters_out = transpile_file(&project, "/adapters.by", &Config::test_default()); assert!( adapters_out.contains("class _by_impl__A__B(_by_Implementation, A):"), "defining module should emit the witness class, got:\n{adapters_out}" @@ -1383,7 +1525,7 @@ mod cross_file { /// itself — that import is the whole difference from the single-file case #[test] fn imported_from_converts_and_imports_the_target() { - let db = project_db(&[ + let project = project_db(&[ ( "/temps.by", "class Celsius:\n degrees: float = 0.0\n\n\ @@ -1396,7 +1538,7 @@ mod cross_file { "from temps import Celsius, report\n\nreport(Celsius())\n", ), ]); - let out = transpile_file(&db, "/main.by", &Config::test_default()); + let out = transpile_file(&project, "/main.by", &Config::test_default()); assert!( out.contains("from temps import Fahrenheit as _by_conv__Fahrenheit"), "the target class should be imported under its alias, got:\n{out}" @@ -1412,7 +1554,7 @@ mod cross_file { /// rebind whatever this file already means by it #[test] fn an_already_imported_target_still_uses_its_alias() { - let db = project_db(&[ + let project = project_db(&[ ( "/temps.by", "class Celsius:\n degrees: float = 0.0\n\n\ @@ -1425,7 +1567,7 @@ mod cross_file { def report(t: Fahrenheit) -> None: ...\n\nreport(Celsius())\n", ), ]); - let out = transpile_file(&db, "/main.by", &Config::test_default()); + let out = transpile_file(&project, "/main.by", &Config::test_default()); assert!( out.contains("report(_by_conv__Fahrenheit.__from__(Celsius()))"), "argument should be converted through the alias, got:\n{out}" @@ -1441,7 +1583,7 @@ mod cross_file { /// the call would reach the wrong object at runtime #[test] fn a_local_class_of_the_same_name_does_not_capture_the_conversion() { - let db = project_db(&[ + let project = project_db(&[ ( "/temps.by", "class Celsius:\n degrees: float = 0.0\n\n\ @@ -1455,7 +1597,7 @@ mod cross_file { class Fahrenheit:\n unrelated: int = 0\n\nreport(Celsius())\n", ), ]); - let out = transpile_file(&db, "/main.by", &Config::test_default()); + let out = transpile_file(&project, "/main.by", &Config::test_default()); assert!( out.contains("from temps import Fahrenheit as _by_conv__Fahrenheit"), "the alias keeps the local class intact, got:\n{out}" @@ -1472,7 +1614,7 @@ mod cross_file { /// (`target/mod.by` → `target.mod` for the checker, `mod` at runtime) #[test] fn an_imported_witness_is_spelled_as_the_file_imports_it() { - let db = project_db(&[ + let project = project_db(&[ ( "/nested/iface.by", "abstract class A:\n abstract def f(self) -> int: ...\n\nclass B:\n a: int = 3\n", @@ -1486,7 +1628,7 @@ mod cross_file { "import adapters\nfrom iface import A, B\n\ndef takes_a(x: A) -> int:\n return x.f()\n\nb = B()\ntakes_a(b)\n", ), ]); - let out = transpile_file(&db, "/nested/main.by", &Config::test_default()); + let out = transpile_file(&project, "/nested/main.by", &Config::test_default()); assert!( out.contains("from adapters import _by_impl__A__B"), "the import must use the file's own spelling, got:\n{out}" @@ -1501,7 +1643,7 @@ mod cross_file { /// must keep the dots #[test] fn a_relatively_imported_witness_keeps_its_dots() { - let db = project_db(&[ + let project = project_db(&[ ("/pkg/__init__.by", "\n"), ( "/pkg/iface.by", @@ -1516,7 +1658,7 @@ mod cross_file { "from .adapters import A\nfrom .iface import B\n\ndef takes_a(x: A) -> int:\n return x.f()\n\ndef main():\n takes_a(B())\n", ), ]); - let out = transpile_file(&db, "/pkg/main.by", &Config::test_default()); + let out = transpile_file(&project, "/pkg/main.by", &Config::test_default()); assert!( out.contains("from .adapters import _by_impl__A__B"), "the relative spelling must be kept, got:\n{out}" @@ -1527,7 +1669,7 @@ mod cross_file { /// so it must bring the module's implementations into scope #[test] fn a_from_import_makes_an_implementation_applicable() { - let db = project_db(&[ + let project = project_db(&[ ( "/iface.by", "abstract class A:\n abstract def f(self) -> int: ...\n\nclass B:\n a: int = 3\n", @@ -1541,7 +1683,7 @@ mod cross_file { "from adapters import A\nfrom iface import B\n\ndef takes_a(x: A) -> int:\n return x.f()\n\nb = B()\ntakes_a(b)\n", ), ]); - let out = transpile_file(&db, "/main.by", &Config::test_default()); + let out = transpile_file(&project, "/main.by", &Config::test_default()); assert!( out.contains("from adapters import _by_impl__A__B"), "witness import should be emitted, got:\n{out}" @@ -1556,7 +1698,7 @@ mod cross_file { /// nothing to wrap — the checker reports the assignment instead #[test] fn implementation_without_the_import_wraps_nothing() { - let db = project_db(&[ + let project = project_db(&[ ( "/iface.by", "abstract class A:\n abstract def f(self) -> int: ...\n\nclass B:\n a: int = 3\n", @@ -1570,7 +1712,7 @@ mod cross_file { "from iface import A, B\n\ndef takes_a(a: A) -> int:\n return a.f()\n\nb = B()\ntakes_a(b)\n", ), ]); - let out = transpile_file(&db, "/main.by", &Config::test_default()); + let out = transpile_file(&project, "/main.by", &Config::test_default()); assert!(out.contains("takes_a(b)"), "got:\n{out}"); assert!(!out.contains("__by_impl__"), "got:\n{out}"); } @@ -1582,10 +1724,11 @@ mod cross_file { #[test] fn line_map_points_runtime_line_to_by_source() { let src = "from collections.abc import Iterator\n\nx: int & str\n\ndef boom() -> int:\n return 1 // 0\n"; - let db = project_db(&[("/m.by", src)]); - let file = system_path_to_file(&db, "/m.by").expect("file not in db"); + let project = project_db(&[("/m.by", src)]); + let file = system_path_to_file(project.db(), "/m.by").expect("file not in db"); let (out, map) = - transpile_typed_with_map(&db, file, &Config::test_default()).expect("transpile failed"); + transpile_typed_with_map(project.db(), file, &Config::test_default(), None) + .expect("transpile failed"); let out_idx = out .lines() diff --git a/crates/by_transforms/src/transforms/ast_driver.rs b/crates/by_transforms/src/transforms/ast_driver.rs index f997a6930e..301d0637e0 100644 --- a/crates/by_transforms/src/transforms/ast_driver.rs +++ b/crates/by_transforms/src/transforms/ast_driver.rs @@ -36,16 +36,16 @@ use ruff_text_size::{Ranged, TextRange}; use super::{ annotation, anon_named_tuple, auto_quote, callable, character_type, checked_cast, coalesce, coalesce_chain, compat, context_params, conversion, decl_site_variance, decorator_keyword, - dedent_string, destructure, dynamic_keyword, empty_declarations, export_import, extension, - float_const, force_unwrap, frameworks, generic_call, generics, grapheme_string, identity_swap, - if_let, implementation, implicit_receiver, implicit_typing, inferred_annotation, init_method, - just_float, kw_subscript, literal_string, literal_types, local_once, main_function, match_type, - modifiers, mutable_defaults, none_chain, optional_type, overload, parametric_is, postfix_await, - propagate, properties, protocol_type, raises_clause, reified_generic, repeated_underscore, - sentinel, some_ctor, soundness, statement_expression, string_tag, super_keyword, - symbolic_type_op, top_star, trailing_lambda, tuple_index, type_fn, type_is, type_reification, - typed_dict_literal, typed_lambda, typeof_keyword, unique_loop_bindings, unpack, - use_site_variance, + dedent_string, destructure, django_lookup, dynamic_keyword, empty_declarations, export_import, + extension, float_const, force_unwrap, frameworks, generic_call, generics, grapheme_string, + identity_swap, if_let, implementation, implicit_receiver, implicit_typing, inferred_annotation, + init_method, just_float, kw_subscript, literal_string, literal_types, local_once, + main_function, match_type, modifiers, mutable_defaults, none_chain, optional_type, overload, + parametric_is, postfix_await, propagate, properties, protocol_type, raises_clause, + reified_generic, repeated_underscore, sentinel, some_ctor, soundness, statement_expression, + string_tag, super_keyword, symbolic_type_op, top_star, trailing_lambda, tuple_index, type_fn, + type_is, type_reification, typed_dict_literal, typed_lambda, typeof_keyword, + unique_loop_bindings, unpack, use_site_variance, }; use crate::Config; use crate::type_info::TypeInfo; @@ -555,6 +555,7 @@ pub(crate) fn run_against_source<'a>( let implementation_block_pass = implementation::ImplementationBlockPass::new(source_ref); let conversion_pass = conversion::ConversionPass::new(source_ref); let implicit_receiver_pass = implicit_receiver::ImplicitReceiverPass; + let django_lookup_pass = django_lookup::DjangoLookupPass; let frameworks_pass = frameworks::FrameworksPass::new(source_ref); let variance_pass = decl_site_variance::VarianceStripPass::new(source_ref); let anon_named_tuple_pass = @@ -698,6 +699,11 @@ pub(crate) fn run_against_source<'a>( // members → its receiver parameter. same shape as the extension rewrite // above, which wins when both could apply &implicit_receiver_pass, + // django lookups written as expressions (`filter(author.name == "x")`) + // become keyword arguments. the argument is replaced whole, with the + // value passing through as `Src`, so lowerings inside it still compose; + // disjoint from the call rewrites above, which target the callee + &django_lookup_pass, &dynamic_keyword_pass, // import-only companion to the ty-side implicit `Character` resolution; // emits no text edits, so ordering among the type passes is free diff --git a/crates/by_transforms/src/transforms/context_sensitive.rs b/crates/by_transforms/src/transforms/context_sensitive.rs index cdb9507cf2..6ddd8c402b 100644 --- a/crates/by_transforms/src/transforms/context_sensitive.rs +++ b/crates/by_transforms/src/transforms/context_sensitive.rs @@ -86,7 +86,7 @@ mod tests { fn mapped_by_line(source: &str, needle: &str) -> Option { let (db, file) = make_in_memory_db(source); let (output, line_map) = - transpile_typed_with_map(&db, file, &Config::test_default()).unwrap(); + transpile_typed_with_map(&db, file, &Config::test_default(), None).unwrap(); let index = output .lines() .position(|line| line.trim() == needle) diff --git a/crates/by_transforms/src/transforms/django_lookup.rs b/crates/by_transforms/src/transforms/django_lookup.rs new file mode 100644 index 0000000000..aa90866965 --- /dev/null +++ b/crates/by_transforms/src/transforms/django_lookup.rs @@ -0,0 +1,83 @@ +//! Lowering for django lookups written as expressions. +//! +//! `Book.objects.filter(author.name == "Ursula", published > date(1970, 1, 1))` +//! lowers to `Book.objects.filter(author__name="Ursula", +//! published__gt=date(1970, 1, 1))`. +//! +//! Which arguments are lookups is ty's answer, not this pass's: the names in a +//! lookup path resolve to model fields rather than to anything in scope, and the +//! checker and the lowering read that from one query, so the query the file was +//! checked against is the query it lowers to. Every argument ty did not read as +//! a lookup passes through untouched. +//! +//! The value is re-emitted as a source span, so lowerings inside it still apply. + +use ruff_python_ast::visitor::{Visitor, walk_expr}; +use ruff_python_ast::{Expr, Stmt}; + +use super::ast_driver::{Fragment, PassContext, TypeAwarePass}; +use crate::type_info::TypeInfo; + +struct DjangoLookupLower<'a> { + types: &'a dyn TypeInfo, + edits: Vec<(ruff_text_size::TextRange, Vec)>, +} + +impl<'ast> Visitor<'ast> for DjangoLookupLower<'_> { + fn visit_expr(&mut self, expr: &'ast Expr) { + if let Expr::Call(call) = expr { + for lookup in self.types.django_lookup_arguments(call) { + self.edits.push(( + lookup.argument, + vec![ + Fragment::Lit(format!("{}=", lookup.key)), + Fragment::Src(lookup.value), + ], + )); + } + } + walk_expr(self, expr); + } +} + +pub(crate) struct DjangoLookupPass; + +impl TypeAwarePass for DjangoLookupPass { + fn run(&self, stmts: &[Stmt], types: &dyn TypeInfo, ctx: &mut PassContext) { + let mut inner = DjangoLookupLower { + types, + edits: Vec::new(), + }; + for stmt in stmts { + inner.visit_stmt(stmt); + } + ctx.template_edits.extend(inner.edits); + } +} + +#[cfg(test)] +mod tests { + use indoc::indoc; + + use crate::{Config, transpile}; + + /// A file with no django in it must come out untouched. The lowering is + /// keyed entirely on ty resolving the callee to a lookup method on a model, + /// so a comparison argument to any other call is an ordinary argument — + /// which is what makes the rewrite additive rather than a rule about the + /// word `filter`. + #[test] + fn a_comparison_argument_to_an_ordinary_call_is_untouched() { + for source in [ + "def filter(*args): ...\nfilter(1 == 2)\n", + "class M:\n def filter(self, *args): ...\n\nM().filter(1 == 2)\n", + indoc! {" + def use(objects) -> None: + objects.filter(objects == 1) + "}, + ] { + let out = transpile(source, &Config::test_default()).expect("transpile should succeed"); + assert!(out.contains("== "), "for `{source}` got:\n{out}"); + } + } +} diff --git a/crates/by_transforms/src/transforms/frameworks.rs b/crates/by_transforms/src/transforms/frameworks.rs index 0fa15326c4..1b9a7b87e6 100644 --- a/crates/by_transforms/src/transforms/frameworks.rs +++ b/crates/by_transforms/src/transforms/frameworks.rs @@ -137,7 +137,7 @@ mod tests { fn transpile_result(db: &TestDb, path: &str) -> Result { let file = system_path_to_file(db, path).expect("file not in db"); - transpile_typed(db, file, &Config::test_default()).map_err(|err| err.to_string()) + transpile_typed(db, file, &Config::test_default(), None).map_err(|err| err.to_string()) } /// a project db with a mock django package in site-packages (`Model` must @@ -248,7 +248,7 @@ mod tests { soundness: crate::config::SoundnessPositions::all(), ..Config::default() }; - let out = transpile_typed(&db, file, &config).expect("model should transpile"); + let out = transpile_typed(&db, file, &config, None).expect("model should transpile"); assert!( out.contains("class User(DeclarativeBase):"), "class structure should survive, got:\n{out}" @@ -316,7 +316,7 @@ mod tests { soundness: crate::config::SoundnessPositions::all(), ..Config::default() }; - let out = transpile_typed(&db, file, &config).expect("model should transpile"); + let out = transpile_typed(&db, file, &config, None).expect("model should transpile"); assert!( out.contains("class Author(models.Model):"), "class structure should survive, got:\n{out}" @@ -424,7 +424,8 @@ mod tests { min_version: ruff_python_ast::PythonVersion::PY313, ..Config::test_default() }; - let out = transpile_typed(&db, file, &config).expect("generic model should transpile"); + let out = + transpile_typed(&db, file, &config, None).expect("generic model should transpile"); assert!( !out.contains("@generic"), "a model class must never be wrapped with `@generic`, got:\n{out}" @@ -462,7 +463,7 @@ mod tests { soundness: crate::config::SoundnessPositions::all(), ..Config::default() }; - let out = transpile_typed(&db, file, &config).expect("model should transpile"); + let out = transpile_typed(&db, file, &config, None).expect("model should transpile"); assert!( out.contains("class User(BaseModel):"), "class structure should survive, got:\n{out}" diff --git a/crates/by_transforms/src/transforms/kw_subscript.rs b/crates/by_transforms/src/transforms/kw_subscript.rs index d8f121d010..db87794a56 100644 --- a/crates/by_transforms/src/transforms/kw_subscript.rs +++ b/crates/by_transforms/src/transforms/kw_subscript.rs @@ -393,7 +393,7 @@ mod tests { .expect("write file failed"); db.init_program().expect("program init failed"); let file = system_path_to_file(&db, "/proj/main.by").expect("file not in db"); - transpile_typed(&db, file, &Config::test_default()).expect("transpile failed") + transpile_typed(&db, file, &Config::test_default(), None).expect("transpile failed") } #[test] diff --git a/crates/by_transforms/src/transforms/mod.rs b/crates/by_transforms/src/transforms/mod.rs index 06ae738382..bf93d481f9 100644 --- a/crates/by_transforms/src/transforms/mod.rs +++ b/crates/by_transforms/src/transforms/mod.rs @@ -15,6 +15,7 @@ pub(crate) mod decl_site_variance; pub(crate) mod decorator_keyword; pub(crate) mod dedent_string; pub(crate) mod destructure; +pub(crate) mod django_lookup; pub(crate) mod dynamic_keyword; pub(crate) mod empty_declarations; pub(crate) mod enums; diff --git a/crates/by_transforms/src/type_info.rs b/crates/by_transforms/src/type_info.rs index 97876115c4..80333f071c 100644 --- a/crates/by_transforms/src/type_info.rs +++ b/crates/by_transforms/src/type_info.rs @@ -181,6 +181,16 @@ pub(crate) trait TypeInfo { /// it used unqualified. both lower to the block's receiver parameter fn implicit_receiver_name(&self, name: &ExprName) -> Option; + /// how each positional argument of a django lookup method that spells a + /// `__` lookup as an expression lowers to a keyword — + /// `filter(author.name == "x")` → `filter(author__name="x")`. empty for + /// every other call, and for any argument the checker did not read as a + /// lookup, which must then be left exactly as written + fn django_lookup_arguments( + &self, + call: &ruff_python_ast::ExprCall, + ) -> Vec; + /// the enum a *context-sensitively* resolved name must be qualified with — /// `Red` in a `Color` context lowers to `Color.Red`. `None` for every name /// that resolves the ordinary way @@ -587,6 +597,13 @@ impl TypeInfo for SemanticModel<'_> { SemanticModel::implicit_receiver_name(self, name) } + fn django_lookup_arguments( + &self, + call: &ruff_python_ast::ExprCall, + ) -> Vec { + SemanticModel::django_lookup_arguments(self, call) + } + fn context_sensitive_qualifier(&self, name: &ExprName) -> Option { SemanticModel::context_sensitive_qualifier(self, name) } diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F821_basedpython.by b/crates/ruff_linter/resources/test/fixtures/pyflakes/F821_basedpython.by index b3c0d796c6..8202da0c4b 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyflakes/F821_basedpython.by +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F821_basedpython.by @@ -192,3 +192,25 @@ print(ImplementedAsInterface(Implemented())) # check` reports it as an unresolved type implementation UndefinedInterface for UndefinedImplemented: # F821 pass + + +# a django lookup expression: the transpiler lowers the field path to the +# keyword it spells (`filter(author.name == "x")` -> `filter(author__name="x")`), +# so the path is never evaluated and none of these names is undefined +class Model: + objects: object + + +Book = Model() +Book.objects.filter(author.name == "Ursula") +Book.objects.filter(published > 1) +Book.objects.filter(data["key"] == 1) +Book.objects.exclude(title == "x", author.pk >= 3) +Book.objects.filter(pk in [1, 2, 3]) +Book.objects.get(pk == 1) + +# but the suppression is only for that shape, so each of these is still reported +Book.objects.filter(bare_name) # F821 +Book.objects.annotate(not_a_lookup_method.x == 1) # F821 +Book.objects.filter(data[undefined_index] == 1) # F821 +Book.objects.filter(no_lookup_for_ne != 1) # F821 diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F821_F821_basedpython.by.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F821_F821_basedpython.by.snap index ea01903560..4e991368cb 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F821_F821_basedpython.by.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F821_F821_basedpython.by.snap @@ -176,3 +176,43 @@ F821 Undefined name `UndefinedInterface` | ^^^^^^^^^^^^^^^^^^ 194 | pass | + +F821 Undefined name `bare_name` + --> F821_basedpython.by:213:21 + | +212 | # but the suppression is only for that shape, so each of these is still reported +213 | Book.objects.filter(bare_name) # F821 + | ^^^^^^^^^ +214 | Book.objects.annotate(not_a_lookup_method.x == 1) # F821 +215 | Book.objects.filter(data[undefined_index] == 1) # F821 + | + +F821 Undefined name `not_a_lookup_method` + --> F821_basedpython.by:214:23 + | +212 | # but the suppression is only for that shape, so each of these is still reported +213 | Book.objects.filter(bare_name) # F821 +214 | Book.objects.annotate(not_a_lookup_method.x == 1) # F821 + | ^^^^^^^^^^^^^^^^^^^ +215 | Book.objects.filter(data[undefined_index] == 1) # F821 +216 | Book.objects.filter(no_lookup_for_ne != 1) # F821 + | + +F821 Undefined name `undefined_index` + --> F821_basedpython.by:215:26 + | +213 | Book.objects.filter(bare_name) # F821 +214 | Book.objects.annotate(not_a_lookup_method.x == 1) # F821 +215 | Book.objects.filter(data[undefined_index] == 1) # F821 + | ^^^^^^^^^^^^^^^ +216 | Book.objects.filter(no_lookup_for_ne != 1) # F821 + | + +F821 Undefined name `no_lookup_for_ne` + --> F821_basedpython.by:216:21 + | +214 | Book.objects.annotate(not_a_lookup_method.x == 1) # F821 +215 | Book.objects.filter(data[undefined_index] == 1) # F821 +216 | Book.objects.filter(no_lookup_for_ne != 1) # F821 + | ^^^^^^^^^^^^^^^^ + | diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 0665709a72..b5a2a10934 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -785,6 +785,7 @@ impl<'a> SemanticModel<'a> { } else if self.is_basedpython_class_base_self_ref(name) || self.is_basedpython_transpile_resolved_name(name) || self.is_basedpython_type_is_lhs(name) + || self.is_basedpython_django_lookup_path(name) { // basedpython resolves these forms at transpile time, so the name is // not actually undefined at runtime @@ -896,6 +897,71 @@ impl<'a> SemanticModel<'a> { matches!(compare.left.as_ref(), Expr::Name(left) if left.range == name.range) } + /// True if `name` leads the field path of a django lookup expression — + /// `filter(author.name == "x")`, which the transpiler lowers to the keyword + /// `author__name="x"`, so the path is never evaluated at runtime. + /// + /// This is deliberately syntactic, and deliberately broader than the + /// transpiler's own rule: whether the leading segment really names a field + /// of the model is a question only the type checker can answer, and it does + /// answer it. Reporting an unresolvable path there rather than here keeps + /// one answer to it instead of two that can disagree. + fn is_basedpython_django_lookup_path(&self, name: &ast::ExprName) -> bool { + if !self.in_basedpython_file() { + return false; + } + let mut ancestors = self.current_expressions().skip(1); + + // climb the attribute / subscript chain the name leads. the name has to + // *lead* it — in `data[key]`, `key` is the index, not the path + let mut path = name.range; + let mut parent = ancestors.next(); + while let Some(expr) = parent { + let value = match expr { + Expr::Attribute(attribute) => attribute.value.range(), + Expr::Subscript(subscript) => subscript.value.range(), + _ => break, + }; + if value != path { + break; + } + path = expr.range(); + parent = ancestors.next(); + } + + // the path must be the left operand of one comparison django can spell + let Some(Expr::Compare(compare)) = parent else { + return false; + }; + if compare.left.range() != path + || !matches!( + compare.ops.as_ref(), + [ast::CmpOp::Eq + | ast::CmpOp::Gt + | ast::CmpOp::GtE + | ast::CmpOp::Lt + | ast::CmpOp::LtE + | ast::CmpOp::In] + ) + { + return false; + } + + // and that comparison must be a positional argument of a lookup method + let Some(Expr::Call(call)) = ancestors.next() else { + return false; + }; + let Expr::Attribute(func) = call.func.as_ref() else { + return false; + }; + matches!(func.attr.as_str(), "filter" | "exclude" | "get" | "aget") + && call + .arguments + .args + .iter() + .any(|argument| argument.range() == compare.range()) + } + /// True if `name` is a forward self-reference that the basedpython /// transpiler will auto-quote, so resolution should not flag it as /// undefined. This covers two positions: diff --git a/crates/ty/Cargo.toml b/crates/ty/Cargo.toml index 69f540e00f..fa8d48571b 100644 --- a/crates/ty/Cargo.toml +++ b/crates/ty/Cargo.toml @@ -29,6 +29,7 @@ by_transforms = { workspace = true } ruff_ranged_value = { workspace = true } ty_combine = { workspace = true } ty_module_resolver = { workspace = true } +ty_ide = { workspace = true } ty_project = { workspace = true, features = ["zstd", "junit"] } ty_python_semantic = { workspace = true, features = ["serde"] } ty_server = { workspace = true } diff --git a/crates/ty/docs/rules.md b/crates/ty/docs/rules.md index 88359fe5b3..dd561e8def 100644 --- a/crates/ty/docs/rules.md +++ b/crates/ty/docs/rules.md @@ -3420,6 +3420,120 @@ def func() -> int: return "a" # error: [invalid-return-type] ``` +## `invalid-route-arguments` + + +Default level: error · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks the arguments a `{% url %}` passes against the route's own pattern. + +**Why is this bad?** + +Django raises `NoReverseMatch` when it renders the template, so the page +500s. A route's pattern names the arguments it takes and the converter +each of them goes through, so a missing, extra or misnamed argument — and +a literal the converter would reject — is a reversal that cannot match. + +Only a route whose whole pattern is known is checked, and a name several +routes share is reported only when none of them accepts the arguments. + +**Examples** + +```django +{# path("/", detail, name="detail") #} +{% url 'detail' %} {# error: `pk` is missing #} +{% url 'detail' pk='x' %} {# error: `pk` goes through `int` #} +{% url 'detail' pk=1 %} {# ok #} +``` + +## `invalid-route-handler` + + +Default level: error · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks that the view a route names can take the arguments the route's +pattern gives it. + +**Why is this bad?** + +A route pattern names its arguments, and django hands each of them to the +view as a keyword argument. A view that names them differently, or does +not take them at all, raises `TypeError` when the url is requested — a +failure nothing reports until the page 500s. + +Only a project whose url tree could be walked in full is checked, since a +route's arguments include the ones the patterns it is mounted behind +contribute. A view taking `*args` or `**kwargs` accepts anything and is +never reported, nor is one reached through a decorator the type checker +cannot see through. A class-based view is checked through the handler +methods it declares itself: what it inherits from django takes `**kwargs`. + +**Examples** + +```python +path("books//", views.detail, name="detail") +``` + +```python +def detail(request, id): ... # error: the route names `pk` +def detail(request): ... # error: `pk` has nowhere to go +def detail(request, pk): ... # ok +``` + +## `invalid-route-parameter-type` + + +Default level: warn · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks the type a view declares for a route argument against the type the +route's converter produces. + +**Why is this bad?** + +A path converter parses the url before django calls the view: +`` hands the view an `int` and `` a `uuid.UUID`. A view +declaring something else is annotated with a type its argument never has, +so everything written against the annotation is written against a value +that is not there. + +Unlike [`invalid-route-handler`](#invalid-route-handler) this does not stop the request: django +calls the view and the wrong value arrives silently. +An unannotated parameter declares nothing and is never reported, +and neither is one matched by a regular expression, which goes through no +converter at all. + +**Examples** + +```python +path("books//", views.detail, name="detail") +``` + +```python +def detail(request, pk: str): ... # warning: the converter gives an `int` +def detail(request, pk: int): ... # ok +``` + ## `invalid-super-argument` @@ -6010,6 +6124,75 @@ class F(NamedTuple): - [Python documentation: super()](https://docs.python.org/3/library/functions.html#super) +## `template-member-alters-data` + + +Default level: warn · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks for a `{{ }}` lookup landing on a method django refuses to call +from a template. + +**Why is this bad?** + +Django calls whatever a lookup lands on, except a method marked +`alters_data = True` — a template is not allowed to write to the +database. Rather than call it, django renders `string_if_invalid` — the +empty string by default — silently, with no error and no output. + +The methods django marks are the ones that write: `save`, `delete` and +their `async` twins on a model, and the `create`/`update`/`bulk_*` +family on a queryset, a manager and the manager behind a relation. +Overriding one keeps the mark, so an override is reported too, exactly +as django's own `AltersData` propagates it at runtime. + +**Examples** + +```django +{{ book.save }} {# warning: renders nothing #} +{{ book.title }} {# ok #} +``` + +## `template-member-needs-arguments` + + +Default level: warn · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks for a `{{ }}` lookup landing on a method that cannot be called +without arguments. + +**Why is this bad?** + +Django calls whatever a lookup lands on. A method that needs an argument +cannot be called, so django renders `string_if_invalid` — the empty string +by default — silently, with no error and no output. + +**Examples** + +```python +class Book: + def headline(self, length: int) -> str: ... + def title(self) -> str: ... +``` + +```django +{{ book.headline }} {# warning: renders nothing #} +{{ book.title }} {# ok #} +``` + ## `too-many-positional-arguments` @@ -6320,6 +6503,35 @@ class C(Generic[T]): - [Typing spec: Scoping rules for type variables](https://typing.python.org/en/latest/spec/generics.html#scoping-rules-for-type-variables) +## `unclosed-template-block` + + +Default level: error · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks for a block tag in a django template whose closing tag is missing. + +**Why is this bad?** + +Django raises `TemplateSyntaxError` when it compiles the template, so the +page does not render at all. + +Only a tag known to open a block is reported: one of django's own, or one +the project registers with `@register.simple_block_tag`. + +**Examples** + +```django +{% if user.is_staff %} {# error: no `{% endif %}` #} +

hello

+``` + ## `undeclared-raise` @@ -6475,6 +6687,181 @@ def test_thing(no_such_fixture) -> None: # requests an unknown fixture ... ``` +## `unknown-template-block` + + +Default level: warn · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks for a `{% block %}` overriding a block no ancestor template +declares. + +**Why is this bad?** + +A child template's blocks are rendered by the parent, so a block the +parent never declares is never rendered — silently, with no error and no +output. + +Only a block written at the top level of a template that `{% extends %}` +something is reported. A block nested inside another one is rendered as +part of its enclosing block and needs no declaration above it. + +**Examples** + +```django +{# base.html declares `content`, and nothing else #} +{% extends "base.html" %} +{% block sidebar %}hello{% endblock %} {# warning: never rendered #} +``` + +## `unknown-template-filter` + + +Default level: error · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks for a filter no library the template can reach registers. + +**Why is this bad?** + +Django raises `TemplateSyntaxError` when it compiles the template, so the +page does not render at all. + +A filter whose library the template has simply not loaded is reported as +[`unloaded-template-library`](#unloaded-template-library) instead. + +**Examples** + +```django +{{ book.title|uppercase }} {# error: the filter is `upper` #} +``` + +## `unknown-template-library` + + +Default level: error · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks for a `{% load %}` of a tag library the project does not have. + +**Why is this bad?** + +Django raises `TemplateSyntaxError` when it compiles the template, so the +page does not render at all. + +A library is a `templatetags` module of the project or of one of the apps +`INSTALLED_APPS` names. Nothing is reported unless the settings module was +found and every installed app resolved, since a library that cannot be +reached is not a library that is missing. + +**Examples** + +```django +{% load blog_xtras %} {# error: the library is `blog_extras` #} +``` + +## `unknown-template-tag` + + +Default level: error · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks for a tag no library the template can reach registers. + +**Why is this bad?** + +Django raises `TemplateSyntaxError` when it compiles the template, so the +page does not render at all. + +A tag whose library the template has simply not loaded is reported as +[`unloaded-template-library`](#unloaded-template-library) instead. + +**Examples** + +```django +{% iff user.is_staff %} {# error: no such tag #} +``` + +## `unloaded-template-library` + + +Default level: error · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks for a tag or filter used without the `{% load %}` it needs. + +**Why is this bad?** + +Django raises `TemplateSyntaxError` when it compiles the template, so the +page does not render at all — the tag exists, but not in this template. + +A library named by `TEMPLATES[*]["OPTIONS"]["builtins"]` is loaded into +every template already and is never reported. + +**Examples** + +```django +{% static 'css/site.css' %} {# error: needs `{% load static %}` #} +``` + +## `unmatched-template-close` + + +Default level: error · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks for a closing tag in a django template that closes nothing. + +**Why is this bad?** + +Django raises `TemplateSyntaxError` when it compiles the template, so the +page does not render at all. + +**Examples** + +```django +{% for book in books %} +

{{ book.title }}

+{% endwith %} {# error: nothing here opened a `{% with %}` #} +{% endfor %} +``` + ## `unresolved-attribute` @@ -6670,6 +7057,96 @@ Using an undefined variable will raise a `NameError` at runtime. print(x) # error ``` +## `unresolved-route` + + +Default level: error · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks for a `{% url %}` naming a route the url configuration does not +have. + +**Why is this bad?** + +Django raises `NoReverseMatch` when it renders the template, so the page +500s. + +The route names are the ones the walk from `ROOT_URLCONF` finds. A project +that does not say where its url tree starts, or whose tree could not be +walked in full, reports nothing. + +**Examples** + +```django + {# error: it is `blog:detail` #} +``` + +## `unresolved-static-file` + + +Default level: warn · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks for a `{% static %}` naming a file that is not there. + +**Why is this bad?** + +Django's default storage builds the url from the name without checking it, +so the page renders and the asset 404s — a failure nothing reports. + +A name whose directory holds no discovered file at all is left alone: that +is what a bundle built into `static/` at deploy time looks like, and it is +not something the source tree can answer. + +**Examples** + +```django +{% load static %} + {# warning: it is `css/site.css` #} +``` + +## `unresolved-template` + + +Default level: error · +Added in 0.0.69 · +Related issues · +View source + + + +**What it does** + +Checks for an `{% extends %}` or `{% include %}` naming a template that +is not there. + +**Why is this bad?** + +Django raises `TemplateDoesNotExist` when it renders the template, so the +page 500s. + +Nothing is reported unless the project's template directories are known: +a project with no readable settings *and* no directory named `templates` +is one whose template set cannot be established. + +**Examples** + +```django +{% extends "blog/bass.html" %} {# error: the template is `blog/base.html` #} +``` + ## `unspecialized-reified-generic` diff --git a/crates/ty/src/by_commands.rs b/crates/ty/src/by_commands.rs index ce1d072f5d..fa0bbacba1 100644 --- a/crates/ty/src/by_commands.rs +++ b/crates/ty/src/by_commands.rs @@ -11,7 +11,7 @@ use ruff_db::diagnostic::{ Span, }; use ruff_db::files::system_path_to_file; -use ruff_db::system::{OsSystem, SystemPath}; +use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf}; use ty_project::{Db, ProjectDatabase, ProjectMetadata}; use walkdir::WalkDir; @@ -123,7 +123,7 @@ pub(crate) fn cmd_run( return Ok(ExitStatus::Failure); } - let (db, handles) = build_project_db(&cwd, &files)?; + let (db, handles, rebuilder) = build_project_db(&cwd, &files)?; // an explicit module always wins; otherwise the project's configured entry // point stands in for it. resolving before the (much slower) check means a @@ -147,6 +147,7 @@ pub(crate) fn cmd_run( &handles, &config, CheckGate::AllErrors, + &rebuilder, |bpy, src, line_map| { let rel = bpy.strip_prefix(&cwd).unwrap_or(bpy); let py = tmp.path().join(rel).with_extension("py"); @@ -218,12 +219,13 @@ pub(crate) fn cmd_build(min_version: &str, lowering: &LoweringArgs) -> anyhow::R return Ok(ExitStatus::Success); } - let (db, handles) = build_project_db(&cwd, &files)?; + let (db, handles, rebuilder) = build_project_db(&cwd, &files)?; if !render_check_and_transpile( &db, &handles, &config, CheckGate::ParseErrorsOnly, + &rebuilder, |bpy, src, _line_map| { let py = out .join(bpy.strip_prefix(&cwd).unwrap()) @@ -313,6 +315,11 @@ pub(crate) fn cmd_transpile( let system = OsSystem::new(project_root); let project_metadata = ProjectMetadata::discover(project_root, &system) .with_context(|| format!("failed to discover project at {project_root}"))?; + let rebuilder = Rebuilder { + metadata: project_metadata.clone(), + root: project_root.to_path_buf(), + included: vec![sys_path.to_path_buf()], + }; let mut db = ProjectDatabase::use_defaults(project_metadata, system); let file = system_path_to_file(&db, sys_path) .with_context(|| format!("file not found in db: {sys_path}"))?; @@ -333,7 +340,8 @@ pub(crate) fn cmd_transpile( return Ok(ExitStatus::Failure); } - match by_transforms::transpile_typed(&db, file, &config) { + let rebuild = || Some(rebuilder.rebuild()); + match by_transforms::transpile_typed(&db, file, &config, Some(&rebuild)) { Ok(out) => { if !diagnostics.is_empty() { render_diagnostics(&db, &diagnostics)?; @@ -428,12 +436,13 @@ fn forward_dir(dir: &Path, config: &Config) -> anyhow::Result { return Ok(ExitStatus::Success); } - let (db, handles) = build_project_db(dir, &files)?; + let (db, handles, rebuilder) = build_project_db(dir, &files)?; let ok = render_check_and_transpile( &db, &handles, config, CheckGate::ParseErrorsOnly, + &rebuilder, |bpy, src, _line_map| { let py = bpy.with_extension("py"); fs::write(&py, src).with_context(|| format!("{}", py.display()))?; @@ -613,13 +622,42 @@ fn bpy_files(root: &Path) -> Vec { .collect() } +/// Everything needed to build a project db a second time. +/// +/// The transpiler asks for one when a pre-pass rewrites the source it hands to +/// phase 0: it then serves the rewritten file out of that db, keeping the +/// project's metadata, search paths and sibling files. The rebuilt db must be +/// independent of the one this command uses — see +/// [`by_transforms::RebuildProject`]. +struct Rebuilder { + metadata: ProjectMetadata, + root: SystemPathBuf, + included: Vec, +} + +impl Rebuilder { + fn rebuild(&self) -> Box { + let mut db = + ProjectDatabase::use_defaults(self.metadata.clone(), OsSystem::new(&self.root)); + db.project() + .set_included_paths(&mut db, self.included.clone()); + Box::new(db) + } +} + +/// A project db, the `(source_path, File)` pairs for the `.by` files it was +/// built for, and the means to build the same project again. +type ProjectBuild = ( + ProjectDatabase, + Vec<(PathBuf, ruff_db::files::File)>, + Rebuilder, +); + /// Build a project db rooted at `cwd` with every `.by` file under it set as /// an included path, returning the db alongside `(source_path, File)` pairs -/// in the same order as the input slice. -fn build_project_db( - cwd: &Path, - files: &[PathBuf], -) -> anyhow::Result<(ProjectDatabase, Vec<(PathBuf, ruff_db::files::File)>)> { +/// in the same order as the input slice, and the means to build the same +/// project again. +fn build_project_db(cwd: &Path, files: &[PathBuf]) -> anyhow::Result { // the project root must be canonicalized the same way the included files // are (below) so it stays a path *prefix* of them: otherwise a file's // search path isn't recognized as first-party and boundary diagnostics @@ -632,6 +670,7 @@ fn build_project_db( let system = OsSystem::new(sys_cwd); let project_metadata = ProjectMetadata::discover(sys_cwd, &system) .with_context(|| format!("failed to discover project at {sys_cwd}"))?; + let metadata = project_metadata.clone(); let mut db = ProjectDatabase::use_defaults(project_metadata, system); let mut handles = Vec::with_capacity(files.len()); @@ -645,8 +684,13 @@ fn build_project_db( .with_context(|| format!("file not found in db: {sys_path}"))?; handles.push((bpy.clone(), f)); } + let rebuilder = Rebuilder { + metadata, + root: sys_cwd.to_path_buf(), + included: included.clone(), + }; db.project().set_included_paths(&mut db, included); - Ok((db, handles)) + Ok((db, handles, rebuilder)) } /// How much of the check outcome blocks emitting output. @@ -671,6 +715,7 @@ fn render_check_and_transpile( handles: &[(PathBuf, ruff_db::files::File)], config: &Config, gate: CheckGate, + rebuilder: &Rebuilder, mut consume: impl FnMut(&Path, &str, &[Option]) -> anyhow::Result<()>, ) -> anyhow::Result { let mut all_diagnostics: Vec = Vec::new(); @@ -696,8 +741,9 @@ fn render_check_and_transpile( return Ok(false); } + let rebuild = || Some(rebuilder.rebuild()); for (bpy, file) in handles { - match by_transforms::transpile_typed_with_map(db, *file, config) { + match by_transforms::transpile_typed_with_map(db, *file, config, Some(&rebuild)) { Ok((out, line_map)) => consume(bpy, &out, &line_map)?, Err(e) => { all_diagnostics.push(transpile_bug_diagnostic(*file, &e)); diff --git a/crates/ty/src/lib.rs b/crates/ty/src/lib.rs index dded0b06fc..a1c372b659 100644 --- a/crates/ty/src/lib.rs +++ b/crates/ty/src/lib.rs @@ -8,7 +8,7 @@ mod version; use std::io::{BufWriter, Write}; use std::process::{ExitCode, Termination}; -use std::sync::Mutex; +use std::sync::{Arc, Mutex}; use anyhow::Result; use anyhow::{Context, anyhow}; @@ -18,7 +18,7 @@ use crossbeam::channel as crossbeam_channel; use rayon::ThreadPoolBuilder; use ruff_db::cancellation::{Canceled, CancellationToken, CancellationTokenSource}; use ruff_db::diagnostic::{ - Diagnostic, DiagnosticId, DisplayDiagnosticConfig, DisplayDiagnostics, Severity, + Diagnostic, DiagnosticId, DisplayDiagnosticConfig, DisplayDiagnostics, Severity, UnifiedFile, }; use ruff_db::files::File; use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf}; @@ -290,6 +290,13 @@ fn run_check(args: CheckCommand) -> anyhow::Result { project_metadata.apply_override_options(args.into_options()); let mut db = ProjectDatabase::fallible(project_metadata, system)?; + + // the project's django, which the type checker does not read: its templates are + // checked alongside the python files, and what its routes get wrong is folded + // into the python files' own diagnostics. a project without django registers a + // checker that answers with nothing, and pays nothing + db.set_checker(Arc::new(ty_ide::DjangoChecker)); + let project = db.project(); project.set_verbose(&mut db, verbosity >= VerbosityLevel::Verbose); @@ -524,6 +531,11 @@ impl MainLoop { } } MainLoopMode::Fix(mode) => { + // both of these rewrite a file through its python tokens, so a + // diagnostic on a file that is not python is reported unchanged + // rather than fixed + let (result, other_language) = split_other_language(db, result); + let result = match mode { FixMode::AddIgnore => { suppress_all_diagnostics(db, result, &self.cancellation_token) @@ -536,7 +548,12 @@ impl MainLoop { ), }; - if let Ok(result) = result { + if let Ok(mut result) = result { + result.diagnostics.extend(other_language); + result.diagnostics.sort_by(|left, right| { + left.rendering_sort_key(db) + .cmp(&right.rendering_sort_key(db)) + }); let fixed_diagnostics = match mode { FixMode::AddIgnore => None, FixMode::ApplyFixes => Some(result.count), @@ -692,6 +709,32 @@ enum FixMode { ApplyFixes, } +/// Split `diagnostics` into the python ones and the ones a registered +/// [`ty_project::ProjectChecker`] owns the file of. +/// +/// Everything that rewrites a file — applying a fix, adding an ignore comment — +/// works through the file's python tokens, and the second group has none. +fn split_other_language( + db: &ProjectDatabase, + diagnostics: Vec, +) -> (Vec, Vec) { + let Some(checker) = db.project_checker() else { + return (diagnostics, Vec::new()); + }; + + diagnostics.into_iter().partition(|diagnostic| { + let owned = diagnostic + .primary_span_ref() + .and_then(|span| match span.file() { + UnifiedFile::Ty(file) => file.path(db).as_system_path(), + UnifiedFile::Ruff(_) => None, + }) + .is_some_and(|path| checker.owns(db, path)); + + !owned + }) +} + fn exit_status_from_diagnostics( diagnostics: &[Diagnostic], terminal_settings: &TerminalSettings, diff --git a/crates/ty/tests/cli/django.rs b/crates/ty/tests/cli/django.rs new file mode 100644 index 0000000000..971e38c5ec --- /dev/null +++ b/crates/ty/tests/cli/django.rs @@ -0,0 +1,417 @@ +//! `by check` over a django project. +//! +//! A template is not python and is not in the project's file set, so these are +//! also the tests that say `by check` reaches a file the type checker never sees — +//! and that it still never reads one as python. + +use crate::CliTest; +use insta_cmd::assert_cmd_snapshot; + +/// A django project whose settings, templates and url tree can all be read. +/// +/// `django` is written *outside* the project root and reached through an extra +/// search path, which is what makes it an installed package rather than source of +/// the project's own — the same difference site-packages makes. +fn django_project(files: &[(&str, &str)]) -> anyhow::Result { + let mut all: Vec<(&str, &str)> = vec![ + ( + "ty.toml", + r#" + [environment] + extra-paths = ["../libs"] + "#, + ), + ( + "manage.py", + r#" + import os + + os.environ.setdefault("DJANGO_SETTINGS_MODULE", "project.settings") + "#, + ), + ("project/__init__.py", ""), + ( + "project/settings.py", + r#" + INSTALLED_APPS = ["blog"] + + TEMPLATES = [{"DIRS": [], "APP_DIRS": True, "OPTIONS": {}}] + + ROOT_URLCONF = "project.urls" + "#, + ), + ( + "project/urls.py", + r#" + from django.urls import include, path + + urlpatterns = [path("blog/", include("blog.urls"))] + "#, + ), + ("blog/__init__.py", ""), + ( + "blog/views.py", + " + def index(request): ... + + + def detail(request, pk: int): ... + ", + ), + ( + "blog/urls.py", + r#" + from django.urls import path + + from blog import views + + app_name = "blog" + + urlpatterns = [ + path("", views.index, name="index"), + path("/", views.detail, name="detail"), + ] + "#, + ), + ( + "blog/templates/blog/base.html", + "{% block content %}{% endblock %}", + ), + ]; + all.extend_from_slice(files); + + let case = CliTest::with_files(all)?; + + write_installed( + &case, + &[ + ("django/__init__.py", ""), + ( + "django/urls/__init__.py", + "def path(route, view, kwargs=None, name=None): ...\n\ + def include(arg, namespace=None): ...\n", + ), + ], + )?; + + Ok(case) +} + +/// Write packages to the `libs` directory beside the project root, which +/// [`django_project`] puts on the search path. +fn write_installed(case: &CliTest, files: &[(&str, &str)]) -> anyhow::Result<()> { + let libs = case + .root() + .parent() + .expect("the project directory always has a parent") + .join("libs"); + + for (path, content) in files { + let path = libs.join(path); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + std::fs::write(path, content)?; + } + + Ok(()) +} + +#[test] +fn a_template_is_checked() -> anyhow::Result<()> { + let case = django_project(&[( + "blog/templates/blog/post.html", + "{% if book %}\n

{{ book.title|uppercase }}

\n", + )])?; + + assert_cmd_snapshot!(case.command().arg("--output-format=concise"), @r" + success: false + exit_code: 1 + ----- stdout ----- + blog/templates/blog/post.html:1:1: error[unclosed-template-block] unclosed `if` + blog/templates/blog/post.html:2:18: error[unknown-template-filter] no template filter named `uppercase` + Found 2 diagnostics + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn a_template_is_never_read_as_python() -> anyhow::Result<()> { + // every line of this is a python syntax error, and the only thing reported is + // the one thing wrong with it as a template + let case = django_project(&[( + "blog/templates/blog/post.html", + "
    \n{% for book in books %}\n
  • {{ book.title }}
  • \n", + )])?; + + assert_cmd_snapshot!(case.command().arg("--output-format=concise"), @r" + success: false + exit_code: 1 + ----- stdout ----- + blog/templates/blog/post.html:2:1: error[unclosed-template-block] unclosed `for` + Found 1 diagnostic + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn a_template_named_on_the_command_line_is_still_not_read_as_python() -> anyhow::Result<()> { + // a path passed explicitly is otherwise taken to be something ty can analyze, + // whatever its extension + let case = django_project(&[("blog/templates/blog/post.html", "{% for book in books %}\n")])?; + + assert_cmd_snapshot!( + case.command() + .arg("--output-format=concise") + .arg("blog/templates/blog/post.html"), + @r" + success: false + exit_code: 1 + ----- stdout ----- + blog/templates/blog/post.html:1:1: error[unclosed-template-block] unclosed `for` + Found 1 diagnostic + + ----- stderr ----- + WARN No python files found under the given path(s) + " + ); + + Ok(()) +} + +#[test] +fn a_suppression_comment_in_a_template_silences_it() -> anyhow::Result<()> { + let case = django_project(&[( + "blog/templates/blog/post.html", + "{% if book %}{# ty: ignore[unclosed-template-block] #}\n\ +

    {{ book.title|uppercase }}

    {# ty: ignore[unknown-template-filter] #}\n", + )])?; + + assert_cmd_snapshot!(case.command(), @" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn a_template_rule_is_configured_like_any_other() -> anyhow::Result<()> { + let case = django_project(&[ + ( + "blog/templates/blog/post.html", + "{% if book %}\n

    {{ book.title|uppercase }}

    \n", + ), + ( + "ty.toml", + r#" + [environment] + extra-paths = ["../libs"] + + [rules] + unclosed-template-block = "warn" + unknown-template-filter = "ignore" + "#, + ), + ])?; + + assert_cmd_snapshot!(case.command().arg("--output-format=concise"), @r" + success: false + exit_code: 1 + ----- stdout ----- + blog/templates/blog/post.html:1:1: warning[unclosed-template-block] unclosed `if` + Found 1 diagnostic + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn a_route_its_view_cannot_serve_is_reported() -> anyhow::Result<()> { + let case = django_project(&[( + "blog/urls.py", + r#" + from django.urls import path + + from blog import views + + app_name = "blog" + + urlpatterns = [ + path("/", views.index, name="broken"), + ] + "#, + )])?; + + assert_cmd_snapshot!(case.command().arg("--output-format=concise"), @r" + success: false + exit_code: 1 + ----- stdout ----- + blog/urls.py:9:28: error[invalid-route-handler] `index` takes no argument named `missing` + Found 1 diagnostic + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn a_suppression_that_silenced_a_route_is_not_reported_unused() -> anyhow::Result<()> { + let case = django_project(&[( + "blog/urls.py", + r#" + from django.urls import path + + from blog import views + + app_name = "blog" + + urlpatterns = [ + path("/", views.index, name="broken"), # ty: ignore[invalid-route-handler] + ] + "#, + )])?; + + assert_cmd_snapshot!(case.command(), @" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn an_installed_apps_own_templates_are_not_the_projects_to_report() -> anyhow::Result<()> { + // django loads these exactly as it loads the project's, and everything that + // reads a template reaches them — but they are a dependency's source + let case = django_project(&[ + ( + "project/settings.py", + r#" + INSTALLED_APPS = ["django.contrib.admin", "blog"] + + TEMPLATES = [{"DIRS": [], "APP_DIRS": True, "OPTIONS": {}}] + + ROOT_URLCONF = "project.urls" + "#, + ), + // the same fault, in the project. it is what says the installed one was + // discovered and left alone rather than never discovered at all + ("blog/templates/blog/post.html", "{% if broken %}\n"), + ])?; + + write_installed( + &case, + &[ + ("django/contrib/__init__.py", ""), + ("django/contrib/admin/__init__.py", ""), + ( + "django/contrib/admin/templates/admin/base.html", + "{% if broken %}\n", + ), + ], + )?; + + assert_cmd_snapshot!(case.command().arg("--output-format=concise"), @r" + success: false + exit_code: 1 + ----- stdout ----- + blog/templates/blog/post.html:1:1: error[unclosed-template-block] unclosed `if` + Found 1 diagnostic + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn a_project_without_django_reports_nothing_about_its_templates() -> anyhow::Result<()> { + // `templates/index.html` in a flask or a jinja project is not a django + // template, and nothing here knows what its language even is + let case = CliTest::with_files([ + ("app.py", "x: int = 1"), + ( + "templates/index.html", + "{% if book %}\n{{ book|uppercase }}\n", + ), + ])?; + + assert_cmd_snapshot!(case.command(), @" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + "); + + Ok(()) +} + +#[test] +fn a_setting_has_the_type_the_settings_module_gives_it() -> anyhow::Result<()> { + // the settings module is found from `manage.py`, which is a fact about the + // project rather than about the file being checked — so this is the test that + // says the type checker reaches it at all, and reaches it the same way the + // language server does + let case = django_project(&[( + "blog/reads_settings.py", + " + from django.conf import settings + + reveal_type(settings.ROOT_URLCONF) + reveal_type(settings.INSTALLED_APPS) + reveal_type(settings.NAMED_NOWHERE) + ", + )])?; + + write_installed( + &case, + &[( + "django/conf/__init__.py", + "from typing import Any\n\ + \n\ + class LazySettings:\n\ + \x20 def __getattr__(self, name: str) -> Any: ...\n\ + \n\ + settings = LazySettings()\n", + )], + )?; + + // no path is named: naming one narrows the project to it, and the settings + // module is then no more part of the project than any other file it excludes + assert_cmd_snapshot!(case.command().arg("--output-format=concise"), @r" + success: false + exit_code: 1 + ----- stdout ----- + blog/reads_settings.py:4:1: warning[undefined-reveal] `reveal_type` used without importing it + blog/reads_settings.py:4:13: info[revealed-type] Revealed type: `str` + blog/reads_settings.py:5:1: warning[undefined-reveal] `reveal_type` used without importing it + blog/reads_settings.py:5:13: info[revealed-type] Revealed type: `Any` + blog/reads_settings.py:6:1: warning[undefined-reveal] `reveal_type` used without importing it + blog/reads_settings.py:6:13: info[revealed-type] Revealed type: `Any` + Found 6 diagnostics + + ----- stderr ----- + "); + + Ok(()) +} diff --git a/crates/ty/tests/cli/main.rs b/crates/ty/tests/cli/main.rs index 4d7e1cd364..1327161160 100644 --- a/crates/ty/tests/cli/main.rs +++ b/crates/ty/tests/cli/main.rs @@ -1,6 +1,7 @@ mod analysis_options; mod api_lockfile; mod config_option; +mod django; mod exit_code; mod file_selection; mod fixes; diff --git a/crates/ty/tests/django_lookup_runtime.rs b/crates/ty/tests/django_lookup_runtime.rs new file mode 100644 index 0000000000..dcdf4b49d6 --- /dev/null +++ b/crates/ty/tests/django_lookup_runtime.rs @@ -0,0 +1,323 @@ +//! runtime half of the django lookup-expression contract. +//! +//! `filter(author.name == "x")` lowers to `filter(author__name="x")`, and what +//! matters is not the text but what django is handed: the right keyword names, +//! the right values, once each. this transpiles a project against a minimal +//! django whose manager records the keywords it receives, executes the output, +//! and asserts on the record. +//! +//! the recorded keywords are the same ones a real django builds its SQL from — +//! `docs/basedpython/frameworks/django.md` covers the query each spells. + +use std::fs; +use std::path::Path; +use std::process::Command; + +/// a django small enough to read and complete enough for both halves: ty +/// recognizes the model, the fields and the manager from it (the `KnownClass` +/// names have to be *defined* in django's own module paths, on a third-party +/// search path, hence the site-packages layout), and it runs, recording the +/// keywords a lookup call is handed +const MOCK_DJANGO: &[(&str, &str)] = &[ + ("django/__init__.py", ""), + // `from django.db import models` reads `models` as an attribute of the + // package, which only exists once the submodule is imported + ( + "django/db/__init__.py", + "from django.db import models as models\n", + ), + ( + "django/db/models/__init__.py", + "from django.db.models.base import Model as Model\n\ + from django.db.models.fields import CharField as CharField, DateField as DateField\n\ + from django.db.models.fields.json import JSONField as JSONField\n\ + from django.db.models.fields.related import CASCADE as CASCADE, ForeignKey as ForeignKey\n\ + from django.db.models.manager import Manager as Manager\n\ + from django.db.models.query import Q as Q, QuerySet as QuerySet\n", + ), + ( + "django/db/models/base.py", + "from typing import Any, ClassVar, Self\n\ + \n\ + from django.db.models.manager import Manager\n\ + \n\ + \n\ + class Model:\n\ + \x20 objects: ClassVar[Manager[Self]]\n\ + \x20 pk: Any\n\ + \n\ + \x20 def __init_subclass__(cls, **kwargs: Any) -> None:\n\ + \x20 super().__init_subclass__(**kwargs)\n\ + \x20 cls.objects = Manager()\n", + ), + ( + "django/db/models/manager.py", + "from typing import Any\n\ + \n\ + from django.db.models.query import QuerySet\n\ + \n\ + \n\ + class BaseManager[_M]:\n\ + \x20 def filter(self, *args: Any, **kwargs: Any) -> QuerySet[_M, _M]:\n\ + \x20 return QuerySet(kwargs)\n\ + \n\ + \x20 def exclude(self, *args: Any, **kwargs: Any) -> QuerySet[_M, _M]:\n\ + \x20 return QuerySet(kwargs)\n\ + \n\ + \n\ + class Manager[_M](BaseManager[_M]):\n\ + \x20 pass\n", + ), + ( + "django/db/models/query.py", + "from typing import Any\n\ + \n\ + \n\ + class Q:\n\ + \x20 def __init__(self, **kwargs: Any) -> None: ...\n\ + \n\ + \n\ + class QuerySet[_M, _Row = _M]:\n\ + \x20 def __init__(self, recorded: dict[str, Any] | None = None) -> None:\n\ + \x20 self.recorded = recorded or {}\n", + ), + ( + "django/db/models/fields/__init__.py", + "from typing import Any\n\ + \n\ + \n\ + class Field[_ST, _GT]:\n\ + \x20 _pyi_private_set_type: Any\n\ + \x20 _pyi_private_get_type: Any\n\ + \n\ + \x20 def __init__(self, **kwargs: Any) -> None: ...\n\ + \x20 def __get__(self, instance: Any, owner: Any = None) -> _GT: ...\n\ + \x20 def __set__(self, instance: Any, value: _ST) -> None: ...\n\ + \n\ + \n\ + class CharField[_ST, _GT](Field[_ST, _GT]):\n\ + \x20 _pyi_private_set_type: str\n\ + \x20 _pyi_private_get_type: str\n\ + \n\ + \n\ + class DateField[_ST, _GT](Field[_ST, _GT]):\n\ + \x20 _pyi_private_set_type: Any\n\ + \x20 _pyi_private_get_type: Any\n", + ), + // the key/index transforms live on `JSONField`, which is recognized by the + // module it is declared in, so the mock has to declare it in that module too + ( + "django/db/models/fields/json.py", + "from typing import Any\n\ + \n\ + from django.db.models.fields import Field\n\ + \n\ + \n\ + class JSONField[_ST, _GT](Field[_ST, _GT]):\n\ + \x20 _pyi_private_set_type: Any\n\ + \x20 _pyi_private_get_type: Any\n", + ), + ( + "django/db/models/fields/related.py", + "from typing import Any\n\ + \n\ + from django.db.models.fields import Field\n\ + \n\ + CASCADE = \"cascade\"\n\ + \n\ + \n\ + class ForeignKey[_ST, _GT](Field[_ST, _GT]):\n\ + \x20 _pyi_private_set_type: Any\n\ + \x20 _pyi_private_get_type: Any\n\ + \n\ + \x20 def __init__(self, to: Any, on_delete: Any = None, **kwargs: Any) -> None: ...\n", + ), +]; + +const MODELS: &str = "\ +from django.db import models + + +class Author(models.Model): + name = models.CharField(max_length=100) + + +class Book(models.Model): + title = models.CharField(max_length=100) + author = models.ForeignKey(Author, on_delete=models.CASCADE) + + +class Doc(models.Model): + name = models.CharField(max_length=100) + data = models.JSONField() +"; + +const MAIN: &str = r#" +from django.db.models import Q + +from models import Book, Doc + + +def label(name, queryset): + print(name, sorted(queryset.recorded.items())) + + +calls = 0 + + +def once(value): + global calls + calls += 1 + return value + + +label("exact", Book.objects.filter(title == "Left Hand")) +label("relation", Book.objects.filter(author.name == "Ursula")) +label("pk", Book.objects.filter(pk == 1)) +label("membership", Book.objects.filter(title in ["a", "b"])) +label("combined", Book.objects.filter(author.name == "Ursula", title == "Left Hand")) +label("excluded", Book.objects.exclude(title == "x")) +label("after positional", Book.objects.filter(Q(), title == "x")) +label("beside keyword", Book.objects.filter(title == "a", author__name="b")) +label("evaluated once", Book.objects.filter(title == once("v"))) +label("json key", Doc.objects.filter(data["key"] == 1)) +label("json nested", Doc.objects.filter(data["a"]["b"] == 1)) +label("json index", Doc.objects.filter(data[0] == 1)) +label("json key operator", Doc.objects.filter(data["key"] > 1)) +label("json key and field", Doc.objects.filter(name == "n", data["key"] == 1)) +print("calls", calls) +"#; + +/// a CPython 3.13, provisioned through uv the way the divergence harness does: +/// the mock django uses pep 695 generics and pep 696 defaults, whose runtime +/// floor is 3.13 +#[cfg(not(windows))] +fn python() -> Option { + if let Ok(path) = std::env::var("PYTHON") { + return Some(path); + } + let find = || { + let output = Command::new("uv") + .args(["python", "find", "3.13"]) + .output() + .ok()?; + output + .status + .success() + .then(|| String::from_utf8_lossy(&output.stdout).trim().to_owned()) + }; + if let Some(path) = find() { + return Some(path); + } + Command::new("uv") + .args(["python", "install", "3.13"]) + .output() + .ok()?; + find() +} + +#[cfg(windows)] +fn python() -> Option { + std::env::var("PYTHON").ok() +} + +fn write(root: &Path, relative: &str, contents: &str) { + let path = root.join(relative); + fs::create_dir_all(path.parent().expect("a relative path has a parent")).expect("create dir"); + fs::write(path, contents).expect("write file"); +} + +fn transpile(project: &Path, module: &str) -> String { + let output = Command::new(env!("CARGO_BIN_EXE_by")) + .args(["transpile", module]) + .current_dir(project) + .output() + .expect("failed to spawn by"); + assert!( + output.status.success(), + "by transpile {module} failed:\n{}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout).expect("transpiled output is utf-8") +} + +#[test] +#[expect( + clippy::print_stderr, + reason = "a skipped test must say why it skipped, or it reads as a pass" +)] +fn lookup_expressions_hand_django_the_keywords_they_spell() { + let Some(python) = python() else { + eprintln!( + "skipping django lookup runtime test: no python 3.13 found \ + (set PYTHON to one, or make `uv` available)" + ); + return; + }; + + let project = tempfile::tempdir().expect("tempdir"); + let root = project.path(); + // ty only reads django's `KnownClass` names off a third-party search path, + // so the mock lives in a venv's site-packages rather than beside the source + let site_packages = "\ +.venv/lib/python3.13/site-packages"; + for (relative, contents) in MOCK_DJANGO { + write(root, &format!("{site_packages}/{relative}"), contents); + } + write( + root, + ".venv/pyvenv.cfg", + "home = /usr/bin\nversion = 3.13.0\n", + ); + write( + root, + "pyproject.toml", + "[project]\nname = \"probe\"\nversion = \"0\"\nrequires-python = \">=3.13\"\n\n\ + [tool.ty.environment]\npython = \".venv\"\npython-version = \"3.13\"\n", + ); + write(root, "models.by", MODELS); + write(root, "main.by", MAIN); + + let lowered = project.path().join("lowered"); + fs::create_dir_all(&lowered).expect("create dir"); + for module in ["models", "main"] { + fs::write( + lowered.join(format!("{module}.py")), + transpile(root, &format!("{module}.by")), + ) + .expect("write lowered module"); + } + + let output = Command::new(&python) + .arg("main.py") + .current_dir(&lowered) + .env("PYTHONPATH", root.join(site_packages).as_os_str()) + .output() + .expect("failed to spawn python"); + assert!( + output.status.success(), + "lowered program failed:\n--- stdout ---\n{}\n--- stderr ---\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + + assert_eq!( + String::from_utf8_lossy(&output.stdout).trim(), + "\ +exact [('title', 'Left Hand')] +relation [('author__name', 'Ursula')] +pk [('pk', 1)] +membership [('title__in', ['a', 'b'])] +combined [('author__name', 'Ursula'), ('title', 'Left Hand')] +excluded [('title', 'x')] +after positional [('title', 'x')] +beside keyword [('author__name', 'b'), ('title', 'a')] +evaluated once [('title', 'v')] +json key [('data__key', 1)] +json nested [('data__a__b', 1)] +json index [('data__0', 1)] +json key operator [('data__key__gt', 1)] +json key and field [('data__key', 1), ('name', 'n')] +calls 1" + ); +} diff --git a/crates/ty_ide/Cargo.toml b/crates/ty_ide/Cargo.toml index 17d16d76a4..f44d4e2ae4 100644 --- a/crates/ty_ide/Cargo.toml +++ b/crates/ty_ide/Cargo.toml @@ -49,6 +49,7 @@ tracing = { workspace = true } [dev-dependencies] ruff_python_parser = { workspace = true } +ruff_ranged_value = { workspace = true } ty_project = { workspace = true, features = ["testing"] } camino = { workspace = true } diff --git a/crates/ty_ide/src/code_action.rs b/crates/ty_ide/src/code_action.rs index 908994a012..743f2195b3 100644 --- a/crates/ty_ide/src/code_action.rs +++ b/crates/ty_ide/src/code_action.rs @@ -1,5 +1,7 @@ use crate::completion; +use crate::django_template::django_template_code_actions; +use ruff_db::system::SystemPathBuf; use ruff_db::{files::File, parsed::parsed_module}; use ruff_diagnostics::Edit; use ruff_python_ast::find_node::covering_node; @@ -15,19 +17,43 @@ pub struct QuickFix { pub title: String, pub edits: Vec, pub preferred: bool, + /// A file the action creates, for a fix to a reference to something that + /// isn't there yet. + pub create: Option, } +impl QuickFix { + fn new(title: String, edits: Vec, preferred: bool) -> Self { + Self { + title, + edits, + preferred, + create: None, + } + } +} + +/// The quick fixes offered for a diagnostic of `file`. +/// +/// `template` says the file is a django template rather than python, which the +/// caller knows and this cannot: nothing about a template may be answered by +/// parsing it as python. pub fn code_actions( db: &dyn Db, file: File, diagnostic_range: TextRange, diagnostic_id: &str, + template: bool, ) -> Vec { let registry = db.lint_registry(); let Ok(lint_id) = registry.get(diagnostic_id) else { return Vec::new(); }; + if template { + return django_template_code_actions(db, file, diagnostic_range, lint_id); + } + let mut actions = Vec::new(); // Suggest imports/qualifications for unresolved references (often ideal) @@ -40,11 +66,11 @@ pub fn code_actions( } // Suggest just suppressing the lint (always a valid option, but never ideal) - actions.push(QuickFix { - title: format!("Ignore '{}' for this line", lint_id.name()), - edits: suppress_single(db, file, lint_id, diagnostic_range).into_edits(), - preferred: false, - }); + actions.push(QuickFix::new( + format!("Ignore '{}' for this line", lint_id.name()), + suppress_single(db, file, lint_id, diagnostic_range).into_edits(), + false, + )); actions } @@ -61,11 +87,7 @@ fn unresolved_fixes( Some( completion::unresolved_fixes(db, file, &parsed, symbol, node) .into_iter() - .map(|import| QuickFix { - title: import.label, - edits: vec![import.edit], - preferred: true, - }), + .map(|import| QuickFix::new(import.label, vec![import.edit], true)), ) } @@ -969,7 +991,15 @@ mod tests { .context(0) .format(DiagnosticFormat::Full); - for mut action in code_actions(&self.db, self.file, self.diagnostic_range, &lint.name) { + // every fixture this harness writes is python; the template actions + // have their own tests + for mut action in code_actions( + &self.db, + self.file, + self.diagnostic_range, + &lint.name, + false, + ) { let mut diagnostic = Diagnostic::new( DiagnosticId::Lint(LintName::of("code-action")), ruff_db::diagnostic::Severity::Info, diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 145446f44b..d0b42f00d0 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -27,6 +27,7 @@ use ty_python_semantic::{ types::{CycleDetector, KnownClass, Type}, }; +use crate::django_template::django_string_completions; use crate::docstring::Docstring; use crate::goto::Definitions; use crate::importer::{ImportRequest, Importer}; @@ -62,6 +63,7 @@ pub fn completion<'db>( let mut completions = Completions::new(db, CollectionContext::none(), UserQuery::fuzzy(None)); + add_django_name_completions(db, &context.cursor, &mut completions); add_string_literal_completions( &model, string_expr, @@ -311,6 +313,12 @@ pub struct Completion<'db> { pub insert: Option, /// The format of [`Self::insert`]. pub insert_text_format: CompletionInsertTextFormat, + /// The range that [`Self::insert`] replaces. + /// + /// This is only set when the client's own idea of the word under the + /// cursor would be wrong, as it is for a django template path or a + /// namespaced url name. + pub replace: Option, /// The type of this completion, if available. /// /// Generally speaking, this is always available @@ -391,6 +399,7 @@ struct CompletionBuilder<'db> { name: CompactString, qualified: Option, insert: Option, + replace: Option, ty: Option>, kind: Option, module_name: Option<&'db ModuleName>, @@ -415,6 +424,7 @@ impl<'db> CompletionBuilder<'db> { name: name.into(), qualified: None, insert: None, + replace: None, ty: None, kind: None, module_name: None, @@ -533,6 +543,7 @@ impl<'db> CompletionBuilder<'db> { qualified: self.qualified, insert, insert_text_format, + replace: self.replace, ty: self.ty, kind, module_name: self.module_name, @@ -557,6 +568,11 @@ impl<'db> CompletionBuilder<'db> { self } + fn replace(mut self, range: TextRange) -> CompletionBuilder<'db> { + self.replace = Some(range); + self + } + fn ty(mut self, ty: impl Into>>) -> CompletionBuilder<'db> { self.ty = ty.into(); self @@ -2276,6 +2292,26 @@ fn position_of(component: FormatSpecComponent) -> usize { .unwrap_or(0) } +/// Adds the django names the string literal under the cursor could be spelling. +/// +/// A template name and a url name are ordinary strings to python, so only the +/// literal's position tells them apart from any other. Nothing is looked up +/// unless that position is one django reads a name from. +fn add_django_name_completions<'db>( + db: &'db dyn Db, + cursor: &ContextCursor<'_>, + completions: &mut Completions<'db>, +) { + for candidate in django_string_completions(db, cursor.covering_node.ancestors()) { + completions.add_skip_query( + Completion::builder(candidate.name) + .kind(candidate.kind) + .replace(candidate.range) + .context_specific(true), + ); + } +} + fn add_string_literal_completions<'db>( model: &SemanticModel<'db>, string_expr: &ast::ExprStringLiteral, @@ -8381,6 +8417,272 @@ x = y = func(lambda: "") ); } + /// A django project whose `blog/views.py` carries the cursor. + fn django_test_builder(views: &str) -> CompletionTestBuilder { + CursorTest::builder() + .source("blog/templates/blog/post.html", "

    ") + .source("blog/templates/blog/list.html", "
      ") + .source( + "blog/urls.py", + r#" +app_name = "blog" + +class BookViewSet: ... + +router = DefaultRouter() +router.register("books", BookViewSet, basename="book") + +urlpatterns = [ + path("/", detail, name="detail"), + path("", index, name="index"), + path("//", paged, name="paged"), +] +"#, + ) + .source("blog/views.py", views) + .completion_test_builder() + } + + #[test] + fn django_render_offers_the_projects_templates() { + let builder = django_test_builder( + r#" +def show(request): + return render(request, "") +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @" + blog/list.html + blog/post.html + "); + } + + #[test] + fn django_render_offers_templates_for_the_keyword_form_too() { + let builder = django_test_builder( + r#" +def show(request): + return shortcuts.render(request, template_name="") +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @" + blog/list.html + blog/post.html + "); + } + + #[test] + fn django_template_response_names_a_template_just_as_render_does() { + let builder = django_test_builder( + r#" +def show(request): + return TemplateResponse(request, "") +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @" + blog/list.html + blog/post.html + "); + } + + #[test] + fn django_a_view_classs_template_name_names_a_template() { + let builder = django_test_builder( + r#" +class PostDetail(DetailView): + template_name = "" +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @" + blog/list.html + blog/post.html + "); + } + + #[test] + fn django_a_template_name_outside_a_class_names_nothing() { + let builder = django_test_builder( + r#" +template_name = "" +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @""); + } + + #[test] + fn django_reverse_offers_the_projects_url_names() { + let builder = django_test_builder( + r#" +def show(request): + return reverse("") +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @" + blog:api-root + blog:book-detail + blog:book-list + blog:detail + blog:index + blog:paged + "); + } + + #[test] + fn django_reverse_lazy_offers_url_names_too() { + let builder = django_test_builder( + r#" +url = urls.reverse_lazy("") +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @" + blog:api-root + blog:book-detail + blog:book-list + blog:detail + blog:index + blog:paged + "); + } + + #[test] + fn django_redirect_offers_url_names() { + let builder = django_test_builder( + r#" +def show(request): + return redirect("") +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @" + blog:api-root + blog:book-detail + blog:book-list + blog:detail + blog:index + blog:paged + "); + } + + #[test] + fn django_redirect_offers_nothing_for_a_url_path() { + // a redirect takes a path as readily as a route name, and a path is the + // one thing a route name never is + let builder = django_test_builder( + r#" +def show(request): + return redirect("/blog/") +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @""); + } + + #[test] + fn django_a_string_in_no_recognised_position_offers_nothing() { + let builder = django_test_builder( + r#" +def show(request): + return render(request, "blog/post.html", {"title": ""}) +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @""); + } + + #[test] + fn django_the_wrong_argument_of_a_render_offers_nothing() { + let builder = django_test_builder( + r#" +def show(request): + return render("", "blog/post.html") +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @""); + } + + #[test] + fn django_reverse_offers_the_arguments_the_route_takes() { + let builder = django_test_builder( + r#" +def show(request): + return reverse("blog:paged", kwargs={""}) +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @" + page + slug + "); + } + + #[test] + fn django_reverse_does_not_offer_an_argument_already_given() { + let builder = django_test_builder( + r#" +def show(request): + return reverse("blog:paged", kwargs={"slug": "a", "": 1}) +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @"page"); + } + + #[test] + fn django_reverse_offers_no_argument_for_a_route_that_takes_none() { + let builder = django_test_builder( + r#" +def show(request): + return reverse("blog:index", kwargs={""}) +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @""); + } + + #[test] + fn django_reverse_offers_no_argument_for_a_route_that_is_not_there() { + let builder = django_test_builder( + r#" +def show(request): + return reverse("blog:missing", kwargs={""}) +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @""); + } + + #[test] + fn django_a_value_in_a_reverses_kwargs_names_no_argument() { + let builder = django_test_builder( + r#" +def show(request): + return reverse("blog:paged", kwargs={"slug": ""}) +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @""); + } + + #[test] + fn django_a_dict_that_is_not_a_reverses_kwargs_names_no_argument() { + let builder = django_test_builder( + r#" +def show(request): + return reverse("blog:paged", query={""}) +"#, + ); + + assert_snapshot!(builder.build().snapshot(), @""); + } + #[test] fn typevar_with_upper_bound() { let builder = completion_test_builder( diff --git a/crates/ty_ide/src/django_template.rs b/crates/ty_ide/src/django_template.rs new file mode 100644 index 0000000000..5b84477e98 --- /dev/null +++ b/crates/ty_ide/src/django_template.rs @@ -0,0 +1,1141 @@ +//! ide support for django template files +//! +//! django templates are not python, so none of the machinery the rest of this +//! crate is built on — the parser, the semantic index, type inference — applies +//! to them. what *is* shared is the project: a template's variables come from +//! the view that renders it, its `{% url %}` names come from the project's url +//! configuration, and its custom tags and filters come from the project's +//! `templatetags` modules. so this module owns a small template front end of its +//! own ([`lexer`], [`index`]) and spends the rest of its effort joining it to the +//! python side ([`project`]). +//! +//! the join runs both ways. [`python`] is the other direction: the template name +//! a view renders and the route name a view reverses are plain strings to python, +//! and it is what lets the python services see them for what they are. + +mod builtins; +mod code_lens; +mod completion; +mod diagnostics; +mod folding; +mod goto; +mod hover; +mod index; +mod inlay_hints; +mod lexer; +mod project; +mod python; +mod references; +mod rename; +mod resolve; +mod routes; +mod semantic_tokens; +mod signature_help; +mod symbols; +mod uses; + +pub use code_lens::{DjangoCodeLens, DjangoLensAction, DjangoLensTarget}; +pub use completion::{TemplateCompletion, TemplateEdit}; +pub use hover::{DisplayTemplateHover, TemplateHover}; +pub use inlay_hints::{TemplateInlayHint, TemplateInlayHintKind}; +pub(crate) use python::{ + string_completions as django_string_completions, string_definition as django_string_definition, +}; +pub use rename::{PreparedTemplateRename, TemplateRename, TemplateRenameOutcome}; +pub use signature_help::TemplateSignature; +pub use symbols::{DjangoSymbol, TemplateSymbol}; + +use ruff_db::diagnostic::Diagnostic; +use ruff_db::files::{File, system_path_to_file}; +use ruff_db::source::source_text; +use ruff_db::system::SystemPath; +use ruff_text_size::{TextRange, TextSize}; +use ty_project::Db; +use ty_project::glob::IncludeResult; + +use ty_python_semantic::lint::LintId; + +use crate::code_action::QuickFix; +use crate::semantic_tokens::SemanticTokens; +use crate::{FoldingRange, InlayHintSettings, NavigationTargets, RangedValue, ReferenceTarget}; + +use index::TemplateIndex; + +/// how many `{% extends %}` hops a parent chain is followed +const MAX_INHERITANCE_DEPTH: usize = 16; + +/// the file extensions a django template is conventionally written with +/// +/// django itself puts no constraint on the extension — the template loader takes +/// whatever path it is given — so this list only decides what the server will +/// *offer* template support for when the editor hasn't already told it the file +/// is a template. +/// +/// `.jinja` is deliberately not here. jinja is a different language that reads +/// alike, and everything in this module answers as django: a `{% set %}`, a +/// `{% macro %}` or a `|default("x")` is correct jinja that django's tag and +/// filter tables know nothing about, so claiming a jinja file means reporting +/// correct code as wrong. we do not support jinja, so we do not claim its files. +const TEMPLATE_EXTENSIONS: &[&str] = &["html", "htm", "txt", "xml", "django", "dj"]; + +/// the directory name django's app-directories loader looks in +const TEMPLATE_DIRECTORY: &str = "templates"; + +/// whether `path` looks like a django template +/// +/// an editor that knows the file's language (vs code's `django-html`, vim's +/// `htmldjango`) tells the server directly and this is never consulted. it is +/// the fallback for the much more common case of a `.html` file that the editor +/// reports as plain html, and it deliberately requires the file to be *inside* a +/// `templates` directory so that ordinary html in a project is left alone. +pub fn is_django_template_path(path: &SystemPath) -> bool { + let has_template_extension = path + .extension() + .is_some_and(|extension| TEMPLATE_EXTENSIONS.contains(&extension)); + + has_template_extension + && path + .ancestors() + .any(|ancestor| ancestor.file_name() == Some(TEMPLATE_DIRECTORY)) +} + +/// the index of `file`, parsed as a django template +/// +/// the query is tracked so that the several ide features that need it — the +/// completions, the semantic tokens, goto — parse each template once per edit +/// rather than once per request. +#[salsa::tracked(returns(ref), heap_size=ruff_memory_usage::heap_size)] +fn template_index(db: &dyn Db, file: File) -> TemplateIndex { + TemplateIndex::from_source(source_text(db, file).as_str()) +} + +/// the semantic tokens of `file`, read as a django template +/// +/// `range` restricts the result to the tokens it touches, for the ranged +/// semantic-tokens request. +pub fn django_template_semantic_tokens( + db: &dyn Db, + file: File, + range: Option, +) -> SemanticTokens { + let source = source_text(db, file); + SemanticTokens::new(semantic_tokens::semantic_tokens( + db, + template_index(db, file), + source.as_str(), + range, + )) +} + +/// where the name at `offset` of `file` is defined, read as a django template +pub fn django_template_goto_definition( + db: &dyn Db, + file: File, + offset: TextSize, +) -> Option> { + let source = source_text(db, file); + goto::goto_definition(db, file, template_index(db, file), source.as_str(), offset) +} + +/// what the thing at `offset` of `file` is, read as a django template +pub fn django_template_hover( + db: &dyn Db, + file: File, + offset: TextSize, +) -> Option> { + let source = source_text(db, file); + hover::hover(db, file, template_index(db, file), source.as_str(), offset) +} + +/// the outline of `file`, read as a django template +pub fn django_template_document_symbols(db: &dyn Db, file: File) -> Vec { + symbols::document_symbols(template_index(db, file)) +} + +/// every django thing of the project whose name matches `query` +pub(crate) fn django_workspace_symbols( + db: &dyn Db, + query: &crate::symbols::QueryPattern, +) -> Vec { + symbols::workspace_symbols(db, query) +} + +/// the foldable ranges of `file`, read as a django template +pub fn django_template_folding_ranges(db: &dyn Db, file: File) -> Vec { + let source = source_text(db, file); + folding::folding_ranges(template_index(db, file), source.as_str()) +} + +/// the templates `index` extends, nearest ancestor first +/// +/// a template whose parent chain has a cycle in it would not render, but the +/// editor must not hang on one, so the walk stops at the first template it has +/// already been through — and at [`MAX_INHERITANCE_DEPTH`] regardless. +fn ancestors<'db>( + db: &'db dyn Db, + file: File, + index: &TemplateIndex, +) -> Vec<(File, &'db TemplateIndex)> { + let mut collected = Vec::new(); + let mut seen = vec![file]; + let mut parent = index.extends().map(|reference| reference.name.clone()); + + while collected.len() < MAX_INHERITANCE_DEPTH { + let Some(name) = parent.take() else { break }; + let Some(parent_file) = project::resolve_template(db, &name) else { + break; + }; + if seen.contains(&parent_file) { + break; + } + seen.push(parent_file); + + let parent_index = template_index(db, parent_file); + collected.push((parent_file, parent_index)); + parent = parent_index + .extends() + .map(|reference| reference.name.clone()); + } + + collected +} + +/// everything wrong with `file`, read as a django template +/// +/// the type checker never sees a template — it is not python — so this is the +/// whole of what a template document can report. +pub fn django_template_diagnostics(db: &dyn Db, file: File) -> Vec { + if !project::has_django(db, db.project()) { + return Vec::new(); + } + + let source = source_text(db, file); + diagnostics::diagnostics(db, file, template_index(db, file), source.as_str()) +} + +/// everything wrong with the django a python file writes +/// +/// this is what the type checker's own pass over the file cannot say: whether a +/// route's view can take the arguments the route gives it is a question about the +/// project's whole url tree, which is read here rather than there. +/// +/// the file's suppression comments are deliberately *not* applied: these are +/// folded into the type checker's own diagnostics, which is where a `ty: ignore` +/// is honoured and counted used — see [`ty_python_semantic::check_file_with`]. +pub fn django_python_diagnostics(db: &dyn Db, file: File) -> Vec { + routes::diagnostics(db, file) +} + +/// django's checks, as something [`ty_project::Project::check`] can run +/// +/// registering this is what makes the command line and an editor report the same +/// rules in the same places: both reach the two functions above, and both reach +/// them through the same suppression and configuration. +#[derive(Debug, Default, Clone, Copy)] +pub struct DjangoChecker; + +impl ty_project::ProjectChecker for DjangoChecker { + fn owns(&self, db: &dyn Db, path: &SystemPath) -> bool { + is_django_template_path(path) && project::has_django(db, db.project()) + } + + fn files(&self, db: &dyn Db) -> Vec { + let project = db.project(); + if !project::has_django(db, project) { + return Vec::new(); + } + + project::template_files(db, project) + .iter() + // an installed app's templates are django's or a dependency's, and a + // project is no more answerable for those than for the python beside them + .filter(|discovered| discovered.own) + // the same include, exclude and `ty check ` filtering the python + // files of the project go through + .filter(|discovered| { + matches!( + project.is_file_included(db, &discovered.path), + IncludeResult::Included { .. } + ) + }) + .filter_map(|discovered| system_path_to_file(db, &discovered.path).ok()) + .collect() + } + + fn check_file(&self, db: &dyn Db, file: File) -> Vec { + django_template_diagnostics(db, file) + } + + fn check_python_file(&self, db: &dyn Db, file: File) -> Vec { + django_python_diagnostics(db, file) + } + + fn django_settings_file(&self, db: &dyn Db) -> Option { + *project::settings_file(db, db.project()) + } +} + +/// the quick fixes offered for a template diagnostic at `range` +pub(crate) fn django_template_code_actions( + db: &dyn Db, + file: File, + range: TextRange, + lint: LintId, +) -> Vec { + let source = source_text(db, file); + diagnostics::code_actions( + db, + file, + template_index(db, file), + source.as_str(), + range, + lint, + ) +} + +/// whether the django name at `offset` of `file` can be renamed +/// +/// `template` says the file is a django template rather than python, which the +/// caller knows and this cannot. either language may write one of these names, +/// and `None` — a position that names nothing django knows — is what leaves a +/// python file's position to the python services. +pub fn django_prepare_rename( + db: &dyn Db, + file: File, + offset: TextSize, + template: bool, +) -> Option { + rename::prepare(db, file, offset, template) +} + +/// every edit renaming the django name at `offset` of `file` to `new_name` makes +pub fn django_rename( + db: &dyn Db, + file: File, + offset: TextSize, + new_name: &str, + template: bool, +) -> Option { + rename::rename(db, file, offset, new_name, template) +} + +/// every place the django name at `offset` of `file` is written +/// +/// `template` says the file is a django template rather than python, as for the +/// rename above. `include_declaration` is the client's, as LSP specifies: with it +/// off, the block the base declares, the file a template name loads and the +/// `path(…, name=…)` a route is declared by are all left out of the answer. +pub fn django_references( + db: &dyn Db, + file: File, + offset: TextSize, + include_declaration: bool, + template: bool, +) -> Option> { + references::references(db, file, offset, include_declaration, template) +} + +/// what the filter argument at `offset` of `file` takes, read as a django +/// template +pub fn django_template_signature_help( + db: &dyn Db, + file: File, + offset: TextSize, +) -> Option { + let source = source_text(db, file); + signature_help::signature_help(db, template_index(db, file), source.as_str(), offset) +} + +/// the hints `range` of `file` shows, read as a django template +pub fn django_template_inlay_hints( + db: &dyn Db, + file: File, + range: TextRange, + settings: &InlayHintSettings, +) -> Vec { + let source = source_text(db, file); + inlay_hints::inlay_hints( + db, + file, + template_index(db, file), + source.as_str(), + range, + settings, + ) +} + +/// the project's `manage.py`, django's own entry point +/// +/// the lenses above say what to run through it; this is what a caller that has to +/// actually run one needs, and a project without one is a project none of them +/// apply to. +pub fn django_manage_script(db: &dyn Db) -> Option { + *project::manage_file(db, db.project()) +} + +/// the lenses `file` shows, read as a django template +/// +/// this is the view side of the join: a template is told what renders it, which +/// is the one thing about itself it cannot say. +pub fn django_template_code_lenses(db: &dyn Db, file: File) -> Vec { + code_lens::template_code_lenses(db, file) +} + +/// the lenses `file` shows, read as one of the project's python modules +/// +/// these are the `manage.py` invocations that apply to the file, and a module +/// django gives no role to has none of them. +pub fn django_python_code_lenses(db: &dyn Db, file: File) -> Vec { + code_lens::python_code_lenses(db, file) +} + +/// the completions for `offset` in `file`, read as a django template +pub fn django_template_completions( + db: &dyn Db, + file: File, + offset: TextSize, +) -> Vec { + let source = source_text(db, file); + completion::completions(db, file, template_index(db, file), source.as_str(), offset) +} + +#[cfg(test)] +pub(crate) mod tests { + use ruff_db::Db as _; + use ruff_db::files::{File, FileRootKind, system_path_to_file}; + use ruff_db::system::{DbWithTestSystem, DbWithWritableSystem, SystemPath, SystemPathBuf}; + use ruff_python_ast::PythonVersion; + use ruff_python_trivia::textwrap::dedent; + use ruff_ranged_value::RangedValue; + use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; + use std::ops::Range; + use ty_module_resolver::SearchPathSettings; + use ty_project::metadata::options::{Options, Rules}; + use ty_project::{ProjectMetadata, TestDb}; + use ty_python_core::platform::PythonPlatform; + use ty_python_core::program::{FallibleStrategy, Program, ProgramSettings}; + use ty_python_semantic::PythonVersionWithSource; + use ty_python_semantic::lint::Level; + + use crate::MarkupKind; + + use crate::InlayHintSettings; + + use super::{ + DjangoLensAction, PreparedTemplateRename, TemplateRenameOutcome, TemplateSymbol, + django_prepare_rename, django_python_code_lenses, django_python_diagnostics, + django_references, django_rename, django_template_code_lenses, django_template_completions, + django_template_diagnostics, django_template_document_symbols, + django_template_folding_ranges, django_template_goto_definition, django_template_hover, + django_template_inlay_hints, django_template_signature_help, is_django_template_path, + }; + + /// a mock django whose implicit builtins are there to be read + /// + /// what it registers differs from the builtin table deliberately, and in both + /// directions: it has a `{% squish %}` and a `|shorten` the table has never + /// heard of, and it has no `{% lorem %}` or `|slugify` though the table does. + /// that is what makes it stand in for a django the table has drifted from. + pub(crate) const DJANGO_BUILTINS: &[(&str, &str)] = &[ + ("django/__init__.py", ""), + ("django/template/__init__.py", ""), + ( + "django/template/defaulttags.py", + " + from django.template import Library + + register = Library() + + @register.tag('for') + def do_for(parser, token): ... + + @register.tag('if') + def do_if(parser, token): ... + + @register.tag + def squish(parser, token): + '''squishes its body.''' + ", + ), + ( + "django/template/defaultfilters.py", + " + from django.template import Library + + register = Library() + + @register.filter(is_safe=True) + def upper(value): ... + + @register.filter + def shorten(value, arg): ... + ", + ), + ( + "django/template/loader_tags.py", + " + from django.template import Library + + register = Library() + + @register.tag('block') + def do_block(parser, token): ... + + @register.tag('extends') + def do_extends(parser, token): ... + + @register.tag('include') + def do_include(parser, token): ... + ", + ), + ]; + + /// a mock django whose model base and admin machinery are there to be + /// resolved + /// + /// what matters about it is where the classes are *declared*: a `Model` or a + /// `ModelAdmin` counts as django's own because the module it comes from is + /// django's, never because of the name written at the point of use. + pub(crate) const DJANGO_ADMIN: &[(&str, &str)] = &[ + ("django/__init__.py", ""), + ("django/db/__init__.py", ""), + ("django/db/models/__init__.py", "class Model: ...\n"), + ("django/contrib/__init__.py", ""), + ( + "django/contrib/admin/__init__.py", + " + class AdminSite: + def register(self, model_or_iterable, admin_class=None, **options): ... + + class ModelAdmin: ... + + class InlineModelAdmin: ... + + class TabularInline(InlineModelAdmin): ... + + site = AdminSite() + + def register(*models, site=None): ... + ", + ), + ]; + + /// a project whose files are written out, with the cursor marked by + /// `` in at most one of them + /// + /// a test that has no cursor to place — a diagnostic's, which reads a whole + /// file rather than a position in one — leaves the marker out, and the file + /// under test is then the last one written. + pub(crate) struct TemplateTest { + pub(crate) db: TestDb, + pub(crate) file: File, + pub(crate) offset: TextSize, + /// the project root the sources were written under, for a test that + /// names a second file of its own + root: SystemPathBuf, + } + + impl TemplateTest { + /// build a project from `(path, contents)` pairs + pub(crate) fn new(sources: &[(&str, &str)]) -> Self { + let mut db = TestDb::new(ProjectMetadata::new("test", SystemPathBuf::from("/"))); + db.init_program_with_python_version(PythonVersion::latest_ty()) + .unwrap(); + + Self::write(db, SystemPath::new("/"), sources) + } + + /// the same, with `installed` written to a site-packages directory + /// + /// site-packages sits *outside* the project root, which is what makes + /// what is written there third-party rather than the project's own — + /// under the root it would be found by the first-party scans instead. + pub(crate) fn with_site_packages( + sources: &[(&str, &str)], + installed: &[(&str, &str)], + ) -> Self { + Self::with_rules(sources, installed, &[]) + } + + /// the same again, with some rules set to a level of their own + /// + /// the rule selection is part of the project's metadata, so a test that + /// needs a rule turned off has to say so before the project is built. + pub(crate) fn with_rules( + sources: &[(&str, &str)], + installed: &[(&str, &str)], + rules: &[(&str, Level)], + ) -> Self { + let root = SystemPathBuf::from("/src"); + let site_packages = SystemPathBuf::from("/site-packages"); + + let mut metadata = ProjectMetadata::new("test", root.clone()); + if !rules.is_empty() { + metadata.apply_override_options(Options { + rules: Some( + rules + .iter() + .map(|(rule, level)| { + ( + RangedValue::cli((*rule).to_string()), + RangedValue::cli(*level), + ) + }) + .collect::(), + ), + ..Options::default() + }); + } + + let mut db = TestDb::new(metadata); + + for (path, contents) in installed { + db.write_file(site_packages.join(path), dedent(contents).as_ref()) + .unwrap(); + } + db.memory_file_system().create_directory_all(&root).unwrap(); + db.memory_file_system() + .create_directory_all(&site_packages) + .unwrap(); + + let search_paths = SearchPathSettings { + src_roots: vec![root.clone()], + site_packages_paths: vec![site_packages.clone()], + ..SearchPathSettings::empty() + } + .to_search_paths(db.system(), db.vendored(), &FallibleStrategy) + .expect("valid search paths"); + + Program::from_settings( + &db, + ProgramSettings { + python_version: PythonVersionWithSource::default(), + python_platform: PythonPlatform::default(), + search_paths, + }, + ); + + db.files().try_add_root(&db, &root, FileRootKind::Project); + db.files() + .try_add_root(&db, &site_packages, FileRootKind::SearchPath); + + Self::write(db, &root, sources) + } + + /// write every source under `root`, taking the one `` marker out + fn write(mut db: TestDb, root: &SystemPath, sources: &[(&str, &str)]) -> Self { + const MARKER: &str = ""; + + let mut cursor = None; + let mut last = None; + + for (path, contents) in sources { + let contents = dedent(contents).into_owned(); + + let (contents, offset) = match contents.find(MARKER) { + Some(index) => { + let mut without = contents[..index].to_string(); + without.push_str(&contents[index + MARKER.len()..]); + (without, Some(TextSize::try_from(index).unwrap())) + } + None => (contents, None), + }; + + let path = root.join(path); + db.write_file(&path, &contents).unwrap(); + let file = system_path_to_file(&db, &path).unwrap(); + last = Some(file); + + if let Some(offset) = offset { + assert!(cursor.is_none(), "more than one `` marker"); + cursor = Some((file, offset)); + } + } + + let (file, offset) = cursor + .unwrap_or_else(|| (last.expect("a source to be written"), TextSize::default())); + Self { + db, + file, + offset, + root: root.to_path_buf(), + } + } + + /// rewrite one of the project's files + /// + /// this is for a test that varies a source the fixture already wrote — + /// the url configuration a route is mounted from, most often. + pub(crate) fn rewrite(&mut self, path: &str, contents: &str) { + self.db + .write_file(self.root.join(path), dedent(contents).as_ref()) + .unwrap(); + } + + /// every django diagnostic of the python file at `path`, rendered as + /// `rule severity: message [text]` + /// + /// what the type checker itself reports about the file is deliberately + /// left out: this is the django join, and mixing the two would make every + /// test below depend on how a mock django happens to be annotated. + pub(crate) fn python_diagnostics(&self, path: &str) -> Vec { + let file = system_path_to_file(&self.db, self.root.join(path)) + .expect("the file to have been written"); + let source = ruff_db::source::source_text(&self.db, file); + + django_python_diagnostics(&self.db, file) + .into_iter() + .map(|diagnostic| { + let range = diagnostic + .primary_span() + .and_then(|span| span.range()) + .unwrap_or_default(); + + format!( + "{} {:?}: {} [{}]", + diagnostic.id(), + diagnostic.severity(), + diagnostic.primary_message(), + &source[range] + ) + }) + .collect() + } + + /// the same, as the project check reports it + /// + /// [`python_diagnostics`](Self::python_diagnostics) is the raw scan, which + /// answers before any `ty: ignore` is read. this is that scan folded into + /// the type checker's own pass, which is where a suppression comment is + /// honoured and counted used — so it is also what says whether + /// `unused-ignore-comment` then fires on the comment that did the silencing. + pub(crate) fn checked_python_diagnostics(&self, path: &str) -> Vec { + const REPORTED: &[&str] = &[ + "invalid-route-handler", + "invalid-route-parameter-type", + "unused-ignore-comment", + ]; + + let file = system_path_to_file(&self.db, self.root.join(path)) + .expect("the file to have been written"); + let external = django_python_diagnostics(&self.db, file); + + ty_python_semantic::check_file_with(&self.db, file, external) + .expect("the file to be readable") + .iter() + // the mock django the fixtures install is annotated no further than + // each test needs, so what the type checker itself says about a view + // is no business of a test about the django join + .filter(|diagnostic| { + diagnostic + .id() + .as_lint() + .is_some_and(|name| REPORTED.contains(&name.as_str())) + }) + .map(|diagnostic| format!("{}: {}", diagnostic.id(), diagnostic.primary_message())) + .collect() + } + + /// every diagnostic of the file under test, rendered as + /// `rule severity: message [text]` + pub(crate) fn diagnostics(&self) -> Vec { + let source = ruff_db::source::source_text(&self.db, self.file); + + django_template_diagnostics(&self.db, self.file) + .into_iter() + .map(|diagnostic| { + let range = diagnostic + .primary_span() + .and_then(|span| span.range()) + .unwrap_or_default(); + + format!( + "{} {:?}: {} [{}]", + diagnostic.id(), + diagnostic.severity(), + diagnostic.primary_message(), + &source[range] + ) + }) + .collect() + } + + /// the workspace symbols matching `query`, as `container name [path]` + /// + /// what python contributes is rendered under `python`, so that a test + /// says both what django added and what it left alone. the order is the + /// order the files were walked in, which is no order at all, so it is + /// sorted here rather than asserted on. + pub(crate) fn workspace_symbols(&self, query: &str) -> Vec { + let mut found: Vec = crate::workspace_symbols(&self.db, query) + .into_iter() + .map(|found| { + format!( + "{} {} [{}]", + found.container.unwrap_or("python"), + found.symbol.name, + // the memory file system reports the host's separator + found.file.path(&self.db).to_string().replace('\\', "/"), + ) + }) + .collect(); + found.sort(); + found + } + + /// the labels of the completions at the cursor, in the order offered + pub(crate) fn completions(&self) -> Vec { + django_template_completions(&self.db, self.file, self.offset) + .into_iter() + .map(|completion| completion.label) + .collect() + } + + /// the labels of the completions at the cursor django will not render + pub(crate) fn unusable(&self) -> Vec { + django_template_completions(&self.db, self.file, self.offset) + .into_iter() + .filter(|completion| completion.unusable) + .map(|completion| completion.label) + .collect() + } + + /// the completions at the cursor, rendered as `label — detail` + pub(crate) fn detailed(&self) -> Vec { + django_template_completions(&self.db, self.file, self.offset) + .into_iter() + .map(|completion| match completion.detail { + Some(detail) => format!("{} — {detail}", completion.label), + None => completion.label, + }) + .collect() + } + + /// where goto-definition at the cursor lands, as `path:text` + pub(crate) fn definitions(&self) -> Vec { + let Some(targets) = django_template_goto_definition(&self.db, self.file, self.offset) + else { + return Vec::new(); + }; + + targets + .into_iter() + .map(|target| { + let source = ruff_db::source::source_text(&self.db, target.file()); + format!( + "{}:{}", + // the memory file system reports the host's separator + target.file().path(&self.db).to_string().replace('\\', "/"), + &source[target.focus_range()] + ) + }) + .collect() + } + + /// the hover at the cursor, as markdown, or `""` where there is none + pub(crate) fn hover(&self) -> String { + django_template_hover(&self.db, self.file, self.offset) + .map(|hover| hover.display(MarkupKind::Markdown).to_string()) + .unwrap_or_default() + } + + /// the signature help at the cursor, as + /// `label [parameter] — documentation` + pub(crate) fn signature(&self) -> String { + let Some(signature) = django_template_signature_help(&self.db, self.file, self.offset) + else { + return "no signature".to_string(); + }; + + let parameter = signature + .parameter + .map(|parameter| format!(" [{parameter}]")) + .unwrap_or_default(); + let documentation = signature + .documentation + .map(|documentation| format!(" — {documentation}")) + .unwrap_or_default(); + + format!("{}{parameter}{documentation}", signature.label) + } + + /// every hint of the whole template, as ``kind at `text`: `label` `` + pub(crate) fn hints(&self) -> Vec { + let source = ruff_db::source::source_text(&self.db, self.file); + self.rendered_hints( + TextRange::up_to(source.text_len()), + &InlayHintSettings::default(), + ) + } + + /// the same, restricted to a byte range of the template + pub(crate) fn hints_in(&self, range: Range) -> Vec { + self.rendered_hints( + TextRange::new(range.start.into(), range.end.into()), + &InlayHintSettings::default(), + ) + } + + /// the same, with only the settings `enable` turns on + pub(crate) fn hints_with( + &self, + enable: impl FnOnce(&mut InlayHintSettings), + ) -> Vec { + let mut settings = InlayHintSettings::none(); + enable(&mut settings); + + let source = ruff_db::source::source_text(&self.db, self.file); + self.rendered_hints(TextRange::up_to(source.text_len()), &settings) + } + + fn rendered_hints(&self, range: TextRange, settings: &InlayHintSettings) -> Vec { + let source = ruff_db::source::source_text(&self.db, self.file); + + django_template_inlay_hints(&self.db, self.file, range, settings) + .into_iter() + .map(|hint| { + let line_start = source.as_str()[..usize::from(hint.position)] + .rfind('\n') + .map_or(0, |index| index + 1); + let anchored = source.as_str()[line_start..usize::from(hint.position)] + .rsplit([' ', '{', '%']) + .next() + .unwrap_or_default(); + + format!("{:?} at `{anchored}`: `{}`", hint.kind, hint.label) + }) + .collect() + } + + /// the template's outline, one line per symbol, nesting indented + pub(crate) fn symbols(&self) -> Vec { + fn render(symbols: &[TemplateSymbol], depth: usize, lines: &mut Vec) { + for symbol in symbols { + lines.push(format!( + "{:indent$}{:?} {}", + "", + symbol.kind, + symbol.name, + indent = depth * 2 + )); + render(&symbol.children, depth + 1, lines); + } + } + + let mut lines = Vec::new(); + render( + &django_template_document_symbols(&self.db, self.file), + 0, + &mut lines, + ); + lines + } + + /// whether the file under test is read as a template rather than python + /// + /// this is the same question the server answers from what the editor told + /// it, and falls back to the same path check when it wasn't told. + fn is_template(&self) -> bool { + match self.file.path(&self.db) { + ruff_db::files::FilePath::System(path) => is_django_template_path(path), + _ => false, + } + } + + /// what the editor is offered for a rename at the cursor + pub(crate) fn prepare_rename(&self) -> String { + match django_prepare_rename(&self.db, self.file, self.offset, self.is_template()) { + None => "no rename".to_string(), + Some(PreparedTemplateRename::Refused(why)) => format!("refused: {why}"), + Some(PreparedTemplateRename::Ready { range, placeholder }) => { + let source = ruff_db::source::source_text(&self.db, self.file); + format!("rename `{placeholder}`, replacing `{}`", &source[range]) + } + } + } + + /// every edit a rename at the cursor would make, as `path:line old -> new` + pub(crate) fn rename(&self, new_name: &str) -> Vec { + let renamed = django_rename( + &self.db, + self.file, + self.offset, + new_name, + self.is_template(), + ); + + let rename = match renamed { + None => return vec!["no rename".to_string()], + Some(TemplateRenameOutcome::Refused(why)) => { + return vec![format!("refused: {why}")]; + } + Some(TemplateRenameOutcome::Edits(rename)) => rename, + }; + + let mut lines: Vec = rename + .edits + .iter() + .map(|edit| { + let source = ruff_db::source::source_text(&self.db, edit.file()); + let line = source.as_str()[..usize::from(edit.range().start())] + .matches('\n') + .count() + + 1; + + format!( + "{}:{line} {} -> {new_name}", + // the memory file system reports the host's separator + edit.file().path(&self.db).to_string().replace('\\', "/"), + &source[edit.range()] + ) + }) + .collect(); + + if let Some((from, to)) = rename.file_rename { + lines.push(format!( + "move {} -> {}", + from.as_str().replace('\\', "/"), + to.as_str().replace('\\', "/") + )); + } + + lines + } + + /// every reference at the cursor, as `path:line text` + /// + /// a declaration is marked, since which occurrences are declarations is + /// what `includeDeclaration` turns on and off. + pub(crate) fn references(&self) -> Vec { + self.references_with_declaration(true) + } + + /// the same, as a client that asked for the uses alone gets them + pub(crate) fn references_without_declaration(&self) -> Vec { + self.references_with_declaration(false) + } + + fn references_with_declaration(&self, include_declaration: bool) -> Vec { + let found = django_references( + &self.db, + self.file, + self.offset, + include_declaration, + self.is_template(), + ); + + found + .unwrap_or_default() + .into_iter() + .map(|target| { + let source = ruff_db::source::source_text(&self.db, target.file()); + let line = source.as_str()[..usize::from(target.range().start())] + .matches('\n') + .count() + + 1; + + format!( + "{}{}:{line} {}", + match target.kind() { + crate::ReferenceKind::Other => "declaration ", + _ => "", + }, + // the memory file system reports the host's separator + target.file().path(&self.db).to_string().replace('\\', "/"), + &source[target.range()], + ) + }) + .collect() + } + + /// every lens of the file under test, as `title -> what it does` + /// + /// which of the two implementations answers is the same question the + /// server asks, so the harness asks it the same way. + pub(crate) fn lenses(&self) -> Vec { + let lenses = if self.is_template() { + django_template_code_lenses(&self.db, self.file) + } else { + django_python_code_lenses(&self.db, self.file) + }; + + lenses + .into_iter() + .map(|lens| { + let action = match lens.action { + DjangoLensAction::Run(arguments) => { + format!("manage.py {}", arguments.join(" ")) + } + DjangoLensAction::Navigate(targets) => targets + .iter() + .map(|target| { + let source = ruff_db::source::source_text(&self.db, target.file); + format!( + "{}:{}", + // the memory file system reports the host's separator + target.file.path(&self.db).to_string().replace('\\', "/"), + &source[target.range], + ) + }) + .collect::>() + .join(", "), + }; + + format!("{} -> {action}", lens.title) + }) + .collect() + } + + /// each foldable range, as the tags that open and close it + pub(crate) fn folds(&self) -> Vec { + let source = ruff_db::source::source_text(&self.db, self.file); + + django_template_folding_ranges(&self.db, self.file) + .into_iter() + .map(|fold| { + let text = &source[fold.range]; + let first = text.lines().next().unwrap_or_default().trim(); + let last = text.lines().next_back().unwrap_or_default().trim(); + format!("{first} … {last}") + }) + .collect() + } + } + + #[test] + fn a_template_is_an_html_file_under_a_templates_directory() { + assert!(is_django_template_path(SystemPath::new( + "/app/templates/blog/post.html" + ))); + assert!(is_django_template_path(SystemPath::new( + "/templates/base.txt" + ))); + } + + #[test] + fn ordinary_html_outside_a_templates_directory_is_not_a_template() { + assert!(!is_django_template_path(SystemPath::new( + "/app/static/index.html" + ))); + assert!(!is_django_template_path(SystemPath::new("/README.html"))); + } + + #[test] + fn a_python_file_is_never_a_template() { + assert!(!is_django_template_path(SystemPath::new( + "/app/templates/views.py" + ))); + } + + #[test] + fn a_jinja_file_is_not_claimed_as_a_django_template() { + // see `TEMPLATE_EXTENSIONS`: answering a jinja file as django reports its + // correct code as wrong + assert!(!is_django_template_path(SystemPath::new( + "/app/templates/page.jinja" + ))); + assert!(!is_django_template_path(SystemPath::new( + "/app/templates/page.html.jinja" + ))); + } +} diff --git a/crates/ty_ide/src/django_template/builtins.rs b/crates/ty_ide/src/django_template/builtins.rs new file mode 100644 index 0000000000..7f9fabb506 --- /dev/null +++ b/crates/ty_ide/src/django_template/builtins.rs @@ -0,0 +1,868 @@ +//! django's builtin template tags and filters +//! +//! these are the ones `django.template.defaulttags`, `django.template.defaultfilters` +//! and the libraries shipped in `django.templatetags` register. a project's own +//! tags and filters are discovered from its source instead — see [`super::project`]. +//! +//! the tables are a *fallback*, not an authority. django's own tags and filters +//! are discovered from the installed django like anything else, and where that +//! discovery succeeds it decides which names exist and which library each comes +//! from — see [`provided_by_django`]. a table written by hand drifts as django +//! moves, and has: `{% partialdef %}` was third-party until django 6.0 made it a +//! builtin, and the table said the wrong thing for a while. +//! +//! what the tables carry that discovery cannot is the block structure (which tag +//! closes which, and which tags may appear in between) and the documentation. +//! both are needed: a `{% for %}` without the knowledge that `{% empty %}` +//! belongs inside it would either close the block early or never offer +//! `{% empty %}` at all. + +use ty_project::Db; + +use super::project::{self, RegistrationKind}; + +/// a builtin template tag +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct Tag { + pub(crate) name: &'static str, + /// the tag closing the block this one opens, for a block tag + pub(crate) closed_by: Option<&'static str>, + /// the tags that may appear between this tag and the one that closes it + pub(crate) branches: &'static [&'static str], + /// the `{% load %}` library providing this tag, or `None` when it is always + /// available + pub(crate) library: Option<&'static str>, + pub(crate) documentation: &'static str, +} + +/// a builtin template filter +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct Filter { + pub(crate) name: &'static str, + /// the `{% load %}` library providing this filter, or `None` when it is + /// always available + pub(crate) library: Option<&'static str>, + pub(crate) documentation: &'static str, +} + +/// look a builtin tag up by name +pub(crate) fn tag(name: &str) -> Option<&'static Tag> { + TAGS.iter().find(|tag| tag.name == name) +} + +/// look a builtin filter up by name +pub(crate) fn filter(name: &str) -> Option<&'static Filter> { + FILTERS.iter().find(|filter| filter.name == name) +} + +/// the tag closing the block `name` opens, whether `name` is builtin or one of +/// the project's own block tags +pub(crate) fn end_tag_for(name: &str) -> Option<&'static str> { + tag(name).and_then(|tag| tag.closed_by) +} + +/// how django's own build hands a name to a template +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum Provided<'a> { + /// every template has it, with no `{% load %}` written + Always, + /// the library that has to be loaded before it can be used + By(&'a str), +} + +impl<'a> Provided<'a> { + /// the library a template has to load first, where there is one + pub(crate) fn library(self) -> Option<&'a str> { + match self { + Self::Always => None, + Self::By(library) => Some(library), + } + } +} + +/// how django provides the tag or filter `name`, or `None` where it does not +/// provide it at all +/// +/// where the project's own django can be read, it is that django which decides +/// both whether a name exists and which library it comes from; the tables above +/// then only supply what discovery has no way to see. where it cannot be read — +/// no settings module, no resolvable django — the tables are all there is, and +/// answer on their own exactly as they did before. +pub(crate) fn provided_by_django<'db>( + db: &'db dyn Db, + name: &str, + is_filter: bool, +) -> Option> { + let registered = project::registrations(db, db.project()) + .iter() + .find(|registration| { + registration.django + && registration.name == name + && (registration.kind == RegistrationKind::Filter) == is_filter + }) + .map(|registration| { + if registration.always_loaded { + Provided::Always + } else { + Provided::By(registration.library.as_str()) + } + }); + + // a django that was read and does not register the name is a django the name + // is not in, and its silence is as much an answer as its registrations are + if registered.is_some() || project::django_is_authoritative(db, db.project()) { + return registered; + } + + let library = if is_filter { + filter(name).map(|filter| filter.library) + } else { + tag(name).map(|tag| tag.library) + }?; + + Some(match library { + None => Provided::Always, + Some(library) => Provided::By(library), + }) +} + +/// the libraries a `{% load %}` can name +/// +/// every one of them ships with django. a tag or filter the project registers is +/// discovered from its source instead, and named by its module — as are django's +/// own, wherever django itself can be read. +pub(crate) const LIBRARIES: &[&str] = &["cache", "i18n", "l10n", "static", "tz"]; + +pub(crate) const TAGS: &[Tag] = &[ + Tag { + name: "autoescape", + closed_by: Some("endautoescape"), + branches: &[], + library: None, + documentation: "controls html auto-escaping for the enclosed block. takes `on` or `off`.", + }, + Tag { + name: "block", + closed_by: Some("endblock"), + branches: &[], + library: None, + documentation: "defines a named block a child template can override.", + }, + Tag { + name: "comment", + closed_by: Some("endcomment"), + branches: &[], + library: None, + documentation: "ignores everything between the tags.", + }, + Tag { + name: "csrf_token", + closed_by: None, + branches: &[], + library: None, + documentation: "renders the hidden csrf token input. required in every `POST` form.", + }, + Tag { + name: "cycle", + closed_by: None, + branches: &[], + library: None, + documentation: "emits the next of its arguments each time it is reached.", + }, + Tag { + name: "debug", + closed_by: None, + branches: &[], + library: None, + documentation: "outputs the whole current context, for debugging.", + }, + Tag { + name: "extends", + closed_by: None, + branches: &[], + library: None, + documentation: "declares this template a child of another. must be the first tag in the file.", + }, + Tag { + name: "filter", + closed_by: Some("endfilter"), + branches: &[], + library: None, + documentation: "runs the enclosed block's output through the given filters.", + }, + Tag { + name: "firstof", + closed_by: None, + branches: &[], + library: None, + documentation: "outputs the first of its arguments that is truthy.", + }, + Tag { + name: "for", + closed_by: Some("endfor"), + branches: &["empty"], + library: None, + documentation: "loops over each item of a sequence. `{% empty %}` supplies the body for an empty one.", + }, + Tag { + name: "if", + closed_by: Some("endif"), + branches: &["elif", "else"], + library: None, + documentation: "renders its body when the condition is truthy.", + }, + Tag { + name: "ifchanged", + closed_by: Some("endifchanged"), + branches: &["else"], + library: None, + documentation: "renders its body only when the value has changed since the last loop iteration.", + }, + Tag { + name: "include", + closed_by: None, + branches: &[], + library: None, + documentation: "renders another template here, with the current context or a `with` one.", + }, + Tag { + name: "load", + closed_by: None, + branches: &[], + library: None, + documentation: "loads a template tag library, making its tags and filters available.", + }, + Tag { + name: "lorem", + closed_by: None, + branches: &[], + library: None, + documentation: "emits placeholder lorem ipsum text.", + }, + Tag { + name: "now", + closed_by: None, + branches: &[], + library: None, + documentation: "formats the current date and time with the given format string.", + }, + Tag { + name: "partial", + closed_by: None, + branches: &[], + library: None, + documentation: "renders a fragment defined by a `{% partialdef %}`.", + }, + Tag { + name: "partialdef", + closed_by: Some("endpartialdef"), + branches: &[], + library: None, + documentation: "defines a named, reusable fragment of this template. `inline` also renders it in place.", + }, + Tag { + name: "querystring", + closed_by: None, + branches: &[], + library: None, + documentation: "renders a url-encoded query string from the request's, with the given changes applied.", + }, + Tag { + name: "regroup", + closed_by: None, + branches: &[], + library: None, + documentation: "regroups a list of objects by a common attribute.", + }, + Tag { + name: "resetcycle", + closed_by: None, + branches: &[], + library: None, + documentation: "restarts a `{% cycle %}` from its first argument.", + }, + Tag { + name: "spaceless", + closed_by: Some("endspaceless"), + branches: &[], + library: None, + documentation: "strips the whitespace between html tags in its body.", + }, + Tag { + name: "templatetag", + closed_by: None, + branches: &[], + library: None, + documentation: "outputs one of the template language's own delimiters, such as `openblock`.", + }, + Tag { + name: "url", + closed_by: None, + branches: &[], + library: None, + documentation: "reverses a named url pattern into its path.", + }, + Tag { + name: "verbatim", + closed_by: Some("endverbatim"), + branches: &[], + library: None, + documentation: "outputs its body without rendering any template syntax in it.", + }, + Tag { + name: "widthratio", + closed_by: None, + branches: &[], + library: None, + documentation: "scales a value against a maximum, for bar-chart widths.", + }, + Tag { + name: "with", + closed_by: Some("endwith"), + branches: &[], + library: None, + documentation: "binds names to values for the enclosed block.", + }, + // `cache` + Tag { + name: "cache", + closed_by: Some("endcache"), + branches: &[], + library: Some("cache"), + documentation: "caches the rendered body for the given number of seconds, keyed by the given name.", + }, + // `i18n` + Tag { + name: "blocktranslate", + closed_by: Some("endblocktranslate"), + branches: &["plural"], + library: Some("i18n"), + documentation: "marks a block of text for translation, with placeholders for variables.", + }, + Tag { + name: "blocktrans", + closed_by: Some("endblocktrans"), + branches: &["plural"], + library: Some("i18n"), + documentation: "the older spelling of `{% blocktranslate %}`.", + }, + Tag { + name: "get_available_languages", + closed_by: None, + branches: &[], + library: Some("i18n"), + documentation: "binds the list of configured `(code, name)` language pairs to a variable.", + }, + Tag { + name: "get_current_language", + closed_by: None, + branches: &[], + library: Some("i18n"), + documentation: "binds the active language's code to a variable.", + }, + Tag { + name: "get_current_language_bidi", + closed_by: None, + branches: &[], + library: Some("i18n"), + documentation: "binds whether the active language is right-to-left to a variable.", + }, + Tag { + name: "get_language_info", + closed_by: None, + branches: &[], + library: Some("i18n"), + documentation: "binds a language's name, code and direction to a variable.", + }, + Tag { + name: "get_language_info_list", + closed_by: None, + branches: &[], + library: Some("i18n"), + documentation: "binds the language info of each of the given codes to a variable.", + }, + Tag { + name: "language", + closed_by: Some("endlanguage"), + branches: &[], + library: Some("i18n"), + documentation: "renders its body with the given language active.", + }, + Tag { + name: "translate", + closed_by: None, + branches: &[], + library: Some("i18n"), + documentation: "translates a string literal or variable.", + }, + Tag { + name: "trans", + closed_by: None, + branches: &[], + library: Some("i18n"), + documentation: "the older spelling of `{% translate %}`.", + }, + // `l10n` + Tag { + name: "localize", + closed_by: Some("endlocalize"), + branches: &[], + library: Some("l10n"), + documentation: "turns locale-aware number formatting on or off for its body.", + }, + // `static` + Tag { + name: "get_media_prefix", + closed_by: None, + branches: &[], + library: Some("static"), + documentation: "binds `MEDIA_URL` to a variable.", + }, + Tag { + name: "get_static_prefix", + closed_by: None, + branches: &[], + library: Some("static"), + documentation: "binds `STATIC_URL` to a variable.", + }, + Tag { + name: "static", + closed_by: None, + branches: &[], + library: Some("static"), + documentation: "builds the url of a static file.", + }, + // `tz` + Tag { + name: "get_current_timezone", + closed_by: None, + branches: &[], + library: Some("tz"), + documentation: "binds the active time zone's name to a variable.", + }, + Tag { + name: "localtime", + closed_by: Some("endlocaltime"), + branches: &[], + library: Some("tz"), + documentation: "turns conversion of datetimes to local time on or off for its body.", + }, + Tag { + name: "timezone", + closed_by: Some("endtimezone"), + branches: &[], + library: Some("tz"), + documentation: "renders its body with the given time zone active.", + }, +]; + +pub(crate) const FILTERS: &[Filter] = &[ + Filter { + name: "add", + library: None, + documentation: "adds the argument to the value.", + }, + Filter { + name: "addslashes", + library: None, + documentation: "backslash-escapes quotes.", + }, + Filter { + name: "capfirst", + library: None, + documentation: "upper-cases the first character.", + }, + Filter { + name: "center", + library: None, + documentation: "centres the value in a field of the given width.", + }, + Filter { + name: "cut", + library: None, + documentation: "removes every occurrence of the argument.", + }, + Filter { + name: "date", + library: None, + documentation: "formats a date with the given format string.", + }, + Filter { + name: "default", + library: None, + documentation: "uses the argument when the value is falsy.", + }, + Filter { + name: "default_if_none", + library: None, + documentation: "uses the argument only when the value is `None`.", + }, + Filter { + name: "dictsort", + library: None, + documentation: "sorts a list of mappings by the given key.", + }, + Filter { + name: "dictsortreversed", + library: None, + documentation: "sorts a list of mappings by the given key, descending.", + }, + Filter { + name: "divisibleby", + library: None, + documentation: "whether the value divides by the argument.", + }, + Filter { + name: "escape", + library: None, + documentation: "html-escapes the value.", + }, + Filter { + name: "escapejs", + library: None, + documentation: "escapes the value for use in a javascript string.", + }, + Filter { + name: "escapeseq", + library: None, + documentation: "html-escapes each element of a sequence.", + }, + Filter { + name: "filesizeformat", + library: None, + documentation: "formats a byte count as `13 KB`.", + }, + Filter { + name: "first", + library: None, + documentation: "the first element.", + }, + Filter { + name: "floatformat", + library: None, + documentation: "rounds a float to the given number of decimal places.", + }, + Filter { + name: "force_escape", + library: None, + documentation: "html-escapes the value immediately rather than lazily.", + }, + Filter { + name: "get_digit", + library: None, + documentation: "the nth digit of an integer, counted from the right.", + }, + Filter { + name: "iriencode", + library: None, + documentation: "converts an iri to a url-safe string.", + }, + Filter { + name: "join", + library: None, + documentation: "joins a sequence with the argument, like python's `str.join`.", + }, + Filter { + name: "json_script", + library: None, + documentation: "renders the value as json inside a `