From b11e769a2ac883a4ce895b8e1fcc5dcc09ed7e5c Mon Sep 17 00:00:00 2001 From: "Giuseppe.Esposito" Date: Mon, 2 Nov 2026 15:10:56 +0100 Subject: [PATCH 01/13] Link Drupal hook attributes to the hook documentation The hook_*() functions of *.api.php files document hooks, core's and contributed extensions' included, and a #[Hook] attribute in an extension's src/Hook jumps to that documentation, shows it on hover, and lists with the other implementations in its references. The Drupal walk also reads every extension's *.info.yml for the hooks to come. Assisted-by: claude-code/claude-opus-5-5 --- src/classmap_scanner/discovery.rs | 3 + src/classmap_scanner/discovery_tests.rs | 3 +- src/completion/laravel_string_keys/mod.rs | 7 +- src/diagnostics/laravel_string_keys.rs | 1 + src/drupal/hooks.rs | 142 +++++++++++++++++++++ src/drupal/mod.rs | 81 ++++++++++-- src/hover/laravel.rs | 4 +- src/indexing/preload.rs | 9 ++ src/parser/ast_update.rs | 5 +- src/symbol_map/extraction/class_like.rs | 2 + src/symbol_map/extraction/drupal.rs | 121 ++++++++++++++++++ src/symbol_map/extraction/mod.rs | 44 ++++++- src/symbol_map/extraction/twig.rs | 2 +- src/symbol_map/extraction/twig_context.rs | 2 +- src/symbol_map/mod.rs | 1 - src/symbol_map/string_keys.rs | 12 +- src/symfony/bundles.rs | 3 +- src/symfony/mod.rs | 6 +- src/symfony/route_world.rs | 3 +- src/twig/callables.rs | 3 +- src/virtual_members/laravel/string_keys.rs | 6 +- src/wordpress/hook_sources.rs | 3 +- tests/integration/drupal_hooks.rs | 111 ++++++++++++++++ tests/integration/main.rs | 1 + 24 files changed, 542 insertions(+), 33 deletions(-) create mode 100644 src/drupal/hooks.rs create mode 100644 tests/integration/drupal_hooks.rs diff --git a/src/classmap_scanner/discovery.rs b/src/classmap_scanner/discovery.rs index 550112171..8e6578404 100644 --- a/src/classmap_scanner/discovery.rs +++ b/src/classmap_scanner/discovery.rs @@ -687,6 +687,9 @@ pub fn scan_drupal_directories( } if is_drupal_php_file(path) || filters.is_php_file(path) { php_files.push((path.to_path_buf(), crate::ClassCompletionOrigin::Project)); + if crate::drupal::is_hook_php_file(path, web_root) { + declaration_files.push(path.to_path_buf()); + } } else if crate::drupal::is_declaration_file(path, web_root) { declaration_files.push(path.to_path_buf()); } diff --git a/src/classmap_scanner/discovery_tests.rs b/src/classmap_scanner/discovery_tests.rs index c76ebc4da..f859a3e83 100644 --- a/src/classmap_scanner/discovery_tests.rs +++ b/src/classmap_scanner/discovery_tests.rs @@ -711,7 +711,7 @@ fn drupal_scan_honors_exclude_globs() { } #[test] -fn scan_drupal_directories_collects_services_files_outside_tests() { +fn scan_drupal_directories_collects_declaration_files_outside_tests() { let dir = tempfile::tempdir().unwrap(); let web_root = dir.path(); for (path, content) in [ @@ -735,6 +735,7 @@ fn scan_drupal_directories_collects_services_files_outside_tests() { files, vec![ web_root.join("core/core.services.yml"), + web_root.join("modules/contrib/token/token.info.yml"), web_root.join("modules/contrib/token/token.services.yml"), web_root.join("sites/default/services.yml"), ] diff --git a/src/completion/laravel_string_keys/mod.rs b/src/completion/laravel_string_keys/mod.rs index a99410ba2..e2b3b5cbe 100644 --- a/src/completion/laravel_string_keys/mod.rs +++ b/src/completion/laravel_string_keys/mod.rs @@ -44,7 +44,9 @@ pub(crate) fn string_key_item_kind(kind: &StringKeyKind) -> CompletionItemKind { StringKeyKind::DrupalEntityType => CompletionItemKind::Class, StringKeyKind::ContainerParameter => CompletionItemKind::Property, StringKeyKind::SymfonyConfigKey => CompletionItemKind::Field, - StringKeyKind::SymfonyEvent | StringKeyKind::WordPressHook => CompletionItemKind::Event, + StringKeyKind::SymfonyEvent | StringKeyKind::WordPressHook | StringKeyKind::DrupalHook => { + CompletionItemKind::Event + } StringKeyKind::MessengerBus => CompletionItemKind::Reference, StringKeyKind::YiiRouteArgument | StringKeyKind::DrupalRouteParameter => { CompletionItemKind::Field @@ -187,7 +189,8 @@ impl Backend { | StringKeyKind::MessengerBus | StringKeyKind::YiiRoute | StringKeyKind::DrupalEntityType - | StringKeyKind::DrupalRoute => self.symfony_declared_names(*kind).into(), + | StringKeyKind::DrupalRoute + | StringKeyKind::DrupalHook => self.symfony_declared_names(*kind).into(), StringKeyKind::TwigFilter | StringKeyKind::TwigFunction => { self.twig_callable_names(*kind).into() } diff --git a/src/diagnostics/laravel_string_keys.rs b/src/diagnostics/laravel_string_keys.rs index 27e1377b1..74d44184e 100644 --- a/src/diagnostics/laravel_string_keys.rs +++ b/src/diagnostics/laravel_string_keys.rs @@ -100,6 +100,7 @@ impl CheckedStringKind { StringKeyKind::WordPressHook | StringKeyKind::DrupalEntityType | StringKeyKind::DrupalRouteParameter + | StringKeyKind::DrupalHook | StringKeyKind::TranslationDomain | StringKeyKind::Section | StringKeyKind::Stack diff --git a/src/drupal/hooks.rs b/src/drupal/hooks.rs new file mode 100644 index 000000000..52edc717d --- /dev/null +++ b/src/drupal/hooks.rs @@ -0,0 +1,142 @@ +//! Drupal's hooks: the extensions that implement them, and what a PHP file +//! of a Drupal project can hold of them. +//! +//! A hook is documented by a `hook_()` function in an `*.api.php` +//! file, which Drupal never loads, and implemented by a method a `#[Hook]` +//! attribute marks in an extension's `src/Hook`, or by a function named +//! `_` in an extension's procedural files. The +//! extensions come from their `*.info.yml`. + +use std::path::{Path, PathBuf}; + +use crate::Backend; + +/// One module, install profile, or theme, read from its `*.info.yml`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct Extension { + /// The directory holding the `.info.yml`. + pub dir: PathBuf, + /// The machine name, from the file name. + pub name: String, + /// Themes implement hooks the theme manager invokes; modules and + /// install profiles the ones the module handler does. + pub is_theme: bool, +} + +/// What a PHP file of a Drupal project holds of hooks, which only its path +/// says: the extraction is told before it reads the file. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub(crate) enum DrupalFile { + /// Anything else: its hook classes, if any, are read by namespace. + #[default] + Other, + /// An `*.api.php` file, whose `hook_*()` functions document hooks. + Api, + /// A file of a test module, which no real site enables: it neither + /// documents nor implements a hook. + Test, +} + +/// Whether the file name at the end of `path` is an extension's +/// `*.info.yml`. +pub(crate) fn is_info_file_name(name: &str) -> bool { + name.ends_with(".info.yml") +} + +/// The extension an `*.info.yml` at `path` declares, from its `type:` +/// line; `None` for anything but a module, profile, or theme. +pub(crate) fn read_extension(path: &Path) -> Option { + let name = path + .file_name()? + .to_str()? + .strip_suffix(".info.yml")? + .to_string(); + let content = std::fs::read_to_string(path).ok()?; + let kind = content.lines().find_map(|line| { + let value = line.strip_prefix("type:")?.trim(); + let value = crate::framework::strip_yaml_quotes(value).0; + Some(value.to_string()) + })?; + let is_theme = match kind.as_str() { + "module" | "profile" => false, + "theme" => true, + _ => return None, + }; + Some(Extension { + dir: path.parent()?.to_path_buf(), + name, + is_theme, + }) +} + +/// The extension a hook class of namespace `Drupal\\Hook\…` +/// belongs to, which is where Drupal discovers hook classes. +pub(crate) fn hook_class_extension(fqn: &str) -> Option<&str> { + let mut segments = fqn.trim_start_matches('\\').split('\\'); + (segments.next() == Some("Drupal")).then_some(())?; + let extension = segments.next()?; + (segments.next() == Some("Hook") && segments.next().is_some()).then_some(extension) +} + +impl Backend { + /// Remember the extensions the Drupal walk found. + pub(crate) fn note_drupal_extensions(&self, extensions: Vec) { + self.drupal.extensions.write().extend(extensions); + } + + /// What the PHP file at `uri` holds of hooks. + pub(crate) fn drupal_file(&self, uri: &str) -> DrupalFile { + if !self.is_drupal_project() { + return DrupalFile::Other; + } + let Some(path) = crate::util::uri_to_path(uri) else { + return DrupalFile::Other; + }; + if self.is_drupal_test_path(&path) { + return DrupalFile::Test; + } + let is_api = path + .file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| name.ends_with(".api.php")); + if is_api { + DrupalFile::Api + } else { + DrupalFile::Other + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn hook_classes_belong_to_the_extension_of_their_namespace() { + assert_eq!( + hook_class_extension("Drupal\\acme\\Hook\\AcmeHooks"), + Some("acme") + ); + assert_eq!( + hook_class_extension("\\Drupal\\acme\\Hook\\Form\\FormHooks"), + Some("acme") + ); + assert_eq!(hook_class_extension("Drupal\\acme\\Hooks"), None); + assert_eq!(hook_class_extension("Drupal\\acme\\Form\\Hook"), None); + } + + #[test] + fn info_files_give_the_extension_and_its_kind() { + let dir = tempfile::tempdir().unwrap(); + let write = |name: &str, content: &str| { + let path = dir.path().join(name); + std::fs::write(&path, content).unwrap(); + path + }; + let module = read_extension(&write("acme.info.yml", "name: Acme\ntype: module\n")).unwrap(); + assert_eq!((module.name.as_str(), module.is_theme), ("acme", false)); + let theme = read_extension(&write("olivero.info.yml", "type: 'theme'\n")).unwrap(); + assert!(theme.is_theme); + assert!(read_extension(&write("engine.info.yml", "type: theme_engine\n")).is_none()); + } +} diff --git a/src/drupal/mod.rs b/src/drupal/mod.rs index 2be41c734..b2097ea45 100644 --- a/src/drupal/mod.rs +++ b/src/drupal/mod.rs @@ -15,6 +15,7 @@ use std::sync::atomic::Ordering; use parking_lot::RwLock; +pub(crate) mod hooks; pub(crate) mod links; pub(crate) mod routing; pub(crate) mod types; @@ -35,6 +36,11 @@ pub(crate) struct DrupalState { declaration_files: RwLock>, /// The project's routes by the methods they dispatch to, once read. route_index: RwLock>>, + /// The PHP files of core and contributed extensions that document or + /// implement hooks, which the workspace walk never reaches either. + hook_php_files: RwLock>, + /// The modules, install profiles, and themes of every web root. + extensions: RwLock>, } /// A kind of file Drupal reads declarations from. @@ -202,11 +208,12 @@ pub(crate) fn matches_route_pattern(pattern: &str, name: &str) -> bool { rest.len() >= last.len() && rest.ends_with(last) } -/// Whether `path` is a file Drupal reads declarations from. +/// Whether `path` is a file Drupal reads declarations from: a services, +/// routing, or links file, or an extension's `*.info.yml`. pub(crate) fn is_declaration_file(path: &Path, web_root: &Path) -> bool { path.file_name() .and_then(|name| name.to_str()) - .is_some_and(|name| declaration_file_kind(name).is_some()) + .is_some_and(|name| declaration_file_kind(name).is_some() || hooks::is_info_file_name(name)) && !is_test_fixture(path, web_root) && !is_unloaded(path, web_root) } @@ -222,6 +229,28 @@ pub(crate) fn is_views_route_name(name: &str) -> bool { ) } +/// Whether `path` is a PHP file that can document or implement hooks: an +/// `*.api.php`, an extension's procedural file, or a class under an +/// extension's `src/Hook`. +pub(crate) fn is_hook_php_file(path: &Path, web_root: &Path) -> bool { + let Some(name) = path.file_name().and_then(|name| name.to_str()) else { + return false; + }; + let by_name = [ + ".api.php", ".module", ".install", ".inc", ".profile", ".theme", + ] + .iter() + .any(|suffix| name.ends_with(suffix)) + || name == "theme-settings.php"; + let in_hook_dir = name.ends_with(".php") + && path + .components() + .collect::>() + .windows(2) + .any(|pair| pair[0].as_os_str() == "src" && pair[1].as_os_str() == "Hook"); + (by_name || in_hook_dir) && !is_test_fixture(path, web_root) && !is_unloaded(path, web_root) +} + /// Whether `path` lies under a `tests` folder of the web root: it belongs to /// a test module, which no real site enables. Roughly half of core's /// declaration files are. @@ -251,11 +280,48 @@ impl Backend { pub(crate) fn clear_drupal_web_roots(&self) { self.drupal.web_roots.write().clear(); self.drupal.declaration_files.write().clear(); + self.drupal.hook_php_files.write().clear(); + self.drupal.extensions.write().clear(); } - /// Remember the declaration files the Drupal walk found. + /// Remember the declaration files the Drupal walk found: the YAML for + /// the workspace index, the extensions their `*.info.yml` declare, and + /// the PHP files that document or implement hooks. pub(crate) fn note_drupal_declaration_files(&self, files: Vec) { - self.drupal.declaration_files.write().extend(files); + let mut yaml = Vec::new(); + let mut php = Vec::new(); + let mut extensions = Vec::new(); + for file in files { + let name = file + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or_default(); + if hooks::is_info_file_name(name) { + extensions.extend(hooks::read_extension(&file)); + } else if name.ends_with(".yml") { + yaml.push(file); + } else { + php.push(file); + } + } + self.drupal.declaration_files.write().extend(yaml); + self.drupal.hook_php_files.write().extend(php); + self.note_drupal_extensions(extensions); + } + + /// The PHP files of core and contributed extensions that document or + /// implement hooks, for the workspace index to parse. + pub(crate) fn drupal_hook_php_files(&self) -> Vec { + self.drupal.hook_php_files.read().clone() + } + + /// Whether `path` lies under a `tests` folder of one of the web roots. + pub(crate) fn is_drupal_test_path(&self, path: &Path) -> bool { + self.drupal + .web_roots + .read() + .iter() + .any(|root| path.starts_with(root) && is_test_fixture(path, root)) } /// The declaration files the Drupal walk found, for the workspace index @@ -386,12 +452,7 @@ impl Backend { path.file_name() .and_then(|name| name.to_str()) .is_some_and(|name| name.ends_with(".api.php")) - || self - .drupal - .web_roots - .read() - .iter() - .any(|root| path.starts_with(root) && is_test_fixture(&path, root)) + || self.is_drupal_test_path(&path) } /// Whether `uri` belongs to a test module of a Drupal project and is not diff --git a/src/hover/laravel.rs b/src/hover/laravel.rs index 2b8e2f669..af186a6cb 100644 --- a/src/hover/laravel.rs +++ b/src/hover/laravel.rs @@ -292,10 +292,12 @@ impl Backend { | StringKeyKind::SymfonyTranslation | StringKeyKind::TranslationDomain | StringKeyKind::DrupalEntityType - | StringKeyKind::DrupalRoute => { + | StringKeyKind::DrupalRoute + | StringKeyKind::DrupalHook => { let label = match kind { StringKeyKind::ServiceId => "Service", StringKeyKind::DrupalEntityType => "Entity type", + StringKeyKind::DrupalHook => "Hook", StringKeyKind::ContainerParameter => "Parameter", StringKeyKind::SymfonyRoute | StringKeyKind::DrupalRoute => "Route", StringKeyKind::SymfonyTranslation => "Translation", diff --git a/src/indexing/preload.rs b/src/indexing/preload.rs index 17620f1d2..933d6cd5f 100644 --- a/src/indexing/preload.rs +++ b/src/indexing/preload.rs @@ -312,6 +312,15 @@ impl Backend { } // Drupal's own walk found the declaration files the gitignore-aware // walk skips; the custom ones both walks reach are listed once. + let drupal_php = self.drupal_hook_php_files(); + if !drupal_php.is_empty() { + let mut seen: HashSet = php_files.iter().cloned().collect(); + php_files.extend( + drupal_php + .into_iter() + .filter(|path| seen.insert(path.clone())), + ); + } let drupal_files = self.drupal_declaration_files(); if !drupal_files.is_empty() { let mut seen: HashSet = resource_files.iter().cloned().collect(); diff --git a/src/parser/ast_update.rs b/src/parser/ast_update.rs index e1cdda391..2e6b06644 100644 --- a/src/parser/ast_update.rs +++ b/src/parser/ast_update.rs @@ -15,7 +15,7 @@ use crate::ParseErrorEntry; use crate::atom::atom; use crate::blade::call_site_inference::BladeScope; use crate::names::OwnedResolvedNames; -use crate::symbol_map::{SymbolMap, extract_symbol_map_for_index}; +use crate::symbol_map::SymbolMap; use crate::types::{ClassInfo, DefineInfo, FunctionInfo, NamespaceSpan}; use mago_allocator::LocalArena; @@ -557,7 +557,7 @@ impl Backend { } else { None }; - extract_symbol_map_for_index( + crate::symbol_map::extraction::extract_symbol_map_for_file( program, content, &owned_resolved, @@ -568,6 +568,7 @@ impl Backend { twig: self.is_twig_project(), }, yii_dir.as_deref(), + self.drupal_file(uri), ) }; // A router that loads its routes from somewhere else than the diff --git a/src/symbol_map/extraction/class_like.rs b/src/symbol_map/extraction/class_like.rs index 35973ab33..5ac344f14 100644 --- a/src/symbol_map/extraction/class_like.rs +++ b/src/symbol_map/extraction/class_like.rs @@ -25,6 +25,7 @@ pub(super) fn extract_from_class<'a>(class: &'a Class<'a>, ctx: &mut ExtractionC emit_yii_route_argument_spans(class, ctx); emit_drupal_entity_type_spans(class, ctx); emit_drupal_route_attribute_spans(class, ctx); + emit_drupal_hook_attribute_spans(class, ctx); emit_drupal_route_builder_spans(&class.members, class.name.span.start.offset, ctx); if let Some(ref extends) = class.extends { @@ -922,6 +923,7 @@ pub(super) fn extract_from_function<'a>(func: &'a Function<'a>, ctx: &mut Extrac is_docblock_reference: false, }, }); + emit_drupal_hook_doc_span(func, ctx); // Docblock. We extract type spans and template params now, but // defer `@param $var` variable spans until after we know diff --git a/src/symbol_map/extraction/drupal.rs b/src/symbol_map/extraction/drupal.rs index e31fb87c8..0bbac7f7d 100644 --- a/src/symbol_map/extraction/drupal.rs +++ b/src/symbol_map/extraction/drupal.rs @@ -691,6 +691,84 @@ fn extension_of(fqn: &str) -> Option<&str> { (!matches!(extension, "Core" | "Component") && segments.next().is_some()).then_some(extension) } +const HOOK_ATTRIBUTE: &str = "Drupal\\Core\\Hook\\Attribute\\Hook"; + +/// Record the hook a `hook_()` function of an `*.api.php` file +/// documents. The span covers the name after `hook_`, so the function's +/// own span still answers on `hook_`. +pub(in crate::symbol_map::extraction) fn emit_drupal_hook_doc_span( + function: &Function<'_>, + ctx: &mut ExtractionCtx<'_>, +) { + if !ctx.drupal || ctx.drupal_file != crate::drupal::hooks::DrupalFile::Api { + return; + } + let name = bytes_to_str(function.name.value); + let Some(hook) = name.strip_prefix("hook_").filter(|hook| !hook.is_empty()) else { + return; + }; + let start = function.name.span.start.offset + 5; + ctx.spans.push(SymbolSpan { + start, + end: start + hook.len() as u32, + kind: SymbolKind::StringKey { + kind: StringKeyKind::DrupalHook, + key: hook.to_string(), + is_write: true, + is_optional: false, + }, + }); +} + +/// Record the hooks the `#[Hook]` attributes of a hook class implement: +/// one on a method implements the hook for that method, one on the class +/// for `__invoke()` or the `method:` it names. Drupal discovers hook +/// classes under an extension's `src/Hook`, whose namespace is +/// `Drupal\\Hook`. +pub(in crate::symbol_map::extraction) fn emit_drupal_hook_attribute_spans( + class: &Class<'_>, + ctx: &mut ExtractionCtx<'_>, +) { + if !ctx.drupal || ctx.drupal_file == crate::drupal::hooks::DrupalFile::Test { + return; + } + let is_hook_class = ctx + .resolved_name_at(class.name.span.start.offset) + .and_then(crate::drupal::hooks::hook_class_extension) + .is_some(); + if !is_hook_class { + return; + } + let methods = class.members.iter().filter_map(|member| match member { + ClassLikeMember::Method(method) => Some(&method.attribute_lists), + _ => None, + }); + let mut found = Vec::new(); + for attribute_lists in std::iter::once(&class.attribute_lists).chain(methods) { + for attribute in attributes_named(attribute_lists, HOOK_ATTRIBUTE, ctx) { + let hook = attribute_arguments(attribute, &["hook", "method", "module"]) + .into_iter() + .find(|(parameter, _)| *parameter == "hook") + .and_then(|(_, value)| resolved_string_literal(value, ctx.content)); + if let Some((key, start, end)) = hook.filter(|(key, _, _)| !key.is_empty()) { + found.push((key.to_string(), start as u32, end as u32)); + } + } + } + for (key, start, end) in found { + ctx.spans.push(SymbolSpan { + start, + end, + kind: SymbolKind::StringKey { + kind: StringKeyKind::DrupalHook, + key, + is_write: false, + is_optional: false, + }, + }); + } +} + /// Record the route name `new Url('…')` reads. pub(in crate::symbol_map::extraction) fn emit_drupal_url_spans( class: &Expression<'_>, @@ -1436,6 +1514,49 @@ class NodeRouteProvider extends DefaultHtmlRouteProvider { assert_eq!(built(php), (Vec::new(), Vec::new())); } + #[test] + fn api_files_document_hooks_after_the_hook_prefix() { + let php = " NodeStorage::class, 'form' => ['default' => 'X']],\n)]\nclass Node {}\n"; diff --git a/src/symbol_map/extraction/mod.rs b/src/symbol_map/extraction/mod.rs index 3b2ec56fa..3fd031da5 100644 --- a/src/symbol_map/extraction/mod.rs +++ b/src/symbol_map/extraction/mod.rs @@ -108,6 +108,8 @@ struct ExtractionCtx<'a> { drupal_route_aliases: Vec, /// The Drupal routes the file's `#[Route]` attributes declare. drupal_routes: Vec, + /// What the file holds of Drupal hooks, which its path says. + drupal_file: crate::drupal::hooks::DrupalFile, /// Current conditional nesting depth (if/else, switch, while, for, etc.). /// Incremented when entering a conditional block, decremented when leaving. cond_nesting_depth: u16, @@ -403,7 +405,14 @@ impl Frameworks { /// every navigable symbol occurrence. #[cfg(test)] pub(crate) fn extract_symbol_map(program: &Program<'_>, content: &str) -> SymbolMap { - extract_symbol_map_inner(program, content, None, Frameworks::ALL, None) + extract_symbol_map_inner( + program, + content, + None, + Frameworks::ALL, + None, + crate::drupal::hooks::DrupalFile::Other, + ) } /// Build a [`SymbolMap`] using the semantic names produced by `mago-names`. @@ -419,6 +428,7 @@ pub(crate) fn extract_symbol_map_with_resolved_names( Some(resolved_names), Frameworks::ALL, None, + crate::drupal::hooks::DrupalFile::Other, ); map.refresh_conditional_string_key_spans(None, |_| false); map @@ -429,6 +439,7 @@ pub(crate) fn extract_symbol_map_with_resolved_names( /// /// `frameworks` says which frameworks' names to record, and `yii_dir` is the /// file's directory when the project is a Yii application. +#[cfg(test)] pub(crate) fn extract_symbol_map_for_index( program: &Program<'_>, content: &str, @@ -436,7 +447,34 @@ pub(crate) fn extract_symbol_map_for_index( frameworks: Frameworks, yii_dir: Option<&std::path::Path>, ) -> SymbolMap { - extract_symbol_map_inner(program, content, Some(resolved_names), frameworks, yii_dir) + extract_symbol_map_inner( + program, + content, + Some(resolved_names), + frameworks, + yii_dir, + crate::drupal::hooks::DrupalFile::Other, + ) +} + +/// [`extract_symbol_map_for_index`] for a file whose path says what it +/// holds of Drupal hooks. +pub(crate) fn extract_symbol_map_for_file( + program: &Program<'_>, + content: &str, + resolved_names: &OwnedResolvedNames, + frameworks: Frameworks, + yii_dir: Option<&std::path::Path>, + drupal_file: crate::drupal::hooks::DrupalFile, +) -> SymbolMap { + extract_symbol_map_inner( + program, + content, + Some(resolved_names), + frameworks, + yii_dir, + drupal_file, + ) } fn extract_symbol_map_inner( @@ -445,6 +483,7 @@ fn extract_symbol_map_inner( resolved_names: Option<&OwnedResolvedNames>, frameworks: Frameworks, yii_dir: Option<&std::path::Path>, + drupal_file: crate::drupal::hooks::DrupalFile, ) -> SymbolMap { let Frameworks { symfony, @@ -484,6 +523,7 @@ fn extract_symbol_map_inner( drupal_route_patterns: Vec::new(), drupal_route_aliases: Vec::new(), drupal_routes: Vec::new(), + drupal_file, cond_nesting_depth: 0, cond_block_end_stack: Vec::new(), has_laravel_container_attrs: None, diff --git a/src/symbol_map/extraction/twig.rs b/src/symbol_map/extraction/twig.rs index 28c47d89e..7a579b302 100644 --- a/src/symbol_map/extraction/twig.rs +++ b/src/symbol_map/extraction/twig.rs @@ -289,7 +289,7 @@ mod tests { let program = mago_syntax::parser::parse_file_content(&arena, file_id, php.as_bytes()); let resolver = mago_names::resolver::NameResolver::new(&arena); let names = crate::names::OwnedResolvedNames::from_resolved(&resolver.resolve(program)); - crate::symbol_map::extract_symbol_map_for_index( + crate::symbol_map::extraction::extract_symbol_map_for_index( program, php, &names, diff --git a/src/symbol_map/extraction/twig_context.rs b/src/symbol_map/extraction/twig_context.rs index 80a83eb9d..7bfb271bc 100644 --- a/src/symbol_map/extraction/twig_context.rs +++ b/src/symbol_map/extraction/twig_context.rs @@ -349,7 +349,7 @@ mod tests { let program = mago_syntax::parser::parse_file_content(&arena, file_id, php.as_bytes()); let resolver = mago_names::resolver::NameResolver::new(&arena); let names = crate::names::OwnedResolvedNames::from_resolved(&resolver.resolve(program)); - crate::symbol_map::extract_symbol_map_for_index( + crate::symbol_map::extraction::extract_symbol_map_for_index( program, php, &names, diff --git a/src/symbol_map/mod.rs b/src/symbol_map/mod.rs index 5eb83b9cb..ef6c21741 100644 --- a/src/symbol_map/mod.rs +++ b/src/symbol_map/mod.rs @@ -48,7 +48,6 @@ mod string_keys; pub(crate) use call_sites::*; #[cfg(test)] pub(crate) use extraction::extract_symbol_map; -pub(crate) use extraction::extract_symbol_map_for_index; pub(crate) use laravel_spans::*; pub(crate) use map::*; pub(crate) use morph_columns::{MorphColumnReceiver, MorphColumnSite}; diff --git a/src/symbol_map/string_keys.rs b/src/symbol_map/string_keys.rs index c01b36923..9fb49033d 100644 --- a/src/symbol_map/string_keys.rs +++ b/src/symbol_map/string_keys.rs @@ -172,6 +172,11 @@ pub(crate) enum StringKeyKind { /// the route's path, and read by the keys of the parameters a route /// call or a links file's `route_parameters:` passes. DrupalRouteParameter, + /// A Drupal hook name: declared by the `hook_()` function that + /// documents it in an `*.api.php` file and by the module handler's and + /// theme manager's calls that invoke it, and read by the implementations + /// a `#[Hook]` attribute or a procedural function's name make. + DrupalHook, } impl StringKeyKind { @@ -218,9 +223,10 @@ impl StringKeyKind { | Self::TwigVariable => KeyFramework::Twig, Self::YiiView | Self::YiiRoute | Self::YiiRouteArgument => KeyFramework::Yii, Self::WordPressHook => KeyFramework::WordPress, - Self::DrupalEntityType | Self::DrupalRoute | Self::DrupalRouteParameter => { - KeyFramework::Drupal - } + Self::DrupalEntityType + | Self::DrupalRoute + | Self::DrupalRouteParameter + | Self::DrupalHook => KeyFramework::Drupal, } } diff --git a/src/symfony/bundles.rs b/src/symfony/bundles.rs index f94a2d1fc..9a02f4fdd 100644 --- a/src/symfony/bundles.rs +++ b/src/symfony/bundles.rs @@ -180,7 +180,7 @@ fn read_php_file(uri: &str, content: &str) -> SymbolMap { let program = mago_syntax::parser::parse_file_content(&arena, file_id, content.as_bytes()); let resolver = mago_names::resolver::NameResolver::new(&arena); let names = crate::names::OwnedResolvedNames::from_resolved(&resolver.resolve(program)); - let map = crate::symbol_map::extract_symbol_map_for_index( + let map = crate::symbol_map::extraction::extract_symbol_map_for_file( program, content, &names, @@ -191,6 +191,7 @@ fn read_php_file(uri: &str, content: &str) -> SymbolMap { twig: false, }, None, + crate::drupal::hooks::DrupalFile::Other, ); let mut configurators = HashMap::new(); for statement in program.statements.iter() { diff --git a/src/symfony/mod.rs b/src/symfony/mod.rs index dfa35c444..23982c2ef 100644 --- a/src/symfony/mod.rs +++ b/src/symfony/mod.rs @@ -125,9 +125,9 @@ pub(crate) struct SymfonyState { /// Whether `map` declares a service ID, a container parameter, a route, a /// translation key, a template, a Twig filter or function, a configuration -/// key, an event listener, a Messenger bus, a Drupal entity type, or a -/// Drupal route, outright or through a configurator call still to be -/// confirmed, or registers a Messenger handler, or imports routes, or +/// key, an event listener, a Messenger bus, a Drupal entity type, a Drupal +/// route, or a Drupal hook, outright or through a configurator call still +/// to be confirmed, or registers a Messenger handler, or imports routes, or /// declares a Twig loader. /// /// Only the kinds `enabled` takes count: outside Symfony, a project that diff --git a/src/symfony/route_world.rs b/src/symfony/route_world.rs index e96db61b9..5b7df6726 100644 --- a/src/symfony/route_world.rs +++ b/src/symfony/route_world.rs @@ -202,7 +202,7 @@ fn read_routing_file(path: &Path, content: &str) -> Option { let program = mago_syntax::parser::parse_file_content(&arena, file_id, content.as_bytes()); let resolver = mago_names::resolver::NameResolver::new(&arena); let names = crate::names::OwnedResolvedNames::from_resolved(&resolver.resolve(program)); - let map = crate::symbol_map::extract_symbol_map_for_index( + let map = crate::symbol_map::extraction::extract_symbol_map_for_file( program, content, &names, @@ -213,6 +213,7 @@ fn read_routing_file(path: &Path, content: &str) -> Option { twig: false, }, None, + crate::drupal::hooks::DrupalFile::Other, ); let mut spans: Vec<_> = map .spans diff --git a/src/twig/callables.rs b/src/twig/callables.rs index fb6658f86..205a29902 100644 --- a/src/twig/callables.rs +++ b/src/twig/callables.rs @@ -140,7 +140,7 @@ fn read_extension_file(uri: &str, content: &str) -> Vec { let program = mago_syntax::parser::parse_file_content(&arena, file_id, content.as_bytes()); let resolver = mago_names::resolver::NameResolver::new(&arena); let names = crate::names::OwnedResolvedNames::from_resolved(&resolver.resolve(program)); - let map = crate::symbol_map::extract_symbol_map_for_index( + let map = crate::symbol_map::extraction::extract_symbol_map_for_file( program, content, &names, @@ -151,6 +151,7 @@ fn read_extension_file(uri: &str, content: &str) -> Vec { twig: true, }, None, + crate::drupal::hooks::DrupalFile::Other, ); map.twig_callables .into_iter() diff --git a/src/virtual_members/laravel/string_keys.rs b/src/virtual_members/laravel/string_keys.rs index f72a38f66..cf9225cd1 100644 --- a/src/virtual_members/laravel/string_keys.rs +++ b/src/virtual_members/laravel/string_keys.rs @@ -80,7 +80,8 @@ pub(crate) fn resolve_laravel_string_key( | StringKeyKind::YiiRoute | StringKeyKind::DrupalEntityType | StringKeyKind::DrupalRoute - | StringKeyKind::DrupalRouteParameter => backend.symfony_declaration_locations(*kind, key), + | StringKeyKind::DrupalRouteParameter + | StringKeyKind::DrupalHook => backend.symfony_declaration_locations(*kind, key), StringKeyKind::YiiView => backend.resolve_yii_view_definitions(key), StringKeyKind::TwigBlock => backend.twig_block_definitions(uri, None, key), StringKeyKind::TwigVariable => backend.twig_variable_definitions(uri, None, key), @@ -318,7 +319,8 @@ pub(crate) fn find_laravel_string_key_references( | StringKeyKind::YiiRoute | StringKeyKind::DrupalEntityType | StringKeyKind::DrupalRoute - | StringKeyKind::DrupalRouteParameter => { + | StringKeyKind::DrupalRouteParameter + | StringKeyKind::DrupalHook => { return find_string_key_usages(kind, key, backend, snapshot, include_declaration); } // The catalogues named after a domain declare it, and the paths of diff --git a/src/wordpress/hook_sources.rs b/src/wordpress/hook_sources.rs index e161db471..bf1f99091 100644 --- a/src/wordpress/hook_sources.rs +++ b/src/wordpress/hook_sources.rs @@ -109,7 +109,7 @@ pub(crate) fn hook_firings(content: &str) -> Vec { let program = mago_syntax::parser::parse_file_content(&arena, file_id, bytes); let resolver = mago_names::resolver::NameResolver::new(&arena); let names = crate::names::OwnedResolvedNames::from_resolved(&resolver.resolve(program)); - let map = crate::symbol_map::extract_symbol_map_for_index( + let map = crate::symbol_map::extraction::extract_symbol_map_for_file( program, content, &names, @@ -120,6 +120,7 @@ pub(crate) fn hook_firings(content: &str) -> Vec { twig: false, }, None, + crate::drupal::hooks::DrupalFile::Other, ); map.spans .into_iter() diff --git a/tests/integration/drupal_hooks.rs b/tests/integration/drupal_hooks.rs new file mode 100644 index 000000000..11699f99e --- /dev/null +++ b/tests/integration/drupal_hooks.rs @@ -0,0 +1,111 @@ +//! Drupal's hooks: the `hook_*()` functions of `*.api.php` files that +//! document them, and the implementations that `#[Hook]` attributes and +//! procedural function names make. + +use crate::common::hover_text_at; +use crate::drupal_services::{definitions_at, position_in, references_at, uri_for, workspace_in}; + +const FORM_API: &str = " (phpantom_lsp::Backend, tempfile::TempDir) { + let dir = tempfile::tempdir().unwrap(); + for (path, content) in [ + (".git/HEAD", "ref: refs/heads/main\n"), + (".gitignore", "/web/core/\n"), + ( + "web/modules/custom/acme/acme.info.yml", + "name: Acme\ntype: module\ncore_version_requirement: ^11\n", + ), + ] + .iter() + .chain(extra) + { + let full = dir.path().join(path); + std::fs::create_dir_all(full.parent().unwrap()).unwrap(); + std::fs::write(full, content).unwrap(); + } + workspace_in(dir, open).await +} + +#[tokio::test] +async fn a_hook_attribute_goes_to_the_documentation() { + let (backend, dir) = drupal( + &[(FORM_API_PATH, FORM_API), HOOK_ATTRIBUTE], + &[ + (HOOKS_PATH, HOOKS), + ( + "web/modules/custom/acme/src/Form/Elsewhere.php", + NOT_A_HOOK_CLASS, + ), + ], + ) + .await; + let php = uri_for(&dir, HOOKS_PATH); + let api = uri_for(&dir, FORM_API_PATH); + // Finding references indexes the workspace, core's `*.api.php` included. + references_at(&backend, &php, position_in(HOOKS, "'cron'", 2)).await; + + for needle in ["'form_alter'", "hook: 'cron'"] { + let inside = needle.find('\'').unwrap() + 2; + let found = definitions_at(&backend, &php, position_in(HOOKS, needle, inside)).await; + assert_eq!(found.len(), 1, "{needle}: {found:?}"); + assert_eq!(found[0].uri, api, "{needle}"); + } + + let position = position_in(HOOKS, "'form_alter'", 2); + let hover = hover_text_at(&backend, &php, position.line, position.character) + .await + .unwrap_or_default(); + assert!(hover.contains("Hook"), "{hover}"); + + // From the documentation: the implementations, not the class outside + // `src/Hook`. + let found = references_at(&backend, &api, position_in(FORM_API, "cron", 1)).await; + assert_eq!( + found, + vec![(api, 7), (php.clone(), 9), (php, 13)], + "{found:?}" + ); +} diff --git a/tests/integration/main.rs b/tests/integration/main.rs index 1ef0f33dd..e0ecde861 100644 --- a/tests/integration/main.rs +++ b/tests/integration/main.rs @@ -201,6 +201,7 @@ mod document_links; mod document_symbols; mod document_symbols_blade; mod drupal_entities; +mod drupal_hooks; mod drupal_mixed_workspace; mod drupal_routes; mod drupal_services; From 31804cfb20c0b2236395c5897945a0d41ec6dcd0 Mon Sep 17 00:00:00 2001 From: "Giuseppe.Esposito" Date: Mon, 2 Nov 2026 15:18:38 +0100 Subject: [PATCH 02/13] Link Drupal's procedural hook implementations to their hooks A global function of a module's or theme's procedural file named after an extension implements the hook the rest of its name says, the way Drupal's hook collector reads it: the file's own extension first, then the other extensions of its group, legacy functions skipped, and nothing read after a scan stop. The hook part of the name jumps to the documentation and lists with the other implementations, while the extension part keeps the function's own references. Assisted-by: claude-code/claude-opus-5-5 --- src/drupal/hooks.rs | 131 +++++++++++++++++++++- src/drupal/mod.rs | 4 +- src/symbol_map/extraction/class_like.rs | 1 + src/symbol_map/extraction/drupal.rs | 89 +++++++++++++++ src/symbol_map/extraction/mod.rs | 4 + tests/integration/drupal_hooks.rs | 139 ++++++++++++++++++++++++ 6 files changed, 362 insertions(+), 6 deletions(-) diff --git a/src/drupal/hooks.rs b/src/drupal/hooks.rs index 52edc717d..69d42ef73 100644 --- a/src/drupal/hooks.rs +++ b/src/drupal/hooks.rs @@ -7,7 +7,9 @@ //! `_` in an extension's procedural files. The //! extensions come from their `*.info.yml`. +use std::collections::HashMap; use std::path::{Path, PathBuf}; +use std::sync::Arc; use crate::Backend; @@ -35,6 +37,89 @@ pub(crate) enum DrupalFile { /// A file of a test module, which no real site enables: it neither /// documents nor implements a hook. Test, + /// An extension's procedural file, whose global functions named + /// `_` implement hooks. + Procedural(Procedural), +} + +/// The extension a procedural file belongs to, and the names its +/// functions can start with. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct Procedural { + /// The file's own extension, tried first. + pub extension: String, + /// Every extension of the file's group (modules and profiles, or + /// themes), longest name first, tried next. + pub group: Arc<[String]>, + /// The file is a `.post_update.php`, whose functions implement + /// `hook_post_update_NAME` and nothing Drupal collects. + pub post_update: bool, +} + +/// The extensions of the workspace, by directory, and the names of each +/// group, longest first. +#[derive(Default)] +pub(crate) struct ExtensionIndex { + by_dir: HashMap, + modules: Arc<[String]>, + themes: Arc<[String]>, +} + +impl ExtensionIndex { + fn add(&mut self, extensions: Vec) { + for extension in extensions { + self.by_dir + .insert(extension.dir, (extension.name, extension.is_theme)); + } + let group = |is_theme: bool| -> Arc<[String]> { + let mut names: Vec = self + .by_dir + .values() + .filter(|(_, theme)| *theme == is_theme) + .map(|(name, _)| name.clone()) + .collect(); + names.sort_by(|a, b| b.len().cmp(&a.len()).then_with(|| a.cmp(b))); + names.dedup(); + names.into() + }; + self.modules = group(false); + self.themes = group(true); + } + + /// The extension a file belongs to: the one of its nearest ancestor + /// directory holding an `.info.yml`, with that directory. + fn owner<'a>(&'a self, path: &'a Path) -> Option<(&'a Path, &'a str, bool)> { + path.ancestors().skip(1).find_map(|dir| { + self.by_dir + .get(dir) + .map(|(name, is_theme)| (dir, name.as_str(), *is_theme)) + }) + } +} + +/// Whether a file at `relative` to its extension's directory is one the +/// hook collector reads functions from. +fn is_procedural_file(relative: &Path, is_theme: bool) -> bool { + let skipped: &[&str] = if is_theme { + &["tests", "js", "css", "templates"] + } else { + &["tests", "js", "css"] + }; + let in_skipped = relative.parent().is_some_and(|dir| { + dir.components() + .any(|part| skipped.iter().any(|skip| part.as_os_str() == *skip)) + }); + let Some(name) = relative.file_name().and_then(|name| name.to_str()) else { + return false; + }; + let suffixes: &[&str] = if is_theme { + &[".inc", ".theme"] + } else { + &[".module", ".inc", ".install", ".profile"] + }; + let eligible = suffixes.iter().any(|suffix| name.ends_with(suffix)) + || (is_theme && name == "theme-settings.php"); + eligible && !in_skipped } /// Whether the file name at the end of `path` is an extension's @@ -81,7 +166,7 @@ pub(crate) fn hook_class_extension(fqn: &str) -> Option<&str> { impl Backend { /// Remember the extensions the Drupal walk found. pub(crate) fn note_drupal_extensions(&self, extensions: Vec) { - self.drupal.extensions.write().extend(extensions); + self.drupal.extensions.write().add(extensions); } /// What the PHP file at `uri` holds of hooks. @@ -100,10 +185,48 @@ impl Backend { .and_then(|name| name.to_str()) .is_some_and(|name| name.ends_with(".api.php")); if is_api { - DrupalFile::Api - } else { - DrupalFile::Other + return DrupalFile::Api; } + self.procedural_file(&path) + .map_or(DrupalFile::Other, DrupalFile::Procedural) + } + + /// The extension and group of a procedural file, by the extension that + /// owns it, or before the walk has read any `.info.yml` by the file's + /// own name (`acme.module`). + fn procedural_file(&self, path: &Path) -> Option { + let name = path.file_name()?.to_str()?; + let extensions = self.drupal.extensions.read(); + let (extension, is_theme, relative) = match extensions.owner(path) { + Some((dir, extension, is_theme)) => ( + extension.to_string(), + is_theme, + path.strip_prefix(dir).ok()?, + ), + None => { + let (stem, suffix) = name.rsplit_once('.')?; + let is_theme = match suffix { + "module" | "install" | "profile" => false, + "theme" => true, + _ => return None, + }; + (stem.to_string(), is_theme, Path::new(name)) + } + }; + let post_update = !is_theme && name.ends_with(".post_update.php"); + if !post_update && !is_procedural_file(relative, is_theme) { + return None; + } + let group = if is_theme { + Arc::clone(&extensions.themes) + } else { + Arc::clone(&extensions.modules) + }; + Some(Procedural { + extension, + group, + post_update, + }) } } diff --git a/src/drupal/mod.rs b/src/drupal/mod.rs index b2097ea45..980603a8e 100644 --- a/src/drupal/mod.rs +++ b/src/drupal/mod.rs @@ -40,7 +40,7 @@ pub(crate) struct DrupalState { /// implement hooks, which the workspace walk never reaches either. hook_php_files: RwLock>, /// The modules, install profiles, and themes of every web root. - extensions: RwLock>, + extensions: RwLock, } /// A kind of file Drupal reads declarations from. @@ -281,7 +281,7 @@ impl Backend { self.drupal.web_roots.write().clear(); self.drupal.declaration_files.write().clear(); self.drupal.hook_php_files.write().clear(); - self.drupal.extensions.write().clear(); + *self.drupal.extensions.write() = hooks::ExtensionIndex::default(); } /// Remember the declaration files the Drupal walk found: the YAML for diff --git a/src/symbol_map/extraction/class_like.rs b/src/symbol_map/extraction/class_like.rs index 5ac344f14..d325941d0 100644 --- a/src/symbol_map/extraction/class_like.rs +++ b/src/symbol_map/extraction/class_like.rs @@ -924,6 +924,7 @@ pub(super) fn extract_from_function<'a>(func: &'a Function<'a>, ctx: &mut Extrac }, }); emit_drupal_hook_doc_span(func, ctx); + emit_drupal_procedural_hook_span(func, ctx); // Docblock. We extract type spans and template params now, but // defer `@param $var` variable spans until after we know diff --git a/src/symbol_map/extraction/drupal.rs b/src/symbol_map/extraction/drupal.rs index 0bbac7f7d..f612f51d5 100644 --- a/src/symbol_map/extraction/drupal.rs +++ b/src/symbol_map/extraction/drupal.rs @@ -720,6 +720,95 @@ pub(in crate::symbol_map::extraction) fn emit_drupal_hook_doc_span( }); } +/// The attributes that keep Drupal's hook collector from reading a +/// procedural function: it implements its hook through a hook class. +const LEGACY_HOOK_ATTRIBUTES: [&str; 3] = [ + "Drupal\\Core\\Hook\\Attribute\\LegacyHook", + "Drupal\\Core\\Hook\\Attribute\\LegacyModuleImplementsAlter", + "Drupal\\Core\\Hook\\Attribute\\LegacyRequirementsHook", +]; +const SCAN_STOP_ATTRIBUTE: &str = "Drupal\\Core\\Hook\\Attribute\\ProceduralHookScanStop"; + +/// Record the hook a global function of an extension's procedural file +/// implements, named `_` the way Drupal's hook collector +/// reads it: the file's own extension first, then every extension of its +/// group, longest first. The span covers the hook after the extension, so +/// the function's own span still answers on the extension part. +pub(in crate::symbol_map::extraction) fn emit_drupal_procedural_hook_span( + function: &Function<'_>, + ctx: &mut ExtractionCtx<'_>, +) { + let crate::drupal::hooks::DrupalFile::Procedural(procedural) = &ctx.drupal_file else { + return; + }; + if !ctx.drupal || ctx.drupal_hook_scan_stopped { + return; + } + if attributes_named(&function.attribute_lists, SCAN_STOP_ATTRIBUTE, ctx) + .next() + .is_some() + { + ctx.drupal_hook_scan_stopped = true; + return; + } + let is_legacy = LEGACY_HOOK_ATTRIBUTES.iter().any(|legacy| { + attributes_named(&function.attribute_lists, legacy, ctx) + .next() + .is_some() + }); + let namespaced = ctx + .resolved_name_at(function.name.span.start.offset) + .is_some_and(|fqn| fqn.trim_start_matches('\\').contains('\\')); + if is_legacy || namespaced { + return; + } + let name = bytes_to_str(function.name.value); + let hook_of = |extension: &str| { + name.strip_prefix(extension)? + .strip_prefix('_') + .filter(|hook| !hook.is_empty()) + }; + // Only the file's own extension names an update function or a post + // update, which Drupal reads by the extension that owns the file. + let found = if procedural.post_update { + hook_of(&procedural.extension) + .filter(|hook| hook.starts_with("post_update_")) + .map(|hook| (procedural.extension.len(), hook)) + } else { + hook_of(&procedural.extension) + .map(|hook| (procedural.extension.len(), hook)) + .or_else(|| { + procedural.group.iter().find_map(|extension| { + hook_of(extension) + .filter(|hook| !starts_with_update_number(hook)) + .map(|hook| (extension.len(), hook)) + }) + }) + }; + let Some((prefix, hook)) = found else { + return; + }; + let start = function.name.span.start.offset + prefix as u32 + 1; + let key = hook.to_string(); + ctx.spans.push(SymbolSpan { + start, + end: start + key.len() as u32, + kind: SymbolKind::StringKey { + kind: StringKeyKind::DrupalHook, + key, + is_write: false, + is_optional: false, + }, + }); +} + +/// `update_` followed by a digit: an update function, which only the +/// extension that owns the file names. +fn starts_with_update_number(hook: &str) -> bool { + hook.strip_prefix("update_") + .is_some_and(|rest| rest.starts_with(|c: char| c.is_ascii_digit())) +} + /// Record the hooks the `#[Hook]` attributes of a hook class implement: /// one on a method implements the hook for that method, one on the class /// for `__invoke()` or the `method:` it names. Drupal discovers hook diff --git a/src/symbol_map/extraction/mod.rs b/src/symbol_map/extraction/mod.rs index 3fd031da5..b606ad609 100644 --- a/src/symbol_map/extraction/mod.rs +++ b/src/symbol_map/extraction/mod.rs @@ -110,6 +110,9 @@ struct ExtractionCtx<'a> { drupal_routes: Vec, /// What the file holds of Drupal hooks, which its path says. drupal_file: crate::drupal::hooks::DrupalFile, + /// A procedural function with `#[ProceduralHookScanStop]` was read: + /// Drupal's hook collector reads no function after it. + drupal_hook_scan_stopped: bool, /// Current conditional nesting depth (if/else, switch, while, for, etc.). /// Incremented when entering a conditional block, decremented when leaving. cond_nesting_depth: u16, @@ -524,6 +527,7 @@ fn extract_symbol_map_inner( drupal_route_aliases: Vec::new(), drupal_routes: Vec::new(), drupal_file, + drupal_hook_scan_stopped: false, cond_nesting_depth: 0, cond_block_end_stack: Vec::new(), has_laravel_container_attrs: None, diff --git a/tests/integration/drupal_hooks.rs b/tests/integration/drupal_hooks.rs index 11699f99e..20fe66746 100644 --- a/tests/integration/drupal_hooks.rs +++ b/tests/integration/drupal_hooks.rs @@ -109,3 +109,142 @@ async fn a_hook_attribute_goes_to_the_documentation() { "{found:?}" ); } + +const MODULE: &str = r#" Date: Mon, 2 Nov 2026 15:29:25 +0100 Subject: [PATCH 03/13] Navigate between Drupal hook invocations and implementations A module handler's or theme manager's invokeAll(), invoke(), alter(), and the other invoking calls name the hooks they fire, an alter type naming its _alter hook. Go-to-definition on one lists the implementations it reaches, of its group and of the extension invoke() names, and then the documentation; on an implementation it lists the invocations that reach it. Install-time hooks and update functions only link to their documentation. Assisted-by: claude-code/claude-opus-5-5 --- src/blade/typed_receiver.rs | 2 +- src/definition/resolve.rs | 6 + src/drupal/hooks.rs | 240 ++++++++++++++++++ src/mem_audit.rs | 1 + src/symbol_map/extraction/drupal.rs | 189 +++++++++++++- .../extraction/expressions/calls.rs | 1 + src/symbol_map/extraction/mod.rs | 5 + src/symbol_map/map.rs | 3 + src/symbol_map/string_keys.rs | 40 +++ src/symfony/mod.rs | 5 +- src/virtual_members/laravel/string_keys.rs | 4 +- tests/integration/drupal_hooks.rs | 110 ++++++++ 12 files changed, 589 insertions(+), 17 deletions(-) diff --git a/src/blade/typed_receiver.rs b/src/blade/typed_receiver.rs index 913a92d00..31864e6ba 100644 --- a/src/blade/typed_receiver.rs +++ b/src/blade/typed_receiver.rs @@ -120,7 +120,7 @@ impl Backend { /// Resolve the receiver of every candidate site and keep the spans of /// the ones that turn out to be renders. - fn confirm_typed_receiver_sites( + pub(crate) fn confirm_typed_receiver_sites( &self, uri: &str, content: &str, diff --git a/src/definition/resolve.rs b/src/definition/resolve.rs index 5e6cd5ff6..809ffce1e 100644 --- a/src/definition/resolve.rs +++ b/src/definition/resolve.rs @@ -415,6 +415,12 @@ impl Backend { if !self.string_key_kind_enabled(*kind) { return None; } + // A hook leads from where it is fired to where it is + // implemented, and back, as well as to its documentation. + if *kind == crate::symbol_map::StringKeyKind::DrupalHook { + let locs = self.drupal_hook_definitions(uri, symbol.start, key); + return (!locs.is_empty()).then_some(locs); + } let locs = laravel::resolve_laravel_string_key(self, kind, key, uri); if locs.is_empty() { None } else { Some(locs) } } diff --git a/src/drupal/hooks.rs b/src/drupal/hooks.rs index 69d42ef73..4afd005c7 100644 --- a/src/drupal/hooks.rs +++ b/src/drupal/hooks.rs @@ -54,8 +54,22 @@ pub(crate) struct Procedural { /// The file is a `.post_update.php`, whose functions implement /// `hook_post_update_NAME` and nothing Drupal collects. pub post_update: bool, + /// The extension is a theme. + pub is_theme: bool, } +/// Hooks Drupal's hook collector leaves out of procedural code: they are +/// called at install time by name, and only link to their documentation. +pub(crate) const NOT_COLLECTED: [&str; 7] = [ + "install", + "install_tasks", + "install_tasks_alter", + "schema", + "uninstall", + "update_last_removed", + "update_dependencies", +]; + /// The extensions of the workspace, by directory, and the names of each /// group, longest first. #[derive(Default)] @@ -86,6 +100,14 @@ impl ExtensionIndex { self.themes = group(true); } + /// Whether the extension `name` is a theme, when the walk found it. + fn is_theme(&self, name: &str) -> Option { + self.by_dir + .values() + .find(|(known, _)| known == name) + .map(|(_, is_theme)| *is_theme) + } + /// The extension a file belongs to: the one of its nearest ancestor /// directory holding an `.info.yml`, with that directory. fn owner<'a>(&'a self, path: &'a Path) -> Option<(&'a Path, &'a str, bool)> { @@ -226,8 +248,226 @@ impl Backend { extension, group, post_update, + is_theme, + }) + } +} + +/// Where a hook is named, and as what. +#[derive(Debug, Clone)] +struct HookOccurrence { + uri: String, + start: u32, + end: u32, + role: Occurrence, +} + +#[derive(Debug, Clone, PartialEq)] +enum Occurrence { + Documentation, + /// An invocation of the module handler's (or the theme manager's) + /// hooks, limited to one extension when it names one. + Invocation { + themes: bool, + extension: Option, + }, + Implementation { + extension: crate::atom::Atom, + is_theme: bool, + collected: bool, + }, +} + +impl Occurrence { + /// Whether an invocation reaches an implementation: of its group, of + /// the extension it names if any, and collected by Drupal. + fn reaches(invocation: &Self, implementation: &Self) -> bool { + match (invocation, implementation) { + ( + Self::Invocation { themes, extension }, + Self::Implementation { + extension: implemented_for, + is_theme, + collected, + }, + ) => { + *collected + && themes == is_theme + && extension.is_none_or(|extension| extension == *implemented_for) + } + _ => false, + } + } +} + +impl Backend { + /// Every place the hook `key` is named, with what it is there. + fn hook_occurrences(&self, key: &str) -> Vec { + use crate::symbol_map::{HookRole, ReceiverClass, StringKeyKind, SymbolKind}; + let reference_key = + crate::reference_index::string_key_reference_key(StringKeyKind::DrupalHook, key); + let snapshot = self.user_file_symbol_maps_for_reference_keys(&[reference_key]); + let extensions = self.drupal.extensions.read(); + let mut found = Vec::new(); + for (uri, map) in &snapshot { + let typed = if map + .typed_receiver_sites + .iter() + .any(|site| site.may_name(StringKeyKind::DrupalHook, key)) + { + self.typed_receiver_spans_for(uri, map) + } else { + std::sync::Arc::default() + }; + for span in map.spans.iter().chain(typed.iter()) { + let SymbolKind::StringKey { + kind: StringKeyKind::DrupalHook, + key: named, + is_write, + .. + } = &span.kind + else { + continue; + }; + if named != key { + continue; + } + let site = map + .drupal_hooks + .binary_search_by_key(&span.start, |site| site.start) + .ok() + .map(|at| &map.drupal_hooks[at].role); + let role = match site { + None if *is_write => Occurrence::Documentation, + None => continue, + Some(HookRole::Implementation { + extension, + is_theme, + collected, + }) => Occurrence::Implementation { + extension: *extension, + is_theme: is_theme + .or_else(|| extensions.is_theme(extension)) + .unwrap_or(false), + collected: *collected, + }, + Some(HookRole::Invocation { extension }) => { + // A method both have is asked about again as the + // theme manager's. + let themes = map + .typed_receiver_sites + .iter() + .find(|site| site.start == span.start) + .is_some_and(|site| match site.receiver { + ReceiverClass::ThemeHooks => true, + ReceiverClass::Hooks => { + let mut theme = site.clone(); + theme.receiver = ReceiverClass::ThemeHooks; + self.confirms_typed_receiver_site(uri, &theme) + } + _ => false, + }); + Occurrence::Invocation { + themes, + extension: *extension, + } + } + }; + found.push(HookOccurrence { + uri: uri.clone(), + start: span.start, + end: span.end, + role, + }); + } + } + found.sort_by(|a, b| (&a.uri, a.start).cmp(&(&b.uri, b.start))); + found + } + + /// Whether the type engine confirms `site` of the file at `uri`. + fn confirms_typed_receiver_site( + &self, + uri: &str, + site: &crate::symbol_map::TypedReceiverSite, + ) -> bool { + self.get_file_content_arc(uri).is_some_and(|content| { + !self + .confirm_typed_receiver_sites(uri, &content, std::slice::from_ref(site)) + .is_empty() }) } + + /// Where go-to-definition on the hook `key`, named at `start` in `uri`, + /// leads: from an invocation, the implementations it reaches; from an + /// implementation, the invocations that reach it; from the + /// documentation, the implementations; then the documentation. + pub(crate) fn drupal_hook_definitions( + &self, + uri: &str, + start: u32, + key: &str, + ) -> Vec { + let occurrences = self.hook_occurrences(key); + let Some(here) = occurrences + .iter() + .find(|occurrence| occurrence.uri == uri && occurrence.start == start) + .map(|occurrence| occurrence.role.clone()) + else { + return Vec::new(); + }; + let mut targets: Vec<&HookOccurrence> = occurrences + .iter() + .filter(|other| match (&here, &other.role) { + (Occurrence::Invocation { .. }, Occurrence::Implementation { .. }) => { + Occurrence::reaches(&here, &other.role) + } + (Occurrence::Implementation { .. }, Occurrence::Invocation { .. }) => { + Occurrence::reaches(&other.role, &here) + } + (Occurrence::Documentation, Occurrence::Implementation { .. }) => true, + _ => false, + }) + .collect(); + targets.extend(occurrences.iter().filter(|other| { + other.role == Occurrence::Documentation && here != Occurrence::Documentation + })); + self.hook_locations(&targets) + } + + /// The documentation of the hook `key`. + pub(crate) fn drupal_hook_documentation( + &self, + key: &str, + ) -> Vec { + let occurrences = self.hook_occurrences(key); + let docs: Vec<&HookOccurrence> = occurrences + .iter() + .filter(|occurrence| occurrence.role == Occurrence::Documentation) + .collect(); + self.hook_locations(&docs) + } + + fn hook_locations( + &self, + occurrences: &[&HookOccurrence], + ) -> Vec { + occurrences + .iter() + .filter_map(|occurrence| { + let parsed = crate::util::parse_url(&occurrence.uri)?; + let content = self.get_file_content_arc(&occurrence.uri)?; + let lines = crate::text_position::LineIndex::new(&content); + Some(tower_lsp_server::gen_lsp_types::Location { + uri: parsed, + range: tower_lsp_server::gen_lsp_types::Range { + start: lines.position(occurrence.start as usize), + end: lines.position(occurrence.end as usize), + }, + }) + }) + .collect() + } } #[cfg(test)] diff --git a/src/mem_audit.rs b/src/mem_audit.rs index 8aa5429e5..70695f1a4 100644 --- a/src/mem_audit.rs +++ b/src/mem_audit.rs @@ -1404,6 +1404,7 @@ pub(crate) fn report(backend: &Backend, runner_content_bytes: usize) { sym.add(sm.entity_types.capacity() * size_of::()); sym.add(sm.drupal_route_patterns.capacity() * size_of::()); sym.add(sm.drupal_route_aliases.capacity() * size_of::()); + sym.add(sm.drupal_hooks.capacity() * size_of::()); for name in sm .drupal_route_patterns .iter() diff --git a/src/symbol_map/extraction/drupal.rs b/src/symbol_map/extraction/drupal.rs index f612f51d5..3a371dd73 100644 --- a/src/symbol_map/extraction/drupal.rs +++ b/src/symbol_map/extraction/drupal.rs @@ -6,7 +6,10 @@ use super::*; use crate::symbol_map::extraction::symfony::{ RouteDeclaration, RouteName, is_route_attribute, push_route_spans, route_declarations, }; -use crate::symbol_map::{DeclaredParameter, DrupalRouteSite, EntityTypeSite, StringKeyKind}; +use crate::symbol_map::{ + DeclaredParameter, DrupalRouteSite, EntityTypeSite, ReceiverClass, StringKeyKind, + TypedReceiverSite, +}; use crate::virtual_members::laravel::helpers::resolved_string_literal; const CONTENT_STORAGE: &str = "Drupal\\Core\\Entity\\Sql\\SqlContentEntityStorage"; @@ -789,7 +792,18 @@ pub(in crate::symbol_map::extraction) fn emit_drupal_procedural_hook_span( return; }; let start = function.name.span.start.offset + prefix as u32 + 1; + let collected = !procedural.post_update + && !crate::drupal::hooks::NOT_COLLECTED.contains(&hook) + && !starts_with_update_number(hook) + && !ends_with_update_number(name); + let role = crate::symbol_map::HookRole::Implementation { + extension: crate::atom::atom(&name[..prefix]), + is_theme: Some(procedural.is_theme), + collected, + }; let key = hook.to_string(); + ctx.drupal_hooks + .push(crate::symbol_map::HookSite { start, role }); ctx.spans.push(SymbolSpan { start, end: start + key.len() as u32, @@ -802,6 +816,12 @@ pub(in crate::symbol_map::extraction) fn emit_drupal_procedural_hook_span( }); } +/// `update_` followed by digits only, at the end: an update function. +fn ends_with_update_number(name: &str) -> bool { + let digits = name.trim_end_matches(|c: char| c.is_ascii_digit()); + digits.len() < name.len() && digits.ends_with("update_") +} + /// `update_` followed by a digit: an update function, which only the /// extension that owns the file names. fn starts_with_update_number(hook: &str) -> bool { @@ -813,7 +833,8 @@ fn starts_with_update_number(hook: &str) -> bool { /// one on a method implements the hook for that method, one on the class /// for `__invoke()` or the `method:` it names. Drupal discovers hook /// classes under an extension's `src/Hook`, whose namespace is -/// `Drupal\\Hook`. +/// `Drupal\\Hook`; a `module:` argument implements the hook for +/// that extension instead. pub(in crate::symbol_map::extraction) fn emit_drupal_hook_attribute_spans( class: &Class<'_>, ctx: &mut ExtractionCtx<'_>, @@ -821,13 +842,13 @@ pub(in crate::symbol_map::extraction) fn emit_drupal_hook_attribute_spans( if !ctx.drupal || ctx.drupal_file == crate::drupal::hooks::DrupalFile::Test { return; } - let is_hook_class = ctx + let Some(extension) = ctx .resolved_name_at(class.name.span.start.offset) .and_then(crate::drupal::hooks::hook_class_extension) - .is_some(); - if !is_hook_class { + .map(crate::atom::atom) + else { return; - } + }; let methods = class.members.iter().filter_map(|member| match member { ClassLikeMember::Method(method) => Some(&method.attribute_lists), _ => None, @@ -835,16 +856,33 @@ pub(in crate::symbol_map::extraction) fn emit_drupal_hook_attribute_spans( let mut found = Vec::new(); for attribute_lists in std::iter::once(&class.attribute_lists).chain(methods) { for attribute in attributes_named(attribute_lists, HOOK_ATTRIBUTE, ctx) { - let hook = attribute_arguments(attribute, &["hook", "method", "module"]) - .into_iter() - .find(|(parameter, _)| *parameter == "hook") - .and_then(|(_, value)| resolved_string_literal(value, ctx.content)); + let mut hook = None; + let mut module = None; + for (parameter, value) in attribute_arguments(attribute, &["hook", "method", "module"]) + { + match parameter { + "hook" => hook = resolved_string_literal(value, ctx.content), + "module" => module = resolved_string_literal(value, ctx.content), + _ => {} + } + } if let Some((key, start, end)) = hook.filter(|(key, _, _)| !key.is_empty()) { - found.push((key.to_string(), start as u32, end as u32)); + let extension = module + .map(|(module, _, _)| crate::atom::atom(module)) + .unwrap_or(extension); + found.push((key.to_string(), start as u32, end as u32, extension)); } } } - for (key, start, end) in found { + for (key, start, end, extension) in found { + ctx.drupal_hooks.push(crate::symbol_map::HookSite { + start, + role: crate::symbol_map::HookRole::Implementation { + extension, + is_theme: None, + collected: true, + }, + }); ctx.spans.push(SymbolSpan { start, end, @@ -858,6 +896,133 @@ pub(in crate::symbol_map::extraction) fn emit_drupal_hook_attribute_spans( } } +/// A method invoking hooks: where its hook name is, whether that name is +/// an alter type (`form` for `form_alter`), and where the extension it is +/// limited to is. +struct HookInvoker { + method: &'static str, + hook: usize, + alter: bool, + extension: Option, +} + +const fn invoker( + method: &'static str, + hook: usize, + alter: bool, + extension: Option, +) -> HookInvoker { + HookInvoker { + method, + hook, + alter, + extension, + } +} + +/// The module handler's invoking methods, which reach modules and +/// profiles. +const MODULE_INVOKERS: [HookInvoker; 7] = [ + invoker("invoke", 1, false, Some(0)), + invoker("invokeAll", 0, false, None), + invoker("invokeAllWith", 0, false, None), + invoker("invokeDeprecated", 2, false, Some(1)), + invoker("invokeAllDeprecated", 1, false, None), + invoker("alter", 0, true, None), + invoker("alterDeprecated", 1, true, None), +]; + +/// The theme manager's invoking methods, which reach themes. +const THEME_INVOKERS: [HookInvoker; 4] = [ + invoker("invoke", 1, false, Some(0)), + invoker("invokeAllWith", 0, false, None), + invoker("alter", 0, true, None), + invoker("alterForTheme", 1, true, None), +]; + +/// Record the hooks a call may invoke, for the type engine to confirm +/// against its receiver: a module handler's or a theme manager's. An +/// alter type names the hook `_alter`, and a list of them names one +/// hook each. +pub(in crate::symbol_map::extraction) fn record_drupal_hook_invocations( + member_name: &str, + argument_list: &ArgumentList<'_>, + ctx: &mut ExtractionCtx<'_>, +) { + if !ctx.drupal + || !matches!( + member_name.as_bytes().first(), + Some(b'i' | b'a' | b'I' | b'A') + ) + { + return; + } + let positional: Vec<&Expression<'_>> = argument_list + .arguments + .iter() + .map_while(|argument| match argument { + Argument::Positional(positional) => Some(positional.value), + Argument::Named(_) => None, + }) + .collect(); + let find = |invokers: &'static [HookInvoker]| { + invokers + .iter() + .find(|invoker| invoker.method.eq_ignore_ascii_case(member_name)) + }; + // The methods both have take their arguments in the same places; only + // the type engine tells which one a call is on. + let (invoker, receiver) = match (find(&MODULE_INVOKERS), find(&THEME_INVOKERS)) { + (Some(invoker), Some(_)) => (invoker, ReceiverClass::Hooks), + (Some(invoker), None) => (invoker, ReceiverClass::ModuleHooks), + (None, Some(invoker)) => (invoker, ReceiverClass::ThemeHooks), + (None, None) => return, + }; + let Some(hook) = positional.get(invoker.hook) else { + return; + }; + let extension = invoker + .extension + .and_then(|at| positional.get(at)) + .and_then(|value| resolved_string_literal(value, ctx.content)) + .map(|(extension, _, _)| crate::atom::atom(extension)); + let names: Vec<(String, usize, usize)> = match hook { + Expression::Array(array) if invoker.alter => array + .elements + .iter() + .filter_map(|element| match element { + ArrayElement::Value(value) => resolved_string_literal(value.value, ctx.content) + .map(|(name, start, end)| (name.to_string(), start, end)), + _ => None, + }) + .collect(), + _ => resolved_string_literal(hook, ctx.content) + .map(|(name, start, end)| (name.to_string(), start, end)) + .into_iter() + .collect(), + }; + for (name, start, end) in names.into_iter().filter(|(name, _, _)| !name.is_empty()) { + let key = if invoker.alter { + format!("{name}_alter") + } else { + name + }; + ctx.typed_receiver_sites.push(TypedReceiverSite { + start: start as u32, + end: end as u32, + key, + is_write: true, + is_optional: false, + guards: 0, + receiver, + }); + ctx.drupal_hooks.push(crate::symbol_map::HookSite { + start: start as u32, + role: crate::symbol_map::HookRole::Invocation { extension }, + }); + } +} + /// Record the route name `new Url('…')` reads. pub(in crate::symbol_map::extraction) fn emit_drupal_url_spans( class: &Expression<'_>, diff --git a/src/symbol_map/extraction/expressions/calls.rs b/src/symbol_map/extraction/expressions/calls.rs index 311ada797..6ab2aa989 100644 --- a/src/symbol_map/extraction/expressions/calls.rs +++ b/src/symbol_map/extraction/expressions/calls.rs @@ -489,6 +489,7 @@ fn extract_method_call<'a>( emit_laravel_method_call_spans(object, &member_name, argument_list, ctx); record_symfony_receiver_sites(object, &member_name, argument_list, ctx); record_drupal_route_parameter_sites(&member_name, argument_list, ctx); + record_drupal_hook_invocations(&member_name, argument_list, ctx); record_yii_view_sites(object, &member_name, argument_list, ctx); emit_mock_method_spans(object, &member_name, argument_list, ctx); // Recorded with `->` for both spellings so callable resolution diff --git a/src/symbol_map/extraction/mod.rs b/src/symbol_map/extraction/mod.rs index b606ad609..52a1fecba 100644 --- a/src/symbol_map/extraction/mod.rs +++ b/src/symbol_map/extraction/mod.rs @@ -108,6 +108,7 @@ struct ExtractionCtx<'a> { drupal_route_aliases: Vec, /// The Drupal routes the file's `#[Route]` attributes declare. drupal_routes: Vec, + drupal_hooks: Vec, /// What the file holds of Drupal hooks, which its path says. drupal_file: crate::drupal::hooks::DrupalFile, /// A procedural function with `#[ProceduralHookScanStop]` was read: @@ -526,6 +527,7 @@ fn extract_symbol_map_inner( drupal_route_patterns: Vec::new(), drupal_route_aliases: Vec::new(), drupal_routes: Vec::new(), + drupal_hooks: Vec::new(), drupal_file, drupal_hook_scan_stopped: false, cond_nesting_depth: 0, @@ -680,6 +682,8 @@ fn extract_symbol_map_inner( ctx.morph_column_sites.sort_by_key(|s| s.start); ctx.container_values.sort_by_key(|s| s.start); ctx.entity_types.sort_by_key(|s| s.start); + ctx.drupal_hooks.sort_by_key(|s| s.start); + ctx.drupal_hooks.dedup_by_key(|s| s.start); ctx.routes.shrink_to_fit(); let mut member_access_indices: crate::atom::AtomMap> = @@ -724,6 +728,7 @@ fn extract_symbol_map_inner( drupal_route_patterns: ctx.drupal_route_patterns, drupal_route_aliases: ctx.drupal_route_aliases, drupal_routes: ctx.drupal_routes, + drupal_hooks: ctx.drupal_hooks, routes: ctx.routes, declares_twig_loader: ctx.declares_twig_loader, route_imports: ctx diff --git a/src/symbol_map/map.rs b/src/symbol_map/map.rs index 841a102c9..ac7b00a78 100644 --- a/src/symbol_map/map.rs +++ b/src/symbol_map/map.rs @@ -320,6 +320,9 @@ pub(crate) struct SymbolMap { /// The Drupal routes the file declares, in source order. Empty for /// every file that declares none. pub drupal_routes: Vec, + /// What each Drupal hook implementation and invocation of the file is, + /// sorted by `start`. Empty for every file that has none. + pub drupal_hooks: Vec, /// The routes the file declares, in source order. Empty for every file /// that declares none. pub routes: Vec, diff --git a/src/symbol_map/string_keys.rs b/src/symbol_map/string_keys.rs index 9fb49033d..2796485e1 100644 --- a/src/symbol_map/string_keys.rs +++ b/src/symbol_map/string_keys.rs @@ -619,6 +619,30 @@ pub(crate) struct ContainerValueSite { pub value: ContainerValue, } +/// What a Drupal hook span is, keyed back to its [`SymbolKind::StringKey`] +/// span by `start`. A hook span with no site is the documentation. +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct HookSite { + pub start: u32, + pub role: HookRole, +} + +#[derive(Debug, Clone, PartialEq)] +pub(crate) enum HookRole { + /// An implementation, for `extension`. Whether that extension is a + /// theme is known for a procedural function, whose file says, and + /// looked up by name for a hook class. An implementation Drupal does + /// not collect (an install-time hook, an update function) only links + /// to its documentation. + Implementation { + extension: Atom, + is_theme: Option, + collected: bool, + }, + /// An invocation, limited to `extension` when it names one. + Invocation { extension: Option }, +} + /// What a Drupal entity type declaration gives its ID: the entity class, /// and the storage handler that loads it. #[derive(Debug, Clone, PartialEq)] @@ -822,6 +846,8 @@ const DRUPAL_FORM_BASE: &str = "Drupal\\Core\\Form\\FormBase"; const DRUPAL_ENTITY_CONTROLLER: &str = "Drupal\\Core\\Entity\\Controller\\EntityController"; const DRUPAL_ACCESS_MANAGER: &str = "Drupal\\Core\\Access\\AccessManagerInterface"; const DRUPAL_ROUTE_PROVIDER: &str = "Drupal\\Core\\Routing\\RouteProviderInterface"; +pub(crate) const DRUPAL_MODULE_HANDLER: &str = "Drupal\\Core\\Extension\\ModuleHandlerInterface"; +pub(crate) const DRUPAL_THEME_MANAGER: &str = "Drupal\\Core\\Theme\\ThemeManagerInterface"; /// The namespace of the PHP configurators that build Symfony's service /// container. @@ -940,6 +966,14 @@ pub(crate) enum ReceiverClass { /// Drupal's access manager, for the route parameters its /// `checkNamedRoute()` takes after the route name. AccessCheckParameters, + /// Drupal's module handler, whose `invokeAll()`, `alter()`, and the + /// rest invoke the hooks of modules and install profiles. + ModuleHooks, + /// Drupal's theme manager, whose `invoke()`, `alter()`, and the rest + /// invoke the hooks of themes. + ThemeHooks, + /// Either of the two, for the methods they share. + Hooks, } impl ReceiverClass { @@ -1053,6 +1087,12 @@ impl ReceiverClass { Self::AccessCheckParameters => { &[(DRUPAL_ACCESS_MANAGER, StringKeyKind::DrupalRouteParameter)] } + Self::ModuleHooks => &[(DRUPAL_MODULE_HANDLER, StringKeyKind::DrupalHook)], + Self::ThemeHooks => &[(DRUPAL_THEME_MANAGER, StringKeyKind::DrupalHook)], + Self::Hooks => &[ + (DRUPAL_MODULE_HANDLER, StringKeyKind::DrupalHook), + (DRUPAL_THEME_MANAGER, StringKeyKind::DrupalHook), + ], } } diff --git a/src/symfony/mod.rs b/src/symfony/mod.rs index 23982c2ef..582ed056b 100644 --- a/src/symfony/mod.rs +++ b/src/symfony/mod.rs @@ -282,9 +282,10 @@ impl Backend { let mut uris: Vec = self.symfony.declaring_uris.read().iter().cloned().collect(); uris.sort_unstable(); // Only Symfony declares names through calls the type engine has to - // confirm; a Drupal project records none. + // confirm, and Drupal its hooks, which the calls that invoke them + // declare; Drupal's other names never are. let needs_typed = match kind.framework() { - KeyFramework::Drupal => false, + KeyFramework::Drupal => kind == StringKeyKind::DrupalHook, KeyFramework::ServiceContainer => self.is_symfony_project(), KeyFramework::Laravel | KeyFramework::Symfony diff --git a/src/virtual_members/laravel/string_keys.rs b/src/virtual_members/laravel/string_keys.rs index cf9225cd1..c7807d560 100644 --- a/src/virtual_members/laravel/string_keys.rs +++ b/src/virtual_members/laravel/string_keys.rs @@ -80,8 +80,8 @@ pub(crate) fn resolve_laravel_string_key( | StringKeyKind::YiiRoute | StringKeyKind::DrupalEntityType | StringKeyKind::DrupalRoute - | StringKeyKind::DrupalRouteParameter - | StringKeyKind::DrupalHook => backend.symfony_declaration_locations(*kind, key), + | StringKeyKind::DrupalRouteParameter => backend.symfony_declaration_locations(*kind, key), + StringKeyKind::DrupalHook => backend.drupal_hook_documentation(key), StringKeyKind::YiiView => backend.resolve_yii_view_definitions(key), StringKeyKind::TwigBlock => backend.twig_block_definitions(uri, None, key), StringKeyKind::TwigVariable => backend.twig_variable_definitions(uri, None, key), diff --git a/tests/integration/drupal_hooks.rs b/tests/integration/drupal_hooks.rs index 20fe66746..b9d6fefec 100644 --- a/tests/integration/drupal_hooks.rs +++ b/tests/integration/drupal_hooks.rs @@ -248,3 +248,113 @@ async fn install_files_themes_and_other_extensions_implement_hooks() { "{found:?}" ); } + +const INVOKER_STUBS: &[(&str, &str)] = &[ + ( + "web/core/lib/Drupal/Core/Extension/ModuleHandlerInterface.php", + "modules->invokeAll('cron'); + $this->modules->alter('form', $form); + $this->themes->alter(['form', 'other'], $form); + $this->modules->invoke('acme', 'help'); + $this->modules->invoke('other', 'help'); + $this->modules->invoke('acme', 'install'); + } +} +"#; +const INVOKER_PATH: &str = "web/modules/custom/acme/src/Invoker.php"; + +/// An invocation leads to the implementations of its group and extension +/// that Drupal collects, an implementation to the invocations that reach +/// it, and both to the documentation. +#[tokio::test] +async fn invocations_and_implementations_lead_to_each_other() { + let (backend, dir) = drupal( + &[ + INVOKER_STUBS[0], + INVOKER_STUBS[1], + (INVOKED_API_PATH, INVOKED_API), + ("web/core/themes/olivero/olivero.info.yml", "type: theme\n"), + ], + &[ + ("web/modules/custom/acme/acme.module", INVOKED_MODULE), + ("web/modules/custom/acme/acme.install", INVOKED_INSTALL), + ("web/core/themes/olivero/olivero.theme", INVOKED_THEME), + (INVOKER_PATH, INVOKER), + ], + ) + .await; + let php = uri_for(&dir, INVOKER_PATH); + let api = uri_for(&dir, INVOKED_API_PATH); + let module = uri_for(&dir, "web/modules/custom/acme/acme.module"); + let theme = uri_for(&dir, "web/core/themes/olivero/olivero.theme"); + references_at(&backend, &php, position_in(INVOKER, "'cron'", 2)).await; + + let at = |needle: &str| { + let inside = needle.rfind('\'').unwrap() - 1; + position_in(INVOKER, needle, inside) + }; + let lines = |found: Vec| { + found + .into_iter() + .map(|location| (location.uri, location.range.start.line)) + .collect::>() + }; + for (needle, expected) in [ + ( + "invokeAll('cron'", + vec![(module.clone(), 1), (api.clone(), 1)], + ), + ( + "modules->alter('form'", + vec![(module.clone(), 3), (api.clone(), 3)], + ), + ( + "themes->alter(['form'", + vec![(theme.clone(), 1), (api.clone(), 3)], + ), + ( + "invoke('acme', 'help'", + vec![(module.clone(), 5), (api.clone(), 5)], + ), + ("invoke('other', 'help'", vec![(api.clone(), 5)]), + ("invoke('acme', 'install'", vec![(api.clone(), 7)]), + ] { + let found = lines(definitions_at(&backend, &php, at(needle)).await); + assert_eq!(found, expected, "{needle}"); + } + + let found = lines( + definitions_at( + &backend, + &module, + position_in(INVOKED_MODULE, "acme_cron", 6), + ) + .await, + ); + assert_eq!(found, vec![(php, 10), (api, 1)]); +} From 9749b18421ba7e19f1093b2f367c6ed03feee435 Mon Sep 17 00:00:00 2001 From: "Giuseppe.Esposito" Date: Mon, 2 Nov 2026 15:34:16 +0100 Subject: [PATCH 04/13] Link Drupal hooks with computed names to their documented patterns An implementation of a hook core names from parts, such as acme_form_user_login_form_alter or acme_node_presave, reaches the documented pattern it fills in, hook_form_FORM_ID_alter or hook_ENTITY_TYPE_presave, where ENTITY_TYPE has to be a declared entity type. The pattern's references and go-to-definition list the implementations of every hook it stands for. Assisted-by: claude-code/claude-opus-5-5 --- src/drupal/hooks.rs | 252 ++++++++++++++++++++- src/virtual_members/laravel/string_keys.rs | 7 +- tests/integration/drupal_hooks.rs | 55 +++++ 3 files changed, 303 insertions(+), 11 deletions(-) diff --git a/src/drupal/hooks.rs b/src/drupal/hooks.rs index 4afd005c7..9a66189bc 100644 --- a/src/drupal/hooks.rs +++ b/src/drupal/hooks.rs @@ -253,6 +253,92 @@ impl Backend { } } +/// Whether a documented hook name has placeholders, written in uppercase +/// (`form_FORM_ID_alter`): it stands for every hook that fills them in. +pub(crate) fn is_hook_pattern(name: &str) -> bool { + name.bytes().any(|byte| byte.is_ascii_uppercase()) +} + +/// One piece of a hook name pattern. +enum Piece<'a> { + Text(&'a str), + /// An uppercase run, such as `FORM_ID` or `ENTITY_TYPE`. + Placeholder(&'a str), +} + +/// A pattern split into text and placeholders: an uppercase run starts a +/// placeholder at the start of the pattern or after an underscore, and goes +/// on through underscores followed by more uppercase. +fn pieces(pattern: &str) -> Vec> { + let bytes = pattern.as_bytes(); + let mut pieces = Vec::new(); + let (mut text_start, mut i) = (0, 0); + while i < bytes.len() { + let starts = bytes[i].is_ascii_uppercase() && (i == 0 || bytes[i - 1] == b'_'); + if !starts { + i += 1; + continue; + } + let mut end = i; + loop { + while end < bytes.len() + && (bytes[end].is_ascii_uppercase() || bytes[end].is_ascii_digit()) + { + end += 1; + } + if end + 1 < bytes.len() && bytes[end] == b'_' && bytes[end + 1].is_ascii_uppercase() { + end += 1; + } else { + break; + } + } + if text_start < i { + pieces.push(Piece::Text(&pattern[text_start..i])); + } + pieces.push(Piece::Placeholder(&pattern[i..end])); + text_start = end; + i = end; + } + if text_start < bytes.len() { + pieces.push(Piece::Text(&pattern[text_start..])); + } + pieces +} + +/// Whether `hook` is one the documented `pattern` stands for: `ENTITY_TYPE` +/// is an entity type ID `is_entity_type` accepts, `N` digits, and any other +/// placeholder a run of lowercase letters, digits, and underscores. +pub(crate) fn matches_hook_pattern( + pattern: &str, + hook: &str, + is_entity_type: &dyn Fn(&str) -> bool, +) -> bool { + fn rest(pieces: &[Piece<'_>], hook: &str, is_entity_type: &dyn Fn(&str) -> bool) -> bool { + let Some((first, others)) = pieces.split_first() else { + return hook.is_empty(); + }; + match first { + Piece::Text(text) => hook + .strip_prefix(text) + .is_some_and(|after| rest(others, after, is_entity_type)), + Piece::Placeholder(name) => (1..=hook.len()) + .filter(|length| hook.is_char_boundary(*length)) + .any(|length| { + let (value, after) = hook.split_at(length); + let fits = match *name { + "ENTITY_TYPE" => is_entity_type(value), + "N" => value.bytes().all(|byte| byte.is_ascii_digit()), + _ => value.bytes().all(|byte| { + byte.is_ascii_lowercase() || byte.is_ascii_digit() || byte == b'_' + }), + }; + fits && rest(others, after, is_entity_type) + }), + } + } + rest(&pieces(pattern), hook, is_entity_type) +} + /// Where a hook is named, and as what. #[derive(Debug, Clone)] struct HookOccurrence { @@ -401,7 +487,9 @@ impl Backend { /// Where go-to-definition on the hook `key`, named at `start` in `uri`, /// leads: from an invocation, the implementations it reaches; from an /// implementation, the invocations that reach it; from the - /// documentation, the implementations; then the documentation. + /// documentation, the implementations, those of every hook a documented + /// pattern stands for included; then the documentation, the patterns + /// the hook matches when none documents it by name. pub(crate) fn drupal_hook_definitions( &self, uri: &str, @@ -416,7 +504,23 @@ impl Backend { else { return Vec::new(); }; - let mut targets: Vec<&HookOccurrence> = occurrences + if here == Occurrence::Documentation { + let implemented: Vec = if is_hook_pattern(key) { + self.hooks_matching(key) + .iter() + .flat_map(|hook| self.hook_occurrences(hook)) + .collect() + } else { + occurrences + }; + let targets: Vec<&HookOccurrence> = implemented + .iter() + .filter(|other| matches!(other.role, Occurrence::Implementation { .. })) + .collect(); + return self.hook_locations(&targets); + } + let docs = self.hook_documentation_occurrences(key, &occurrences); + let targets: Vec<&HookOccurrence> = occurrences .iter() .filter(|other| match (&here, &other.role) { (Occurrence::Invocation { .. }, Occurrence::Implementation { .. }) => { @@ -425,27 +529,123 @@ impl Backend { (Occurrence::Implementation { .. }, Occurrence::Invocation { .. }) => { Occurrence::reaches(&other.role, &here) } - (Occurrence::Documentation, Occurrence::Implementation { .. }) => true, _ => false, }) + .chain(docs.iter()) .collect(); - targets.extend(occurrences.iter().filter(|other| { - other.role == Occurrence::Documentation && here != Occurrence::Documentation - })); self.hook_locations(&targets) } - /// The documentation of the hook `key`. + /// The documentation of the hook `key`: its own, or that of every + /// documented pattern it matches. pub(crate) fn drupal_hook_documentation( &self, key: &str, ) -> Vec { let occurrences = self.hook_occurrences(key); - let docs: Vec<&HookOccurrence> = occurrences + let docs = self.hook_documentation_occurrences(key, &occurrences); + self.hook_locations(&docs.iter().collect::>()) + } + + /// Every place the hook `key` is named, and for a documented pattern + /// every place a hook it stands for is, with the documentation of the + /// patterns a hook matches when `include_declaration` asks for it. + pub(crate) fn drupal_hook_references( + &self, + key: &str, + include_declaration: bool, + ) -> Vec { + let mut found = self.hook_occurrences(key); + if is_hook_pattern(key) { + for hook in self.hooks_matching(key) { + found.extend(self.hook_occurrences(&hook)); + } + } else if include_declaration && !found.iter().any(|o| o.role == Occurrence::Documentation) + { + found.extend(self.hook_documentation_occurrences(key, &[])); + } + if !include_declaration { + found.retain(|occurrence| { + !matches!( + occurrence.role, + Occurrence::Documentation | Occurrence::Invocation { .. } + ) + }); + } + found.sort_by(|a, b| (&a.uri, a.start).cmp(&(&b.uri, b.start))); + found.dedup_by(|a, b| a.uri == b.uri && a.start == b.start); + self.hook_locations(&found.iter().collect::>()) + } + + /// The documentation among `occurrences` of `key`, else that of every + /// documented pattern `key` matches. + fn hook_documentation_occurrences( + &self, + key: &str, + occurrences: &[HookOccurrence], + ) -> Vec { + let own: Vec = occurrences .iter() .filter(|occurrence| occurrence.role == Occurrence::Documentation) + .cloned() .collect(); - self.hook_locations(&docs) + if !own.is_empty() || is_hook_pattern(key) { + return own; + } + let is_entity_type = |id: &str| !self.drupal_entity_types(id).is_empty(); + self.documented_hook_patterns() + .iter() + .filter(|pattern| matches_hook_pattern(pattern, key, &is_entity_type)) + .flat_map(|pattern| self.hook_occurrences(pattern)) + .filter(|occurrence| occurrence.role == Occurrence::Documentation) + .collect() + } + + /// The documented hook names with placeholders, sorted. + fn documented_hook_patterns(&self) -> Vec { + use crate::symbol_map::{StringKeyKind, SymbolKind}; + let mut patterns = Vec::new(); + self.visit_declaring_symbol_maps(|_, map| { + for span in &map.spans { + if let SymbolKind::StringKey { + kind: StringKeyKind::DrupalHook, + key, + is_write: true, + .. + } = &span.kind + && is_hook_pattern(key) + { + patterns.push(key.clone()); + } + } + }); + patterns.sort_unstable(); + patterns.dedup(); + patterns + } + + /// Every hook the workspace names, implemented or documented, that the + /// documented `pattern` stands for. + fn hooks_matching(&self, pattern: &str) -> Vec { + use crate::symbol_map::{StringKeyKind, SymbolKind}; + let is_entity_type = |id: &str| !self.drupal_entity_types(id).is_empty(); + let mut hooks: Vec = Vec::new(); + for (_, map) in self.user_file_symbol_maps() { + for span in &map.spans { + if let SymbolKind::StringKey { + kind: StringKeyKind::DrupalHook, + key, + .. + } = &span.kind + && !is_hook_pattern(key) + && !hooks.contains(key) + && matches_hook_pattern(pattern, key, &is_entity_type) + { + hooks.push(key.clone()); + } + } + } + hooks } fn hook_locations( @@ -474,6 +674,40 @@ impl Backend { mod tests { use super::*; + #[test] + fn patterns_fill_their_placeholders() { + let entity = |id: &str| matches!(id, "node" | "taxonomy_term"); + assert!(matches_hook_pattern( + "form_FORM_ID_alter", + "form_user_login_form_alter", + &entity + )); + assert!(matches_hook_pattern( + "ENTITY_TYPE_presave", + "taxonomy_term_presave", + &entity + )); + assert!(!matches_hook_pattern( + "ENTITY_TYPE_presave", + "acme_presave", + &entity + )); + assert!(matches_hook_pattern("update_N", "update_10001", &entity)); + assert!(!matches_hook_pattern("update_N", "update_last", &entity)); + assert!(matches_hook_pattern( + "entity_query_tag__ENTITY_TYPE__TAG_alter", + "entity_query_tag__node__acme_alter", + &entity + )); + assert!(!matches_hook_pattern( + "form_FORM_ID_alter", + "form_alter", + &entity + )); + assert!(is_hook_pattern("preprocess_HOOK")); + assert!(!is_hook_pattern("preprocess_page")); + } + #[test] fn hook_classes_belong_to_the_extension_of_their_namespace() { assert_eq!( diff --git a/src/virtual_members/laravel/string_keys.rs b/src/virtual_members/laravel/string_keys.rs index c7807d560..6fdf0e729 100644 --- a/src/virtual_members/laravel/string_keys.rs +++ b/src/virtual_members/laravel/string_keys.rs @@ -319,10 +319,13 @@ pub(crate) fn find_laravel_string_key_references( | StringKeyKind::YiiRoute | StringKeyKind::DrupalEntityType | StringKeyKind::DrupalRoute - | StringKeyKind::DrupalRouteParameter - | StringKeyKind::DrupalHook => { + | StringKeyKind::DrupalRouteParameter => { return find_string_key_usages(kind, key, backend, snapshot, include_declaration); } + // A documented pattern stands for every hook that fills it in. + StringKeyKind::DrupalHook => { + return backend.drupal_hook_references(key, include_declaration); + } // The catalogues named after a domain declare it, and the paths of // its handler's routes a route argument, and are added below. A // route's name may be put together by the imports that load it, or diff --git a/tests/integration/drupal_hooks.rs b/tests/integration/drupal_hooks.rs index b9d6fefec..c1f7c22cd 100644 --- a/tests/integration/drupal_hooks.rs +++ b/tests/integration/drupal_hooks.rs @@ -358,3 +358,58 @@ async fn invocations_and_implementations_lead_to_each_other() { ); assert_eq!(found, vec![(php, 10), (api, 1)]); } + +const PATTERN_API: &str = " Date: Mon, 2 Nov 2026 15:42:36 +0100 Subject: [PATCH 05/13] Complete Drupal hook implementations and hook names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Naming a function in a module's or theme's procedural file offers _ for every documented hook, inserting the documented parameters and a tab stop for each placeholder of a pattern. Hook names complete in #[Hook('…')] and in the module handler's and theme manager's invoking calls, an alter call's without its _alter. Assisted-by: claude-code/claude-opus-5-5 --- src/completion/drupal_hooks.rs | 234 ++++++++++++++++++++++ src/completion/handler/mod.rs | 14 ++ src/completion/laravel_string_keys/mod.rs | 6 +- src/completion/mod.rs | 1 + src/completion/symfony_string_keys.rs | 62 ++++++ src/drupal/hooks.rs | 21 +- src/symbol_map/extraction/drupal.rs | 16 ++ src/symbol_map/extraction/mod.rs | 1 + tests/integration/drupal_hooks.rs | 146 ++++++++++++++ 9 files changed, 493 insertions(+), 8 deletions(-) create mode 100644 src/completion/drupal_hooks.rs diff --git a/src/completion/drupal_hooks.rs b/src/completion/drupal_hooks.rs new file mode 100644 index 000000000..b9579055e --- /dev/null +++ b/src/completion/drupal_hooks.rs @@ -0,0 +1,234 @@ +//! Completion of Drupal hook implementations: typing a function name in an +//! extension's procedural file offers `_` for every +//! documented hook, with the documented parameters. + +use tower_lsp_server::gen_lsp_types::{ + CompletionItem, CompletionItemKind, InsertTextFormat, Position, Range, TextEdit, +}; + +use crate::Backend; +use crate::drupal::hooks::DrupalFile; +use crate::types::FileContext; + +/// The function name being typed after `function ` at `cursor`, when the +/// line holds nothing else: what is typed so far, and where it starts. +fn function_name_at(content: &str, cursor: usize) -> Option<(&str, usize)> { + let before = content.get(..cursor)?; + let line_start = before.rfind('\n').map_or(0, |index| index + 1); + let line = &before[line_start..]; + let after_keyword = line.trim_start().strip_prefix("function")?; + let typed = after_keyword.trim_start(); + let starts_with_space = after_keyword.len() > typed.len(); + let is_name = typed + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'_'); + (starts_with_space && is_name).then(|| (typed, cursor - typed.len())) +} + +/// Whether the line that starts at `line_start` starts a statement, rather +/// than continuing an expression: the last line before it that is not +/// blank, a comment, or an attribute ends a statement or opens or closes a +/// block, or there is none but the opening tag. A closure written on a +/// line of its own inside a call's arguments is not a declaration. +fn starts_statement(content: &str, line_start: usize) -> bool { + for line in content[..line_start].lines().rev() { + let line = line.trim(); + if line.is_empty() + || line.starts_with("//") + || line.starts_with('#') + || line.starts_with('*') + || line.starts_with("/*") + || line.ends_with("*/") + { + continue; + } + return line.starts_with(" String { + let mut escaped = String::with_capacity(text.len()); + for c in text.chars() { + if matches!(c, '$' | '}' | '\\') { + escaped.push('\\'); + } + escaped.push(c); + } + escaped +} + +/// The hook name as a snippet: each placeholder of a documented pattern +/// (`FORM_ID`) is a tab stop holding its name. +fn hook_snippet(hook: &str) -> (String, usize) { + let mut snippet = String::new(); + let mut stops = 0; + let bytes = hook.as_bytes(); + let mut i = 0; + while i < bytes.len() { + let starts = bytes[i].is_ascii_uppercase() && (i == 0 || bytes[i - 1] == b'_'); + if !starts { + snippet.push(bytes[i] as char); + i += 1; + continue; + } + let mut end = i; + loop { + while end < bytes.len() + && (bytes[end].is_ascii_uppercase() || bytes[end].is_ascii_digit()) + { + end += 1; + } + if end + 1 < bytes.len() && bytes[end] == b'_' && bytes[end + 1].is_ascii_uppercase() { + end += 1; + } else { + break; + } + } + stops += 1; + snippet.push_str(&format!("${{{stops}:{}}}", &hook[i..end])); + i = end; + } + (snippet, stops) +} + +impl Backend { + /// The hook implementations a function name typed at `cursor` in an + /// extension's procedural file can be: only for a function declared at + /// the top level, not a method, a nested function, or a closure, and + /// only when one of them matches, so that every other completion still + /// answers there. + pub(crate) fn drupal_hook_function_completion( + &self, + uri: &str, + content: &str, + cursor: usize, + position: Position, + ctx: &FileContext, + ) -> Option> { + let DrupalFile::Procedural(procedural) = self.drupal_file(uri) else { + return None; + }; + let (typed, start) = function_name_at(content, cursor)?; + let line_start = content[..cursor].rfind('\n').map_or(0, |index| index + 1); + let offset = u32::try_from(cursor).unwrap_or(u32::MAX); + let in_body = crate::class_lookup::find_class_at_offset(&ctx.classes, offset).is_some() + || self + .symbol_map_for(uri) + .is_some_and(|map| map.find_enclosing_scope(offset) != 0); + if in_body || !starts_statement(content, line_start) { + return None; + } + let start_position = crate::text_position::offset_to_position(content, start); + let mut items = Vec::new(); + for hook in self.drupal_documented_hooks() { + let name = format!("{}_{hook}", procedural.extension); + let matches_typed = name + .as_bytes() + .get(..typed.len()) + .is_some_and(|head| head.eq_ignore_ascii_case(typed.as_bytes())); + if !matches_typed { + continue; + } + let doc = format!("hook_{hook}"); + let parameters = self + .find_or_load_function(&[doc.as_str()]) + .map(|function| self.hook_parameters(&function.parameters, ctx)) + .unwrap_or_default(); + let (hook_part, stops) = hook_snippet(&hook); + let snippet = format!( + "{}_{hook_part}({}) {{\n\t${}\n}}", + snippet_text(&procedural.extension), + snippet_text(¶meters), + stops + 1 + ); + items.push(CompletionItem { + label: name.clone(), + kind: Some(CompletionItemKind::Function), + detail: Some(format!("Implements {doc}()")), + filter_text: Some(name), + insert_text_format: Some(InsertTextFormat::Snippet), + text_edit: Some( + tower_lsp_server::gen_lsp_types::CompletionItemTextEdit::TextEdit(TextEdit { + range: Range { + start: start_position, + end: position, + }, + new_text: snippet, + }), + ), + ..CompletionItem::default() + }); + } + (!items.is_empty()).then_some(items) + } + + /// The documented parameters, as an implementation writes them: the + /// native hints, shortened by the file's imports. + fn hook_parameters( + &self, + parameters: &[crate::types::ParameterInfo], + ctx: &FileContext, + ) -> String { + parameters + .iter() + .map(|parameter| { + let hint = parameter + .native_type_hint + .as_ref() + .map(|hint| { + hint.resolve_names(&|name| { + crate::names::shorten_from_fqn(name, &ctx.use_map, &ctx.namespace) + }) + .to_string() + }) + .map(|hint| format!("{hint} ")) + .unwrap_or_default(); + let sigil = if parameter.is_variadic { + "..." + } else if parameter.is_reference { + "&" + } else { + "" + }; + let default = parameter + .default_value + .as_ref() + .filter(|_| !parameter.is_required && !parameter.is_variadic) + .map(|value| format!(" = {value}")) + .unwrap_or_default(); + format!("{hint}{sigil}{}{default}", parameter.name) + }) + .collect::>() + .join(", ") + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn only_a_function_name_after_the_keyword_is_completed() { + let php = " self.symfony_declared_names(*kind).into(), + | StringKeyKind::DrupalRoute => self.symfony_declared_names(*kind).into(), StringKeyKind::TwigFilter | StringKeyKind::TwigFunction => { self.twig_callable_names(*kind).into() } + // Only the documentation, not the invocations the type engine + // would have to confirm in every file that fires a hook. + StringKeyKind::DrupalHook => self.drupal_documented_hooks().into(), // A translation key is completed in the domain it is looked up // in, a configuration key under the key it is nested in, which // only the Symfony completion knows, and a route argument from diff --git a/src/completion/mod.rs b/src/completion/mod.rs index e3264f13a..3fb2df82f 100644 --- a/src/completion/mod.rs +++ b/src/completion/mod.rs @@ -78,6 +78,7 @@ pub(crate) mod array_callable; pub mod array_shape; pub(crate) mod builder; pub(crate) mod command_params; +pub(crate) mod drupal_hooks; mod drupal_route_params; pub(crate) mod eloquent_model_attributes; pub(crate) mod eloquent_string; diff --git a/src/completion/symfony_string_keys.rs b/src/completion/symfony_string_keys.rs index 61a62bd7e..5c8d98006 100644 --- a/src/completion/symfony_string_keys.rs +++ b/src/completion/symfony_string_keys.rs @@ -90,6 +90,36 @@ impl Backend { position, ); } + if kind == StringKeyKind::DrupalHook { + // An alter call names its hook without the `_alter`. + let alter = detect_string_call_context(content, cursor, code) + .and_then(|call| { + crate::symbol_map::extraction::hook_invocation( + &call.method_name, + call.arg_index, + ) + }) + .is_some_and(|(_, alter)| alter); + let hooks = self + .drupal_documented_hooks() + .into_iter() + .filter_map(|hook| { + if alter { + hook.strip_suffix("_alter").map(str::to_string) + } else { + Some(hook) + } + }) + .collect(); + return self.symfony_names_response( + kind, + hooks, + content, + prefix_start, + cursor, + position, + ); + } if kind == StringKeyKind::SymfonyTranslation { let domain = self.translation_domain_at(uri, content, literal_start); let messages = self.symfony_translation_messages(&domain); @@ -254,6 +284,18 @@ impl Backend { == Some(call.arg_index)) .then_some(StringKeyKind::DrupalRoute); } + if self.is_drupal_project() + && let Some((receiver, _)) = + crate::symbol_map::extraction::hook_invocation(&call.method_name, call.arg_index) + { + return self.receiver_key_for( + subject, + (receiver, &call.method_name, call.arg_index), + content, + cursor, + ctx, + ); + } self.receiver_call_key( subject, &call.method_name, @@ -276,6 +318,19 @@ impl Backend { ctx: &FileContext, ) -> Option { let (receiver, _) = symfony_receiver_method(method, index)?; + self.receiver_key_for(subject, (receiver, method, index), content, cursor, ctx) + } + + /// What the argument `index` of a call of `method` on `subject` names + /// when the type engine confirms the subject is a `receiver`. + fn receiver_key_for( + &self, + subject: &str, + (receiver, method, index): (crate::symbol_map::ReceiverClass, &str, usize), + content: &str, + cursor: usize, + ctx: &FileContext, + ) -> Option { let loaders = self.file_loaders(ctx); let class_loader = loaders.resolved_class_loader(); let rctx = loaders.resolution_ctx_at(content, cursor as u32); @@ -526,6 +581,13 @@ fn attribute_key( ) .then_some((StringKeyKind::YiiRouteArgument, literal_start)); } + if attribute.trim_start_matches('\\') == "Drupal\\Core\\Hook\\Attribute\\Hook" { + return matches!( + slot, + ArgumentSlot::Named("hook") | ArgumentSlot::Positional(0) + ) + .then_some((StringKeyKind::DrupalHook, literal_start)); + } if is_message_handler_attribute(&attribute) { return matches!( slot, diff --git a/src/drupal/hooks.rs b/src/drupal/hooks.rs index 9a66189bc..e4d86c9b9 100644 --- a/src/drupal/hooks.rs +++ b/src/drupal/hooks.rs @@ -603,8 +603,18 @@ impl Backend { /// The documented hook names with placeholders, sorted. fn documented_hook_patterns(&self) -> Vec { + let mut patterns = self.drupal_documented_hooks(); + patterns.retain(|hook| is_hook_pattern(hook)); + patterns + } + + /// Every hook an `*.api.php` file documents, patterns included, sorted. + pub(crate) fn drupal_documented_hooks(&self) -> Vec { use crate::symbol_map::{StringKeyKind, SymbolKind}; - let mut patterns = Vec::new(); + let mut hooks = Vec::new(); + // The documentation is a plain span; an invocation, the other kind + // of declaration, is a call the type engine confirms, and is left + // out. self.visit_declaring_symbol_maps(|_, map| { for span in &map.spans { if let SymbolKind::StringKey { @@ -613,15 +623,14 @@ impl Backend { is_write: true, .. } = &span.kind - && is_hook_pattern(key) { - patterns.push(key.clone()); + hooks.push(key.clone()); } } }); - patterns.sort_unstable(); - patterns.dedup(); - patterns + hooks.sort_unstable(); + hooks.dedup(); + hooks } /// Every hook the workspace names, implemented or documented, that the diff --git a/src/symbol_map/extraction/drupal.rs b/src/symbol_map/extraction/drupal.rs index 3a371dd73..b9792e914 100644 --- a/src/symbol_map/extraction/drupal.rs +++ b/src/symbol_map/extraction/drupal.rs @@ -940,6 +940,22 @@ const THEME_INVOKERS: [HookInvoker; 4] = [ invoker("alterForTheme", 1, true, None), ]; +/// What a call of `method` invokes in its positional argument `index`: +/// the receiver it needs, and whether the argument is an alter type. +pub(crate) fn hook_invocation(method: &str, index: usize) -> Option<(ReceiverClass, bool)> { + let find = |invokers: &'static [HookInvoker]| { + invokers + .iter() + .find(|invoker| invoker.method.eq_ignore_ascii_case(method) && invoker.hook == index) + }; + match (find(&MODULE_INVOKERS), find(&THEME_INVOKERS)) { + (Some(invoker), Some(_)) => Some((ReceiverClass::Hooks, invoker.alter)), + (Some(invoker), None) => Some((ReceiverClass::ModuleHooks, invoker.alter)), + (None, Some(invoker)) => Some((ReceiverClass::ThemeHooks, invoker.alter)), + (None, None) => None, + } +} + /// Record the hooks a call may invoke, for the type engine to confirm /// against its receiver: a module handler's or a theme manager's. An /// alter type names the hook `_alter`, and a list of them names one diff --git a/src/symbol_map/extraction/mod.rs b/src/symbol_map/extraction/mod.rs index 52a1fecba..297d72f75 100644 --- a/src/symbol_map/extraction/mod.rs +++ b/src/symbol_map/extraction/mod.rs @@ -351,6 +351,7 @@ use wordpress::*; use yii::*; use yii_routes::*; +pub(crate) use drupal::hook_invocation; pub(crate) use symfony::{ configurator_function_kind, is_autowire_attribute, is_bus_name_stamp, is_event_listener_attribute, is_message_handler_attribute, is_template_attribute, diff --git a/tests/integration/drupal_hooks.rs b/tests/integration/drupal_hooks.rs index c1f7c22cd..5e0a8c84d 100644 --- a/tests/integration/drupal_hooks.rs +++ b/tests/integration/drupal_hooks.rs @@ -413,3 +413,149 @@ async fn computed_hook_names_reach_their_documented_pattern() { .await; assert_eq!(found, vec![(api, 3), (module, 3)], "{found:?}"); } + +/// Hooks complete where an implementation is named and where a hook name is +/// written, an alter call's without its `_alter`. +#[tokio::test] +async fn hooks_complete_with_their_documented_signature() { + let module = " { + items + } + Some(tower_lsp_server::gen_lsp_types::CompletionResponse::CompletionList(list)) => { + list.items + } + None => Vec::new(), + }; + let form_alter = items + .iter() + .find(|item| item.label == "acme_form_alter") + .expect("acme_form_alter offered"); + let Some(tower_lsp_server::gen_lsp_types::CompletionItemTextEdit::TextEdit(edit)) = + &form_alter.text_edit + else { + panic!("{form_alter:?}"); + }; + assert_eq!( + edit.new_text, + "acme_form_alter(&\\$form, Drupal\\\\Core\\\\Form\\\\FormStateInterface \\$form_state, \\$form_id) {\n\t$1\n}" + ); + + for (path, content, needle, inside, expected) in [ + (HOOKS_PATH, hooks.as_str(), "#[Hook('')]", 8, "form_alter"), + (INVOKER_PATH, invoker.as_str(), "invokeAll('')", 11, "cron"), + ( + INVOKER_PATH, + invoker.as_str(), + "modules->alter(''", + 16, + "form", + ), + ] { + let position = position_in(content, needle, inside); + let labels = crate::common::response_labels( + crate::common::completion_response( + &backend, + &uri_for(&dir, path), + position.line, + position.character, + None, + ) + .await, + ); + assert!( + labels.contains(&expected.to_string()), + "{needle}: {labels:?}" + ); + } +} + +/// The completion labels right after `typed` in `acme.module`, whose +/// content is `module`, once the documented hooks are known. +async fn module_completion_labels(module: &str, typed: &str) -> Vec { + let (backend, dir) = drupal( + &[(FORM_API_PATH, FORM_API)], + &[("web/modules/custom/acme/acme.module", module)], + ) + .await; + let module_uri = uri_for(&dir, "web/modules/custom/acme/acme.module"); + // The same warm-up as the test above, on the first `acme_` name, so + // the hooks are known. + references_at(&backend, &module_uri, position_in(module, "acme_", 1)).await; + let position = position_in(module, typed, typed.len()); + crate::common::response_labels( + crate::common::completion_response( + &backend, + &module_uri, + position.line, + position.character, + None, + ) + .await, + ) +} + +/// A method without a modifier in a `.module` file is not a hook +/// implementation, so its name gets the class-body completion instead: the +/// parent's method it can override. +#[tokio::test] +async fn a_method_in_a_module_file_is_not_offered_hooks() { + let module = " Date: Mon, 2 Nov 2026 15:47:08 +0100 Subject: [PATCH 06/13] Type untyped Drupal hook parameters from the hook documentation A parameter a procedural hook implementation or a #[Hook] method leaves without a type or an @param takes the type the hook's documentation gives the parameter at its place, through a documented pattern when the hook's name is computed. Assisted-by: claude-code/claude-opus-5-5 --- src/drupal/hooks.rs | 68 +++++++++++ .../variable/forward_walk/param_seeding.rs | 34 ++++++ tests/integration/drupal_hooks.rs | 110 +++++++++++++++++- tests/integration/drupal_services.rs | 27 +++++ 4 files changed, 237 insertions(+), 2 deletions(-) diff --git a/src/drupal/hooks.rs b/src/drupal/hooks.rs index e4d86c9b9..8c0c33f61 100644 --- a/src/drupal/hooks.rs +++ b/src/drupal/hooks.rs @@ -633,6 +633,74 @@ impl Backend { hooks } + /// The documentation function of the hook the function or method + /// declared at `decl_start` of `content` implements: a procedural + /// function's (`name` `None`, read from the content), or a hook class + /// method's (`name`, on the class `class_fqn`). Read from the symbol + /// map of the file that declares it, which has to be `content`. + pub(crate) fn drupal_hook_documentation_function( + &self, + content: &str, + decl_start: u32, + method: Option<&str>, + class_fqn: &str, + ) -> Option { + use crate::symbol_map::{StringKeyKind, SymbolKind}; + if !self.is_drupal_project() { + return None; + } + let rest = content.get(decl_start as usize..)?; + let keyword = rest.find("function")?; + let after = &rest[keyword + "function".len()..]; + let name_from = after.len() - after.trim_start().len(); + let name_start = decl_start as usize + keyword + "function".len() + name_from; + let name_len = content[name_start..] + .bytes() + .take_while(|byte| byte.is_ascii_alphanumeric() || *byte == b'_') + .count(); + let name = &content[name_start..name_start + name_len]; + let uri = match method { + None => self + .symbols + .global_functions + .read() + .get(name) + .map(|(uri, _)| uri.clone())?, + Some(_) => self.class_declaration_uri(class_fqn.trim_start_matches('\\'))?, + }; + let map = self + .symbol_map_for(&uri) + .filter(|map| map.matches_source(content))?; + // A procedural function names its hook after its extension; a + // method's `#[Hook]` stands among the attributes before its name. + let (from, to) = match method { + None => (name_start as u32, (name_start + name_len) as u32), + Some(_) => (decl_start, name_start as u32), + }; + let at = map.spans.partition_point(|span| span.start < from); + let hook = map.spans[at..] + .iter() + .take_while(|span| span.start < to) + .find_map(|span| match &span.kind { + SymbolKind::StringKey { + kind: StringKeyKind::DrupalHook, + key, + is_write: false, + .. + } => Some(key.clone()), + _ => None, + })?; + let exact = format!("hook_{hook}"); + if let Some(function) = self.find_or_load_function(&[exact.as_str()]) { + return Some(function); + } + let is_entity_type = |id: &str| !self.drupal_entity_types(id).is_empty(); + self.documented_hook_patterns() + .iter() + .filter(|pattern| matches_hook_pattern(pattern, &hook, &is_entity_type)) + .find_map(|pattern| self.find_or_load_function(&[format!("hook_{pattern}").as_str()])) + } + /// Every hook the workspace names, implemented or documented, that the /// documented `pattern` stands for. fn hooks_matching(&self, pattern: &str) -> Vec { diff --git a/src/type_engine/variable/forward_walk/param_seeding.rs b/src/type_engine/variable/forward_walk/param_seeding.rs index ab43252ad..afb794e95 100644 --- a/src/type_engine/variable/forward_walk/param_seeding.rs +++ b/src/type_engine/variable/forward_walk/param_seeding.rs @@ -100,6 +100,10 @@ pub(crate) fn seed_params<'b>( // The parameters Drupal upcasts to an entity before a controller runs, // looked up once, when there is a parameter it could be. let mut drupal_upcasts: Option> = None; + // A Drupal hook implementation that leaves a parameter untyped takes + // the type its hook's documentation gives it; looked up once, when + // there is such a parameter. + let mut hook_documentation: Option> = None; for (index, param) in parameters.enumerate() { let pname = bytes_to_str(param.variable.name).to_string(); @@ -152,6 +156,36 @@ pub(crate) fn seed_params<'b>( } } + if native_type.is_none() + && let Some(backend) = ctx.file.backend + && super::super::resolution::declared_param_docblock_type( + ctx.file.content, + method_span_start as usize, + &pname, + ) + .is_none() + && let Some(documented) = hook_documentation + .get_or_insert_with(|| { + backend.drupal_hook_documentation_function( + ctx.file.content, + method_span_start, + method_name, + &ctx.current_class.fqn(), + ) + }) + .as_ref() + .and_then(|function| function.parameters.get(index)) + .and_then(|parameter| { + parameter + .type_hint + .clone() + .or_else(|| parameter.native_type_hint.clone()) + }) + { + scope.seed(&pname, ctx.resolved_types_for(documented)); + continue; + } + let param_results = resolve_param_type( &pname, param, diff --git a/tests/integration/drupal_hooks.rs b/tests/integration/drupal_hooks.rs index 5e0a8c84d..758e72202 100644 --- a/tests/integration/drupal_hooks.rs +++ b/tests/integration/drupal_hooks.rs @@ -3,7 +3,9 @@ //! procedural function names make. use crate::common::hover_text_at; -use crate::drupal_services::{definitions_at, position_in, references_at, uri_for, workspace_in}; +use crate::drupal_services::{ + definitions_at, position_in, references_at, uri_for, workspace_in, workspace_opened_early, +}; const FORM_API: &str = " (phpantom_lsp::Backend, tempfile::TempDir) { + workspace_in(drupal_dir(extra), open).await +} + +/// The directory of [`drupal`]'s workspace, before `open` joins it. +fn drupal_dir(extra: &[(&str, &str)]) -> tempfile::TempDir { let dir = tempfile::tempdir().unwrap(); for (path, content) in [ (".git/HEAD", "ref: refs/heads/main\n"), @@ -66,7 +73,7 @@ async fn drupal( std::fs::create_dir_all(full.parent().unwrap()).unwrap(); std::fs::write(full, content).unwrap(); } - workspace_in(dir, open).await + dir } #[tokio::test] @@ -559,3 +566,102 @@ async fn a_closure_in_a_module_file_is_not_offered_hooks() { "{labels:?}" ); } + +/// An implementation that leaves a parameter untyped reads it as the hook's +/// documentation types it, unless it types it itself. +#[tokio::test] +async fn untyped_hook_parameters_take_the_documented_types() { + let api = " (Backend, tempfile::TempDir) { + std::fs::write(dir.path().join("composer.json"), COMPOSER).unwrap(); + for (path, content) in STUBS.iter().chain(files) { + let full = dir.path().join(path); + std::fs::create_dir_all(full.parent().unwrap()).unwrap(); + std::fs::write(full, content).unwrap(); + } + let backend = Backend::new_test(); + #[allow(deprecated)] // `root_uri` is what this generation of clients sends + let params = InitializeParams { + root_uri: Some(Uri::from_file_path(dir.path()).unwrap()), + ..Default::default() + }; + backend.initialize(params).await.unwrap(); + for (path, content) in files { + open_document(&backend, &uri_for(&dir, path), "php", content).await; + } + backend.initialized(InitializedParams {}).await; + (backend, dir) +} + pub(crate) fn uri_for(dir: &tempfile::TempDir, rel: &str) -> Uri { Uri::from_file_path(dir.path().join(rel)).unwrap() } From c39fb8c48870b200d158b13c7fdd7e5dce551732 Mon Sep 17 00:00:00 2001 From: "Giuseppe.Esposito" Date: Mon, 2 Nov 2026 15:54:58 +0100 Subject: [PATCH 07/13] Link computed Drupal hooks to the core calls that fire them A table of the core methods that fire hooks under computed names, ported from PHPlumb, lets go-to-definition on an implementation such as acme_form_user_login_form_alter or acme_node_presave list the invoking calls there that build a matching name, of its module or theme group and with the most written-out characters per method, before the documentation. An ENTITY_TYPE row only stands for the entity types its storage stores. Assisted-by: claude-code/claude-opus-5-5 --- src/drupal/computed_hooks.rs | 370 ++++++++++++++++++++++++ src/drupal/hooks.rs | 22 ++ src/drupal/mod.rs | 1 + src/mem_audit.rs | 7 + src/symbol_map/extraction/class_like.rs | 2 + src/symbol_map/extraction/drupal.rs | 118 ++++++++ src/symbol_map/extraction/mod.rs | 3 + src/symbol_map/map.rs | 3 + src/symbol_map/string_keys.rs | 19 ++ tests/integration/drupal_hooks.rs | 88 ++++++ 10 files changed, 633 insertions(+) create mode 100644 src/drupal/computed_hooks.rs diff --git a/src/drupal/computed_hooks.rs b/src/drupal/computed_hooks.rs new file mode 100644 index 000000000..576bf7440 --- /dev/null +++ b/src/drupal/computed_hooks.rs @@ -0,0 +1,370 @@ +//! The core methods that fire hooks under computed names, ported from +//! PHPlumb: an implementation of `hook_form_FORM_ID_alter` is fired by the +//! `alter()` call `FormBuilder::prepareForm()` makes with the name it +//! builds, which nothing in the call itself spells. +//! +//! The table is tied to core's internals, so it is kept apart from the rest +//! of the hook support: dropping this module leaves every other hook +//! feature working. + +use crate::Backend; + +/// Which entity types an `ENTITY_TYPE` placeholder of a row stands for. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Storage { + Any, + /// Entity types whose storage is not a config entity storage. + Content, + /// Entity types stored by a config entity storage. + Config, +} + +/// Each row: the computed hook names, the core methods that fire them, and +/// the entity types an `ENTITY_TYPE` placeholder stands for. +pub(crate) const COMPUTED: &[(&[&str], &[&str], Storage)] = &[ + ( + &[ + "ENTITY_TYPE_create", + "ENTITY_TYPE_presave", + "ENTITY_TYPE_insert", + "ENTITY_TYPE_update", + "ENTITY_TYPE_predelete", + "ENTITY_TYPE_delete", + "ENTITY_TYPE_revision_delete", + "ENTITY_TYPE_translation_create", + "ENTITY_TYPE_translation_insert", + "ENTITY_TYPE_translation_delete", + "ENTITY_TYPE_field_values_init", + ], + &["Drupal\\Core\\Entity\\EntityStorageBase::invokeHook"], + Storage::Content, + ), + ( + &[ + "ENTITY_TYPE_create", + "ENTITY_TYPE_presave", + "ENTITY_TYPE_insert", + "ENTITY_TYPE_update", + "ENTITY_TYPE_predelete", + "ENTITY_TYPE_delete", + "ENTITY_TYPE_revision_delete", + "ENTITY_TYPE_translation_create", + "ENTITY_TYPE_translation_insert", + "ENTITY_TYPE_translation_delete", + "ENTITY_TYPE_field_values_init", + ], + &["Drupal\\Core\\Config\\Entity\\ConfigEntityStorage::invokeHook"], + Storage::Config, + ), + ( + &[ + "entity_create", + "entity_presave", + "entity_insert", + "entity_update", + "entity_predelete", + "entity_delete", + "entity_revision_delete", + "entity_translation_create", + "entity_translation_insert", + "entity_translation_delete", + "entity_field_values_init", + ], + &[ + "Drupal\\Core\\Entity\\EntityStorageBase::invokeHook", + "Drupal\\Core\\Config\\Entity\\ConfigEntityStorage::invokeHook", + ], + Storage::Any, + ), + ( + &["ENTITY_TYPE_load"], + &["Drupal\\Core\\Entity\\EntityStorageBase::postLoad"], + Storage::Any, + ), + ( + &["ENTITY_TYPE_storage_load"], + &["Drupal\\Core\\Entity\\ContentEntityStorageBase::invokeStorageLoadHook"], + Storage::Any, + ), + ( + &["ENTITY_TYPE_revision_create"], + &["Drupal\\Core\\Entity\\ContentEntityStorageBase::createRevision"], + Storage::Any, + ), + ( + &["ENTITY_TYPE_duplicate"], + &[ + "Drupal\\Core\\Entity\\EntityBase::createDuplicate", + "Drupal\\Core\\Entity\\ContentEntityBase::createDuplicate", + ], + Storage::Any, + ), + ( + &["ENTITY_TYPE_access"], + &["Drupal\\Core\\Entity\\EntityAccessControlHandler::access"], + Storage::Any, + ), + ( + &["ENTITY_TYPE_create_access"], + &["Drupal\\Core\\Entity\\EntityAccessControlHandler::createAccess"], + Storage::Any, + ), + ( + &["ENTITY_TYPE_view", "ENTITY_TYPE_view_alter"], + &["Drupal\\Core\\Entity\\EntityViewBuilder::buildMultiple"], + Storage::Any, + ), + ( + &["ENTITY_TYPE_build_defaults_alter"], + &["Drupal\\Core\\Entity\\EntityViewBuilder::viewMultiple"], + Storage::Any, + ), + ( + &["ENTITY_TYPE_view_mode_alter"], + &["Drupal\\Core\\Entity\\EntityViewBuilder::getBuildDefaults"], + Storage::Any, + ), + ( + &["ENTITY_TYPE_form_mode_alter"], + &["Drupal\\Core\\Entity\\Entity\\EntityFormDisplay::collectRenderDisplay"], + Storage::Any, + ), + ( + &["ENTITY_TYPE_prepare_form"], + &["Drupal\\Core\\Entity\\EntityForm::prepareInvokeAll"], + Storage::Any, + ), + ( + &[ + "entity_query_ENTITY_TYPE_alter", + "entity_query_tag__TAG_alter", + "entity_query_tag__ENTITY_TYPE__TAG_alter", + ], + &["Drupal\\Core\\Entity\\Query\\QueryBase::alter"], + Storage::Any, + ), + ( + &[ + "form_alter", + "form_FORM_ID_alter", + "form_BASE_FORM_ID_alter", + ], + &["Drupal\\Core\\Form\\FormBuilder::prepareForm"], + Storage::Any, + ), + ( + &["query_alter", "query_TAG_alter"], + &[ + "Drupal\\Core\\Database\\Query\\Select::preExecute", + "Drupal\\Core\\Entity\\Plugin\\EntityReferenceSelection\\DefaultSelection::reAlterQuery", + ], + Storage::Any, + ), + ( + &["field_widget_single_element_WIDGET_TYPE_form_alter"], + &["Drupal\\Core\\Field\\WidgetBase::formSingleElement"], + Storage::Any, + ), + ( + &["field_widget_complete_WIDGET_TYPE_form_alter"], + &["Drupal\\Core\\Field\\WidgetBase::form"], + Storage::Any, + ), + ( + &["block_view_BASE_BLOCK_ID_alter"], + &["Drupal\\block\\BlockViewBuilder::buildPreRenderableBlock"], + Storage::Any, + ), + ( + &["block_build_BASE_BLOCK_ID_alter"], + &["Drupal\\block\\BlockViewBuilder::viewMultiple"], + Storage::Any, + ), + ( + &[ + "theme_suggestions_HOOK", + "theme_suggestions_alter", + "theme_suggestions_HOOK_alter", + ], + &["Drupal\\Core\\Theme\\ThemeManager::buildThemeHookSuggestions"], + Storage::Any, + ), + ( + &["preprocess_HOOK"], + &["Drupal\\Core\\Theme\\Registry::collectModulePreprocess"], + Storage::Any, + ), + ( + &[ + "plugin_filter_TYPE_alter", + "plugin_filter_TYPE__CONSUMER_alter", + ], + &["Drupal\\Core\\Plugin\\FilteredPluginManagerTrait::getFilteredDefinitions"], + Storage::Any, + ), + ( + &[ + "language_fallback_candidates_alter", + "language_fallback_candidates_OPERATION_alter", + ], + &["Drupal\\language\\ConfigurableLanguageManager::getFallbackCandidates"], + Storage::Any, + ), + ( + &["migrate_MIGRATION_ID_prepare_row"], + &["Drupal\\migrate\\Plugin\\migrate\\source\\SourcePluginBase::prepareRow"], + Storage::Any, + ), + ( + &["jsonapi_ENTITY_TYPE_filter_access"], + &["Drupal\\jsonapi\\Access\\TemporaryQueryGuard::getAccessResultsFromEntityFilterHook"], + Storage::Any, + ), +]; + +/// Whether `class::method`, a resolved class name and a method, is one of +/// the table's core methods. +pub(crate) fn is_computing_method(class: &str, method: &str) -> bool { + let class = class.trim_start_matches('\\'); + COMPUTED.iter().any(|(_, sites, _)| { + sites.iter().any(|site| { + site.split_once("::") + .is_some_and(|(site_class, site_method)| { + site_class.eq_ignore_ascii_case(class) + && site_method.eq_ignore_ascii_case(method) + }) + }) + }) +} + +/// Whether `text` matches `shape`, where each `*` stands for a non-empty +/// run. +fn shape_matches(shape: &str, text: &str) -> bool { + let Some((head, tail)) = shape.split_once('*') else { + return shape == text; + }; + let Some(after) = text.strip_prefix(head) else { + return false; + }; + (1..=after.len()) + .filter(|length| after.is_char_boundary(*length)) + .any(|length| shape_matches(tail, &after[length..])) +} + +impl Backend { + /// Whether the entity type `id` is declared, and stored the way + /// `storage` asks: by a config entity storage or not. + fn is_entity_type_of(&self, id: &str, storage: Storage) -> bool { + const CONFIG_STORAGE: &str = "Drupal\\Core\\Config\\Entity\\ConfigEntityStorage"; + let declared = self.drupal_entity_types(id); + if storage == Storage::Any { + return !declared.is_empty(); + } + declared.iter().any(|(_, handler)| { + let is_config = handler.is_some_and(|handler| { + let mut class = Some(handler.to_string()); + let mut depth = 0; + while let Some(name) = class.take() { + if name + .trim_start_matches('\\') + .eq_ignore_ascii_case(CONFIG_STORAGE) + { + return true; + } + depth += 1; + if depth > 20 { + break; + } + class = self + .find_or_load_class(&name) + .and_then(|info| info.parent_class.map(|parent| parent.to_string())); + } + false + }); + is_config == (storage == Storage::Config) + }) + } + + /// The calls in core that fire the computed hook `hook`, implemented by + /// a theme when `is_theme`: for each of the table's methods that builds + /// a name `hook` matches, the invoking calls there of the + /// implementation's group whose built name matches it with the most + /// written-out characters, as files and byte ranges. + pub(crate) fn computed_hook_call_sites( + &self, + hook: &str, + is_theme: bool, + ) -> Vec<(String, u32, u32)> { + let mut found = Vec::new(); + for (patterns, sites, storage) in COMPUTED { + let is_entity_type = |id: &str| self.is_entity_type_of(id, *storage); + let matches = patterns + .iter() + .any(|pattern| super::hooks::matches_hook_pattern(pattern, hook, &is_entity_type)); + if !matches { + continue; + } + for site in *sites { + let Some((class, method)) = site.split_once("::") else { + continue; + }; + // Loading the class parses its file, which records the calls. + if self.find_or_load_class(class).is_none() { + continue; + } + let Some(uri) = self.class_declaration_uri(class) else { + continue; + }; + let Some(map) = self.symbol_map_for(&uri) else { + continue; + }; + let scored: Vec<(usize, u32, u32)> = map + .drupal_computed_hooks + .iter() + .filter(|call| call.method.eq_ignore_ascii_case(method)) + .filter(|call| call.alter == hook.ends_with("_alter")) + .filter(|call| call.themes.is_none_or(|themes| themes == is_theme)) + .filter(|call| call.shape.contains('*') && shape_matches(&call.shape, hook)) + .map(|call| { + let score = call.shape.len() - call.shape.matches('*').count(); + (score, call.start, call.end) + }) + .collect(); + let Some(best) = scored.iter().map(|(score, _, _)| *score).max() else { + continue; + }; + for (_, start, end) in scored.into_iter().filter(|(score, _, _)| *score == best) { + if !found.contains(&(uri.clone(), start, end)) { + found.push((uri.clone(), start, end)); + } + } + } + } + found + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn shapes_match_with_a_non_empty_run_per_star() { + assert!(shape_matches("form_*_alter", "form_user_login_alter")); + assert!(shape_matches("*_alter", "form_alter")); + assert!(!shape_matches("form_*_alter", "form__alter")); + assert!(!shape_matches("form_*_alter", "query_x_alter")); + } + + #[test] + fn computing_methods_are_matched_by_class_and_method() { + assert!(is_computing_method( + "\\Drupal\\Core\\Form\\FormBuilder", + "prepareForm" + )); + assert!(!is_computing_method( + "Drupal\\Core\\Form\\FormBuilder", + "buildForm" + )); + } +} diff --git a/src/drupal/hooks.rs b/src/drupal/hooks.rs index 8c0c33f61..af411b6fe 100644 --- a/src/drupal/hooks.rs +++ b/src/drupal/hooks.rs @@ -520,6 +520,27 @@ impl Backend { return self.hook_locations(&targets); } let docs = self.hook_documentation_occurrences(key, &occurrences); + // Then the calls in core that fire it under a computed name. + let computed: Vec = match &here { + Occurrence::Implementation { + is_theme, + collected: true, + .. + } => self + .computed_hook_call_sites(key, *is_theme) + .into_iter() + .map(|(uri, start, end)| HookOccurrence { + uri, + start, + end, + role: Occurrence::Invocation { + themes: *is_theme, + extension: None, + }, + }) + .collect(), + _ => Vec::new(), + }; let targets: Vec<&HookOccurrence> = occurrences .iter() .filter(|other| match (&here, &other.role) { @@ -531,6 +552,7 @@ impl Backend { } _ => false, }) + .chain(computed.iter()) .chain(docs.iter()) .collect(); self.hook_locations(&targets) diff --git a/src/drupal/mod.rs b/src/drupal/mod.rs index 980603a8e..d6d97172e 100644 --- a/src/drupal/mod.rs +++ b/src/drupal/mod.rs @@ -15,6 +15,7 @@ use std::sync::atomic::Ordering; use parking_lot::RwLock; +pub(crate) mod computed_hooks; pub(crate) mod hooks; pub(crate) mod links; pub(crate) mod routing; diff --git a/src/mem_audit.rs b/src/mem_audit.rs index 70695f1a4..57e07fc3f 100644 --- a/src/mem_audit.rs +++ b/src/mem_audit.rs @@ -1405,6 +1405,13 @@ pub(crate) fn report(backend: &Backend, runner_content_bytes: usize) { sym.add(sm.drupal_route_patterns.capacity() * size_of::()); sym.add(sm.drupal_route_aliases.capacity() * size_of::()); sym.add(sm.drupal_hooks.capacity() * size_of::()); + sym.add( + sm.drupal_computed_hooks.capacity() + * size_of::(), + ); + for site in &sm.drupal_computed_hooks { + sym.add(site.shape.capacity()); + } for name in sm .drupal_route_patterns .iter() diff --git a/src/symbol_map/extraction/class_like.rs b/src/symbol_map/extraction/class_like.rs index d325941d0..082419df2 100644 --- a/src/symbol_map/extraction/class_like.rs +++ b/src/symbol_map/extraction/class_like.rs @@ -26,6 +26,7 @@ pub(super) fn extract_from_class<'a>(class: &'a Class<'a>, ctx: &mut ExtractionC emit_drupal_entity_type_spans(class, ctx); emit_drupal_route_attribute_spans(class, ctx); emit_drupal_hook_attribute_spans(class, ctx); + emit_drupal_computed_hook_sites(&class.members, class.name.span.start.offset, ctx); emit_drupal_route_builder_spans(&class.members, class.name.span.start.offset, ctx); if let Some(ref extends) = class.extends { @@ -201,6 +202,7 @@ pub(super) fn extract_from_trait<'a>(trait_def: &'a Trait<'a>, ctx: &mut Extract extract_from_attribute_lists(&trait_def.attribute_lists, ctx, 0); emit_drupal_route_builder_spans(&trait_def.members, trait_def.name.span.start.offset, ctx); + emit_drupal_computed_hook_sites(&trait_def.members, trait_def.name.span.start.offset, ctx); if let Some((doc_text, doc_offset)) = get_docblock_text_with_offset(ctx.trivias, ctx.content, trait_def) diff --git a/src/symbol_map/extraction/drupal.rs b/src/symbol_map/extraction/drupal.rs index b9792e914..58d7dac1c 100644 --- a/src/symbol_map/extraction/drupal.rs +++ b/src/symbol_map/extraction/drupal.rs @@ -1039,6 +1039,124 @@ pub(in crate::symbol_map::extraction) fn record_drupal_hook_invocations( } } +/// Record the invoking calls of the core methods that fire hooks under +/// computed names, with the shape of the name each builds (see +/// [`crate::drupal::computed_hooks`]). A hook name held in a variable is +/// all `*`; an alter call adds `_alter`, and a list of alter types gives +/// one shape each. +pub(in crate::symbol_map::extraction) fn emit_drupal_computed_hook_sites( + members: &Sequence<'_, ClassLikeMember<'_>>, + name_offset: u32, + ctx: &mut ExtractionCtx<'_>, +) { + if !ctx.drupal || ctx.drupal_file == crate::drupal::hooks::DrupalFile::Test { + return; + } + let Some(class) = ctx.resolved_name_at(name_offset).map(str::to_string) else { + return; + }; + for member in members.iter() { + let ClassLikeMember::Method(method) = member else { + continue; + }; + let method_name = bytes_to_str(method.name.value); + if !crate::drupal::computed_hooks::is_computing_method(&class, method_name) { + continue; + } + let mut calls = Vec::new(); + mago_syntax::walker::Walker::walk_method(&InvokingCallWalker, method, &mut calls); + for (receiver, callee, arguments) in calls { + let positional: Vec<&Expression<'_>> = arguments + .arguments + .iter() + .map_while(|argument| match argument { + Argument::Positional(positional) => Some(positional.value), + Argument::Named(_) => None, + }) + .collect(); + let invoker = MODULE_INVOKERS + .iter() + .chain(THEME_INVOKERS.iter()) + .find(|invoker| invoker.method.eq_ignore_ascii_case(callee)); + let Some(invoker) = invoker else { + continue; + }; + let Some(hook) = positional.get(invoker.hook) else { + continue; + }; + // The receiver's spelling says which group a call reaches: the + // theme manager calls its own `alterForTheme()`, and the module + // handler and the theme manager are injected under their names. + let receiver_text = expr_to_subject_text(receiver); + let themes = if callee.eq_ignore_ascii_case("alterForTheme") + || receiver_text.contains("themeManager") + || (receiver_text == "$this" + && class + .trim_start_matches('\\') + .ends_with("Theme\\ThemeManager")) + { + Some(true) + } else if receiver_text.contains("moduleHandler") { + Some(false) + } else { + None + }; + let parts: Vec<&Expression<'_>> = match hook { + Expression::Array(array) => array + .elements + .iter() + .filter_map(|element| match element { + ArrayElement::Value(value) => Some(value.value), + _ => None, + }) + .collect(), + _ => vec![*hook], + }; + for part in parts { + let mut shape = String::new(); + push_name_parts(part, &mut shape); + if invoker.alter { + shape.push_str("_alter"); + } + let span = part.span(); + ctx.drupal_computed_hooks + .push(crate::symbol_map::ComputedHookSite { + method: crate::atom::atom(method_name), + start: span.start.offset, + end: span.end.offset, + shape, + alter: invoker.alter, + themes, + }); + } + } + } +} + +/// The method calls of a method body. +struct InvokingCallWalker; + +/// A method call: its receiver, its name, and its arguments. +type FoundCall<'ast, 'arena> = ( + &'ast Expression<'arena>, + &'arena str, + &'ast ArgumentList<'arena>, +); + +impl<'ast, 'arena> mago_syntax::walker::Walker<'ast, 'arena, Vec>> + for InvokingCallWalker +{ + fn walk_in_method_call( + &self, + call: &'ast MethodCall<'arena>, + calls: &mut Vec>, + ) { + if let ClassLikeMemberSelector::Identifier(name) = &call.method { + calls.push((call.object, bytes_to_str(name.value), &call.argument_list)); + } + } +} + /// Record the route name `new Url('…')` reads. pub(in crate::symbol_map::extraction) fn emit_drupal_url_spans( class: &Expression<'_>, diff --git a/src/symbol_map/extraction/mod.rs b/src/symbol_map/extraction/mod.rs index 297d72f75..09b9943d6 100644 --- a/src/symbol_map/extraction/mod.rs +++ b/src/symbol_map/extraction/mod.rs @@ -109,6 +109,7 @@ struct ExtractionCtx<'a> { /// The Drupal routes the file's `#[Route]` attributes declare. drupal_routes: Vec, drupal_hooks: Vec, + drupal_computed_hooks: Vec, /// What the file holds of Drupal hooks, which its path says. drupal_file: crate::drupal::hooks::DrupalFile, /// A procedural function with `#[ProceduralHookScanStop]` was read: @@ -529,6 +530,7 @@ fn extract_symbol_map_inner( drupal_route_aliases: Vec::new(), drupal_routes: Vec::new(), drupal_hooks: Vec::new(), + drupal_computed_hooks: Vec::new(), drupal_file, drupal_hook_scan_stopped: false, cond_nesting_depth: 0, @@ -730,6 +732,7 @@ fn extract_symbol_map_inner( drupal_route_aliases: ctx.drupal_route_aliases, drupal_routes: ctx.drupal_routes, drupal_hooks: ctx.drupal_hooks, + drupal_computed_hooks: ctx.drupal_computed_hooks, routes: ctx.routes, declares_twig_loader: ctx.declares_twig_loader, route_imports: ctx diff --git a/src/symbol_map/map.rs b/src/symbol_map/map.rs index ac7b00a78..a28baaaff 100644 --- a/src/symbol_map/map.rs +++ b/src/symbol_map/map.rs @@ -323,6 +323,9 @@ pub(crate) struct SymbolMap { /// What each Drupal hook implementation and invocation of the file is, /// sorted by `start`. Empty for every file that has none. pub drupal_hooks: Vec, + /// The invoking calls of the core methods that fire hooks under + /// computed names. Empty for every other file. + pub drupal_computed_hooks: Vec, /// The routes the file declares, in source order. Empty for every file /// that declares none. pub routes: Vec, diff --git a/src/symbol_map/string_keys.rs b/src/symbol_map/string_keys.rs index 2796485e1..e5e6bf581 100644 --- a/src/symbol_map/string_keys.rs +++ b/src/symbol_map/string_keys.rs @@ -643,6 +643,25 @@ pub(crate) enum HookRole { Invocation { extension: Option }, } +/// An invoking call inside one of the core methods that fire hooks under +/// computed names (see `drupal::computed_hooks`), with the shape of the +/// name it builds: `*` for each part not written out, and `_alter` after +/// an alter type. +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct ComputedHookSite { + /// The method the call is in. + pub method: Atom, + /// The byte range of the call's hook argument. + pub start: u32, + pub end: u32, + pub shape: String, + /// The call is an alter call. + pub alter: bool, + /// The call reaches themes (a theme manager's) or modules (a module + /// handler's), when its receiver says which. + pub themes: Option, +} + /// What a Drupal entity type declaration gives its ID: the entity class, /// and the storage handler that loads it. #[derive(Debug, Clone, PartialEq)] diff --git a/tests/integration/drupal_hooks.rs b/tests/integration/drupal_hooks.rs index 758e72202..485cb6875 100644 --- a/tests/integration/drupal_hooks.rs +++ b/tests/integration/drupal_hooks.rs @@ -665,3 +665,91 @@ async fn a_hook_file_opened_before_the_walk_ends_takes_the_documented_types() { assert!(text.contains("FormStateInterface"), "{path}: {text}"); } } + +const FORM_BUILDER: &str = r#"moduleHandler->alter($hooks, $form, $form_state, $form_id); + $this->themeManager->alter($hooks, $form, $form_state, $form_id); + } +} +"#; + +const STORAGES: &[(&str, &str)] = &[ + ( + "web/core/lib/Drupal/Core/Entity/EntityStorageBase.php", + "moduleHandler()->invokeAll($this->entityTypeId . '_' . $hook, [$entity]);\n $this->moduleHandler()->invokeAll('entity_' . $hook, [$entity]);\n }\n}\n", + ), + ( + "web/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php", + "moduleHandler()->invokeAll($this->entityTypeId . '_' . $hook, [$entity]);\n }\n}\n", + ), + ( + "web/core/modules/node/src/Entity/Node.php", + "| { + found + .into_iter() + .map(|location| (location.uri, location.range.start.line)) + .collect::>() + }; + let api = uri_for(&dir, "web/core/lib/Drupal/Core/Form/form.api.php"); + + let found = lines( + definitions_at( + &backend, + &module, + position_in(PATTERN_MODULE, "acme_form_user_login_form_alter", 6), + ) + .await, + ); + assert_eq!( + found, + vec![(uri_for(&dir, builder_path), 7), (api.clone(), 1)] + ); + + let found = lines( + definitions_at( + &backend, + &module, + position_in(PATTERN_MODULE, "acme_node_presave", 6), + ) + .await, + ); + assert_eq!( + found, + vec![(uri_for(&dir, STORAGES[0].0), 4), (api, 3)], + "only the content storage stores nodes" + ); +} From 32c1b17d41f00b412e8bef029b44bb6e57a4bbda Mon Sep 17 00:00:00 2001 From: "Giuseppe.Esposito" Date: Mon, 2 Nov 2026 15:56:18 +0100 Subject: [PATCH 08/13] Show Drupal hooks in the playground and the changelog The playground's module documents, invokes, and implements a hook of its own and implements core's form alter hook procedurally, with Try comments at each site, and the assertions check each claim. DR6 leaves the backlog. Assisted-by: claude-code/claude-opus-5-5 --- docs/CHANGELOG.md | 1 + docs/todo.md | 3 +- docs/todo/drupal.md | 43 ++++++++----------- examples/drupal/README.md | 9 ++++ examples/drupal/assertions.php | 12 ++++++ .../web/modules/custom/acme/acme.api.php | 19 ++++++++ .../web/modules/custom/acme/acme.module | 14 ++++++ .../web/modules/custom/acme/src/Greeter.php | 8 +++- .../custom/acme/src/Hook/AcmeHooks.php | 14 ++++++ 9 files changed, 95 insertions(+), 28 deletions(-) create mode 100644 examples/drupal/web/modules/custom/acme/acme.api.php diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index c10393384..260de8dab 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -48,6 +48,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Symfony services that bundles and the container builder declare.** The service IDs and parameters an installed bundle declares in its own configuration complete, jump to their declaration, and type `$container->get()`, and so do the ones a kernel, an extension, or a compiler pass declares with `register()`, `setAlias()`, `setDefinition()`, `setParameter()`, or `getKernelParameters()`. A parameter set under a name built at runtime, like `'app.' . $key`, leaves the names it can produce unreported. A configurator's `parent()` and the `%name%` placeholders in its arguments are read as the service and parameter they name. - **Drupal services.** In a Drupal project, the service IDs and container parameters that core, contributed, and custom modules declare in their `*.services.yml` complete, jump to their declaration, and list their references wherever a services file, `$container->get()`, or `\Drupal::service()` names them, and a lookup is typed by the class the service is built from, or by the class or interface a lookup names by `::class`. Rename leaves core and contributed extensions alone, as it does vendor code. Contributed by @giuseppe-esposito-87. - **Drupal entity types.** In a Drupal project, an entity type ID declared by an entity class's attribute or annotation completes, jumps to its declaration, and lists its references wherever the entity type manager is asked for it, and `getStorage()` is typed as that entity type's storage, so `load()`, `loadMultiple()`, `create()`, and the other loads return the entity class. An entity query started by `\Drupal::entityQuery()` or a storage's `getQuery()` gives the IDs of that entity type, or their number after `count()`. Contributed by @giuseppe-esposito-87. +- **Drupal hooks.** In a Drupal project, a hook implementation, procedural or by `#[Hook]`, jumps to the hook's `*.api.php` documentation and to the calls that fire it, core's computed hook names included, and an invocation jumps to the implementations it reaches. Hook names and implementations complete with their documented signature, and an implementation's untyped parameters take the documented types. Contributed by @giuseppe-esposito-87. - **Drupal route names.** In a Drupal project, a route declared by a routing file, a `#[Route]` attribute, or a route builder completes, jumps to its declaration, and lists its references wherever code, a links file, or a template names it, from `Url::fromRoute()`, `$this->redirect()`, and `$form_state->setRedirect()` to `route_name:` and `path()`. A route name nothing declares is reported. The parameter keys those calls, links files, and templates pass jump to the placeholder of the route's path and complete from it, and a controller's parameter is typed as the entity Drupal upcasts it to, with a warning when a controller's type hint unrelated to the entity keeps Drupal from upcasting it. Contributed by @giuseppe-esposito-87. - **Symfony route names.** In a Symfony project, a route name declared by a `#[Route]` attribute, the routing YAML or XML, or a PHP routing configurator completes, jumps to its declaration, and lists its references wherever a controller or a Twig template builds a URL to it, from `generateUrl()` and `redirectToRoute()` to `path()` and `url()`. Names are followed through the imports that load them, bundles' own routing files included, along with the ones Symfony derives from a route, such as `contact.en` for a localized one. A route name nothing declares is reported, unless the router is configured to load its routes some other way. Contributed by @sidux. - **Symfony translations.** In a Symfony project, a translation key declared in a YAML or XLIFF catalogue completes, jumps to its entry in every locale, and lists its references wherever PHP or a Twig template translates it, from `trans()`, `TranslatableMessage`, and `t()` to the `trans` filter. A key is looked up in the domain the call names, and a domain jumps to its catalogues. A key spelled like one (`demo.title`) that nothing declares is reported, in a domain the project's own catalogues hold and no installed bundle ships. Contributed by @sidux. diff --git a/docs/todo.md b/docs/todo.md index f7a66d611..c1347ce1a 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -33,7 +33,6 @@ contributor even though it's short. | # | Item | Impact | Complexity | | --- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | ----------- | | DR9 | [Drupal directories outside `contrib` and `custom`](todo/drupal.md#dr9-drupal-directories-outside-contrib-and-custom) | Medium-High | Low-Medium | -| DR6 | [Hooks](todo/drupal.md#dr6-hooks) | Medium-High | Medium | | DR8 | [Permission and configuration names](todo/drupal.md#dr8-permission-and-configuration-names) | Low-Medium | Medium | | DR7 | [Render arrays, theme hooks, and libraries](todo/drupal.md#dr7-render-arrays-theme-hooks-and-libraries) | Medium | Medium-High | | | **Release 1.0.0 + IDE extensions** | | | @@ -126,7 +125,6 @@ ships, remove every row that names it. | DR11 | [Form and render callbacks](todo/drupal.md#dr11-form-and-render-callbacks) | Low-Medium | Low-Medium | | DR12 | [Translation placeholders](todo/drupal.md#dr12-translation-placeholders) | Low-Medium | Low-Medium | | DR13 | [Module names and `*.info.yml`](todo/drupal.md#dr13-module-names-and-infoyml) | Low-Medium | Low-Medium | -| DR6 | [Hooks](todo/drupal.md#dr6-hooks) | Medium-High | Medium | | DR10 | [Plugin IDs](todo/drupal.md#dr10-plugin-ids) | Medium | Medium | | DR14 | [Port PHPStan's Drupal extension tests](todo/drupal.md#dr14-port-phpstans-drupal-extension-tests) | Medium | Medium | | DR8 | [Permission and configuration names](todo/drupal.md#dr8-permission-and-configuration-names) | Low-Medium | Medium | @@ -136,6 +134,7 @@ ships, remove every row that names it. | DR16 | [Event subscribers](todo/drupal.md#dr16-event-subscribers) | Medium | Low-Medium | | DR17 | [Warn on unknown service IDs](todo/drupal.md#dr17-warn-on-unknown-service-ids) | Medium | Low-Medium | | DR18 | [Warn on unknown entity type IDs](todo/drupal.md#dr18-warn-on-unknown-entity-type-ids) | Medium | Low-Medium | +| DR19 | [Warn on unknown hook names](todo/drupal.md#dr19-warn-on-unknown-hook-names) | Low-Medium | Medium | | DR20 | [Complete route names built in code](todo/drupal.md#dr20-complete-route-names-built-in-code) | Low-Medium | Medium | | DR21 | [Return types core's docblocks leave wide](todo/drupal.md#dr21-return-types-cores-docblocks-leave-wide) | Medium | Low-Medium | diff --git a/docs/todo/drupal.md b/docs/todo/drupal.md index c2c926c9b..896d5a4da 100644 --- a/docs/todo/drupal.md +++ b/docs/todo/drupal.md @@ -4,9 +4,9 @@ Known gaps and missing features in PHPantom's Drupal support. The web root is detected, core, contrib, and the Drupal file extensions (`.module`, `.install`, `.theme`, …) are indexed past `.gitignore`, and the services the `*.services.yml` files declare, the entity types -entity classes declare, and the routes the routing files, route -attributes, and route builders declare are read. Nothing reads Drupal's -other declarations yet: fields or hooks. +entity classes declare, the routes the routing files, route attributes, +and route builders declare, and the hooks the `*.api.php` files document +and the extensions implement are read. Nothing reads fields yet. The requests in [#48](https://github.com/PHPantom-dev/phpantom_lsp/issues/48) are the @@ -54,28 +54,6 @@ names and report a misspelt one. --- -## DR6. Hooks - -**Impact: Medium-High · Complexity: Medium** - -```php -function acme_form_alter(&$form, FormStateInterface $form_state, $form_id) { … } - -#[Hook('form_alter')] -public function formAlter(array &$form, FormStateInterface $form_state, $form_id): void { … } -``` - -A hook is implemented by naming a function `_` or, since -Drupal 11.1, with a `#[Hook]` attribute on a method. Core and modules -document each hook as a `hook_()` function in a `*.api.php` file. -Link an implementation to that documentation: hover shows it, -go-to-definition reaches it, find references lists the implementations, -and completing `acme_` in a `.module` file offers the hooks with their -documented signatures. Parameter types for an implementation that -omits them can come from the documented signature. - ---- - ## DR7. Render arrays, theme hooks, and libraries **Impact: Medium · Complexity: Medium-High** @@ -309,6 +287,21 @@ site where an implementation of that hook may add one stays silent. --- +## DR19. Warn on unknown hook names + +**Impact: Low-Medium · Complexity: Medium** + +```php +\Drupal::moduleHandler()->invokeAll('acme_order_placd', [$order]); +``` + +Any module may invoke any hook name, and many hooks have no `*.api.php` +documentation, so a name nothing documents is not wrong on that alone. +Report only a hook name that nothing in the site documents, implements, +or invokes anywhere else, which is most likely a typo. + +--- + ## DR20. Complete route names built in code **Impact: Low-Medium · Complexity: Medium** diff --git a/examples/drupal/README.md b/examples/drupal/README.md index daf94f117..65bfea3dd 100644 --- a/examples/drupal/README.md +++ b/examples/drupal/README.md @@ -65,6 +65,15 @@ Run `composer install` here first. Composer puts core and contrib into the same route's title callback: both are a `Note`. A controller hint unrelated to the entity class, which leaves Drupal passing the raw ID, is reported. +- Hooks navigate between their documentation, the calls that fire them, and + their implementations. In `web/modules/custom/acme/acme.module`, hover + `$form_state` in `acme_form_alter()`, typed by core's documentation, or go + to the definition of `form_alter` in the function's name, which lists the + `alter()` call in core's `FormBuilder` and `hook_form_alter()`. In + `src/Greeter.php`, go to the definition of `'acme_greeting'`, which lists + the implementation in `src/Hook/AcmeHooks.php` and the documentation in + `acme.api.php`. Type `function acme_` on a new line of `acme.module` to + complete a hook with its documented signature. ## What the Symfony support already reads diff --git a/examples/drupal/assertions.php b/examples/drupal/assertions.php index dfa82a607..cd0ff05a1 100644 --- a/examples/drupal/assertions.php +++ b/examples/drupal/assertions.php @@ -201,6 +201,18 @@ function attributeArguments(ReflectionClass|ReflectionMethod $reflection, string check('acme_greeting has the template its name implies', is_file("$module/templates/acme-greeting.html.twig")); check('acme_theme preprocesses acme_greeting', function_exists('acme_theme_preprocess_acme_greeting')); +// The module documents, invokes, and implements its own hook, and +// implements core's form alter hooks. +require_once "$module/acme.api.php"; +$greetingAlter = new ReflectionMethod(AcmeHooks::class, 'greetingAlter'); +check('acme.api.php documents hook_acme_greeting_alter()', function_exists('hook_acme_greeting_alter')); +check('Greeter invokes the acme_greeting alter hook', str_contains((string) file_get_contents("$module/src/Greeter.php"), "->alter('acme_greeting', \$greeting, \$name)")); +check("AcmeHooks::greetingAlter() implements it through #[Hook('acme_greeting_alter')]", attributeArguments($greetingAlter, Hook::class) === ['acme_greeting_alter']); +check("greetingAlter()'s untyped name is the documented string", $greetingAlter->getParameters()[1]->getType() === null && (string) (new ReflectionFunction('hook_acme_greeting_alter'))->getParameters()[1]->getType() === 'string'); +check('acme_form_alter() implements hook_form_alter procedurally', function_exists('acme_form_alter')); +check("core documents hook_form_alter() with the FormStateInterface acme_form_alter() leaves untyped", str_contains((string) file_get_contents("$web/core/lib/Drupal/Core/Form/form.api.php"), 'function hook_form_alter(&$form, \\Drupal\\Core\\Form\\FormStateInterface $form_state, $form_id)') && (new ReflectionFunction('acme_form_alter'))->getParameters()[1]->getType() === null); +check("core's FormBuilder::prepareForm() fires the form alter hooks", str_contains((string) file_get_contents("$web/core/lib/Drupal/Core/Form/FormBuilder.php"), '$this->moduleHandler->alter($hooks, $form, $form_state, $form_id);')); + // ─── Summary ──────────────────────────────────────────────────────────────── echo "\n"; diff --git a/examples/drupal/web/modules/custom/acme/acme.api.php b/examples/drupal/web/modules/custom/acme/acme.api.php new file mode 100644 index 000000000..57e153f75 --- /dev/null +++ b/examples/drupal/web/modules/custom/acme/acme.api.php @@ -0,0 +1,19 @@ +disableCache(); + } +} + /** * Implements hook_help(). */ diff --git a/examples/drupal/web/modules/custom/acme/src/Greeter.php b/examples/drupal/web/modules/custom/acme/src/Greeter.php index afad6bdd7..66048f794 100644 --- a/examples/drupal/web/modules/custom/acme/src/Greeter.php +++ b/examples/drupal/web/modules/custom/acme/src/Greeter.php @@ -22,9 +22,15 @@ public function greet(string $name): string // from core's ConfigFactoryInterface. $siteName = $this->configFactory->get('system.site')->get('name'); - return strtr($this->format, [ + $greeting = strtr($this->format, [ '@name' => $name, '@site' => is_string($siteName) ? $siteName : 'Drupal', ]); + // Try: go to the definition of 'acme_greeting', which lists the + // implementation in AcmeHooks and then the documentation in + // acme.api.php. Complete a hook inside `alter('')`. + \Drupal::moduleHandler()->alter('acme_greeting', $greeting, $name); + + return $greeting; } } diff --git a/examples/drupal/web/modules/custom/acme/src/Hook/AcmeHooks.php b/examples/drupal/web/modules/custom/acme/src/Hook/AcmeHooks.php index cf538f3f6..4084b3ff3 100644 --- a/examples/drupal/web/modules/custom/acme/src/Hook/AcmeHooks.php +++ b/examples/drupal/web/modules/custom/acme/src/Hook/AcmeHooks.php @@ -28,4 +28,18 @@ public function theme(): array ], ]; } + + /** + * Implements hook_acme_greeting_alter(). + */ + #[Hook('acme_greeting_alter')] + public function greetingAlter(string &$greeting, $name): void + { + // Try: hover `$name`, typed `string` by the hook's documentation in + // acme.api.php, or go to the definition of 'acme_greeting_alter', + // which lists the alter() call in Greeter and the documentation. + if ($name === 'Ada') { + $greeting .= ' Welcome back.'; + } + } } From 4329d01d087637cfd48ec25916960419e2267266 Mon Sep 17 00:00:00 2001 From: "Giuseppe.Esposito" Date: Mon, 2 Nov 2026 16:19:25 +0100 Subject: [PATCH 09/13] Drupal hooks: keep helpers as functions and read hook documentation once A function named after its module whose hook nothing documents or invokes is read as the function it is, so its references, definition, and rename work on its whole name. The hook documentation and the entity type IDs are read once until a declaring file changes, and a pattern is checked against the rest of a name before its entity type, so pattern matching and hook parameter typing stay cheap. Of the patterns a hook matches, the one that writes out the most documents it. Hover names the documentation, find references without declarations keeps the invocations, and test modules' invocations are left out. Assisted-by: claude-code/claude-opus-5-5 --- src/definition/resolve.rs | 2 +- src/drupal/hooks.rs | 239 +++++++++++++++--- src/drupal/mod.rs | 2 + src/hover/laravel.rs | 6 +- src/symbol_map/extraction/drupal.rs | 1 + src/symfony/mod.rs | 14 +- .../variable/forward_walk/param_seeding.rs | 1 + tests/integration/drupal_hooks.rs | 49 +++- 8 files changed, 266 insertions(+), 48 deletions(-) diff --git a/src/definition/resolve.rs b/src/definition/resolve.rs index 809ffce1e..d962168dc 100644 --- a/src/definition/resolve.rs +++ b/src/definition/resolve.rs @@ -87,7 +87,7 @@ impl Backend { ) -> Option { let map = self.symbol_map_for(uri)?; if let Some(span) = map.lookup(offset) { - return Some(span.clone()); + return Some(self.drupal_hook_or_function(&map, span).clone()); } if map .morph_column_sites diff --git a/src/drupal/hooks.rs b/src/drupal/hooks.rs index af411b6fe..3a85ffef8 100644 --- a/src/drupal/hooks.rs +++ b/src/drupal/hooks.rs @@ -325,20 +325,75 @@ pub(crate) fn matches_hook_pattern( .filter(|length| hook.is_char_boundary(*length)) .any(|length| { let (value, after) = hook.split_at(length); - let fits = match *name { - "ENTITY_TYPE" => is_entity_type(value), + let shaped = match *name { "N" => value.bytes().all(|byte| byte.is_ascii_digit()), _ => value.bytes().all(|byte| { byte.is_ascii_lowercase() || byte.is_ascii_digit() || byte == b'_' }), }; - fits && rest(others, after, is_entity_type) + // The rest of the name is cheaper to check than whether + // a run is a declared entity type. + shaped + && rest(others, after, is_entity_type) + && (*name != "ENTITY_TYPE" || is_entity_type(value)) }), } } rest(&pieces(pattern), hook, is_entity_type) } +/// How much of `pattern` is written out: its length without its +/// placeholders. Of the patterns a hook matches, the ones that write out +/// the most are the ones that document it (`theme_suggestions_HOOK_alter` +/// over `theme_suggestions_HOOK` for `theme_suggestions_node_alter`). +fn written_out(pattern: &str) -> usize { + pieces(pattern) + .iter() + .map(|piece| match piece { + Piece::Text(text) => text.len(), + Piece::Placeholder(_) => 0, + }) + .sum() +} + +/// What the documentation of the workspace's hooks says, read once and +/// kept until a file that declares hooks or entity types changes. +#[derive(Default)] +pub(crate) struct HookIndex { + /// Every documented hook, patterns included, sorted. + documented: Vec, + /// The documented hooks with placeholders, sorted. + patterns: Vec, + /// Every declared entity type ID. + entity_types: std::collections::HashSet, +} + +impl HookIndex { + /// The documented patterns `hook` matches that write out the most. + fn best_patterns(&self, hook: &str) -> Vec<&str> { + let is_entity_type = |id: &str| self.entity_types.contains(id); + let matching: Vec<&str> = self + .patterns + .iter() + .filter(|pattern| matches_hook_pattern(pattern, hook, &is_entity_type)) + .map(String::as_str) + .collect(); + let best = matching.iter().map(|pattern| written_out(pattern)).max(); + matching + .into_iter() + .filter(|pattern| Some(written_out(pattern)) == best) + .collect() + } + + /// Whether `hook` is documented, by name or by a pattern. + fn documents(&self, hook: &str) -> bool { + self.documented + .binary_search_by(|known| known.as_str().cmp(hook)) + .is_ok() + || !self.best_patterns(hook).is_empty() + } +} + /// Where a hook is named, and as what. #[derive(Debug, Clone)] struct HookOccurrence { @@ -393,7 +448,6 @@ impl Backend { let reference_key = crate::reference_index::string_key_reference_key(StringKeyKind::DrupalHook, key); let snapshot = self.user_file_symbol_maps_for_reference_keys(&[reference_key]); - let extensions = self.drupal.extensions.read(); let mut found = Vec::new(); for (uri, map) in &snapshot { let typed = if map @@ -433,7 +487,7 @@ impl Backend { }) => Occurrence::Implementation { extension: *extension, is_theme: is_theme - .or_else(|| extensions.is_theme(extension)) + .or_else(|| self.drupal.extensions.read().is_theme(extension)) .unwrap_or(false), collected: *collected, }, @@ -471,12 +525,18 @@ impl Backend { found } - /// Whether the type engine confirms `site` of the file at `uri`. + /// Whether the type engine confirms `site` of the file at `uri`, read + /// in isolation from whatever pass is running, as the declarations of + /// another file are. fn confirms_typed_receiver_site( &self, uri: &str, site: &crate::symbol_map::TypedReceiverSite, ) -> bool { + let _isolated = ( + crate::type_engine::variable::forward_walk::suspend_diagnostic_scope(), + crate::type_engine::resolver::with_isolated_chain_cache(), + ); self.get_file_content_arc(uri).is_some_and(|content| { !self .confirm_typed_receiver_sites(uri, &content, std::slice::from_ref(site)) @@ -587,12 +647,7 @@ impl Backend { found.extend(self.hook_documentation_occurrences(key, &[])); } if !include_declaration { - found.retain(|occurrence| { - !matches!( - occurrence.role, - Occurrence::Documentation | Occurrence::Invocation { .. } - ) - }); + found.retain(|occurrence| occurrence.role != Occurrence::Documentation); } found.sort_by(|a, b| (&a.uri, a.start).cmp(&(&b.uri, b.start))); found.dedup_by(|a, b| a.uri == b.uri && a.start == b.start); @@ -614,45 +669,128 @@ impl Backend { if !own.is_empty() || is_hook_pattern(key) { return own; } - let is_entity_type = |id: &str| !self.drupal_entity_types(id).is_empty(); - self.documented_hook_patterns() - .iter() - .filter(|pattern| matches_hook_pattern(pattern, key, &is_entity_type)) + self.hook_index() + .best_patterns(key) + .into_iter() .flat_map(|pattern| self.hook_occurrences(pattern)) .filter(|occurrence| occurrence.role == Occurrence::Documentation) .collect() } - /// The documented hook names with placeholders, sorted. - fn documented_hook_patterns(&self) -> Vec { - let mut patterns = self.drupal_documented_hooks(); - patterns.retain(|hook| is_hook_pattern(hook)); - patterns - } - /// Every hook an `*.api.php` file documents, patterns included, sorted. pub(crate) fn drupal_documented_hooks(&self) -> Vec { + self.hook_index().documented.clone() + } + + /// What the documentation of the workspace's hooks says, read once + /// until a declaring file changes (see + /// [`Backend::forget_drupal_hook_index`]). + fn hook_index(&self) -> std::sync::Arc { use crate::symbol_map::{StringKeyKind, SymbolKind}; - let mut hooks = Vec::new(); + if let Some(index) = self.drupal.hook_index.read().as_ref() { + return std::sync::Arc::clone(index); + } + let mut index = HookIndex::default(); // The documentation is a plain span; an invocation, the other kind // of declaration, is a call the type engine confirms, and is left // out. self.visit_declaring_symbol_maps(|_, map| { for span in &map.spans { if let SymbolKind::StringKey { - kind: StringKeyKind::DrupalHook, + kind, key, is_write: true, .. } = &span.kind { - hooks.push(key.clone()); + match kind { + StringKeyKind::DrupalHook => index.documented.push(key.clone()), + StringKeyKind::DrupalEntityType => { + index.entity_types.insert(key.clone()); + } + _ => {} + } } } }); - hooks.sort_unstable(); - hooks.dedup(); - hooks + index.documented.sort_unstable(); + index.documented.dedup(); + index.patterns = index + .documented + .iter() + .filter(|hook| is_hook_pattern(hook)) + .cloned() + .collect(); + let index = std::sync::Arc::new(index); + *self.drupal.hook_index.write() = Some(std::sync::Arc::clone(&index)); + index + } + + /// Forget what [`Backend::hook_index`] read, once a file that declares + /// hooks or entity types may have changed. + pub(crate) fn forget_drupal_hook_index(&self) { + if self.drupal.hook_index.read().is_some() { + *self.drupal.hook_index.write() = None; + } + } + + /// The span the cursor means at `span`: the function itself, when + /// `span` is the hook part of a procedural function's name and nothing + /// but the function backs that hook, which makes it a helper named + /// after its extension rather than a hook implementation. + pub(crate) fn drupal_hook_or_function<'m>( + &self, + map: &'m crate::symbol_map::SymbolMap, + span: &'m crate::symbol_map::SymbolSpan, + ) -> &'m crate::symbol_map::SymbolSpan { + use crate::symbol_map::{HookRole, StringKeyKind, SymbolKind}; + let SymbolKind::StringKey { + kind: StringKeyKind::DrupalHook, + key, + is_write: false, + .. + } = &span.kind + else { + return span; + }; + let procedural = map + .drupal_hooks + .binary_search_by_key(&span.start, |site| site.start) + .ok() + .is_some_and(|at| { + matches!( + map.drupal_hooks[at].role, + HookRole::Implementation { + is_theme: Some(_), + .. + } + ) + }); + if !procedural || self.drupal_hook_is_backed(key) { + return span; + } + // The function's own span starts before the hook part and ends with + // it. + let at = map.spans.partition_point(|other| other.start < span.start); + map.spans[..at] + .iter() + .rev() + .find(|other| { + other.end == span.end && matches!(other.kind, SymbolKind::FunctionCall { .. }) + }) + .unwrap_or(span) + } + + /// Whether the hook `key` is backed by anything but its implementations: + /// documented, by name or by a pattern, or invoked. A procedural + /// function named after its extension whose hook nothing backs is + /// read as the function it is. + pub(crate) fn drupal_hook_is_backed(&self, key: &str) -> bool { + self.hook_index().documents(key) + || self + .hook_occurrences(key) + .iter() + .any(|occurrence| matches!(occurrence.role, Occurrence::Invocation { .. })) } /// The documentation function of the hook the function or method @@ -712,22 +850,26 @@ impl Backend { } => Some(key.clone()), _ => None, })?; - let exact = format!("hook_{hook}"); - if let Some(function) = self.find_or_load_function(&[exact.as_str()]) { - return Some(function); - } - let is_entity_type = |id: &str| !self.drupal_entity_types(id).is_empty(); - self.documented_hook_patterns() - .iter() - .filter(|pattern| matches_hook_pattern(pattern, &hook, &is_entity_type)) - .find_map(|pattern| self.find_or_load_function(&[format!("hook_{pattern}").as_str()])) + let index = self.hook_index(); + let documented = if index + .documented + .binary_search_by(|known| known.as_str().cmp(&hook)) + .is_ok() + { + hook + } else { + index.best_patterns(&hook).first()?.to_string() + }; + self.find_or_load_function(&[format!("hook_{documented}").as_str()]) } /// Every hook the workspace names, implemented or documented, that the - /// documented `pattern` stands for. + /// documented `pattern` stands for, and that no pattern writing out + /// more documents. fn hooks_matching(&self, pattern: &str) -> Vec { use crate::symbol_map::{StringKeyKind, SymbolKind}; - let is_entity_type = |id: &str| !self.drupal_entity_types(id).is_empty(); + let index = self.hook_index(); + let is_entity_type = |id: &str| index.entity_types.contains(id); let mut hooks: Vec = Vec::new(); for (_, map) in self.user_file_symbol_maps() { for span in &map.spans { @@ -739,6 +881,7 @@ impl Backend { && !is_hook_pattern(key) && !hooks.contains(key) && matches_hook_pattern(pattern, key, &is_entity_type) + && index.best_patterns(key).contains(&pattern) { hooks.push(key.clone()); } @@ -776,6 +919,22 @@ mod tests { #[test] fn patterns_fill_their_placeholders() { let entity = |id: &str| matches!(id, "node" | "taxonomy_term"); + let index = HookIndex { + documented: Vec::new(), + patterns: vec![ + "theme_suggestions_HOOK".to_string(), + "theme_suggestions_HOOK_alter".to_string(), + ], + entity_types: Default::default(), + }; + assert_eq!( + index.best_patterns("theme_suggestions_node_alter"), + ["theme_suggestions_HOOK_alter"] + ); + assert_eq!( + index.best_patterns("theme_suggestions_node"), + ["theme_suggestions_HOOK"] + ); assert!(matches_hook_pattern( "form_FORM_ID_alter", "form_user_login_form_alter", diff --git a/src/drupal/mod.rs b/src/drupal/mod.rs index d6d97172e..36db61a4e 100644 --- a/src/drupal/mod.rs +++ b/src/drupal/mod.rs @@ -42,6 +42,8 @@ pub(crate) struct DrupalState { hook_php_files: RwLock>, /// The modules, install profiles, and themes of every web root. extensions: RwLock, + /// What the hook documentation says, until a declaring file changes. + hook_index: RwLock>>, } /// A kind of file Drupal reads declarations from. diff --git a/src/hover/laravel.rs b/src/hover/laravel.rs index af186a6cb..5100f6272 100644 --- a/src/hover/laravel.rs +++ b/src/hover/laravel.rs @@ -303,7 +303,11 @@ impl Backend { StringKeyKind::SymfonyTranslation => "Translation", _ => "Translation domain", }; - let declared = self.symfony_declaration_locations(*kind, key); + let declared = if *kind == StringKeyKind::DrupalHook { + self.drupal_hook_documentation(key) + } else { + self.symfony_declaration_locations(*kind, key) + }; let mut detail = match declared .first() .and_then(|location| self.workspace_relative_path(location.uri.as_str())) diff --git a/src/symbol_map/extraction/drupal.rs b/src/symbol_map/extraction/drupal.rs index 58d7dac1c..08fcaccc7 100644 --- a/src/symbol_map/extraction/drupal.rs +++ b/src/symbol_map/extraction/drupal.rs @@ -966,6 +966,7 @@ pub(in crate::symbol_map::extraction) fn record_drupal_hook_invocations( ctx: &mut ExtractionCtx<'_>, ) { if !ctx.drupal + || ctx.drupal_file == crate::drupal::hooks::DrupalFile::Test || !matches!( member_name.as_bytes().first(), Some(b'i' | b'a' | b'I' | b'A') diff --git a/src/symfony/mod.rs b/src/symfony/mod.rs index 582ed056b..99dfa9a0d 100644 --- a/src/symfony/mod.rs +++ b/src/symfony/mod.rs @@ -213,8 +213,12 @@ impl Backend { .iter() .any(|prefix| uri.starts_with(prefix.as_str())); self.note_drupal_routes(uri, Some(map)); + let was_declaring = self.symfony.declaring_uris.read().contains(uri); + if was_declaring || declares { + self.forget_drupal_hook_index(); + } // Only take the write lock when the answer changes. - if self.symfony.declaring_uris.read().contains(uri) == declares { + if was_declaring == declares { return; } let mut uris = self.symfony.declaring_uris.write(); @@ -229,6 +233,7 @@ impl Backend { pub(crate) fn forget_symfony_declarations(&self, uri: &str) { self.note_drupal_routes(uri, None); if self.symfony.declaring_uris.read().contains(uri) { + self.forget_drupal_hook_index(); self.symfony.declaring_uris.write().remove(uri); } } @@ -282,10 +287,11 @@ impl Backend { let mut uris: Vec = self.symfony.declaring_uris.read().iter().cloned().collect(); uris.sort_unstable(); // Only Symfony declares names through calls the type engine has to - // confirm, and Drupal its hooks, which the calls that invoke them - // declare; Drupal's other names never are. + // confirm. A Drupal project records the calls that invoke its + // hooks, but hooks have an index of their own (see + // `crate::drupal::hooks`), which is not read here. let needs_typed = match kind.framework() { - KeyFramework::Drupal => kind == StringKeyKind::DrupalHook, + KeyFramework::Drupal => false, KeyFramework::ServiceContainer => self.is_symfony_project(), KeyFramework::Laravel | KeyFramework::Symfony diff --git a/src/type_engine/variable/forward_walk/param_seeding.rs b/src/type_engine/variable/forward_walk/param_seeding.rs index afb794e95..87a2bcab3 100644 --- a/src/type_engine/variable/forward_walk/param_seeding.rs +++ b/src/type_engine/variable/forward_walk/param_seeding.rs @@ -158,6 +158,7 @@ pub(crate) fn seed_params<'b>( if native_type.is_none() && let Some(backend) = ctx.file.backend + && backend.is_drupal_project() && super::super::resolution::declared_param_docblock_type( ctx.file.content, method_span_start as usize, diff --git a/tests/integration/drupal_hooks.rs b/tests/integration/drupal_hooks.rs index 485cb6875..cc13d00e1 100644 --- a/tests/integration/drupal_hooks.rs +++ b/tests/integration/drupal_hooks.rs @@ -403,14 +403,18 @@ async fn computed_hook_names_reach_their_documented_pattern() { "{needle}" ); } - // `widget` is no entity type. + // `widget` is no entity type, so the function is a helper and leads to + // nothing but itself. let found = definitions_at( &backend, &module, position_in(PATTERN_MODULE, "acme_widget_presave", 6), ) .await; - assert!(found.is_empty(), "{found:?}"); + assert!( + found.iter().all(|location| location.uri == module), + "{found:?}" + ); let found = references_at( &backend, @@ -753,3 +757,44 @@ async fn computed_hooks_reach_the_core_call_that_fires_them() { "only the content storage stores nodes" ); } + +/// A function named after its module that no documentation and no +/// invocation backs is a helper: its whole name is the function's. Hover +/// on an implementation names its documentation, a pattern's included, and +/// a hook matching two patterns is documented by the one that writes out +/// more of it. +#[tokio::test] +async fn helpers_stay_functions_and_hooks_reach_their_most_specific_documentation() { + let api = " = found + .iter() + .map(|l| (l.uri.clone(), l.range.start.line)) + .collect(); + assert_eq!(lines, vec![(api_uri, 3)]); + + let position = position_in(module, "acme_theme_suggestions_node_alter", 7); + let hover = hover_text_at(&backend, &php, position.line, position.character) + .await + .unwrap_or_default(); + assert!(hover.contains("theme.api.php"), "{hover}"); +} From 79373da459327420db7fd43ddef4f9bd82cb6e4a Mon Sep 17 00:00:00 2001 From: "Giuseppe.Esposito" Date: Mon, 2 Nov 2026 16:20:07 +0100 Subject: [PATCH 10/13] Drupal hooks: look a class up in the computed-name table once A class that declares none of the table's core methods is turned away before its methods are compared with the table. Assisted-by: claude-code/claude-opus-5-5 --- src/drupal/computed_hooks.rs | 12 ++++++++++++ src/symbol_map/extraction/drupal.rs | 6 +++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/drupal/computed_hooks.rs b/src/drupal/computed_hooks.rs index 576bf7440..f1b6196c3 100644 --- a/src/drupal/computed_hooks.rs +++ b/src/drupal/computed_hooks.rs @@ -222,6 +222,18 @@ pub(crate) const COMPUTED: &[(&[&str], &[&str], Storage)] = &[ ), ]; +/// Whether `class`, a resolved class name, declares one of the table's core +/// methods. +pub(crate) fn is_computing_class(class: &str) -> bool { + let class = class.trim_start_matches('\\'); + COMPUTED.iter().any(|(_, sites, _)| { + sites.iter().any(|site| { + site.split_once("::") + .is_some_and(|(site_class, _)| site_class.eq_ignore_ascii_case(class)) + }) + }) +} + /// Whether `class::method`, a resolved class name and a method, is one of /// the table's core methods. pub(crate) fn is_computing_method(class: &str, method: &str) -> bool { diff --git a/src/symbol_map/extraction/drupal.rs b/src/symbol_map/extraction/drupal.rs index 08fcaccc7..0ddd62a55 100644 --- a/src/symbol_map/extraction/drupal.rs +++ b/src/symbol_map/extraction/drupal.rs @@ -1053,7 +1053,11 @@ pub(in crate::symbol_map::extraction) fn emit_drupal_computed_hook_sites( if !ctx.drupal || ctx.drupal_file == crate::drupal::hooks::DrupalFile::Test { return; } - let Some(class) = ctx.resolved_name_at(name_offset).map(str::to_string) else { + let Some(class) = ctx + .resolved_name_at(name_offset) + .filter(|class| crate::drupal::computed_hooks::is_computing_class(class)) + .map(str::to_string) + else { return; }; for member in members.iter() { From b1db4d26571d4321194b0f2b5820b5648a42ec8b Mon Sep 17 00:00:00 2001 From: "Giuseppe.Esposito" Date: Mon, 2 Nov 2026 16:36:15 +0100 Subject: [PATCH 11/13] Drupal hooks: only functions and methods take documented parameter types A property hook's parameter no longer reads the documentation of a hook implementation that follows it, and only a declaration's own function keyword is read for its name. Finding the hooks a pattern stands for keeps a set of the names seen, and hook completion reuses the pattern's own placeholder scanner. Assisted-by: claude-code/claude-opus-5-5 --- src/completion/drupal_hooks.rs | 29 ++++--------------- src/drupal/hooks.rs | 13 +++++++-- .../variable/forward_walk/param_seeding.rs | 4 +++ .../forward_walk/readonly_properties.rs | 1 + tests/integration/drupal_hooks.rs | 20 +++++++++++++ 5 files changed, 41 insertions(+), 26 deletions(-) diff --git a/src/completion/drupal_hooks.rs b/src/completion/drupal_hooks.rs index b9579055e..379b163cc 100644 --- a/src/completion/drupal_hooks.rs +++ b/src/completion/drupal_hooks.rs @@ -64,31 +64,14 @@ fn snippet_text(text: &str) -> String { fn hook_snippet(hook: &str) -> (String, usize) { let mut snippet = String::new(); let mut stops = 0; - let bytes = hook.as_bytes(); - let mut i = 0; - while i < bytes.len() { - let starts = bytes[i].is_ascii_uppercase() && (i == 0 || bytes[i - 1] == b'_'); - if !starts { - snippet.push(bytes[i] as char); - i += 1; - continue; - } - let mut end = i; - loop { - while end < bytes.len() - && (bytes[end].is_ascii_uppercase() || bytes[end].is_ascii_digit()) - { - end += 1; - } - if end + 1 < bytes.len() && bytes[end] == b'_' && bytes[end + 1].is_ascii_uppercase() { - end += 1; - } else { - break; + for piece in crate::drupal::hooks::pieces(hook) { + match piece { + crate::drupal::hooks::Piece::Text(text) => snippet.push_str(text), + crate::drupal::hooks::Piece::Placeholder(name) => { + stops += 1; + snippet.push_str(&format!("${{{stops}:{name}}}")); } } - stops += 1; - snippet.push_str(&format!("${{{stops}:{}}}", &hook[i..end])); - i = end; } (snippet, stops) } diff --git a/src/drupal/hooks.rs b/src/drupal/hooks.rs index 3a85ffef8..8c02610f8 100644 --- a/src/drupal/hooks.rs +++ b/src/drupal/hooks.rs @@ -260,7 +260,7 @@ pub(crate) fn is_hook_pattern(name: &str) -> bool { } /// One piece of a hook name pattern. -enum Piece<'a> { +pub(crate) enum Piece<'a> { Text(&'a str), /// An uppercase run, such as `FORM_ID` or `ENTITY_TYPE`. Placeholder(&'a str), @@ -269,7 +269,7 @@ enum Piece<'a> { /// A pattern split into text and placeholders: an uppercase run starts a /// placeholder at the start of the pattern or after an underscore, and goes /// on through underscores followed by more uppercase. -fn pieces(pattern: &str) -> Vec> { +pub(crate) fn pieces(pattern: &str) -> Vec> { let bytes = pattern.as_bytes(); let mut pieces = Vec::new(); let (mut text_start, mut i) = (0, 0); @@ -811,6 +811,12 @@ impl Backend { } let rest = content.get(decl_start as usize..)?; let keyword = rest.find("function")?; + // The declaration's own keyword, after nothing but its attributes + // and modifiers. + let before = &rest[..keyword]; + if before.contains(['{', ';', '(']) && !before.trim_start().starts_with("#[") { + return None; + } let after = &rest[keyword + "function".len()..]; let name_from = after.len() - after.trim_start().len(); let name_start = decl_start as usize + keyword + "function".len() + name_from; @@ -870,6 +876,7 @@ impl Backend { use crate::symbol_map::{StringKeyKind, SymbolKind}; let index = self.hook_index(); let is_entity_type = |id: &str| index.entity_types.contains(id); + let mut seen: std::collections::HashSet = std::collections::HashSet::new(); let mut hooks: Vec = Vec::new(); for (_, map) in self.user_file_symbol_maps() { for span in &map.spans { @@ -879,7 +886,7 @@ impl Backend { .. } = &span.kind && !is_hook_pattern(key) - && !hooks.contains(key) + && seen.insert(key.clone()) && matches_hook_pattern(pattern, key, &is_entity_type) && index.best_patterns(key).contains(&pattern) { diff --git a/src/type_engine/variable/forward_walk/param_seeding.rs b/src/type_engine/variable/forward_walk/param_seeding.rs index 87a2bcab3..410859dc1 100644 --- a/src/type_engine/variable/forward_walk/param_seeding.rs +++ b/src/type_engine/variable/forward_walk/param_seeding.rs @@ -65,6 +65,7 @@ pub(crate) fn seed_body_scope<'b>( span_start, owner.method_name, owner.has_scope_attr, + true, ctx, ); seed_superglobals(&mut scope); @@ -87,6 +88,7 @@ pub(crate) fn seed_params<'b>( method_span_start: u32, method_name: Option<&str>, has_scope_attr: bool, + can_implement_hook: bool, ctx: &ForwardWalkCtx<'_>, ) { // While a body is being read for its return type, the call site that @@ -157,6 +159,7 @@ pub(crate) fn seed_params<'b>( } if native_type.is_none() + && can_implement_hook && let Some(backend) = ctx.file.backend && backend.is_drupal_project() && super::super::resolution::declared_param_docblock_type( @@ -370,6 +373,7 @@ pub(crate) fn seed_property_hook_scope( hook.span().start.offset, None, false, + false, ctx, ); } else if hook.name.value.eq_ignore_ascii_case(b"set") { diff --git a/src/type_engine/variable/forward_walk/readonly_properties.rs b/src/type_engine/variable/forward_walk/readonly_properties.rs index db31e256a..5e4c98bb9 100644 --- a/src/type_engine/variable/forward_walk/readonly_properties.rs +++ b/src/type_engine/variable/forward_walk/readonly_properties.rs @@ -149,6 +149,7 @@ fn constructor_narrowed_properties( ctor.span().start.offset, Some("__construct"), false, + false, &walk_ctx, ); // Each property starts out on its declared type, so an assignment diff --git a/tests/integration/drupal_hooks.rs b/tests/integration/drupal_hooks.rs index cc13d00e1..e0cdd3de7 100644 --- a/tests/integration/drupal_hooks.rs +++ b/tests/integration/drupal_hooks.rs @@ -798,3 +798,23 @@ async fn helpers_stay_functions_and_hooks_reach_their_most_specific_documentatio .unwrap_or_default(); assert!(hover.contains("theme.api.php"), "{hover}"); } + +/// Only a function or a method implements a hook: a property hook's +/// parameter keeps no type when a hook implementation follows it. +#[tokio::test] +async fn a_property_hook_is_no_hook_implementation() { + let api = "count = 1;\n }\n }\n}\nfunction acme_form_alter(&$form, $form_state, $form_id) {\n}\n"; + let (backend, dir) = drupal( + &[(FORM_API_PATH, api)], + &[("web/modules/custom/acme/acme.module", module)], + ) + .await; + let php = uri_for(&dir, "web/modules/custom/acme/acme.module"); + references_at(&backend, &php, position_in(module, "acme_form_alter", 6)).await; + let position = position_in(module, "$value;", 2); + let hover = hover_text_at(&backend, &php, position.line, position.character) + .await + .unwrap_or_default(); + assert!(!hover.contains("array"), "{hover}"); +} From c7467a9bd13ccf814b3f4845d07313e0e819a22b Mon Sep 17 00:00:00 2001 From: "Giuseppe.Esposito" Date: Tue, 3 Nov 2026 11:45:10 +0100 Subject: [PATCH 12/13] Check hooks in a mixed workspace and the Drupal hook invokers The mixed workspace checks that a WordPress hook reaches only the plugin's calls when a Drupal module documents a hook of the same name, and that a module file's methods still complete as usual. A unit test checks that every invoking method of the module handler and the theme manager reaches a receiver offering that class's hooks. Assisted-by: claude-code/claude-opus-5-5 --- src/symbol_map/extraction/drupal.rs | 33 +++++++++++ tests/integration/drupal_mixed_workspace.rs | 65 ++++++++++++++++++++- 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/src/symbol_map/extraction/drupal.rs b/src/symbol_map/extraction/drupal.rs index 0ddd62a55..c23427170 100644 --- a/src/symbol_map/extraction/drupal.rs +++ b/src/symbol_map/extraction/drupal.rs @@ -1453,6 +1453,39 @@ mod tests { use super::*; use crate::names::OwnedResolvedNames; + /// Every invoking method of the module handler and the theme manager, + /// at the argument it names a hook in, reaches a receiver that offers + /// that class's hooks, whichever other table also has the method. + #[test] + fn every_hook_invoker_reaches_its_drupal_class() { + for (invokers, class) in [ + ( + &MODULE_INVOKERS[..], + "Drupal\\Core\\Extension\\ModuleHandlerInterface", + ), + ( + &THEME_INVOKERS[..], + "Drupal\\Core\\Theme\\ThemeManagerInterface", + ), + ] { + for invoker in invokers { + let (receiver, alter) = hook_invocation(invoker.method, invoker.hook) + .unwrap_or_else(|| { + panic!("{}#{} names no receiver", invoker.method, invoker.hook) + }); + assert_eq!(alter, invoker.alter, "{}#{}", invoker.method, invoker.hook); + assert!( + receiver + .targets() + .contains(&(class, StringKeyKind::DrupalHook)), + "{}#{}: {receiver:?} has no {class} target", + invoker.method, + invoker.hook + ); + } + } + } + /// The entity type IDs, classes, and storages `php` declares. fn declared(php: &str) -> Vec<(String, String, Option)> { let arena = mago_allocator::LocalArena::new(); diff --git a/tests/integration/drupal_mixed_workspace.rs b/tests/integration/drupal_mixed_workspace.rs index 412f2a7b6..dfd6a0642 100644 --- a/tests/integration/drupal_mixed_workspace.rs +++ b/tests/integration/drupal_mixed_workspace.rs @@ -158,6 +158,27 @@ const SYMFONY_TEMPLATE: &str = "Home\n"; const SYMFONY_ROUTES_PATH: &str = "symfony/config/routes.yaml"; const SYMFONY_TEMPLATE_PATH: &str = "symfony/templates/home.html.twig"; +/// A Drupal hook documented under the name a WordPress hook of the plugin +/// has, and a module file with a class in it. +const HOOK_FILES: &[(&str, &str)] = &[ + (DRUPAL_API_PATH, DRUPAL_API), + (DRUPAL_MODULE_PATH, DRUPAL_MODULE), +]; + +const DRUPAL_API: &str = "gre + } +} +"#; +const DRUPAL_API_PATH: &str = "drupal/web/modules/custom/acme/acme.api.php"; +const DRUPAL_MODULE_PATH: &str = "drupal/web/modules/custom/acme/acme.module"; + // ─── Symfony, in `symfony/` ───────────────────────────────────────────────── const SYMFONY_COMPOSER: &str = r#"{ @@ -335,13 +356,14 @@ fn mixed_files() -> Vec<(String, &'static str)> { ENTITY_FILES .iter() .chain(ROUTE_FILES) + .chain(HOOK_FILES) .map(|(path, content)| (path.to_string(), *content)), ); files } -/// An initialized workspace holding `files`, with every PHP, YAML, and -/// Twig file among them open. +/// An initialized workspace holding `files`, with every PHP (`.module` +/// files included), YAML, and Twig file among them open. async fn workspace(files: &[(String, &str)]) -> (Backend, tempfile::TempDir) { let dir = tempfile::tempdir().unwrap(); for (path, content) in files { @@ -352,7 +374,7 @@ async fn workspace(files: &[(String, &str)]) -> (Backend, tempfile::TempDir) { let backend = Backend::new_test(); initialize_with_options(&backend, dir.path(), None).await; for (path, content) in files { - let language = if path.ends_with(".php") { + let language = if path.ends_with(".php") || path.ends_with(".module") { "php" } else if path.ends_with(".yml") || path.ends_with(".yaml") { "yaml" @@ -654,6 +676,43 @@ async fn drupal_templates_work_in_the_drupal_site() { assert!(labels.contains(&"acme_shout".to_string()), "{labels:?}"); } +/// The plugin's hook reaches the plugin's calls only, though a Drupal +/// module documents a hook of the same name. +#[tokio::test] +async fn wordpress_hooks_stay_wordpress_beside_drupal_hooks() { + let (backend, dir) = workspace(&mixed_files()).await; + let fires = uri_for(&dir, WORDPRESS_FIRES_PATH); + let listens = uri_for(&dir, WORDPRESS_LISTENS_PATH); + let found = references( + &backend, + &listens, + position_in(WORDPRESS_LISTENS, "'acme_order_placed'", 3), + ) + .await; + let mut expected = vec![ + (fires, line_of(WORDPRESS_FIRES, "do_action(")), + (listens, line_of(WORDPRESS_LISTENS, "add_action(")), + ]; + expected.sort(); + assert_eq!(found, expected); +} + +/// In a module file, a method's body still gets the ordinary completion. +#[tokio::test] +async fn a_module_files_methods_complete_as_usual() { + let (backend, dir) = workspace(&mixed_files()).await; + let labels = completion_labels( + &backend, + &uri_for(&dir, DRUPAL_MODULE_PATH), + position_in(DRUPAL_MODULE, "$this->gre", 10), + ) + .await; + assert!( + labels.iter().any(|label| label.starts_with("greet")), + "{labels:?}" + ); +} + /// A Drupal lookup reads every project's service declarations, the /// Symfony application's PHP configurator among them; confirming its calls /// from the Drupal site must not resolve them against the Drupal site's From 78d65a86a4cc56cbb777672ac5c2770d03d482be Mon Sep 17 00:00:00 2001 From: Anders Jenbo Date: Thu, 5 Nov 2026 16:42:09 +0100 Subject: [PATCH 13/13] Drupal hooks: resolve completed hints and type entity hook parameters Resolve a completed hook signature's hints with the *.api.php file's own imports, so an imported class is written the way the implementing file reaches it. Type an entity interface parameter of a hook_ENTITY_TYPE_*() implementation as the entity class of its type, which removes false unknown-member warnings on core. Drop the hook index only when a file that documents hooks or declares entity types changes, rather than on every edit of a file that invokes a hook. Check a config entity's storage handler with the shared subtype test instead of a depth-capped parent walk. File B734, a typed by-reference parameter leaving the caller's variable at its old type. Co-Authored-By: Claude Opus 5.5 --- docs/CHANGELOG.md | 2 +- docs/todo/bugs.md | 24 +++ src/completion/drupal_hooks.rs | 29 +++- src/drupal/computed_hooks.rs | 23 +-- src/drupal/hooks.rs | 152 +++++++++++++++--- src/symfony/mod.rs | 6 +- .../variable/forward_walk/param_seeding.rs | 10 +- tests/integration/drupal_hooks.rs | 115 +++++++++++++ 8 files changed, 307 insertions(+), 54 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 260de8dab..2b7c4410f 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -48,7 +48,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Symfony services that bundles and the container builder declare.** The service IDs and parameters an installed bundle declares in its own configuration complete, jump to their declaration, and type `$container->get()`, and so do the ones a kernel, an extension, or a compiler pass declares with `register()`, `setAlias()`, `setDefinition()`, `setParameter()`, or `getKernelParameters()`. A parameter set under a name built at runtime, like `'app.' . $key`, leaves the names it can produce unreported. A configurator's `parent()` and the `%name%` placeholders in its arguments are read as the service and parameter they name. - **Drupal services.** In a Drupal project, the service IDs and container parameters that core, contributed, and custom modules declare in their `*.services.yml` complete, jump to their declaration, and list their references wherever a services file, `$container->get()`, or `\Drupal::service()` names them, and a lookup is typed by the class the service is built from, or by the class or interface a lookup names by `::class`. Rename leaves core and contributed extensions alone, as it does vendor code. Contributed by @giuseppe-esposito-87. - **Drupal entity types.** In a Drupal project, an entity type ID declared by an entity class's attribute or annotation completes, jumps to its declaration, and lists its references wherever the entity type manager is asked for it, and `getStorage()` is typed as that entity type's storage, so `load()`, `loadMultiple()`, `create()`, and the other loads return the entity class. An entity query started by `\Drupal::entityQuery()` or a storage's `getQuery()` gives the IDs of that entity type, or their number after `count()`. Contributed by @giuseppe-esposito-87. -- **Drupal hooks.** In a Drupal project, a hook implementation, procedural or by `#[Hook]`, jumps to the hook's `*.api.php` documentation and to the calls that fire it, core's computed hook names included, and an invocation jumps to the implementations it reaches. Hook names and implementations complete with their documented signature, and an implementation's untyped parameters take the documented types. Contributed by @giuseppe-esposito-87. +- **Drupal hooks.** In a Drupal project, a hook implementation, procedural or by `#[Hook]`, jumps to the hook's `*.api.php` documentation and to the calls that fire it, core's computed hook names included, and an invocation jumps to the implementations it reaches. Hook names and implementations complete with their documented signature, and an implementation's untyped parameters take the documented types, narrowed to the entity class for a hook of one entity type. Contributed by @giuseppe-esposito-87. - **Drupal route names.** In a Drupal project, a route declared by a routing file, a `#[Route]` attribute, or a route builder completes, jumps to its declaration, and lists its references wherever code, a links file, or a template names it, from `Url::fromRoute()`, `$this->redirect()`, and `$form_state->setRedirect()` to `route_name:` and `path()`. A route name nothing declares is reported. The parameter keys those calls, links files, and templates pass jump to the placeholder of the route's path and complete from it, and a controller's parameter is typed as the entity Drupal upcasts it to, with a warning when a controller's type hint unrelated to the entity keeps Drupal from upcasting it. Contributed by @giuseppe-esposito-87. - **Symfony route names.** In a Symfony project, a route name declared by a `#[Route]` attribute, the routing YAML or XML, or a PHP routing configurator completes, jumps to its declaration, and lists its references wherever a controller or a Twig template builds a URL to it, from `generateUrl()` and `redirectToRoute()` to `path()` and `url()`. Names are followed through the imports that load them, bundles' own routing files included, along with the ones Symfony derives from a route, such as `contact.en` for a localized one. A route name nothing declares is reported, unless the router is configured to load its routes some other way. Contributed by @sidux. - **Symfony translations.** In a Symfony project, a translation key declared in a YAML or XLIFF catalogue completes, jumps to its entry in every locale, and lists its references wherever PHP or a Twig template translates it, from `trans()`, `TranslatableMessage`, and `t()` to the `trans` filter. A key is looked up in the domain the call names, and a domain jumps to its catalogues. A key spelled like one (`demo.title`) that nothing declares is reported, in a domain the project's own catalogues hold and no installed bundle ships. Contributed by @sidux. diff --git a/docs/todo/bugs.md b/docs/todo/bugs.md index 3697fcee4..a09301bb9 100644 --- a/docs/todo/bugs.md +++ b/docs/todo/bugs.md @@ -208,3 +208,27 @@ only the replacement declares is reported: about ninety times on core, all for `language_manager`. Read the literal `setClass()` calls in `ServiceProvider::alter()` methods and type the service as the union of the declared class and every replacement. + +### B734. Passing a variable to a typed by-reference parameter leaves its old type + +**Impact: Low-Medium · Complexity: Low-Medium** + +```php +function mail(array &$message): void { + $message['body'][] = new Markup(); +} + +$message = ['body' => []]; +mail($message); +$message['body'][0]; // null, should be mixed (or Markup) +``` + +After a call, a variable passed to a by-reference parameter keeps the +type it had before the call when the parameter declares a type, native +or `@param`, and no `@param-out`. An untyped by-reference parameter +takes the type the callee's body writes into it, so only the typed case +is wrong. PHPStan gives the variable the parameter's declared type when +there is no `@param-out`. A test of Drupal's update module reports +`Cannot access method 'render' on type 'never'` after calling a +`hook_mail()` implementation, which reads `$message` as `array` from the +hook's documentation. diff --git a/src/completion/drupal_hooks.rs b/src/completion/drupal_hooks.rs index 379b163cc..d873f03c6 100644 --- a/src/completion/drupal_hooks.rs +++ b/src/completion/drupal_hooks.rs @@ -105,6 +105,10 @@ impl Backend { } let start_position = crate::text_position::offset_to_position(content, start); let mut items = Vec::new(); + // The imports and namespace of each `*.api.php` file, which its + // documented hints are written against. + let mut api_scopes: std::collections::HashMap = + std::collections::HashMap::new(); for hook in self.drupal_documented_hooks() { let name = format!("{}_{hook}", procedural.extension); let matches_typed = name @@ -117,7 +121,20 @@ impl Backend { let doc = format!("hook_{hook}"); let parameters = self .find_or_load_function(&[doc.as_str()]) - .map(|function| self.hook_parameters(&function.parameters, ctx)) + .map(|function| { + let api_uri = self + .global_functions() + .read() + .get(doc.as_str()) + .map(|(uri, _)| uri.clone()) + .unwrap_or_default(); + let (use_map, namespace) = + api_scopes.entry(api_uri).or_insert_with_key(|api_uri| { + let api = self.file_context(api_uri); + (api.use_map, api.namespace) + }); + self.hook_parameters(&function.parameters, (&*use_map, &*namespace), ctx) + }) .unwrap_or_default(); let (hook_part, stops) = hook_snippet(&hook); let snippet = format!( @@ -148,21 +165,27 @@ impl Backend { } /// The documented parameters, as an implementation writes them: the - /// native hints, shortened by the file's imports. + /// native hints, read with the imports and namespace of the `*.api.php` + /// file that writes them (`api`), then shortened by the implementing + /// file's. fn hook_parameters( &self, parameters: &[crate::types::ParameterInfo], + (api_use_map, api_namespace): (&std::collections::HashMap, &Option), ctx: &FileContext, ) -> String { parameters .iter() .map(|parameter| { + // A function's native hint is kept as written. let hint = parameter .native_type_hint .as_ref() .map(|hint| { hint.resolve_names(&|name| { - crate::names::shorten_from_fqn(name, &ctx.use_map, &ctx.namespace) + let fqn = + crate::names::resolve_to_fqn(name, api_use_map, api_namespace); + crate::names::shorten_class_fqn(&fqn, &ctx.use_map, &ctx.namespace) }) .to_string() }) diff --git a/src/drupal/computed_hooks.rs b/src/drupal/computed_hooks.rs index f1b6196c3..852659c58 100644 --- a/src/drupal/computed_hooks.rs +++ b/src/drupal/computed_hooks.rs @@ -272,26 +272,13 @@ impl Backend { if storage == Storage::Any { return !declared.is_empty(); } + let loader = self.resolved_class_loader_with(&[]); declared.iter().any(|(_, handler)| { let is_config = handler.is_some_and(|handler| { - let mut class = Some(handler.to_string()); - let mut depth = 0; - while let Some(name) = class.take() { - if name - .trim_start_matches('\\') - .eq_ignore_ascii_case(CONFIG_STORAGE) - { - return true; - } - depth += 1; - if depth > 20 { - break; - } - class = self - .find_or_load_class(&name) - .and_then(|info| info.parent_class.map(|parent| parent.to_string())); - } - false + self.find_or_load_class(handler.as_str()) + .is_some_and(|class| { + crate::class_lookup::is_subtype_of(&class, CONFIG_STORAGE, &loader) + }) }); is_config == (storage == Storage::Config) }) diff --git a/src/drupal/hooks.rs b/src/drupal/hooks.rs index 8c02610f8..54d673221 100644 --- a/src/drupal/hooks.rs +++ b/src/drupal/hooks.rs @@ -313,17 +313,29 @@ pub(crate) fn matches_hook_pattern( hook: &str, is_entity_type: &dyn Fn(&str) -> bool, ) -> bool { - fn rest(pieces: &[Piece<'_>], hook: &str, is_entity_type: &dyn Fn(&str) -> bool) -> bool { + match_hook_pattern(pattern, hook, is_entity_type).is_some() +} + +/// [`matches_hook_pattern`], with the entity type ID the `ENTITY_TYPE` +/// placeholder of `pattern` stands for in `hook`, when it has one. +fn match_hook_pattern<'h>( + pattern: &str, + hook: &'h str, + is_entity_type: &dyn Fn(&str) -> bool, +) -> Option> { + fn rest<'h>( + pieces: &[Piece<'_>], + hook: &'h str, + is_entity_type: &dyn Fn(&str) -> bool, + ) -> Option> { let Some((first, others)) = pieces.split_first() else { - return hook.is_empty(); + return hook.is_empty().then_some(None); }; match first { - Piece::Text(text) => hook - .strip_prefix(text) - .is_some_and(|after| rest(others, after, is_entity_type)), + Piece::Text(text) => rest(others, hook.strip_prefix(text)?, is_entity_type), Piece::Placeholder(name) => (1..=hook.len()) .filter(|length| hook.is_char_boundary(*length)) - .any(|length| { + .find_map(|length| { let (value, after) = hook.split_at(length); let shaped = match *name { "N" => value.bytes().all(|byte| byte.is_ascii_digit()), @@ -331,11 +343,16 @@ pub(crate) fn matches_hook_pattern( byte.is_ascii_lowercase() || byte.is_ascii_digit() || byte == b'_' }), }; + if !shaped { + return None; + } // The rest of the name is cheaper to check than whether // a run is a declared entity type. - shaped - && rest(others, after, is_entity_type) - && (*name != "ENTITY_TYPE" || is_entity_type(value)) + let captured = rest(others, after, is_entity_type)?; + if *name != "ENTITY_TYPE" { + return Some(captured); + } + is_entity_type(value).then_some(Some(value)) }), } } @@ -366,6 +383,8 @@ pub(crate) struct HookIndex { patterns: Vec, /// Every declared entity type ID. entity_types: std::collections::HashSet, + /// The files the index was read from. + sources: std::collections::HashSet, } impl HookIndex { @@ -394,6 +413,53 @@ impl HookIndex { } } +/// The documentation function of the hook an implementation implements, +/// and the class of the entity type a documented `ENTITY_TYPE` +/// placeholder stands for in the implemented hook's name. +pub(crate) struct HookDocumentation { + pub function: crate::types::FunctionInfo, + pub entity_class: Option, +} + +const ENTITY_INTERFACE: &str = "Drupal\\Core\\Entity\\EntityInterface"; + +impl HookDocumentation { + /// The documented type of the parameter at `index`, with each entity + /// interface it names (`EntityInterface`, `NodeInterface`) that the + /// entity class of the implemented hook implements read as that class: + /// `hook_node_presave()` is passed a `Node`, which + /// `hook_ENTITY_TYPE_presave()` documents as an `EntityInterface`. + pub(crate) fn parameter_type( + &self, + index: usize, + class_loader: &crate::class_lookup::ResolvedClassLoader<'_>, + ) -> Option { + // The effective type, which the parser resolved against the + // `*.api.php` file's own imports; a function's native hint is kept + // as written. + let documented = self.function.parameters.get(index)?.type_hint.clone()?; + let Some(entity) = self + .entity_class + .and_then(|class| class_loader(crate::class_lookup::Fqn::new(&class))) + else { + return Some(documented); + }; + Some(documented.resolve_names(&|name| { + let is_entity_interface = name.trim_start_matches('\\') == ENTITY_INTERFACE + || class_loader(crate::class_lookup::Fqn::new(name)).is_some_and(|class| { + crate::class_lookup::is_subtype_of(&class, ENTITY_INTERFACE, class_loader) + }); + if is_entity_interface + && crate::class_lookup::is_subtype_of(&entity, name, class_loader) + { + entity.fqn().to_string() + } else { + name.to_string() + } + })) + } +} + /// Where a hook is named, and as what. #[derive(Debug, Clone)] struct HookOccurrence { @@ -684,7 +750,7 @@ impl Backend { /// What the documentation of the workspace's hooks says, read once /// until a declaring file changes (see - /// [`Backend::forget_drupal_hook_index`]). + /// [`Backend::note_drupal_hook_declarations`]). fn hook_index(&self) -> std::sync::Arc { use crate::symbol_map::{StringKeyKind, SymbolKind}; if let Some(index) = self.drupal.hook_index.read().as_ref() { @@ -694,7 +760,8 @@ impl Backend { // The documentation is a plain span; an invocation, the other kind // of declaration, is a call the type engine confirms, and is left // out. - self.visit_declaring_symbol_maps(|_, map| { + self.visit_declaring_symbol_maps(|uri, map| { + let mut read = false; for span in &map.spans { if let SymbolKind::StringKey { kind, @@ -708,10 +775,14 @@ impl Backend { StringKeyKind::DrupalEntityType => { index.entity_types.insert(key.clone()); } - _ => {} + _ => continue, } + read = true; } } + if read { + index.sources.insert(uri.to_string()); + } }); index.documented.sort_unstable(); index.documented.dedup(); @@ -726,10 +797,36 @@ impl Backend { index } - /// Forget what [`Backend::hook_index`] read, once a file that declares - /// hooks or entity types may have changed. - pub(crate) fn forget_drupal_hook_index(&self) { - if self.drupal.hook_index.read().is_some() { + /// Forget what [`Backend::hook_index`] read when the file at `uri`, + /// now holding `map` (`None` once it is gone), documents a hook or + /// declares an entity type, or did when the index was read. Any other + /// edit, such as one to a file that only invokes hooks, keeps it. + pub(crate) fn note_drupal_hook_declarations( + &self, + uri: &str, + map: Option<&crate::symbol_map::SymbolMap>, + ) { + use crate::symbol_map::{StringKeyKind, SymbolKind}; + let stale = match self.drupal.hook_index.read().as_ref() { + None => return, + Some(index) => { + index.sources.contains(uri) + || map.is_some_and(|map| { + map.spans.iter().any(|span| { + matches!( + span.kind, + SymbolKind::StringKey { + kind: StringKeyKind::DrupalHook + | StringKeyKind::DrupalEntityType, + is_write: true, + .. + } + ) + }) + }) + } + }; + if stale { *self.drupal.hook_index.write() = None; } } @@ -804,7 +901,7 @@ impl Backend { decl_start: u32, method: Option<&str>, class_fqn: &str, - ) -> Option { + ) -> Option { use crate::symbol_map::{StringKeyKind, SymbolKind}; if !self.is_drupal_project() { return None; @@ -857,16 +954,28 @@ impl Backend { _ => None, })?; let index = self.hook_index(); - let documented = if index + let (documented, entity_type) = if index .documented .binary_search_by(|known| known.as_str().cmp(&hook)) .is_ok() { - hook + (hook.clone(), None) } else { - index.best_patterns(&hook).first()?.to_string() + let pattern = index.best_patterns(&hook).first()?.to_string(); + let is_entity_type = |id: &str| index.entity_types.contains(id); + let entity_type = match_hook_pattern(&pattern, &hook, &is_entity_type).flatten(); + (pattern, entity_type) }; - self.find_or_load_function(&[format!("hook_{documented}").as_str()]) + let function = self.find_or_load_function(&[format!("hook_{documented}").as_str()])?; + let entity_class = entity_type.and_then(|id| { + self.drupal_entity_types(id) + .first() + .map(|(class, _)| *class) + }); + Some(HookDocumentation { + function, + entity_class, + }) } /// Every hook the workspace names, implemented or documented, that the @@ -933,6 +1042,7 @@ mod tests { "theme_suggestions_HOOK_alter".to_string(), ], entity_types: Default::default(), + sources: Default::default(), }; assert_eq!( index.best_patterns("theme_suggestions_node_alter"), diff --git a/src/symfony/mod.rs b/src/symfony/mod.rs index 99dfa9a0d..81c8b2462 100644 --- a/src/symfony/mod.rs +++ b/src/symfony/mod.rs @@ -213,10 +213,8 @@ impl Backend { .iter() .any(|prefix| uri.starts_with(prefix.as_str())); self.note_drupal_routes(uri, Some(map)); + self.note_drupal_hook_declarations(uri, Some(map)); let was_declaring = self.symfony.declaring_uris.read().contains(uri); - if was_declaring || declares { - self.forget_drupal_hook_index(); - } // Only take the write lock when the answer changes. if was_declaring == declares { return; @@ -232,8 +230,8 @@ impl Backend { /// Forget a file that no longer exists. pub(crate) fn forget_symfony_declarations(&self, uri: &str) { self.note_drupal_routes(uri, None); + self.note_drupal_hook_declarations(uri, None); if self.symfony.declaring_uris.read().contains(uri) { - self.forget_drupal_hook_index(); self.symfony.declaring_uris.write().remove(uri); } } diff --git a/src/type_engine/variable/forward_walk/param_seeding.rs b/src/type_engine/variable/forward_walk/param_seeding.rs index 410859dc1..d0a122638 100644 --- a/src/type_engine/variable/forward_walk/param_seeding.rs +++ b/src/type_engine/variable/forward_walk/param_seeding.rs @@ -105,7 +105,7 @@ pub(crate) fn seed_params<'b>( // A Drupal hook implementation that leaves a parameter untyped takes // the type its hook's documentation gives it; looked up once, when // there is such a parameter. - let mut hook_documentation: Option> = None; + let mut hook_documentation: Option> = None; for (index, param) in parameters.enumerate() { let pname = bytes_to_str(param.variable.name).to_string(); @@ -178,12 +178,8 @@ pub(crate) fn seed_params<'b>( ) }) .as_ref() - .and_then(|function| function.parameters.get(index)) - .and_then(|parameter| { - parameter - .type_hint - .clone() - .or_else(|| parameter.native_type_hint.clone()) + .and_then(|documentation| { + documentation.parameter_type(index, ctx.file.class_loader) }) { scope.seed(&pname, ctx.resolved_types_for(documented)); diff --git a/tests/integration/drupal_hooks.rs b/tests/integration/drupal_hooks.rs index e0cdd3de7..9d4c128e1 100644 --- a/tests/integration/drupal_hooks.rs +++ b/tests/integration/drupal_hooks.rs @@ -625,6 +625,121 @@ async fn untyped_hook_parameters_take_the_documented_types() { } } +/// A documented hint the `*.api.php` file imports is completed as the +/// class it names there, written the way the implementing file reaches it. +#[tokio::test] +async fn completed_signatures_read_hints_with_the_api_files_imports() { + let api = " items, + Some(tower_lsp_server::gen_lsp_types::CompletionResponse::CompletionList(list)) => { + list.items + } + None => Vec::new(), + }; + let item = items + .iter() + .find(|item| item.label == "acme_block_view_alter") + .expect("acme_block_view_alter offered"); + let Some(tower_lsp_server::gen_lsp_types::CompletionItemTextEdit::TextEdit(edit)) = + &item.text_edit + else { + panic!("{item:?}"); + }; + assert_eq!(edit.new_text, expected); + } +} + +/// An implementation of a hook documented for every entity type is passed +/// the entity class of the type its name fills in, where the documentation +/// names an entity interface that class implements; any other parameter +/// keeps its documented type. +#[tokio::test] +async fn entity_type_hooks_take_the_entity_class_of_their_type() { + let api = "